CSC/ECE 517 Fall 2022 - E2253.Refactor delayed mailer.rb and scheduled task.rb

From Expertiza_Wiki
Jump to navigation Jump to search

E2253. Refactoring the Delayed Mailer and Scheduler

This page provides a description of the Expertiza based OSS project.



About Expertiza

Expertiza is an open source project based on Ruby on Rails framework. Expertiza is an all inclusive web service that allows instructors to create, edit and manage assignments. This includes the functionality for topic selection and team formation across various projects and assignments. Expertiza allows for a variety of submission types including URL and PDF. Expertiza also allows the instructor to assign peer reviews and team assessments.

Problem Statement

The following tasks were accomplished in this project:

  • Improved the clarity of code by improving variable and method names.
  • Improved the clarity of code by adding comments.
  • Ensured one method didn't perform more than one task.
  • Added RSPEC testcases for testing Assignments Controller and Mail Worker

About Assignment Controller

This class is responsible for the creation and implementation of assignment forms. This includes due dates, questionnaires and assigning reviews.

Previous Information can be found here E2144

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


Current Implementation

Functionality
  • Instructor should be able to create, edit and assign assignments to all account in their course.
  • TA's should be able to create their own assignments and only edit assignments they've created.
  • The Sidekiq Gem is used to automate emails to students about upcoming assignments and when assignments have been altered or delayed.
Drawbacks and Solutions
  • Problem 1:
The method delete_from_delayed_queue performs two tasks
def delete_from_delayed_queue
    queue = Sidekiq::Queue.new('mailers')
    queue.each do |job|
      assignmentId = job.args.first
      job.delete if @assignment.id == assignmentId
    end

    queue = Sidekiq::ScheduledSet.new
    queue.each do |job|
      assignmentId = job.args.first
      job.delete if @assignment.id == assignmentId
    end
  end
  • Solution: The implementation has been changed to include two function that will instead be called back to back:
    • delete_from_delayed_queue
    • delete_from_scheduled_set
  • Problem 2: Duplicate code found within the create and update_assignment_form methods within assignments_controller.rb
  def create
    ...
    ques_array = assignment_form_params[:assignment_questionnaire]
    due_array = assignment_form_params[:due_date]
    ques_array.each do |cur_questionnaire|
      cur_questionnaire[:assignment_id] = exist_assignment.id.to_s
    end
    due_array.each do |cur_due|
      cur_due[:parent_id] = exist_assignment.id.to_s
    end
    assignment_form_params[:assignment_questionnaire] = ques_array
    assignment_form_params[:due_date] = due_array
    ...
  end

  def update_assignment_form(exist_assignment)
    questionnaire_array = assignment_form_params[:assignment_questionnaire]
    questionnaire_array.each { |cur_questionnaire| cur_questionnaire[:assignment_id] = exist_assignment.id.to_s }
    assignment_form_params[:assignment_questionnaire]
    due_array = assignment_form_params[:due_date]
    due_array.each { |cur_due| cur_due[:parent_id] = exist_assignment.id.to_s }
    assignment_form_params[:due_date]
    @assignment_form.update(assignment_form_params, current_user)
  end
  • Solution: Create two methods to create the arrays for due dates and questionnaires.
  • Problem 3:
Tests failing within sidekiq_mail_worker_spec.rb
describe 'Tests mailer with sidekiq' do
    it "should have sent welcome email after user was created" do
      email = ActionMailer::Base.deliveries.first
      expect(email.from[0]).to eq("expertiza.development@gmail.com")
      expect(email.to[0]).to eq("expertiza.development@gmail.com")
      expect(email.subject).to eq("Your Expertiza account and password has been created")
    end

    it 'should send reminder email to required email address with proper content' do
      Sidekiq::Testing.inline!
      ActionMailer::Base.deliveries.clear
      worker = MailWorker.new
      worker.perform("1", "metareview", "2018-12-31 00:00:01")
      expect(ActionMailer::Base.deliveries.size).to eq(1)
      email = ActionMailer::Base.deliveries.first
      expect(email.from[0]).to eq("expertiza.development@gmail.com")
    ...
  • Solution: Fix the naming convention of the calls

