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:

1. Remove DueDate.assign_topic_deadline and use DeadlineHelper.create_topic_deadline method wherever possible.

2. Create a new method for sort in due_date.rb and invoke it from places used in due_date.rb and response_controller.rb.

3. Rename setFlag() to adhere to Rails naming conventions.

4. Refactor DueDate.default_permission.

5. 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

Case 5: Refactor `DeadlineHelper.set_start_due_date` method to smaller methods

The set_start_due_date method in deadline_helper.rbis too long. First create a new method check_dependency.

 def self.check_dependency(set_of_topic, set_of_due_dates, offset)
   set_of_topic.each { |topic_id|
     #if the due dates have already been created and the save dependency is being clicked,
     #then delete existing n create again
     prev_saved_due_dates = TopicDeadline.where(topic_id: topic_id)
     #Only if there is a dependency for the topic
     if !prev_saved_due_dates.nil?
       num_due_dates = prev_saved_due_dates.length
       #for each due date in the current topic he want to compare it to the previous due date
       for x in 0..num_due_dates - 1
         #we don't want the old date to move earlier in time so we save it as the new due date and destroy the old one
         if DateTime.parse(set_of_due_dates[x].due_at.to_s) + offset.to_i < DateTime.parse(prev_saved_due_dates[x].due_at.to_s)
           set_of_due_dates[x] = prev_saved_due_dates[x]
           offset = 0
         end
         prev_saved_due_dates[x].destroy
       end
     end
     set_of_due_dates.each_with_index {|due_date, index|
       create_topic_deadline(due_date, offset, topic_id)
     }
   }
 end

Then call check_dependency method in the original set_start_due_date method.

Before Change

After Change

  def self.set_start_due_date(assignment_id,set_of_topics)

    #Remember, in create_common_start_time_topics function we reversed the graph so reverse it back
    set_of_topics = set_of_topics.reverse

    set_of_topics_due_dates = Array.new
    i=0
    days_between_submissions = Assignment.find(assignment_id)['days_between_submissions'].to_i
    set_of_topics.each { |set_of_topic|
      set_of_due_dates = nil
      if i==0
        #take the first set from the table which user stores
        set_of_due_dates = DueDate.where(assignment_id: assignment_id)
        offset = 0
      else
        set_of_due_dates = TopicDeadline.where(topic_id: set_of_topics[i-1][0])

        set_of_due_dates.sort_by {|a| a.due_at }

        offset = days_between_submissions
      end

      set_of_topic.each { |topic_id|
        #if the due dates have already been created and the save dependency is being clicked,
        #then delete existing n create again
        prev_saved_due_dates = TopicDeadline.where(topic_id: topic_id)

        #Only if there is a dependency for the topic
        if !prev_saved_due_dates.nil?
          num_due_dates = prev_saved_due_dates.length
          #for each due date in the current topic he want to compare it to the previous due date
          for x in 0..num_due_dates - 1
            #we don't want the old date to move earlier in time so we save it as the new due date 
            and destroy the old one
            if DateTime.parse(set_of_due_dates[x].due_at.to_s) + offset.to_i < 
            DateTime.parse(prev_saved_due_dates[x].due_at.to_s)
              set_of_due_dates[x] = prev_saved_due_dates[x]
              offset = 0
            end
            prev_saved_due_dates[x].destroy
          end
        end

        set_of_due_dates.each_with_index {|due_date, index|
          create_topic_deadline(due_date, offset, topic_id)
        }
      }
      i = i+1
    }

  end

  def self.set_start_due_date(assignment_id,set_of_topics)

    #Remember, in create_common_start_time_topics function we reversed the graph so reverse it back
    set_of_topics = set_of_topics.reverse

    set_of_topics_due_dates = Array.new
    i=0
    days_between_submissions = Assignment.find(assignment_id)['days_between_submissions'].to_i
    set_of_topics.each { |set_of_topic|
      set_of_due_dates = nil
      if i==0
        #take the first set from the table which user stores
        set_of_due_dates = DueDate.where(assignment_id: assignment_id)
        offset = 0
      else
        set_of_due_dates = TopicDeadline.where(topic_id: set_of_topics[i-1][0])

        set_of_due_dates.sort_by {|a| a.due_at }

        offset = days_between_submissions
      end

      check_dependency(set_of_topic, set_of_due_dates, offset)

      i = i+1
    }

  end

  def self.check_dependency(set_of_topic, set_of_due_dates, offset)
    set_of_topic.each { |topic_id|
      #if the due dates have already been created and the save dependency is being clicked,
      #then delete existing n create again
      prev_saved_due_dates = TopicDeadline.where(topic_id: topic_id)

      #Only if there is a dependency for the topic
      if !prev_saved_due_dates.nil?
        num_due_dates = prev_saved_due_dates.length
        #for each due date in the current topic he want to compare it to the previous due date
        for x in 0..num_due_dates - 1
          #we don't want the old date to move earlier in time so we save it as the new due date 
          and destroy the old one
          if DateTime.parse(set_of_due_dates[x].due_at.to_s) + offset.to_i < 
          DateTime.parse(prev_saved_due_dates[x].due_at.to_s)
            set_of_due_dates[x] = prev_saved_due_dates[x]
            offset = 0
          end
          prev_saved_due_dates[x].destroy
        end
      end

      set_of_due_dates.each_with_index {|due_date, index|
        create_topic_deadline(due_date, offset, topic_id)
      }
    }
  end

