E1632: Refactor delayed job.rb and scheduled task.rb: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 42: Line 42:
One of them looks like:
One of them looks like:


  class review < DeadlineType
  class Review < DeadlineType
   def email_list(assignment_id)
   def email_list(assignment_id)
     emails = []
     emails = []
Line 55: Line 55:
   end
   end
  end
  end


== Changes to the mail_signed_up_user ==
== Changes to the mail_signed_up_user ==

Revision as of 03:03, 1 November 2016

Expertiza

Expertiza is an open source type of project that has been undertaken by the National Science Foundation. Various articles, codes in multiple languages and links can be submitted using the platform. It is popularly used by students studying at NC State to submit assignments, review works and get feedback. It allows users to switch between multiple environments if an user is not satisfied with a particular environment.

Installation and Contribution

The setup development can be done using OSX, Linux or Docker. While using the OSX method and Linux method (RHEL), shell access and root access is a prerequisite. Docker is another method to install Wikipedia. It is cross Operating System (OS) compatible. Docker pull is needed to pull the docker image. Docker run is then used on port number 3000. Expertiza allows user to contribute to their project. The expertiza project needs to be forked and cloned to create a local directory. Changes can be pushed and committed to a new branch. Finally the code is reviewed and changes are possibly made after a pull request.

Problem Statement

The project is related to the class scheduled_class. The associated MVC framework relate to sending out reminder emails around the deadline of a task. The project objective includes-

  • Removing duplicate code that is common in scheduled_task.rb and delayed_job.rb.
  • Subclass Deadline_Type in the perform method.
  • Refactoring mail_signed_up_users to multiple named methods and modifying it in a more elegant manner.
  • Checks to the new refactored code.

Current Implementation A task queue is implemented in expertiza for all the emails that need to be sent to the users. Delayed_job is used to send this task when the task is out of the queue. Scheduled_task provides information to the admin about the queue present at any given time. Scheduled_task uses assignment_id, deadline_type and due at to provide information for the admin. Current implementations include-

  • Using the perform method linked to other methods mailing can be done for metareviewers, emails can be fetched and reminders can be sent to team members and mails can be sent to all signed up users.
  • Implementation can be used for various types of deadlines. Deadline for team formation, sign-up and dropping outstanding reviews are also implemented.

Drawbacks and Solutions

Problem 1: Perform is too big a method and is implemented in a complicated manner.

Solution: Deadline_type is made a superclass with metareview, review, submission, drop_topic, sign_up, team_formation,drop_one_member_topic, drop_outstanding_reviews as the subclasses. The subclasses have a method email_list which contains the functionality. Thus, inheritance replaces the case structure.

Problem 2: mail_signed_up_users is a long method and contains duplicate code in its branches.

Solution: The mail_signed_up users can be split to two different methods- mail_sign_up_topics_user and and mail_non_sign_up_topics_user.The functionality of the first one is also used by the drop_topic subclass and the submission subclass uses both- the mail_sign_up_topics_user and mail_non_sign_up_topics_user.


Code Modification

Several new models have been added. All the deadline_type subclasses have been added as new models.

One of them looks like:

class Review < DeadlineType
 def email_list(assignment_id)
   emails = []
   reviewer_tuples = ResponseMap.where(['reviewed_object_id = ? AND type = "ReviewResponseMap"', assignment_id])
   for reviewer in reviewer_tuples
     participant = Participant.where(['parent_id = ? AND id = ?', assignment_id, reviewer.reviewer_id])
     uid  = participant.first.user_id
     user = User.find(uid)
     emails << user.email
   end
   emails
 end
end

Changes to the mail_signed_up_user

This includes the splitting of the method to multiple methods to make it more elegant

Earlier Code:


 def mail_signed_up_users
   emails = []
   assignment = Assignment.find(self.assignment_id)
   sign_up_topics = SignUpTopic.where(['assignment_id = ?', self.assignment_id])
   # If there are sign_up topics for an assignement then send a mail toonly signed_up_teams else send a mail to all participants
   if (sign_up_topics.nil? || sign_up_topics.count == 0)
     if assignment.team_assignment?
       teamMails = getTeamMembersMail
       for mail in teamMails
         emails << mail
         email_reminder(emails, self.deadline_type)
       end
     else
       mail_assignment_participants
     end
   else
     for topic in sign_up_topics
       signedUpTeams = SignedUpTeam.where(['topic_id = ?', topic.id])
       unless assignment.team_assignment?
         for signedUser in signedUpTeams
           uid  = signedUser.team_id
           user = User.find(uid)
           emails << user.email
         end
       else
         for signedUser in signedUpTeams
           teamid = signedUser.team_id
           team_members = TeamsUser.where(team_id: teamid)
           for team_member in team_members
             user = User.find(team_member.user_id)
             emails << user.email
           end
         end
       end
     end
     email_reminder(emails, self.deadline_type) if emails.size > 0
   end
 end


Modified Code:

def mail_signed_up_users
  sign_up_topics = SignUpTopic.where(['assignment_id = ?', self.assignment_id])
  # If there are sign_up topics for an assignement then send a mail toonly signed_up_teams else send a mail to all participants
  if (sign_up_topics.nil? || sign_up_topics.count == 0)
   emails = mail_non_sign_up_topic_users
  else
   emails = mail_sign_up_topic_users
  end
 emails
end 

The perform method has been modified

The perform method has been modified to a large extent as it now uses Single Table Inheritance (STI)[1] instead of if-else cases statements.

Earlier Code:

def perform
   assignment = Assignment.find(self.assignment_id)
   if !assignment.nil? && !assignment.id.nil?
     if (self.deadline_type == "metareview")
       mail_metareviewers
       if assignment.team_assignment?
         teamMails = getTeamMembersMail
         email_reminder(teamMails, "teammate review")
       end
     end
     if (self.deadline_type == "review")
       mail_reviewers # to all reviewers
     end
     if (self.deadline_type == "submission")
       mail_signed_up_users # to all signed up users
     end
     if (self.deadline_type == "drop_topic")
       sign_up_topics = SignUpTopic.where(['assignment_id = ?', self.assignment_id])
       if (!sign_up_topics.nil? && sign_up_topics.count != 0)
         mail_signed_up_users # reminder to signed_up users of the assignment
       end
     end
     if (self.deadline_type == "signup")
       sign_up_topics = SignUpTopic.where(['assignment_id = ?', self.assignment_id])
       if (!sign_up_topics.nil? && sign_up_topics.count != 0)
         mail_assignment_participants # reminder to all participants
       end
     end
     if (self.deadline_type == "team_formation")
       assignment = Assignment.find(self.assignment_id)
       if (assignment.team_assignment?)
         emails = get_one_member_team
         email_reminder(emails, self.deadline_type)
       end
     end
     if (self.deadline_type == "drop_one_member_topics")
       assignment = Assignment.find(self.assignment_id)
       drop_one_member_topics if (assignment.team_assignment?)
     end
     if (self.deadline_type == "drop_outstanding_reviews")
       drop_outstanding_reviews
     end
   end
 end

Modified Code:

def perform
   assignment = Assignment.find(self.assignment_id)
   emails = []
   if !assignment.nil? && !assignment.id.nil?
     deadlineObj = self.deadline_type.new
     emails = deadlineObj.email_list
     email_reminder(emails, self.deadline_type) if emails.size > 0
   end
end

Testing

The scheduled_task_spec.rb is the test class for the above changes.