CSC/ECE 517 Spring 2023 - E2302

Topic Overview

Background

History: Previous projects

* E2253 [1]
* E2144 [2]
* E2273 [3]

Our Changes

Upon review of previous PRs, we found that the work was thorough, albeit a few test cases were failing, and comments and methods could be refactored better. Based on the prompts we got, we focussed on refactoring the create method of assignments controller, and assignments form, along with the refactoring of sidekiq-related files.

Below is the summary of all the changes as part of this PR.

Changes to spec/controllers/auth_controller_spec.rb

#	Change	Rationale	Commit Link
1	Refactored the create method into multiple methods	The current create method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context	Commit
2	Used early return in case of errors	We refactored the code such that the errors like the assignment already being present or the directory already being present were caught early in the code. It leads to cleaner code as well.	Commit
3	Renamed variable names	Some of the current variable names were ambiguous as to what they meant, (cur_questionnaire, cur_due, etc.). Renamed those names to be less ambiguous and state their function.	Commit
4	Added comments to other helper methods	Some functions had comments missing.	Commit

Changes to app/mailers/mail_worker.rb

#	Change	Rationale	Commit Link
1	Removed inline comments and added block comments to the function perform	Block comments before the function allows better readability and looks clean	Commit
2	Moved the line to find the participants of a course inside the conditional block	Earlier the list of participants of a course were found even when the deadline_type was 'drop_outstanding_reviews’. We do find the list of emails when it is not 'drop_outstanding_reviews’ and send emails to them.	Commit
3	Combined the multiple attr_accessorlines into a single line	Better readability and shorter function code.	Commit
4	@mail object is not used as it is not reused anywhere else	Any temporary variable that is used once should be avoided as it takes up valuable resources.	Commit

Changes to spec/models/assignment_form_spec.rb

#	Change	Rationale	Commit Link
1	Replaced ‘mailers’ with ‘jobs’ in add_to_delayed_queue test	Test was failing due to checking the wrong Sidekiq queue, currently add_to_delayed_queue adds to job queue NOT mailers queue	Commit

Changes to app/models/assignment_form.rb

#	Change	Rationale	Commit Link
1	Added block comments to methods and removed inline comments	Better readability and looks clean	Commit
2	Created default_assignment_questionaire to handle creation of a default assignment_questionnaire that was originally in assignment questionnaire	assignment_questionnaire originally not only searched for an assignment_questionnaire but created one if one matching type did not exist. Now the second functionality (creating a default assignment_questionnaire) is its own method.	Commit
3	In line 58 of original code replaced [][] method of searching with .dig	This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors	Commit

Changes to spec/sidekiq_mail_worker_spec.rb

#	Change	Rationale	Commit Link
1	Added test case for method perform in mail_worker.rb	This test case was not implemented in the earlier Pull Request	Commit
2	Added test case for method find_participant_emails in mail_worker.rb	This test case was not implemented in the earlier Pull Request	Commit
3	Added test case for method email_reminder in mail_worker.rb	This test case was not implemented in the earlier Pull Request	Commit