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

From Expertiza_Wiki
Jump to navigation Jump to search

Topic Overview

Statement of Problem

Expertiza uses 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.

Prior Work

The prior work which is the basis for this topic is linked here is linked here. This work was done in Fall of 2020, and as previously mentioned, achieved much of the desired functionality for this task. This prior work will be referenced several times within this documentation, as it has a more comprehensive explanation of the underlying design principles behind the views, controller, model, algorithm, and RSpec tests implemented. These details will only be briefly summarized here.

As previously noted, this prior implementation was rejected due to DRY violations, and so correcting these violations--and explaining how those corrections were done--will be the major focus of this project documentation.

History: Previous projects

* E2253 [1]
* E2144 [2]

Problem Statement

  • This project used the Sidekiq gem to asynchronously handle email tasks, which is sort of similar to E2135. Sidekiq was partially merged before, but this project changes the queue setup slightly. The refactor of methods in assignment_form.rb is significant, and several methods were moved to due_date.rb. It also adds reminder functionality to mail_worker, which also conflicts with E2135. Significant spec additions have been made from what exists in beta. Overall, code looks appropriate and well-written. Sidekiq needs to be started in the background while Expertiza is running, but since Sidekiq was already partially merged, I think this should be expected. After viewing the peer reviews and newest Travis run, this appears almost ready to merge. Peer reviewers believe it is a quality project, but note that several methods lack method comments. The travis build has a single small error in models (assignment_form_spec.rb). The code for the prior PR is here[3]

    Explanation of Feature

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

    Existing Mailer

    Currently in expertiza, the mailer functionality... An image showcasing this functionality is given below:

    ...Students may choose a number of topics to bid on, in order of priority, and once the deadline is reached, all students are given a topic...

    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

    Problems and planned changes

    We are planning to accomplish the following tasks in this project:

    Problem 1: Add insightful comments to the methods in mail_worker.rb

    * Proposed Solution: Add insightful comments to each method.
    * Solution Implemented: Added meaningful comments to every major line in the mail worker.rb file to reflect the functionality and purpose of each method.
    

    Problem 2: Fix Bugs in sidekiq_mail_worker_spec.rb

    * Proposed Solution: Fix the failing existing tests.
    * Solution Implemented: Changed the email from expertiza.development@gmail.com to expertiza.debugging@gmail.com. Generated an email with the appropriate body, instead of a nil object. 
    
    it "should have sent welcome email after user was created" do
          Sidekiq::Testing.inline!
          email = Mailer.sync_message(
            to: 'tluo@ncsu.edu',
            subject: 'Your Expertiza account and password has been created',
            body: {
              obj_name: 'assignment',
              type: 'submission',
              location: '1',
              first_name: 'User',
              partial_name: 'update'
            }
          ).deliver_now
          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")
          expect(email.bcc[0]).to eq("psingh22@ncsu.edu")
          expect(email.subject).to eq("Message regarding teammate review for assignment no assignment")
          expect(email.body).to eq("This is a reminder to complete teammate review for assignment no assignment.\nPlease follow the link: http://expertiza.ncsu.edu/student_task/view?id=1\nDeadline is 2018-12-31 00:00:01. If you have already done the teammate review, then please ignore this mail.")
    end
    

    Problem 3: Increase the test coverage for sidekiq_mail_worker_spec.rb

    * Proposed Solution: Add more tests to increase test coverage.
    * Solution Implemented: Added the following tests:
    

    1. Test to check an email is not sent when there is deadline with outstanding reviews.

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

    Test Plan

    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.

    1. Test to check an email is not sent when there is deadline with outstanding reviews.

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

    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

    Conclusions and Future Work

    While the current state of the review bidding code is operational, and is certainly more DRY and stable than the previous incarnation of this work, there are still several issues with the current state of this feature that make it difficult to recommend an immediate merge into expertiza.

    References

    1. Expertiza on GitHub
    2. GitHub Project Repository Fork
    3. E2273 Pull Request
    4. The live Expertiza website