CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb
Topic Overview
Background
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 E2144. This work was done in Fall of 2021, and 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.
This prior implementation was rejected due to lack of method comments and a failing test, and so correcting the test and adding method comments--and explaining how those corrections were done--will be the major focus of this project documentation.
The work in E2253 was rejected because it included code changes in files that were deemed by Dr. Gehringer to be unnecessary and unassociated with the core functionality of this feature. Additionally, some of the comments are shallow and not insightful enough according to Dr. Gehringer.
History: Previous projects
* E2253 [1] * E2144 [2]
Problem Statement
Files involved (some changed should not have been changed):
The code for the prior PR is linked here is linked here.
Explanation of Feature
Mailer/mailing is referring to the sending of emails to email address associated with expertiza account. There are two types of mailing: synchronous and asynchronous. Synchronous mailing occurs after an action is taken e.g., an email that notifies when a change is made to a document. Asynchronous mailing occurs during a set time e.g., a notification email to warn students when a deadline is approaching.
Summary of Work Needed
After discussing with our project mentor and Dr. Gehringer, we discovered that what needed to be done is refining the reason why the previous PR was not deemed production-ready and resolve those issues in order to merge to main:
In terms of functionality, Dr. Gehringer mentioned that asynchronous mailing is having issues e.g., not sending out an assignment deadline notification a day before it is due. If there is time, the team may debug and work to fix this. If the team is unable to resolve, this may be passed on to future teams. Dr. Gehringer did not specify if it is within scope of this project.
Problems and planned changes
We are planning to accomplish the following tasks in this project:
Problem 1: Add insightful comments to the methods in mailer.rb
* Proposed Solution: Add insightful comments to each method. * Solution Implemented: Added meaningful comments to every major line in the mailer.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
2. Test to check if the size of the queue is increased when Mailworker is used.
it "should increase the size of queue by 1 when MailWorker is used" do
Sidekiq::Testing.inline!
Mailer.deliveries.clear
worker = MailWorker.new
worker.perform(1, 'review', '2022-12-05 00:00:01')
expect(Mailer.deliveries.size).to eq(1)
end
Test Plan
Automated Testing using RSPEC
To run the tests, run the following commands from the expertiza directory
rspec ./spec/workers/sidekiq_mail_worker_spec.rb
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:
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
Testing from UI
No changes were made to the functionality of the project, however you can access it through the setup below.
- Visit the VCL hosted site: http://152.7.99.69:8080/
- Login: Instructor6
- Password: Password
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, we needed to run Sidekiq (steps were obtained from this site):
- 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.
NB: To create a generic sidekiq worker to test with, you could use
rails g sidekiq:worker TestWorker
to create a shell worker class.
Sidekiq UI
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the /sidekiq route of the application.
SimicheckWorker Test
To enqueue a Simicheck job:
- Edit an assignment.
- Select the
Use simicheck?box. - Set
Simicheck Delayto0(or to a nonzero value to observe a delay). - Set the SimiCheck Similarity Threshold to a nonzero value.
- Ensure the Late Policy is set to
--None--under the Due Dates tab. If there is a late policy set, per pre-existing code, the enqueued SimicheckWorker job will be deleted and will not appear in the Sidekiq UI (unless it is dequeued quickly, before the code can delete the jobs in the queue). - Click
Save. - Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.
MailWorker and DropOustandingReviews Test
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
- Log into Expertiza as an instructor.
- Navigate to Manage>Assignments in the navigation.
- Click the plus button in the top right corner on the Manage Assignments and create an assignment with default settings.
- Go to the Due Dates tab and assign due dates for both the assignment and the review.
- Select a late policy next to Apply penalty policy:. If a late policy has not been created, press New Late Policy and follow the steps for creating a late policy.
- Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the Manage Assignments page.
- Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. The assignment due date must be changed to a time at least an hour ahead of the current time for the job to be queued.
- To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq.
- Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to Scheduled. After arriving on this page, you should see something like the following:
The job MailWorker that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job DropOutstandingReviewsWorker showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.
Functional Tests
Functional testing is done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the When field, Job field, and Arguments field in the Sidekiq view all came out as expected. A previously implemented test for Simicheck was commented out as advised by Dr. Gehringer, because it has a helper function dependency called PlagiarismCheckerHelper. According to Dr. Gehringer, in PlagiarismCheckerHelper, the request "variable is supposed to contain an http request for running the plagiarism checker. The checker has not been used for several years. It will not hurt anything important if it does not work now."