Expertiza_Wiki - User contributions [en]

File:E2170-assessments for.png

2021-11-08T23:13:04Z

Agcarrut:

CSC/ECE 517 Fall 2021 - E2170. Testing - Response Maps

2021-11-08T23:12:15Z

Agcarrut: /* self.assessments_for(team) */

== Description about project ==
This page is a description of Expertiza OSS project E2170 which aims to adequately test several files regarding the ResponseMap model. Here is a description of what response maps do:
*In Expertiza, response maps establish a relationship between a reviewer, a reviewee, and an object to be reviewed. The reviewer is an assignment_participant, the reviewee is an assignment_team, and the reviewed object is either an assignment or another response_map. Each (reviewer, reviewee) pair will have a separate response map. Every time a new review is performed (e.g, every round), a new Response object is created whose map_id is that response map.

The breakdown of the ResponseMap model can be found [https://expertiza.csc.ncsu.edu/index.php/Response_maps here]. There are several types of ResponseMaps that extend functionality of the original ResponseMap. None of which including the superclass are adequately tested. These files include:

* review_response_map.rb <--------- 92.5% coverage

* metareview_response_map.rb <------ 23.08% coverage

* teammate_review_response_map.rb <- no coverage

* response_map.rb <----------------- 74.47% coverage

This project aims to get all the response maps to at least a 90% level of coverage for every Response Map file by writing unit tests for each uncovered method listed in the next section.

== Team ==

* Connor Smith (cpsmith6)

* Abir Majumder (aamajumd)

* Quinn Dibble (qdibble)

* Alex Carruth (agcarrut)

== Test Files Involved ==
There are 4 test files involved in this project:
* review_response_map_spec.rb

* metareview_response_map_spec.rb

* teammate_review_response_map_spec.rb

* response_map_spec.rb
of these files, only review_response_map_spec.rb currently exists; the rest will be added.

== Methods Tested Per File ==

=== review_response_map_spec.rb ===
Currently this file is 92.5% covered and there are no plans to cover it further.

=== metareview_response_map_spec.rb ===
The functions for metareview_response_map.rb that will be tested are:
====get_all_versions====
This returns a sorted array of all the different versions of what is being reviewed in this response map. This means for a test, we will need to create a reviewer, a reviewee, several types of reviews of different versions to be sorted correctly.

[[File:E2170-get all versions.PNG]]

====contributor====
Returns the team associated with the ReviewResponseMap that is to be metareviewed. We will need to create a ReviewResponseMap that has a team associated with it. Then we must create MetareviewResponseMap that points to a created ReviewResponseMap via the reviewed_object_id attribute. We will test if it indeed returns the team associated with ReviewResponseMap.

[[File:E2170-contributor.PNG]]

====questionnaire====
Returns all questionnaires associated to the assignment of this MetareviewResponseMap that is of type 'MetareviewQuestionnaire'. We will need to create a valid MetareviewResponseMap including at least one MetareviewQuestionnaire that can be returned.

[[File:E2170-questionnaire.PNG]]

====get_title====
Returns string "Metareview". We will test if this string is the same as typed out.

[[File:E2170-get title.PNG]]

====exports====
This function takes 3 parameters: csv (the csv file that will contain the exported metareview information), parent_id (the assignment ID containing the requested metareviews), and _options. The function sends all the information about the metareviews to a csv file. We will test that the information is properly exported to the csv file. 
[[File:E2170-export.jpg]]

====export_fields====
Takes a parameter called "_options" that returns the three field columns associated with metareviews to be exported for use by other controllers. We will test to see that these three columns are properly returned. 
[[File:E2170-export_fields.jpg]]

====import====
This function takes 3 parameters: row_hash (represents the hash of the metareview data we want to import), session, and id (representing the id of the assignment we want to import metareview data for). The function imports the data from the hash map, and if the information is input correctly, the data is extracted from the hash and creates a metareview for the requested assignment, otherwise it flashes an error saying the data was input incorrectly. We will test to make sure the imported data is properly converted into a metareview. 
[[File:E2170-import.jpg]]

====email====
This function takes 3 parameters: defn, _participant (the user who will be receiving the email), and assignment (the assignment associated with the metareview). The function sends an email to the participant when they have a new metareview to complete. We will test to make sure the email is sent to the correct participant. 
[[File:E2170-email.jpg]]

=== teammate_review_response_map_spec.rb ===
The functions for teammate_review_response_map.rb that will be tested are:
==== questionnaire ====
This function searches for a TeammateReviewQuestionnaire object and returns it.

[[File:E2170-teammate review response map-questionnaire.PNG]]

==== get_title ====
This function returns the string "Teammate Review".

[[File:E2170-teammate_review_response_map-get_title.png]]

==== teammate_response_report ====
This function returns the teammate response report given the reviewer ID.

[[File:E2170-teammate_review_response_map-teammate_response_report.PNG]]

==== email ====
This function sends a notification email to a student who has been reviewed by their teammate.

[[File:E2170-teammate_review_response_map-email.png]]

=== response_map_spec.rb ===
The functions for response_map.rb that will be tested are:
==== self.assessments_for(team) ====
Gets all submitted Review Responses or all regular Responses for a team, sorts the responses by version, and returns the latest Responses.

[[File:E2170-assessments_for.png]]

==== comparator ====
Compares two Responses version numbers and returns a -1,0, or 1 depending on which version number is greater. If there is no version number for the first response, -1 is returned, otherwise 1 is returned.

[[File:E2170-comparator.png]]

==== self.reviewer_assessments_for(team, reviewer) ====
Gets the Responses for a team reviewed by a specific reviewer and returns the latest response.

[[File:E2170-reviewer_assessments_for.png]]

==== metareviewed_by? ====
Returns whether the Response Map has been metareviewed

[[File:E2170-metareviewed_by.png]]

==== assign_metareviewer ====
This function is in charge of assigning a metareviewer to this review (i.e. a reviewer to review the review).

[[File:E2170-response_map-assign_metareviewer.png]]

==== find_team_member ====
This function returns the team that the reviewer is part of.

[[File:E2170-response_map-find_team_member.png]]

== Running Tests ==

<pre>
rspec ./spec/controllers/<test file name>
</pre>

CSC/ECE 517 Fall 2021 - E2170. Testing - Response Maps

2021-11-08T23:12:01Z

Agcarrut: /* response_map_spec.rb */

== Description about project ==
This page is a description of Expertiza OSS project E2170 which aims to adequately test several files regarding the ResponseMap model. Here is a description of what response maps do:
*In Expertiza, response maps establish a relationship between a reviewer, a reviewee, and an object to be reviewed. The reviewer is an assignment_participant, the reviewee is an assignment_team, and the reviewed object is either an assignment or another response_map. Each (reviewer, reviewee) pair will have a separate response map. Every time a new review is performed (e.g, every round), a new Response object is created whose map_id is that response map.

The breakdown of the ResponseMap model can be found [https://expertiza.csc.ncsu.edu/index.php/Response_maps here]. There are several types of ResponseMaps that extend functionality of the original ResponseMap. None of which including the superclass are adequately tested. These files include:

* review_response_map.rb <--------- 92.5% coverage

* metareview_response_map.rb <------ 23.08% coverage

* teammate_review_response_map.rb <- no coverage

* response_map.rb <----------------- 74.47% coverage

This project aims to get all the response maps to at least a 90% level of coverage for every Response Map file by writing unit tests for each uncovered method listed in the next section.

== Team ==

* Connor Smith (cpsmith6)

* Abir Majumder (aamajumd)

* Quinn Dibble (qdibble)

* Alex Carruth (agcarrut)

== Test Files Involved ==
There are 4 test files involved in this project:
* review_response_map_spec.rb

* metareview_response_map_spec.rb

* teammate_review_response_map_spec.rb

* response_map_spec.rb
of these files, only review_response_map_spec.rb currently exists; the rest will be added.

== Methods Tested Per File ==

=== review_response_map_spec.rb ===
Currently this file is 92.5% covered and there are no plans to cover it further.

=== metareview_response_map_spec.rb ===
The functions for metareview_response_map.rb that will be tested are:
====get_all_versions====
This returns a sorted array of all the different versions of what is being reviewed in this response map. This means for a test, we will need to create a reviewer, a reviewee, several types of reviews of different versions to be sorted correctly.

[[File:E2170-get all versions.PNG]]

====contributor====
Returns the team associated with the ReviewResponseMap that is to be metareviewed. We will need to create a ReviewResponseMap that has a team associated with it. Then we must create MetareviewResponseMap that points to a created ReviewResponseMap via the reviewed_object_id attribute. We will test if it indeed returns the team associated with ReviewResponseMap.

[[File:E2170-contributor.PNG]]

====questionnaire====
Returns all questionnaires associated to the assignment of this MetareviewResponseMap that is of type 'MetareviewQuestionnaire'. We will need to create a valid MetareviewResponseMap including at least one MetareviewQuestionnaire that can be returned.

[[File:E2170-questionnaire.PNG]]

====get_title====
Returns string "Metareview". We will test if this string is the same as typed out.

[[File:E2170-get title.PNG]]

====exports====
This function takes 3 parameters: csv (the csv file that will contain the exported metareview information), parent_id (the assignment ID containing the requested metareviews), and _options. The function sends all the information about the metareviews to a csv file. We will test that the information is properly exported to the csv file. 
[[File:E2170-export.jpg]]

====export_fields====
Takes a parameter called "_options" that returns the three field columns associated with metareviews to be exported for use by other controllers. We will test to see that these three columns are properly returned. 
[[File:E2170-export_fields.jpg]]

====import====
This function takes 3 parameters: row_hash (represents the hash of the metareview data we want to import), session, and id (representing the id of the assignment we want to import metareview data for). The function imports the data from the hash map, and if the information is input correctly, the data is extracted from the hash and creates a metareview for the requested assignment, otherwise it flashes an error saying the data was input incorrectly. We will test to make sure the imported data is properly converted into a metareview. 
[[File:E2170-import.jpg]]

====email====
This function takes 3 parameters: defn, _participant (the user who will be receiving the email), and assignment (the assignment associated with the metareview). The function sends an email to the participant when they have a new metareview to complete. We will test to make sure the email is sent to the correct participant. 
[[File:E2170-email.jpg]]

=== teammate_review_response_map_spec.rb ===
The functions for teammate_review_response_map.rb that will be tested are:
==== questionnaire ====
This function searches for a TeammateReviewQuestionnaire object and returns it.

[[File:E2170-teammate review response map-questionnaire.PNG]]

==== get_title ====
This function returns the string "Teammate Review".

[[File:E2170-teammate_review_response_map-get_title.png]]

==== teammate_response_report ====
This function returns the teammate response report given the reviewer ID.

[[File:E2170-teammate_review_response_map-teammate_response_report.PNG]]

==== email ====
This function sends a notification email to a student who has been reviewed by their teammate.

[[File:E2170-teammate_review_response_map-email.png]]

=== response_map_spec.rb ===
The functions for response_map.rb that will be tested are:
==== self.assessments_for(team) ====
Gets all submitted Review Responses or all regular Responses for a team, sorts the responses by version, and returns the latest Responses.
[[File:E2170-assessments_for.png]]

==== comparator ====
Compares two Responses version numbers and returns a -1,0, or 1 depending on which version number is greater. If there is no version number for the first response, -1 is returned, otherwise 1 is returned.

[[File:E2170-comparator.png]]

==== self.reviewer_assessments_for(team, reviewer) ====
Gets the Responses for a team reviewed by a specific reviewer and returns the latest response.

[[File:E2170-reviewer_assessments_for.png]]

==== metareviewed_by? ====
Returns whether the Response Map has been metareviewed

[[File:E2170-metareviewed_by.png]]

==== assign_metareviewer ====
This function is in charge of assigning a metareviewer to this review (i.e. a reviewer to review the review).

[[File:E2170-response_map-assign_metareviewer.png]]

==== find_team_member ====
This function returns the team that the reviewer is part of.

[[File:E2170-response_map-find_team_member.png]]

== Running Tests ==

<pre>
rspec ./spec/controllers/<test file name>
</pre>

CSC/ECE 517 Fall 2021 - E2170. Testing - Response Maps

2021-11-08T22:59:21Z

Agcarrut: /* response_map_spec.rb */

== Description about project ==
This page is a description of Expertiza OSS project E2170 which aims to adequately test several files regarding the ResponseMap model. Here is a description of what response maps do:
*In Expertiza, response maps establish a relationship between a reviewer, a reviewee, and an object to be reviewed. The reviewer is an assignment_participant, the reviewee is an assignment_team, and the reviewed object is either an assignment or another response_map. Each (reviewer, reviewee) pair will have a separate response map. Every time a new review is performed (e.g, every round), a new Response object is created whose map_id is that response map.

The breakdown of the ResponseMap model can be found [https://expertiza.csc.ncsu.edu/index.php/Response_maps here]. There are several types of ResponseMaps that extend functionality of the original ResponseMap. None of which including the superclass are adequately tested. These files include:

* review_response_map.rb <--------- 92.5% coverage

* metareview_response_map.rb <------ 23.08% coverage

* teammate_review_response_map.rb <- no coverage

* response_map.rb <----------------- 74.47% coverage

This project aims to get all the response maps to at least a 90% level of coverage for every Response Map file by writing unit tests for each uncovered method listed in the next section.

== Team ==

* Connor Smith (cpsmith6)

* Abir Majumder (aamajumd)

* Quinn Dibble (qdibble)

* Alex Carruth (agcarrut)

== Test Files Involved ==
There are 4 test files involved in this project:
* review_response_map_spec.rb

* metareview_response_map_spec.rb

* teammate_review_response_map_spec.rb

* response_map_spec.rb
of these files, only review_response_map_spec.rb currently exists; the rest will be added.

== Methods Tested Per File ==

=== review_response_map_spec.rb ===
Currently this file is 92.5% covered and there are no plans to cover it further.

=== metareview_response_map_spec.rb ===
The functions for metareview_response_map.rb that will be tested are:
====get_all_versions====
This returns a sorted array of all the different versions of what is being reviewed in this response map. This means for a test, we will need to create a reviewer, a reviewee, several types of reviews of different versions to be sorted correctly.

[[File:E2170-get all versions.PNG]]

====contributor====
Returns the team associated with the ReviewResponseMap that is to be metareviewed. We will need to create a ReviewResponseMap that has a team associated with it. Then we must create MetareviewResponseMap that points to a created ReviewResponseMap via the reviewed_object_id attribute. We will test if it indeed returns the team associated with ReviewResponseMap.

[[File:E2170-contributor.PNG]]

====questionnaire====
Returns all questionnaires associated to the assignment of this MetareviewResponseMap that is of type 'MetareviewQuestionnaire'. We will need to create a valid MetareviewResponseMap including at least one MetareviewQuestionnaire that can be returned.

[[File:E2170-questionnaire.PNG]]

====get_title====
Returns string "Metareview". We will test if this string is the same as typed out.

[[File:E2170-get title.PNG]]

====exports====
This function takes 3 parameters: csv (the csv file that will contain the exported metareview information), parent_id (the assignment ID containing the requested metareviews), and _options. The function sends all the information about the metareviews to a csv file. We will test that the information is properly exported to the csv file. 
[[File:E2170-export.jpg]]

====export_fields====
Takes a parameter called "_options" that returns the three field columns associated with metareviews to be exported for use by other controllers. We will test to see that these three columns are properly returned. 
[[File:E2170-export_fields.jpg]]

====import====
This function takes 3 parameters: row_hash (represents the hash of the metareview data we want to import), session, and id (representing the id of the assignment we want to import metareview data for). The function imports the data from the hash map, and if the information is input correctly, the data is extracted from the hash and creates a metareview for the requested assignment, otherwise it flashes an error saying the data was input incorrectly. We will test to make sure the imported data is properly converted into a metareview. 
[[File:E2170-import.jpg]]

====email====
This function takes 3 parameters: defn, _participant (the user who will be receiving the email), and assignment (the assignment associated with the metareview). The function sends an email to the participant when they have a new metareview to complete. We will test to make sure the email is sent to the correct participant. 
[[File:E2170-email.jpg]]

=== teammate_review_response_map_spec.rb ===
The functions for teammate_review_response_map.rb that will be tested are:
==== questionnaire ====
This function searches for a TeammateReviewQuestionnaire object and returns it.

[[File:E2170-teammate review response map-questionnaire.PNG]]

==== get_title ====
This function returns the string "Teammate Review".

[[File:E2170-teammate_review_response_map-get_title.png]]

==== teammate_response_report ====
This function returns the teammate response report given the reviewer ID.

[[File:E2170-teammate_review_response_map-teammate_response_report.PNG]]

==== email ====
This function sends a notification email to a student who has been reviewed by their teammate.

[[File:E2170-teammate_review_response_map-email.png]]

=== response_map_spec.rb ===
The functions for response_map.rb that will be tested are:
==== self.assessments_for(team) ====
Gets all submitted Review Responses or all regular Responses for a team, sorts the responses by version, and returns the latest Responses.

==== comparator ====
Compares two Responses version numbers and returns a -1,0, or 1 depending on which version number is greater. If there is no version number for the first response, -1 is returned, otherwise 1 is returned.

[[File:E2170-comparator.png]]

==== self.reviewer_assessments_for(team, reviewer) ====
Gets the Responses for a team reviewed by a specific reviewer and returns the latest response.

[[File:E2170-reviewer_assessments_for.png]]

==== metareviewed_by? ====
Returns whether the Response Map has been metareviewed

[[File:E2170-metareviewed_by.png]]

==== assign_metareviewer ====
This function is in charge of assigning a metareviewer to this review (i.e. a reviewer to review the review).

[[File:E2170-response_map-assign_metareviewer.png]]

==== find_team_member ====
This function returns the team that the reviewer is part of.

[[File:E2170-response_map-find_team_member.png]]

== Running Tests ==

<pre>
rspec ./spec/controllers/<test file name>
</pre>

File:E2170-metareviewed by.png

2021-11-08T22:57:11Z

Agcarrut:

File:E2170-reviewer assessments for.png

2021-11-08T22:56:27Z

Agcarrut:

File:E2170-comparator.png

2021-11-08T22:55:22Z

Agcarrut:

CSC/ECE 517 Fall 2021 - E2170. Testing - Response Maps

2021-11-08T22:54:56Z

Agcarrut:

== Description about project ==
This page is a description of Expertiza OSS project E2170 which aims to adequately test several files regarding the ResponseMap model. Here is a description of what response maps do:
*In Expertiza, response maps establish a relationship between a reviewer, a reviewee, and an object to be reviewed. The reviewer is an assignment_participant, the reviewee is an assignment_team, and the reviewed object is either an assignment or another response_map. Each (reviewer, reviewee) pair will have a separate response map. Every time a new review is performed (e.g, every round), a new Response object is created whose map_id is that response map.

The breakdown of the ResponseMap model can be found [https://expertiza.csc.ncsu.edu/index.php/Response_maps here]. There are several types of ResponseMaps that extend functionality of the original ResponseMap. None of which including the superclass are adequately tested. These files include:

* review_response_map.rb <--------- 92.5% coverage

* metareview_response_map.rb <------ 23.08% coverage

* teammate_review_response_map.rb <- no coverage

* response_map.rb <----------------- 74.47% coverage

This project aims to get all the response maps to at least a 90% level of coverage for every Response Map file by writing unit tests for each uncovered method listed in the next section.

== Team ==

* Connor Smith (cpsmith6)

* Abir Majumder (aamajumd)

* Quinn Dibble (qdibble)

* Alex Carruth (agcarrut)

== Test Files Involved ==
There are 4 test files involved in this project:
* review_response_map_spec.rb

* metareview_response_map_spec.rb

* teammate_review_response_map_spec.rb

* response_map_spec.rb
of these files, only review_response_map_spec.rb currently exists; the rest will be added.

== Methods Tested Per File ==

=== review_response_map_spec.rb ===
Currently this file is 92.5% covered and there are no plans to cover it further.

=== metareview_response_map_spec.rb ===
The functions for metareview_response_map.rb that will be tested are:
====get_all_versions====
This returns a sorted array of all the different versions of what is being reviewed in this response map. This means for a test, we will need to create a reviewer, a reviewee, several types of reviews of different versions to be sorted correctly.

[[File:E2170-get all versions.PNG]]

====contributor====
Returns the team associated with the ReviewResponseMap that is to be metareviewed. We will need to create a ReviewResponseMap that has a team associated with it. Then we must create MetareviewResponseMap that points to a created ReviewResponseMap via the reviewed_object_id attribute. We will test if it indeed returns the team associated with ReviewResponseMap.

[[File:E2170-contributor.PNG]]

====questionnaire====
Returns all questionnaires associated to the assignment of this MetareviewResponseMap that is of type 'MetareviewQuestionnaire'. We will need to create a valid MetareviewResponseMap including at least one MetareviewQuestionnaire that can be returned.

[[File:E2170-questionnaire.PNG]]

====get_title====
Returns string "Metareview". We will test if this string is the same as typed out.

[[File:E2170-get title.PNG]]

====exports====
This function takes 3 parameters: csv (the csv file that will contain the exported metareview information), parent_id (the assignment ID containing the requested metareviews), and _options. The function sends all the information about the metareviews to a csv file. We will test that the information is properly exported to the csv file. 
[[File:E2170-export.jpg]]

====export_fields====
Takes a parameter called "_options" that returns the three field columns associated with metareviews to be exported for use by other controllers. We will test to see that these three columns are properly returned. 
[[File:E2170-export_fields.jpg]]

====import====
This function takes 3 parameters: row_hash (represents the hash of the metareview data we want to import), session, and id (representing the id of the assignment we want to import metareview data for). The function imports the data from the hash map, and if the information is input correctly, the data is extracted from the hash and creates a metareview for the requested assignment, otherwise it flashes an error saying the data was input incorrectly. We will test to make sure the imported data is properly converted into a metareview. 
[[File:E2170-import.jpg]]

====email====
This function takes 3 parameters: defn, _participant (the user who will be receiving the email), and assignment (the assignment associated with the metareview). The function sends an email to the participant when they have a new metareview to complete. We will test to make sure the email is sent to the correct participant. 
[[File:E2170-email.jpg]]

=== teammate_review_response_map_spec.rb ===
The functions for teammate_review_response_map.rb that will be tested are:
==== questionnaire ====
This function searches for a TeammateReviewQuestionnaire object and returns it.

[[File:E2170-teammate review response map-questionnaire.PNG]]

==== get_title ====
This function returns the string "Teammate Review".

[[File:E2170-teammate_review_response_map-get_title.png]]

==== teammate_response_report ====
This function returns the teammate response report given the reviewer ID.

[[File:E2170-teammate_review_response_map-teammate_response_report.PNG]]

==== email ====
This function sends a notification email to a student who has been reviewed by their teammate.

[[File:E2170-teammate_review_response_map-email.png]]

=== response_map_spec.rb ===
The functions for response_map.rb that will be tested are:
==== self.assessments_for(team) ====
Gets all submitted Review Responses or all regular Responses for a team, sorts the responses by version, and returns the latest Responses.

==== comparator ====
Compares two Responses version numbers and returns a -1,0, or 1 depending on which version number is greater. If there is no version number for the first response, -1 is returned, otherwise 1 is returned.

[[File:E2170-comparator.png]]

==== self.reviewer_assessments_for(team, reviewer) ====
Gets the responses for a team reviewed by a specific reviewer and returns the latest response.

[[File:E2170-reviewer_assessments_for.png]]

==== metareviewed_by? ====
Returns whether the response map has been metareviewed

[[File:E2170-metareviewed_by.png]]

==== assign_metareviewer ====
This function is in charge of assigning a metareviewer to this review (i.e. a reviewer to review the review).

[[File:E2170-response_map-assign_metareviewer.png]]

==== find_team_member ====
This function returns the team that the reviewer is part of.

[[File:E2170-response_map-find_team_member.png]]

== Running Tests ==

<pre>
rspec ./spec/controllers/<test file name>
</pre>

CSC/ECE 517 Fall 2021 - E2170. Testing - Response Maps

2021-11-02T21:09:33Z

Agcarrut: /* Description about project */

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T02:58:04Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

For every worker, we have added a test case in the <code>spec/workers/sidekiq_mail_worker_spec.rb</code> file: one each for MailWorker, SimicheckWorker, DropOutstandingReviewsWorker.

Further, we moved methods <code>find_min_from_now_duration</code> from <code>assignment_form.rb</code> to <code>due_date.rb</code>. Along with that we introduced some methods to make it easier to understand the conversions and values that are being set.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

If we try to set the due date to be a date that has passed, no new jobs are queued as expected.

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

The following tests were added according to the methods we moved and created:

<pre>
describe "#find_min_from_now_duration" do
it "returns time difference between due_date and now" do
allow(DateTime).to receive(:now).and_return(DateTime.new(2021, 10, 20, 11, 11, 11).in_time_zone)
due_at = Time.parse(DateTime.new(2021, 10, 20, 12, 12, 12).in_time_zone.to_s(:db))
expect(DueDate.find_min_from_now_duration(due_at)).to eq(61)
end
end

describe "#get_dequeue_time_as_seconds_duration_from_now" do
it "returns time difference between now and dequeue of job in seconds" do
allow(DateTime).to receive(:now).and_return(DateTime.new(2021, 10, 20, 11, 00, 00).in_time_zone)
due_at = DateTime.new(2021, 10, 20, 12, 00, 00)
delay_duration = 1.hour
assignment_id = create(:assignment, staggered_deadline: true, name: "testassignment").id
due_date = create(:topic_due_date, deadline_type: @deadline_type,
submission_allowed_id: @deadline_right, review_allowed_id: @deadline_right,
review_of_review_allowed_id: @deadline_right, due_at: due_at, parent_id: assignment_id)
expect(DueDate.get_dequeue_time_as_seconds_duration_from_now(due_date, delay_duration)).to eq(7200)
end
end
</pre>

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, we needed to run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

NB: To create a generic sidekiq worker to test with, you could use
<pre>rails g sidekiq:worker SimicheckWorker</pre>
to create a shell worker class.

===== Sidekiq UI =====
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Ensure the Late Policy is set to <code>--None--</code> under the Due Dates tab. If there is a late policy set, per pre-existing code, the enqueued SimicheckWorker job will be deleted and will not appear in the Sidekiq UI (unless it is dequeued quickly, before the code can delete the jobs in the queue).
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

[[File:simicheck-job-enqueued.png|950px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor.
# Navigate to '''Manage'''>'''Assignments''' in the navigation.
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings.
# Go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Select a late policy next to '''Apply penalty policy:'''. If a late policy has not been created, press '''New Late Policy''' and follow the steps for creating a late policy.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.
# Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. The assignment due date must be changed to a time at least an hour ahead of the current time for the job to be queued.
# To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq.
# Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Automated Tests ===

The following tests were added to test the workers:

<pre>
describe "test different workers" do
it "should increase the size of queue by 1 when MailWorker is used" do
Sidekiq::Testing.fake!
MailWorker.perform_in(42.minutes, 1, "review", "2021-10-27 00:00:01")
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end

it "should increase the size of queue by 1 when SimicheckWorker is used" do
Sidekiq::Testing.fake!
SimicheckWorker.perform_in(42.minutes, 1)
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end

it "should increase the size of queue by 1 when DropOutstandingReviewsWorker is used" do
Sidekiq::Testing.fake!
DropOutstandingReviewsWorker.perform_in(42.minutes, 1, "review", "2021-10-27 00:00:01")
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end
end
</pre>

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T02:56:45Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

For every worker, we have added a test case in the <code>spec/workers/sidekiq_mail_worker_spec.rb</code> file: one each for MailWorker, SimicheckWorker, DropOutstandingReviewsWorker.

Further, we moved methods <code>find_min_from_now_duration</code> from <code>assignment_form.rb</code> to <code>due_date.rb</code>. Along with that we introduced some methods to make it easier to understand the conversions and values that are being set.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

If we try to set the due date to be a date that has passed, no new jobs are queued as expected.

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

The following tests were added according to the methods we moved and created:

<pre>
describe "#find_min_from_now_duration" do
it "returns time difference between due_date and now" do
allow(DateTime).to receive(:now).and_return(DateTime.new(2021, 10, 20, 11, 11, 11).in_time_zone)
due_at = Time.parse(DateTime.new(2021, 10, 20, 12, 12, 12).in_time_zone.to_s(:db))
expect(DueDate.find_min_from_now_duration(due_at)).to eq(61)
end
end

describe "#get_dequeue_time_as_seconds_duration_from_now" do
it "returns time difference between now and dequeue of job in seconds" do
allow(DateTime).to receive(:now).and_return(DateTime.new(2021, 10, 20, 11, 00, 00).in_time_zone)
due_at = DateTime.new(2021, 10, 20, 12, 00, 00)
delay_duration = 1.hour
assignment_id = create(:assignment, staggered_deadline: true, name: "testassignment").id
due_date = create(:topic_due_date, deadline_type: @deadline_type,
submission_allowed_id: @deadline_right, review_allowed_id: @deadline_right,
review_of_review_allowed_id: @deadline_right, due_at: due_at, parent_id: assignment_id)
expect(DueDate.get_dequeue_time_as_seconds_duration_from_now(due_date, delay_duration)).to eq(7200)
end
end
</pre>

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, we needed to run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

NB: To create a generic sidekiq worker to test with, you could use
<pre>rails g sidekiq:worker SimicheckWorker</pre>
to create a shell worker class.

===== Sidekiq UI =====
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Ensure the Late Policy is set to <code>--None--</code> under the Due Dates tab. If there is a late policy set, per pre-existing code, the enqueued SimicheckWorker job will be deleted and will not appear in the Sidekiq UI (unless it is dequeued quickly, before the code can delete the jobs in the queue).
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

[[File:simicheck-job-enqueued.png|950px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor
# Navigate to '''Manage'''>'''Assignments''' in the navigation
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings.
# Go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Select a late policy next to '''Apply penalty policy:'''. If a late policy has not been created, press '''New Late Policy''' and follow the steps for creating a late policy.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.
# Go to the assignment that student7613 had been assigned to
# Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. The assignment due date must be changed to a time at least an hour ahead of the current time for the reminder to be sent.
# To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq.
# Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Automated Tests ===

The following tests were added to test the workers:

<pre>
describe "test different workers" do
it "should increase the size of queue by 1 when MailWorker is used" do
Sidekiq::Testing.fake!
MailWorker.perform_in(42.minutes, 1, "review", "2021-10-27 00:00:01")
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end

it "should increase the size of queue by 1 when SimicheckWorker is used" do
Sidekiq::Testing.fake!
SimicheckWorker.perform_in(42.minutes, 1)
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end

it "should increase the size of queue by 1 when DropOutstandingReviewsWorker is used" do
Sidekiq::Testing.fake!
DropOutstandingReviewsWorker.perform_in(42.minutes, 1, "review", "2021-10-27 00:00:01")
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end
end
</pre>

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T01:51:00Z

Agcarrut: /* Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

For every worker, we have added a test case in the <code>spec/workers/sidekiq_mail_worker_spec.rb</code> file: one each for MailWorker, SimicheckWorker, DropOutstandingReviewsWorker.

Further, we moved methods <code>find_min_from_now_duration</code> from <code>assignment_form.rb</code> to <code>due_date.rb</code>. Along with that we introduced some methods to make it easier to understand the conversions and values that are being set.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

The following tests were added according to the methods we moved and created:

<pre>
describe "#find_min_from_now_duration" do
it "returns time difference between due_date and now" do
allow(DateTime).to receive(:now).and_return(DateTime.new(2021, 10, 20, 11, 11, 11).in_time_zone)
due_at = Time.parse(DateTime.new(2021, 10, 20, 12, 12, 12).in_time_zone.to_s(:db))
expect(DueDate.find_min_from_now_duration(due_at)).to eq(61)
end
end

describe "#get_dequeue_time_as_seconds_duration_from_now" do
it "returns time difference between now and dequeue of job in seconds" do
allow(DateTime).to receive(:now).and_return(DateTime.new(2021, 10, 20, 11, 00, 00).in_time_zone)
due_at = DateTime.new(2021, 10, 20, 12, 00, 00)
delay_duration = 1.hour
assignment_id = create(:assignment, staggered_deadline: true, name: "testassignment").id
due_date = create(:topic_due_date, deadline_type: @deadline_type,
submission_allowed_id: @deadline_right, review_allowed_id: @deadline_right,
review_of_review_allowed_id: @deadline_right, due_at: due_at, parent_id: assignment_id)
expect(DueDate.get_dequeue_time_as_seconds_duration_from_now(due_date, delay_duration)).to eq(7200)
end
end
</pre>

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, we needed to run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

NB: To create a generic sidekiq worker to test with, you could use
<pre>rails g sidekiq:worker SimicheckWorker</pre>
to create a shell worker class.

===== Sidekiq UI =====
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor
# Navigate to '''Manage'''>'''Assignments''' in the navigation
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.

At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again.
<ol start="5">
<li> Go to the assignment that student7613 had been assigned to </li>
<li> Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. The assignment due date must be changed to a time at least an hour ahead of the current time for the reminder to be sent. </li>
<li> To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. </li>
<li> Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following: </li>
</ol>

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Automated Tests ===

The following tests were added to test the workers:

<pre>
describe "test different workers" do
it "should increase the size of queue by 1 when MailWorker is used" do
Sidekiq::Testing.fake!
MailWorker.perform_in(42.minutes, 1, "review", "2021-10-27 00:00:01")
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end

it "should increase the size of queue by 1 when SimicheckWorker is used" do
Sidekiq::Testing.fake!
SimicheckWorker.perform_in(42.minutes, 1)
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end

it "should increase the size of queue by 1 when DropOutstandingReviewsWorker is used" do
Sidekiq::Testing.fake!
DropOutstandingReviewsWorker.perform_in(42.minutes, 1, "review", "2021-10-27 00:00:01")
queue = Sidekiq::Queues["jobs"]
expect(queue.size).to eq(1)
queue.clear
end
end
</pre>

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T01:29:31Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor
# Navigate to '''Manage'''>'''Assignments''' in the navigation
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.

At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again.
<ol start="5">
<li> Go to the assignment that student7613 had been assigned to </li>
<li> Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. The assignment due date must be changed to a time at least an hour ahead of the current time for the reminder to be sent. </li>
<li> To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. </li>
<li> Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following: </li>
</ol>

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T01:28:51Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor
# Navigate to '''Manage'''>'''Assignments''' in the navigation
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.

At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again.
<ol start="5">
<li> Go to the assignment that student7613 had been assigned to </li>
<li> Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. Note: The assignment due date must be changed to a time at least an hour ahead of the current time for the reminder to be sent. </li>
<li> To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. </li>
<li> Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following: </li>
</ol>

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T01:25:09Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor
# Navigate to '''Manage'''>'''Assignments''' in the navigation
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.

At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again.
5. Go to the assignment that student7613 had been assigned to.

6. Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. Note: The assignment due date must be changed to a time at least an hour ahead of the current time for the reminder to be sent.

7. To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq.

8. Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T01:24:49Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor
# Navigate to '''Manage'''>'''Assignments''' in the navigation
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.

At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again.
5. Go to the assignment that student7613 had been assigned to.
6. Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. Note: The assignment due date must be changed to a time at least an hour ahead of the current time for the reminder to be sent.
7. To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq.
8. Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T01:24:23Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these:
# Log into Expertiza as an instructor
# Navigate to '''Manage'''>'''Assignments''' in the navigation
# Click the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Assign student7613 to the assignment you created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page.

At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again.
# Go to the assignment that student7613 had been assigned to.
# Edit the assignment, go to the due dates tab, and change the due date on the review to be a day ahead of its current due date, and press save. Note: The assignment due date must be changed to a time at least an hour ahead of the current time for the reminder to be sent.
# To ensure that the assignments are now queued on Sidekiq, visit the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq.
# Upon arriving on this page, verify that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, you should see something like the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-27T01:20:50Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====
To enqueue a Simicheck job:
# Edit an assignment.
# Select the <code>Use simicheck?</code> box.
# Set <code>Simicheck Delay</code> to <code>0</code> (or to a nonzero value to observe a delay).
# Set the SimiCheck Similarity Threshold to a nonzero value.
# Click <code>Save</code>.
# Observe a Simicheck Worker job enqueued and dequeued in the Sidekiq UI.

[[File:Enqueue-simicheck-job.png|500px|How to Enqueue a Simicheck Worker Job]]

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these, we:
# Logged into Expertiza as an instructor
# Navigated to '''Manage'''>'''Assignments''' in the navigation
# Clicked the plus button in the top right corner on the '''Manage Assignments''' and create an assignment with default settings. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review.
# Assign student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, changed the due date on the review to be a day ahead of its current due date, and pressed save. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T18:09:12Z

Agcarrut: Moved code cleanup

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, changed the due date on the review to be a day ahead of its current due date, and pressed save. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T18:07:58Z

Agcarrut: Added test plan

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

== Test Plan ==

=== Unit Tests ===
In order to properly test that our refactoring had worked properly, we found that we would have to add in multiple tests to account for our new methods as well as refactor old tests to work with the changes that we had made.

=== Functional Tests ===
We decided that functional testing would need to be done by manually checking to see if Sidekiq tasks were queued as expected when changes to assignments were made. The three important functional tests that we decided needed to be verified were that the Simicheck Worker was queuing, the Mail Worker was queueing, and the Drop Outstanding Reviews Worker was queuing. These could be considered verified under the condition that the '''When''' field, '''Job''' field, and '''Arguments''' field in the Sidekiq view all came out as expected.

=== Considering Edge Cases and Other Possibilities ===

== Implementing Test Plan ==

=== Update and add unit tests according to the refactoring changes made ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, changed the due date on the review to be a day ahead of its current due date, and pressed save. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable
* ...

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:57:43Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Update and add unit tests according to the above changes ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. Drop Outstanding Reviews Worker drops all reviews that had not been worked on before the due date was changed. These both can be tested together, as when an assignment with a review is updated, Sidekiq should queue both the jobs for Mail Worker and Drop Outstanding Reviews Worker. To test these, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. Make sure when creating the assignment to go to the '''Due Dates''' tab and assign due dates for both the assignment and the review. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, changed the due date on the review to be a day ahead of its current due date, and pressed save. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring of Mail Worker had worked properly and the Mail Worker was operational. The job '''DropOutstandingReviewsWorker''' showed that our refactoring of Drop Outstanding Reviews Worker had also been done correctly.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable
* ...

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:50:52Z

Agcarrut: /* MailWorker and DropOustandingReviews Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Update and add unit tests according to the above changes ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. To test Mail Worker, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, changed the due date on the review to be a day ahead of its current due date, and pressed save. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring had worked properly and the Mail Worker was operational.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable
* ...

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:24:54Z

Agcarrut: /* DropOutstandingReviewsWorker Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Update and add unit tests according to the above changes ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. To test Mail Worker, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, and changed the due date to be a day ahead of its current due date. Following this, we pressed save on the assignment, and then moved back to the '''Home''' page by hitting '''Home''' on the navigation bar to ensure that everything on the page had been updated correctly. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring had worked properly and the Mail Worker was operational.

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable
* ...

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:24:47Z

Agcarrut: /* MailWorker Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Update and add unit tests according to the above changes ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker and DropOustandingReviews Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. To test Mail Worker, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, and changed the due date to be a day ahead of its current due date. Following this, we pressed save on the assignment, and then moved back to the '''Home''' page by hitting '''Home''' on the navigation bar to ensure that everything on the page had been updated correctly. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring had worked properly and the Mail Worker was operational.

==== DropOutstandingReviewsWorker Test ====

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable
* ...

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:24:20Z

Agcarrut: /* MailWorker Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Update and add unit tests according to the above changes ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. To test Mail Worker, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, and changed the due date to be a day ahead of its current due date. Following this, we pressed save on the assignment, and then moved back to the '''Home''' page by hitting '''Home''' on the navigation bar to ensure that everything on the page had been updated correctly. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq-jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring had worked properly and the Mail Worker was operational.

==== DropOutstandingReviewsWorker Test ====

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable
* ...

File:Sidekiq-jobs.png

2021-10-26T17:24:00Z

Agcarrut:

File:Sidekiq jobs.png

2021-10-26T17:23:16Z

Agcarrut:

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:23:04Z

Agcarrut: /* MailWorker Test */

__TOC__

== Background ==
This project revolves around the 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 <code>perform()</code> to gather email IDs of all participants in an assignment and send them an email reminder. Some test cases exist for this work.

=== Previous Setup ===
Initial work can be found [https://expertiza.csc.ncsu.edu/index.php/E1835_Refactor_delayed_mailer_and_scheduled_task here].

Code in the [https://github.com/expertiza/expertiza/pull/1245/files pull request] was found to be reasonable and working. The method <code>perform()</code> has:
{| class=wikitable style=text-align:right
|+ Metrics for <code>perform</code> method
|-
! Metric !! Current Value !! Limit
|-
! Cognitive Complexity
| 15 || 5
|-
! Cyclomatic Complexity
| 8 || 6
|-
! Assignment Branch Condition size
| 22.93 || 15
|-
! Perceived complexity
| 10 || 7
|}

Previous pull request also has duplicated commit messages and the commits need to be squashed.

Files involved (some changed should not have been changed):
* app/controllers/assignments_controller.rb
* app/mailers/mail_worker.rb
* app/models/assignment_form.rb
* config/sidekiq.yml
* spec/models/assignment_form_spec.rb
* spec/sidekiq_mail_worker_spec.rb
* spec/spec_helper.rb

There are minor styling issues such as missing or trailing whitespaces, lines that are too long, extra empty line, unused arguments.

=== Problem Statement ===

This project revolves around the 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.
The project was partially merged.

What needs to be done:
Feedback from the previous team (E1835) is:
Code in pull request seems pretty reasonable. Some concerns wrt. naming functions, but otherwise reviewers seem to agree that the code is good.

Your pull request has many duplicated commit messages. Please try to <code>squash</code> similar commits.
And using meaningful commit messages later.

You should commit changes to the DB schema (<code>db/schema.rb</code>) only if you have created new DB migrations.
Please double check your code. If you did not aim to change the DB, please revert the DB schema changes. ***** Important, try not to change the DB.

You should not change <code>rails_helper.rb</code> or <code>spec_helper.rb</code> file; please revert these changes.

The previous team’s code needs a refactor in some places, though the actual function of it seems solid. It is very possible that in the three years since this code has been worked on, more mailer functionality has been added. There are other teams this semester working on mailer code as well. Try to keep your code as contained and simple as possible, so that merging these projects is possible.

== Summary of Work Needed ==
After discussing with our project mentor and Dr. Gehringer, we determined the main tasks to be the following:
# Refactor the single Sidekiq worker into multiple workers
# Remove all code related to the <code>drop_one_member_topics</code> deadline type
# Update and add unit tests according to the above changes
# Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected
# Code cleanup

=== Refactor the single Sidekiq worker into multiple workers ===
Previously there was only a single worker class that handled all types of jobs. Its top-level method <code>perform</code> looked like the following.
<pre>
def perform(assignment_id, deadline_type, due_at)
self.assignment_id = assignment_id
self.deadline_type = deadline_type
self.due_at = due_at

assignment = Assignment.find(self.assignment_id)
participant_mails = find_participant_emails

if %w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck].include?(self.deadline_type)
drop_one_member_topics if self.deadline_type == "drop_outstanding_reviews" && assignment.team_assignment
drop_outstanding_reviews if self.deadline_type == "drop_outstanding_reviews"
perform_simicheck_comparisons(self.assignment_id) if self.deadline_type == "compare_files_with_simicheck"
else
# Can we rename deadline_type(metareview) to "teammate review". If, yes then we donot need this if clause below!
deadlineText = if self.deadline_type == "metareview"
"teammate review"
else
self.deadline_type
end

email_reminder(participant_mails, deadlineText) unless participant_mails.empty?
end
end
</pre>

The processing was different depending on the <code>deadline_type</code>. The <code>deadline_type</code>s in
<pre>
%w[drop_one_member_topics drop_outstanding_reviews compare_files_with_simicheck]
</pre>
did not lead to an email reminder being sent, but they were still handled in this <code>MailWorker</code> class.

To make the code more manageable and easier to follow, it made sense to break up the responsibilities of this single <code>MailWorker</code> class into multiple workers:
* <code>SimicheckWorker</code>
* <code>DropOutstandingReviewsWorker</code>
* <code>MailWorker</code>
All three workers extended an "abstract class" called <code>Worker</code> that contained the Sidekiq-related <code>include</code> and the queue name to which all jobs would be sent.

For simplicity, we still had all new workers continue to use a single queue, which we renamed from <code>mailers</code> to <code>jobs</code>. A future improvement could divide these jobs to go to multiple different queues if needed.

=== Remove all code related to the <code>drop_one_member_topics</code> deadline type ===
It was decided that code used for dropping one member or two member teams from topics would never need to be implemented in the Expertiza website. Thus, we removed all code related to the drop_one_member_topics method within the Expertiza beta. This involved removing the <code>drop_one_member_topics</code> function from <code>drop_outstanding_reviews_mail_worker.rb</code>, now renamed <code>drop_outstanding_reviews_worker.rb</code>. The <code>drop_one_member_topics</code> method was called from the <code>prepare_data</code> method, which was then called by the Sidekiq method <code>perform</code> so we removed this call. The <code>drop_one_member_topics</code> was also referenced in <code>assignment_form.rb</code> using <code>add_delayed_job</code> in the <code>add_to_delayed_queue</code> method and in <code>delayed_mailer.html.erb</code> as an option for the handler so both of these code snippets were removed as well.

=== Update and add unit tests according to the above changes ===

=== Test all new worker classes to ensure Sidekiq jobs were being dequeued and run as expected ===
==== Sidekiq setup ====
In order to test the Sidekiq workers, a few steps were needed to set up the workers and run Sidekiq (steps were obtained from [https://tudip.com/blog-post/how-to-use-sidekiq-in-rails-for-background-processing/ this site]):
# Initialize three new workers, each having one of the three new worker types:
## <code>rails g sidekiq:worker SimicheckWorker</code>
## <code>rails g sidekiq:worker DropOutstandingReviewsWorker</code>
## <code>rails g sidekiq:worker MailWorker</code>
# In a terminal separate from the one in which the main rails application is running, run
<blockquote>
<pre>
bundle exec sidekiq
</pre>
</blockquote>

This Sidekiq-specific terminal will show logs and printed statements from the workers.

==== SimicheckWorker Test ====

==== MailWorker Test ====
The Mail Worker sends out notifications to students if the due date of an assignment is changed. To test Mail Worker, we first created an assignment with the default options by logging in as an instructor, going to '''Manage'''>'''Assignments''' in the navigation, and clicking the plus button in the top right corner on the '''Manage Assignments'''. We then assigned student7613 to the assignment we had created by clicking the person with the plus icon in the icons next to the assignment on the '''Manage Assignments''' page. At this point, we realized we need to alter the code to test if the code was properly queuing the Mail Worker because the current if statement to add items to the delayed queue required that it have a late policy. However, the late policy code is not currently working. Thus, in the <code>assignment_form.rb</code> model file, we edited the following code:

[[File:Change_late_policy.png|950px|Late policy code to alter]]

We removed <code>and has_late_policy</code> so <code>add_delayed_queue</code> can run even without late policies being implemented. After saving this file, we started up Expertiza again and went to the assignment that student7613 had been assigned to. We then edited the assignment, went to the due dates tab, and changed the due date to be a day ahead of its current due date. Following this, we pressed save on the assignment, and then moved back to the '''Home''' page by hitting '''Home''' on the navigation bar to ensure that everything on the page had been updated correctly. To ensure that the assignments were now queued on Sidekiq, we visited the Sidekiq UI by going to [Expertiza VCL URL]/sidekiq. Upon arriving on this page, we verified that the jobs added to the queue were correct by clicking to '''Scheduled'''. After arriving on this page, we saw the following:

[[File:Sidekiq_jobs.png|950px|Sidekiq jobs]]

The job '''MailWorker''' that had been queued successfully shows that our refactoring had worked properly and the Mail Worker was operational.

==== DropOutstandingReviewsWorker Test ====

=== Sidekiq UI ===
To see which messages were being enqueued and dequeued in Sidekiq, we use the Sidekiq user interface, which can be accessed via the <code>/sidekiq</code> route of the application.
[[File:Sidekiq-ui.png|950px|The Sidekiq UI]]

=== Code Cleanup ===
Smaller improvements done while implementing the main tasks included:
* Using ActiveSupport::Duration objects in order to calculate the minutes and seconds related to the dequeue times
* Refactoring the time calculations for the dequeue times to the DueDate class
* Correcting the parameters passed to the workers, which were previously incorrect. As an example:
<blockquote>
<pre>
def add_delayed_job(_assignment, deadline_type, due_date, min_left)
delayed_job_id = MailWorker.perform_in(min_left * 60, due_date.parent_id, deadline_type, due_date.due_at)
delayed_job_id
end
</pre>
The first parameter after the time delay must be assignment ID, and <code>due_date.parent_id</code> is not an assignment ID.
</blockquote>
* Removing whitespace issues reported by codeclimate
* Refactoring all variable names to be in snake_case
* Removing identical blocks of code
* Adding method comments to make methods more understandable
* ...

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:12:49Z

Agcarrut: /* MailWorker Test */

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:12:00Z

Agcarrut: /* MailWorker Test */

File:Change late policy.png

2021-10-26T17:11:13Z

Agcarrut:

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:08:05Z

Agcarrut: /* MailWorker Test */

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T17:02:41Z

Agcarrut: /* MailWorker Test */

File:Change it.PNG

2021-10-26T17:00:09Z

Agcarrut:

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-26T16:58:19Z

Agcarrut: /* MailWorker Test */

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-20T20:28:53Z

Agcarrut: Added more code cleanup that was done

CSC/ECE 517 Fall 2021 - E2144. Refactor delayed mailer and scheduled task

2021-10-20T20:25:25Z

Agcarrut: Added info on drop_one_member_topics