CSC/ECE 517 Spring 2023 - E2302: Difference between revisions
Jump to navigation
Jump to search
| Line 46: | Line 46: | ||
|Some functions had comments missing. | |Some functions had comments missing. | ||
|[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> === | |||
{| class="wikitable" style="width: 100%; | |||
! # !! 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 | |||
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 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. | |||
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit] | |||
|- | |||
|3 | |||
|Moved the line to find the participants of a course inside the conditional block | |||
|Better readability and shorter function code. | |||
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 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. | |||
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit] | |||
|- | |||
|5 | |||
|Added test case for method perform in mail_worker.rb | |||
|The test case was not present earlier | |||
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit] | |||
|- | |||
|6 | |||
|Added test case for method find_participant_emails in mail_worker.rb | |||
|The test case was not present earlier | |||
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit] | |||
|- | |||
|7 | |||
|Added test case for method email_reminder in mail_worker.rb | |||
|The test case was not present earlier | |||
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit] | |||
|} | |} | ||
Revision as of 00:59, 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 | We extracted the create 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 | 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.) we 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 | Moved the line to find the participants of a course inside the conditional block | 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 |
| 5 | Added test case for method perform in mail_worker.rb | The test case was not present earlier | Commit |
| 6 | Added test case for method find_participant_emails in mail_worker.rb | The test case was not present earlier | Commit |
| 7 | Added test case for method email_reminder in mail_worker.rb | The test case was not present earlier | Commit |