CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

Background

This project revolves around the Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq’s queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

Previous Setup

Initial work can be found here.

Code in the pull request was found to be reasonable and working. The method perform() has:

Metrics for perform method

Metric	Current Value	Limit
Cognitive Complexity	15	5
Cyclomatic Complexity	8	6
Assignment Branch Condition size	22.93	15
Perceived complexity	10	7

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):

• app/controllers/assignments_controller.rb
• app/mailers/mail_worker.rb
• app/models/assignment_form.rb
• config/sidekiq.yml
• spec/models/assignment_form_spec.rb
• spec/sidekiq_mail_worker_spec.rb
• spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

Problem Statement

This project revolves around the Sidekiq gem for asynchronous processing of email tasks. It has a queue system to hold and then process jobs. Sidekiq's queue replaces DelayedMailer’s queue. The previous team that worked on this also created a method perform() to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work. The project was partially merged. ​

What needs to be done: Feedback from the previous team (E1835) is: Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good. ​

Your pull request has many duplicated commit messages. Please try to squash similar commits. And using meaningful commit messages later. ​

You should commit changes to the DB schema (db/schema.rb) only if you have created new DB migrations. Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB. ​

You should not change rails_helper.rb or spec_helper.rb file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

Summary of Work Needed

After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:

1. Refactor the single Sidekiq worker into multiple workers
2. Remove all code related to the drop_one_member_topics deadline type
3. Update and add unit tests according to the above changes
4. Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
5. Code cleanup

Refactor the single Sidekiq worker into multiple workers

Previously there was only a single worker class that handled all types of jobs. Its top-level method perform looked like the following.

def perform(assignment_id, deadline_type, due_at)
    self.assignment_id = assignment_id
    self.deadline_type = deadline_type
    self.due_at = due_at

    assignment = Assignment.find(self.assignment_id)
    participant_mails = find_participant_emails

    if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
      drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
      drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
      perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
    else
      # Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
      deadlineText = if self.deadline_type == "metareview"
                       "teammate review"
                     else
                       self.deadline_type
      end

      email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
    end
  end

The processing was different depending on the deadline_type. The deadline_types in

%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]

did not lead to an email reminder being sent, but they were still handled in this MailWorker class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single MailWorker class into multiple workers:

• SimicheckWorker
• DropOutstandingReviewsWorker
• MailWorker

All three workers extended an "abstract class" called Worker that contained the Sidekiq-related include and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from mailers to jobs. A future improvement could divide these jobs to go to multiple different queues if needed.

Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected

Sidekiq setup

In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from this site):

1. Initialize three new workers, each having one of the three new worker types:
   1. rails g sidekiq:worker SimicheckWorker
   2. rails g sidekiq:worker DropOutstandingReviewsWorker
   3. rails g sidekiq:worker MailWorker
2. In a terminal separate from the one in which the main rails application is running, run

bundle exec sidekiq

This Sidekiq-specific terminal will show logs and printed statements from the workers.

Sidekiq UI

To see which messages were being enqueued and dequeued in Sidekig, we use the Sidekiq user interface, which can be accessed via the /sidekiq route of the application.