CSC/ECE 517 Spring 2023 - E2302: Difference between revisions

From Expertiza_Wiki
Jump to navigation Jump to search
Line 28: Line 28:
|-
|-
|1
|1
|We extracted the create method into multiple methods
|Refactored the <code>create</code> method into multiple methods
| the current create method is long and have multiple complex references and if statements. We created separate helpers to simplify complex db context  
| The current <code>create</code> method is long and have multiple complex references and if statements. Created separate helpers to simplify complex DB context  
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|-
Line 39: Line 39:
|3
|3
|Renamed variable names
|Renamed variable names
|Some of the current variable names were ambiguous as to what they meant, (cur_questionnaire, cur_due, etc.) we renamed those names to be less ambiguous and state their function.
|Some of the current variable names were ambiguous as to what they meant, <code>(cur_questionnaire, cur_due, etc.)</code>. Renamed those names to be less ambiguous and state their function.
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|[https://github.com/tusharkini/expertiza/commit/4c2f0e04e7e3ab10443af26d2868758b5c696b55 Commit]
|-
|-
Line 47: Line 47:
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}
|}


=== Changes to <code>app/mailers/mail_worker.rb</code> ===
=== Changes to <code>app/mailers/mail_worker.rb</code> ===

Revision as of 01:13, 23 March 2023

E2302. Refactoring the Delayed Mailer and Scheduler

Topic Overview

Background


Prior Work

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 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
Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Spring_2023_-_E2302&oldid=148096"