New Implementation

  • The method delete_from_delayed_queue has been split into 2 methods now.
    • delete_from_delayed_queue- This method will remove the delayed_job from the delayed_jobs queue
    • delete_from_scheduled_set- This method will remove the delayed_job from the scheduled set
  # Deletes the job with id equal to "delayed_job_id" from the delayed_jobs queue
  def delete_from_delayed_queue
    queue = Sidekiq::Queue.new('mailers')
    queue.each do |job|
      assignmentId = job.args.first
      job.delete if @assignment.id == assignmentId
    end
  end

  # Deletes the job with id equal to "delayed_job_id" from the scheduled set
  def delete_from_scheduled_set
    queue = Sidekiq::ScheduledSet.new
    queue.each do |job|
      assignmentId = job.args.first
      job.delete if @assignment.id == assignmentId
    end
  end
  • Created the methods assign_questionnaire_array and assign_due_date_array to handle the creation of the arrays
    • assign_questionnaire_array- This method will create the array of questionnaires
    • delete_from_scheduled_set- This method will create the array of due dates assorted
  def create
    ...
    assignment_form_params[:assignment_questionnaire] = assign_questionnaire_array(exist_assignment)
    assignment_form_params[:due_date] = assign_due_date_array(exist_assignment)
    ...
  end

  def update_assignment_form(exist_assignment)
    assignment_form_params[:assignment_questionnaire] = assign_questionnaire_array(exist_assignment)
    assignment_form_params[:due_date] = assign_due_date_array(exist_assignment)
    @assignment_form.update(assignment_form_params, current_user)
  end

  # creates array of questionnaires
  def assign_questionnaire_array(exist_assignment)
    questionnaire_array = assignment_form_params[:assignment_questionnaire]
    questionnaire_array.each { |cur_questionnaire| cur_questionnaire[:assignment_id] = exist_assignment.id.to_s }
    questionnaire_array
  end

  # creates array of due dates
  def assign_due_date_array(exist_assignment)
    due_array = assignment_form_params[:due_date]
    due_array.each { |cur_due| cur_due[:parent_id] = exist_assignment.id.to_s }
    due_array
  end
  • Fixed bug within tests so that tests run successfully
  describe 'Tests mailer with sidekiq' do
    it "should have sent welcome email after user was created" do
      email = Mailer.deliveries.first
      expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
      expect(email.to[0]).to eq("expertiza.debugging@gmail.com")
      expect(email.subject).to eq("Your Expertiza account and password has been created")
    end

    it 'should send reminder email to required email address with proper content' do
      Sidekiq::Testing.inline!
      Mailer.deliveries.clear
      worker = MailWorker.new
      worker.perform("1", "metareview", "2018-12-31 00:00:01")
      expect(Mailer.deliveries.size).to eq(1)
      email = Mailer.deliveries.first
      expect(email.from[0]).to eq("expertiza.debugging@gmail.com")
    ...

Code improvements

  • Added comments to methods that lacked clarity in order to improve the readability
  • Changed method names to improve readibility
    • get_time_diff_btw_due_date_and_now is now get_time_diff_between_due_date_and_now within assignment_form.rb
  • Changed variable names to improve readability
    • diff_btw_time_left_and_threshold is now diff_between_time_left_and_threshold within assignment_form.rb
    • dd is now dueDate within assignment_form.rb within update
    • participant_mails is now participant_emails within mail_worker.rb
    • review_has_began is now review_has_begun within mail_worker.rb

Testing Plan

New Tests Added to RPEC

The tests below are anti-tests added for the purpose of ensuring emails aren't sent when they shouldn't be. 

    it "should not return email if deadline is compare_files_with_simicheck" do
      Sidekiq::Testing.inline!
      Mailer.delivieries.clear
      worker = MailWorker.new
      worker.perform("1", "compare_files_with_simicheck", "2018-12-31 00:00:01")
      expect(Mailer.delivieries.size).to eq(0)      
    end

    it "should not return email if deadline is drop_outstanding_reviews" do
      Sidekiq::Testing.inline!
      Mailer.delivieries.clear
      worker = MailWorker.new
      worker.perform("1", "drop_outstanding_reviews", "2018-12-31 00:00:01")
      expect(Mailer.delivieries.size).to eq(0)
    end

Automated Testing using RSPEC

The current version of expertiza included tests for the assignments_form and the mail_worker, but they were far from extensive. Thus we added a few tests of our own.

To run the tests, run the following commands from the expertiza directory 

rspec ./spec/models/assignment_form_spec.rb

rspec ./spec/workers/sidekiq_mail_worker_spec.rb

Testing from UI

No changes were made to the functionality of the project, however you can access it through the setup below.

To Test via UI

1) Login with the login above

2) Click on Manage...

3) Click on Assignments

4) On the right side click the +

5) Once you've added a name, scroll to the bottom and click create

6) Repeat steps 2 and 3, to view the assignment added to the list

References

  1. Expertiza on GitHub
  2. GitHub Project Repository Fork
  3. E2253 Pull Request
  4. The live Expertiza website
Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb&oldid=146173"