CSC/ECE 517 Fall 2015/oss E1561 WZL
E1561. Refactoring due_date.rb and deadline_helper.rb
This page provides a description of the Expertiza based OSS project. This project aimed at refactoring due_date.rb and deadline_helper.rb.
Introduction to Expertiza
Expertiza is a Open Source Rails application where students can submit and peer-review learning objects (articles, code, web sites, etc). It is used in select courses at NC State and by professors at several other colleges and universities. This Open Source application can be cloned from Github, the latest active branch is "Rails 4".
Project Desicription<ref>https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus/edit</ref>
Files involved:
due_date.rb response_controller.rb deadline-helper.rb sign_up_sheet.rb
What it does: Manages the deadlines of an assignment, setting due dates for an assignment, copying due dates from one assignment to a new assignment etc.
What's wrong with it:
- It has methods making unnecessary DB calls.
- It contains duplicated methods.
What needs to be done: There are 5 major goals for this project:
- Remove
DueDate.assign_topic_deadline
and useDeadlineHelper.create_topic_deadline
method wherever possible.
- Create a new method for sort in
due_date.rb
and invoke it from places used indue_date.rb
andresponse_controller.rb
.
- Rename
setFlag()
to adhere to Rails naming conventions.
- Refactor
DueDate.default_permission
.
- Refactor
DeadlineHelper.set_start_due_date
method to smaller methods.
Modification
Case 1: Remove DueDate.assign_topic_deadline
and use DeadlineHelper.create_topic_deadline
DueDate.assign_topic_deadline
method which is same as DeadlineHelper.create_topic_deadline
method.
DueDate.assign_topic_deadline Method | DeadlineHelper.create_topic_deadline Method |
---|---|
def self.assign_topic_deadline(due_date,offset,topic_id) topic_deadline = TopicDeadline.new topic_deadline.topic_id = topic_id topic_deadline.due_at = DateTime.parse(due_date.due_at.to_s) + offset.to_i topic_deadline.deadline_type_id = due_date.deadline_type_id topic_deadline.late_policy_id = nil topic_deadline.submission_allowed_id = due_date.submission_allowed_id topic_deadline.review_allowed_id = due_date.review_allowed_id topic_deadline.review_of_review_allowed_id = due_date.review_of_review_allowed_id topic_deadline.round = due_date.round topic_deadline.save end |
def self.create_topic_deadline(due_date,offset,topic_id) topic_deadline = TopicDeadline.new topic_deadline.topic_id = topic_id topic_deadline.due_at = DateTime.parse(due_date.due_at.to_s) + offset.to_i topic_deadline.deadline_type_id = due_date.deadline_type_id topic_deadline.late_policy_id = nil topic_deadline.submission_allowed_id = due_date.submission_allowed_id topic_deadline.review_allowed_id = due_date.review_allowed_id topic_deadline.review_of_review_allowed_id = due_date.review_of_review_allowed_id topic_deadline.round = due_date.round topic_deadline.save end |
First remove the assign_topic_deadline
method in due_date.rb
, then use DeadlineHelper.create_topic_deadline
in sign_up_sheet.rb
instead.
Before Change | After Change |
---|---|
set_of_due_dates = DueDate.where(assignment_id: assignment_id) set_of_due_dates.each { |due_date| DueDate.assign_topic_deadline(due_date, 0, topic.id) |
set_of_due_dates = DueDate.where(assignment_id: assignment_id) set_of_due_dates.each { |due_date| DeadlineHelper.create_topic_deadline(due_date, 0, topic.id) |
Case 2: Create a new method for sort in due_date.rb
and use and invoke it from where use it
First, create a new class method in due_date.rb
.
def self.deadline_sort(due_dates) due_dates.sort { |m1, m2| (m1.due_at and m2.due_at) ? m1.due_at <=> m2.due_at : (m1.due_at ? -1 : 1) } end
Then call the new method when we sort the deadline in in line number 104 of due_date.rb
and in line number 61 of response_controller.rb
.
In due_date.rb:
Before Change | After Change |
---|---|
due_dates = DueDate.where(["assignment_id = ?", assignment_id]) sorted_deadlines = Array.new #sorted so that the earliest deadline is at the first sorted_deadlines = due_dates.sort { |m1, m2| (m1.due_at and m2.due_at) ? m1.due_at <=> m2.due_at : (m1.due_at ? -1 : 1) } |
due_dates = DueDate.where(["assignment_id = ?", assignment_id]) sorted_deadlines = Array.new #sorted so that the earliest deadline is at the first sorted_deadlines = deadline_sort(due_dates) |
In response_controller.rb:
Before Change | After Change |
---|---|
due_dates = DueDate.where(["assignment_id = ?", @assignment.id]) @sorted_deadlines=Array.new @sorted_deadlines=due_dates.sort { |m1, m2| (m1.due_at and m2.due_at) ? m1.due_at <=> m2.due_at : (m1.due_at ? -1 : 1) } |
due_dates = DueDate.where(["assignment_id = ?", @assignment.id]) @sorted_deadlines=Array.new @sorted_deadlines=DueDate.deadline_sort(due_dates) |
Case 3: Rename setFlag()
to adhere to Rails naming conventions
Find the method in due_date.rb
and rename it to set_flag
.
Before Change | After Change |
---|---|
def setFlag() self.flag = true self.save end |
def set_flag self.flag = true self.save end |
Then also change the method name where it is called.
In background_email_reminder.rake
:
Before Change | After Change |
---|---|
date.setFlag |
date.set_flag |
Case 4: Use the class variable in default_permission
method and use conditionally check deadline type using if...else
statements
Before Change | After Change |
---|---|
def self.default_permission(deadline_type, permission_type) permission_id = Hash.new permission_id['OK'] = DeadlineRight.find_by_name('OK').id permission_id['No'] = DeadlineRight.find_by_name('No').id permission_id['Late'] = DeadlineRight.find_by_name('Late').id default_permission = Hash.new default_permission['submission'] = Hash.new default_permission['submission']['submission_allowed'] = permission_id['OK'] default_permission['submission']['can_review'] = permission_id['No'] default_permission['submission']['review_of_review_allowed'] = permission_id['No'] default_permission['review'] = Hash.new default_permission['review']['submission_allowed'] = permission_id['No'] default_permission['review']['can_review'] = permission_id['OK'] default_permission['review']['review_of_review_allowed'] = permission_id['No'] default_permission['metareview'] = Hash.new default_permission['metareview']['submission_allowed'] = permission_id['No'] default_permission['metareview']['can_review'] = permission_id['No'] default_permission['metareview']['review_of_review_allowed'] = permission_id['OK'] default_permission['drop_topic'] = Hash.new default_permission['drop_topic']['submission_allowed'] = permission_id['OK'] default_permission['drop_topic']['can_review'] = permission_id['No'] default_permission['drop_topic']['review_of_review_allowed'] = permission_id['No'] default_permission['signup'] = Hash.new default_permission['signup']['submission_allowed'] = permission_id['OK'] default_permission['signup']['can_review'] = permission_id['No'] default_permission['signup']['review_of_review_allowed'] = permission_id['No'] default_permission['team_formation'] = Hash.new default_permission['team_formation']['submission_allowed'] = permission_id['OK'] default_permission['team_formation']['can_review'] = permission_id['No'] default_permission['team_formation']['review_of_review_allowed'] = permission_id['No'] default_permission[deadline_type][permission_type] end |
@@permission_id = Hash.new @@permission_id['OK'] = DeadlineRight.find_by_name('OK').id @@permission_id['No'] = DeadlineRight.find_by_name('No').id @@permission_id['Late'] = DeadlineRight.find_by_name('Late').id def self.default_permission(deadline_type, permission_type) default_permission = Hash.new if (deadline_type == 'submission') default_permission['submission'] = Hash.new default_permission['submission']['submission_allowed'] = @@permission_id['OK'] default_permission['submission']['can_review'] = @@permission_id['No'] default_permission['submission']['review_of_review_allowed'] = @@permission_id['No'] elsif (deadline_type == 'review') default_permission['review'] = Hash.new default_permission['review']['submission_allowed'] = @@permission_id['No'] default_permission['review']['can_review'] = @@permission_id['OK'] default_permission['review']['review_of_review_allowed'] = @@permission_id['No'] elsif (deadline_type == 'metareview') default_permission['metareview'] = Hash.new default_permission['metareview']['submission_allowed'] = @@permission_id['No'] default_permission['metareview']['can_review'] = @@permission_id['No'] default_permission['metareview']['review_of_review_allowed'] = @@permission_id['OK'] elsif (deadline_type == 'drop_topic') default_permission['drop_topic'] = Hash.new default_permission['drop_topic']['submission_allowed'] = @@permission_id['OK'] default_permission['drop_topic']['can_review'] = @@permission_id['No'] default_permission['drop_topic']['review_of_review_allowed'] = @@permission_id['No'] elsif (deadline_type == 'signup') default_permission['signup'] = Hash.new default_permission['signup']['submission_allowed'] = @@permission_id['OK'] default_permission['signup']['can_review'] = @@permission_id['No'] default_permission['signup']['review_of_review_allowed'] = @@permission_id['No'] elsif (deadline_type == 'team_formation') default_permission['team_formation'] = Hash.new default_permission['team_formation']['submission_allowed'] = @@permission_id['OK'] default_permission['team_formation']['can_review'] = @@permission_id['No'] default_permission['team_formation']['review_of_review_allowed'] = @@permission_id['No'] end default_permission[deadline_type][permission_type] end |
Results Screenshot
References
<references/>