E1632: Refactor delayed job.rb and scheduled task.rb: Difference between revisions
Line 181: | Line 181: | ||
== '''Testing''' == | == '''Testing''' == | ||
The scheduled_task_spec.rb is the test class for the above changes. | The scheduled_task_spec.rb is the test class for the above changes.All the test cases pass for the test case.[https://travis-ci.org/expertiza/expertiza/builds/172184541] |
Revision as of 03:05, 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? deadlineTypeClass = self.deadline_type.split("_").map { |n| n.capitalize}.join("").to_s deadlineObj = Object.const_get(deadlineTypeClass).new emails = deadlineObj.email_list(self.assignment_id) 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.All the test cases pass for the test case.[2]