Results Screenshot

Steps to verify deadline type default

Log into the application with the user having an instructor's role.

1. Click on Assignments.

2. Click on Edit.

3. Click on Due dates.

You will see the default of each deadline type in each row is "Yes", while the others are "No".

Steps to verify save dependency

Log into the application with the user having an instructor's role.

1. Go to Page: http://152.1.13.227:3000/sign_up_sheet/add_signup_topics_staggered/723 (723 can be changed to the id of any assignments).

2. Click on Save dependencies.

Successful loading of this page confirms the save dependency method.

References

CSC/ECE 517 Fall 2015/oss E1561 WZL

Contents

Introduction to Expertiza

Project Desicription<ref>https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus/edit</ref>

Modification

Case 1: Remove `DueDate.assign_topic_deadline` and use `DeadlineHelper.create_topic_deadline`

Case 2: Create a new method for sort in `due_date.rb` and use and invoke it from where use it

Case 3: Rename `setFlag()` to adhere to Rails naming conventions

Case 4: Use the class variable in `default_permission` method and use conditionally check deadline type using `if...else` statements

Case 5: Refactor `DeadlineHelper.set_start_due_date` method to smaller methods

Results Screenshot

Steps to verify deadline type default

Steps to verify save dependency

References

Navigation menu

CSC/ECE 517 Fall 2015/oss E1561 WZL

Introduction to Expertiza

Project Desicription<ref>https://docs.google.com/document/d/1uWs3zyrupTmrOFuv5IbVWCF4NRvCXqJmg8dZ0wCqgus/edit</ref>

Modification

Case 1: Remove DueDate.assign_topic_deadline and use DeadlineHelper.create_topic_deadline

Case 2: Create a new method for sort in due_date.rb and use and invoke it from where use it

Case 3: Rename setFlag() to adhere to Rails naming conventions

Case 4: Use the class variable in default_permission method and use conditionally check deadline type using if...else statements

Case 5: Refactor DeadlineHelper.set_start_due_date method to smaller methods

Results Screenshot

Steps to verify deadline type default

Steps to verify save dependency

References

Navigation menu

Search

Case 1: Remove `DueDate.assign_topic_deadline` and use `DeadlineHelper.create_topic_deadline`

Case 2: Create a new method for sort in `due_date.rb` and use and invoke it from where use it

Case 3: Rename `setFlag()` to adhere to Rails naming conventions

Case 4: Use the class variable in `default_permission` method and use conditionally check deadline type using `if...else` statements

Case 5: Refactor `DeadlineHelper.set_start_due_date` method to smaller methods