View source for CSC/ECE 517 Spring 2023 - E2302

==E2302. Refactoring the Delayed Mailer and Scheduler==

__TOC__

== Expertiza Overview ==

===Background===

Expertiza is an open source software written using Ruby on Rails which functions as a learning management software system. It has man different functions and abilities including the ability to create assignments, quizzes, assignment groups and topics, and also a complete mechanism for providing peer reviews and feedback for other groups and other teammates. 


===Prior Work===

=== History: Previous projects ===

 * E2253 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2253.Refactor_delayed_mailer.rb_and_scheduled_task.rb]
 * E2144 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2021_-_E2144._Refactor_delayed_mailer_and_scheduled_task]
 * E2273 [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2273.Refactor_delayed_mailer.rb_and_scheduled_task.rb]

The latest Pull Request was not merged into the repository due to some failing tests and missing comments. <br>
Tests were failing in the following files-
* <code>spec/models/assignment_form_spec.rb</code>
* <code>spec/controllers/assignments_controller_spec.rb</code><br>
We try to address those failing tests and add block comments before function definition wherever needed. <br>
Additional testing of some related files is also in the scope of the project.

=== 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 <code>spec/controllers/auth_controller_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
! &nbsp;#&nbsp; !! Change !! Rationale !! Commit Link
|-
|1
|Refactored the <code>create</code> method into multiple methods
| 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]
|-
|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.
|[https://github.com/tusharkini/expertiza/commit/3c00648b7228eb0daf8a9afd30f8f3abfb44c2c4 Commit]
|-
|3
|Renamed variable names
|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]
|-
|4
|Added comments to other helper methods
|Some functions had no comments which were needed.
|[https://github.com/tusharkini/expertiza/commit/b72febb7f2370d6ccb0c99939500f6d36178bf00 Commit]
|}

==== Changes to <code>app/mailers/mail_worker.rb</code> ====
{| class="wikitable" style="width: 100%;
! &nbsp;#&nbsp; !! 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 <code>deadline_type</code> was  <code>'drop_outstanding_reviews’</code>. We do find the list of emails when it is not <code>'drop_outstanding_reviews’</code> and send emails to them.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|-
|3
|Combined the multiple <code>attr_accessor</code>lines into a single line
|Better readability and shorter function code.
|[https://github.com/tusharkini/expertiza/commit/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 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/b56ecca4db6e1784a3f92b4fa9619992c1d4dcb2 Commit]
|}

==== Changes to <code>spec/models/assignment_form_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
! &nbsp;#&nbsp; !! Change !! Rationale !! Commit Link
|-
|1
| Replaced ‘mailers’ with ‘jobs’ in <code>add_to_delayed_queue</code> test
| Test was failing due to checking the wrong Sidekiq queue, currently <code>add_to_delayed_queue</code> adds to job queue NOT mailers queue
|[https://github.com/tusharkini/expertiza/commit/95dcdaaa36b837df3d580f3352b8504535cc7c2e Commit]
|}

==== Changes to <code>app/models/assignment_form.rb</code> ====
{| class="wikitable" style="width: 100%;
! &nbsp;#&nbsp; !! Change !! Rationale !! Commit Link
|-
|1
|Added block comments to methods and removed inline comments
|Better readability and looks clean
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|-
|2
|Created <code>default_assignment_questionaire</code> to handle creation of a default <code>assignment_questionnaire</code> that was originally in assignment questionnaire
|<code>assignment_questionnaire</code> originally not only searched for an <code>assignment_questionnaire</code> but created one if one matching type did not exist. Now the second functionality (creating a default <code>assignment_questionnaire</code>) is its own method.
|[https://github.com/expertiza/expertiza/pull/2556/commits/b784e829920059cbff412ad7768833e95469ff2b Commit]
|-
|3
|In line 58 of original code replaced <code>[][]</code> method of searching with <code>.dig</code>
|This particular line could throw errors and .dig is a safer way to search a table without the risk of getting nil errors
|[https://github.com/expertiza/expertiza/pull/2556/commits/975312c88bb65861b1b6f2b268dc8d80c15aa501 Commit]
|}


==== Changes to <code>spec/sidekiq_mail_worker_spec.rb</code> ====
{| class="wikitable" style="width: 100%;
! &nbsp;#&nbsp; !! Change !! Rationale !! Commit Link
|-
|1
|Added test case for method <code>perform</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/8d78d40ea4e4035f95522e025fd08e220b8b9ce4 Commit]
|-
|2
|Added test case for method <code>find_participant_emails</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/190ca781681021f995a8a034a34bb8cede1733e3 Commit]
|-
|3
|Added test case for method <code>email_reminder</code> in <code>mail_worker.rb</code>
|This test case was not implemented in the earlier Pull Request
|[https://github.com/tusharkini/expertiza/commit/53b8cf281215aeeec857886deaa2fae7998eff47 Commit]

|}

=== Test Coverage for some related files===
====  <code>spec/controllers/auth_controller_spec.rb</code> ====
After making the changes to assignment_form and assignment_form_spec we ran the rspec tests with the command bundle exec rspec spec/models/assignment_form_spec.rb
[[File:Assignment_form_test.jpg|1000px]]

====  <code>spec/models/assignment_form_spec.rb</code> ====
Automated testing of sidekiq_mailer_worker_spec.rb
[[File:mailer_spec.jpg|1000px]]

==== <code>spec/sidekiq_mail_worker_spec.rb</code>  ====
Automated testing of assignments_controller_spec.rb
[[File:Assignment_controller_test.jpg|1000px]]

== Testing ==
VCL : http://152.7.178.223:8080/

Login: instructor_Lien

Password: password

To Test via UI

1) Login to VCL

2) Click on Manage

3) Click on Assignments

4) click on +

5) Create an assignment with a non-zero delay in SimiCheck and Similarity Threshold to a non-zero. 

6) Go to the due date and add a due date for tomorrow. 

7) Create the assignment

8) Go to the Assignment page and add participants (ex. Paul & Dan) to the assignment.

9) Check the sidekiq [http://152.7.178.223:8080/sidekiq] page for queued jobs.