CSC/ECE 517 Fall 2022 - E2273.Refactor delayed mailer.rb and scheduled task.rb: Difference between revisions
| Line 41: | Line 41: | ||
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: | 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: | ||
<li> Add method comments | <li> Add method comments | ||
<li> Limit changes to delayed_mailer.rb and scheduled_task.rb | <li> Limit changes to delayed_mailer.rb and scheduled_task.rb (this means removing any changes to assignments_controller.rb and assignments_form.rb | ||
<li> Increase test coverage for above models | <li> Increase test coverage for above models | ||
<li> Fix failing test in assignment_form_spec.rb | <li> Fix failing test in assignment_form_spec.rb | ||
Revision as of 03:29, 21 November 2022
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
The delayed_mailer class is responsible for the ... . 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.
Existing Mailer
Currently in expertiza, the mailer functionality... An image showcasing this functionality is given below:
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.
- Visit the VCL hosted site: http://152.7.99.69:8080/
- Login: Instructor6
- Password: Password
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.