Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-05-04T20:20:26Z

Dbell5: /* Files to be modified */

This wiki page contains description of changes designed, and implemented for [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for CSC/ECE 517, Spring 2020.

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''Controllers'''
**''assignments_controller.rb'': To refresh the topics list when changing tabs
**''popup_controller.rb'': To add a potential error message to the rubric view scores popup

*'''Models'''
**''assignment.rb'': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
**''assignment_form.rb'': Add topic ids to created assignment questionnaires
**''assignment_questionnaire.rb'': Add topic id to assignment questionnaire model
**''review_response_map.rb'': To allow finding review questionnaires by topic id
**''sign_up_topic.rb'': To allow a topic to have many assignment questionnaires attach to it (via topic id)

*'''Views'''
**''assignments/edit.html.erb'': To move topic editing view to its own file that is rendered as part of assignment edit
**''edit/_rubrics.html.erb'': Factor out common code into a function, update to use topic id
**''edit/_topics.html.erb'': Topic editing view that was moved from assignment edit view
**''popup/view_review_scores_popup.html.erb'': Assignments that vary by topic should not be displayed, instead getting error
**''sign_up_sheet/_table_line.html.erb'': Add questionnaires to signup sheet table if assignment varies by topic

*'''Helpers'''
**''assignment_helper.rb'': To add a topic id to the searchable fields for a questionnaire

*'''DB Migrate'''
**''XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb'': Migration to add topic id to assignment questionnaire schema

*'''All of the related test files to accommodate the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''Controllers'''
**''assignments_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''grades_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''popup_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''Models'''
**''assignment.rb'': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
**''assignment_form.rb'': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
**''assignment_participant.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''feedback_response_map.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''on_the_fly_calc.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''self_review_response_map.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''tag_prompt_deployment.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''Views'''
**''edit/_rubrics.html.erb'': Modify to set assignment vary by round/topic fields instead of non persisted flags

*'''Helpers'''
**''assignment_helper.rb'': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
**''grades_helper.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''summary_helper.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''DB Migrate'''
**''XXXXXXXXX_add_vary_by_topic_to_assignments.rb'': Migration to add “vary by topic” field to assignment
**''XXXXXXXXX_add_vary_by_round_to_assignments.rb'': Migration to add “vary by round” field to assignment

*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.
[[File:HW04_table.PNG]]

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==New Implementation==
While trying to integrate the previous team's implementation of this feature, we discovered that their method for querying AssignmentQuestionnaires (AQs) was flawed.

It would always query for the AQs by assignment id, current round number, and current topic id.

However, if the assignment did not have reviews that vary by round or topic, these values would be nil on the AQ, and the previous query would fail.

To fix this, we modified the ''assignment_questionnaire'' function in the AssignmentForm model to check the Assignment's ''vary_by_round'' and ''vary_by_topic'' flags, and then add the corresponding fields to the query.

This means that:
* If an assignment does not vary by round or by topic, the query for AQs will be by assignment id only.
* If the assignment varies by round but not topic, the query for AQs will be by assignment id and round number.

[[File:AQNewImpl.PNG]]

==Testing Plan==
As part of our implementation, we modified existing code as well as added new code. To ensure that existing functionality was not broken, and new functionality worked as expected, we used the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*The following existing RSpec test files have been modified and they pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced the following new RSpec test files. We have retained those file.
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*All these rspec tests passed.

===Manual Testing in Expertiza UI===
Here we describe manual UI Testing steps to edit an existing assignment to allow it have to specialized rubrics for different topic types. These steps are also shown in recorded demo video.
*Login to Expertiza using instructor account (For testing, username: '''instructor6''', password: '''password''')
*Click on Manage > Assignments
*Click on Edit option for any assignment, you should get following view. Make sure '''Has topics?''' box is checked.
[[File:Assignment edit 3.png]]
*Click on Rubrics tab. You will see 2 checkboxes ('''Review rubric varies by round?''', '''Review rubric varies by topic?''')
*Check the box for '''Review rubric varies by topic?'''
*Go to Topics tab and verify that there is dropdown menu beside each Topic.
*Select a rubric from dropdown menu, and click '''Save'''
[[File:Topics rubric 2.png]]
*Go back to Home, and select the same assignment to edit. When you click on Topics tab, you should see the rubric you had selected.

===Coverage===
All rspec tests part of [https://travis-ci.org/github/expertiza/expertiza/builds/679301186 Travis CI build] has passed. The [https://coveralls.io/jobs/61850924 code coverage] has increased by 10.9% to 41.425%.

== Future Work==
After merging the previous team's work with expertiza, several RSpec tests failed.
*This was due to how questionnaires were retrieved/found for an assignment. Staggered deadlines also rely on finding questionnaires using topic id, which isn't handled until after an assignment is made.
*'''Resolved'''
However, undoing filtering results in even more errors across models, controllers, and forms. This feature, however is a quality of life improvement and is less of a priority.
* '''Unresolved'''
Another quality of life improvement is that staggered deadlines and topics cannot be added to an assignment until after the assignment has been created and saved.
* This issue exists on expertiza's beta branch itself, separate from our changes.
*'''Unresolved'''

==Useful Links==
*'''Github''': https://github.com/abhishekupadhyaya/expertiza
*'''Pull Request''': https://github.com/expertiza/expertiza/pull/1720
*'''Demo Recording''': https://www.youtube.com/watch?v=3Ns3DYGtJdQ

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-05-04T20:16:11Z

Dbell5: /* Files to be modified */

This wiki page contains description of changes designed, and implemented for [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for CSC/ECE 517, Spring 2020.

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''Controllers'''
**''assignments_controller.rb'': To refresh the topics list when changing tabs
**''popup_controller.rb'': To add a potential error message to the rubric view scores popup

*'''Models'''
**''assignment.rb'': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
**''assignment_form.rb'': Add topic ids to created assignment questionnaires
**''assignment_questionnaire.rb'': Add topic id to assignment questionnaire model
**''review_response_map.rb'': To allow finding review questionnaires by topic id
**''sign_up_topic.rb'': To allow a topic to have many assignment questionnaires attach to it (via topic id)

*'''Views'''
**''edit.html.erb'': To move topic editing view to its own file that is rendered as part of assignment edit
**''_rubrics.html.erb'': Factor out common code into a function, update to use topic id
**''_topics.html.erb'': Topic editing view that was moved from assignment edit view
**''view_review_scores_popup.html.erb'': Assignments that vary by topic should not be displayed, instead getting error
**''_table_line.html.erb'': Add questionnaires to signup sheet table if assignment varies by topic

*'''Helpers'''
**''assignment_helper.rb'': To add a topic id to the searchable fields for a questionnaire

*'''DB Migrate'''
**''XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb'': Migration to add topic id to assignment questionnaire schema

*'''All of the related test files to accommodate the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''Controllers'''
**''assignments_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''grades_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''popup_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''Models'''
**''assignment.rb'': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
**''assignment_form.rb'': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
**''assignment_participant.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''feedback_response_map.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''on_the_fly_calc.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''self_review_response_map.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''tag_prompt_deployment.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''Views'''
**''_rubrics.html.erb'': Modify to set assignment vary by round/topic fields instead of non persisted flags

*'''Helpers'''
**''assignment_helper.rb'': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
**''grades_helper.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''summary_helper.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''DB Migrate'''
**''XXXXXXXXX_add_vary_by_topic_to_assignments.rb'': Migration to add “vary by topic” field to assignment
**''XXXXXXXXX_add_vary_by_round_to_assignments.rb'': Migration to add “vary by round” field to assignment

*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.
[[File:HW04_table.PNG]]

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==New Implementation==
While trying to integrate the previous team's implementation of this feature, we discovered that their method for querying AssignmentQuestionnaires (AQs) was flawed.

It would always query for the AQs by assignment id, current round number, and current topic id.

However, if the assignment did not have reviews that vary by round or topic, these values would be nil on the AQ, and the previous query would fail.

To fix this, we modified the ''assignment_questionnaire'' function in the AssignmentForm model to check the Assignment's ''vary_by_round'' and ''vary_by_topic'' flags, and then add the corresponding fields to the query.

This means that:
* If an assignment does not vary by round or by topic, the query for AQs will be by assignment id only.
* If the assignment varies by round but not topic, the query for AQs will be by assignment id and round number.

[[File:AQNewImpl.PNG]]

==Testing Plan==
As part of our implementation, we modified existing code as well as added new code. To ensure that existing functionality was not broken, and new functionality worked as expected, we used the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*The following existing RSpec test files have been modified and they pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced the following new RSpec test files. We have retained those file.
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*All these rspec tests passed.

===Manual Testing in Expertiza UI===
Here we describe manual UI Testing steps to edit an existing assignment to allow it have to specialized rubrics for different topic types. These steps are also shown in recorded demo video.
*Login to Expertiza using instructor account (For testing, username: '''instructor6''', password: '''password''')
*Click on Manage > Assignments
*Click on Edit option for any assignment, you should get following view. Make sure '''Has topics?''' box is checked.
[[File:Assignment edit 3.png]]
*Click on Rubrics tab. You will see 2 checkboxes ('''Review rubric varies by round?''', '''Review rubric varies by topic?''')
*Check the box for '''Review rubric varies by topic?'''
*Go to Topics tab and verify that there is dropdown menu beside each Topic.
*Select a rubric from dropdown menu, and click '''Save'''
[[File:Topics rubric 2.png]]
*Go back to Home, and select the same assignment to edit. When you click on Topics tab, you should see the rubric you had selected.

===Coverage===
All rspec tests part of [https://travis-ci.org/github/expertiza/expertiza/builds/679301186 Travis CI build] has passed. The [https://coveralls.io/jobs/61850924 code coverage] has increased by 10.9% to 41.425%.

== Future Work==
After merging the previous team's work with expertiza, several RSpec tests failed.
*This was due to how questionnaires were retrieved/found for an assignment. Staggered deadlines also rely on finding questionnaires using topic id, which isn't handled until after an assignment is made.
*'''Resolved'''
However, undoing filtering results in even more errors across models, controllers, and forms. This feature, however is a quality of life improvement and is less of a priority.
* '''Unresolved'''
Another quality of life improvement is that staggered deadlines and topics cannot be added to an assignment until after the assignment has been created and saved.
* This issue exists on expertiza's beta branch itself, separate from our changes.
*'''Unresolved'''

==Useful Links==
*'''Github''': https://github.com/abhishekupadhyaya/expertiza
*'''Pull Request''': https://github.com/expertiza/expertiza/pull/1720
*'''Demo Recording''': https://www.youtube.com/watch?v=3Ns3DYGtJdQ

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-05-04T20:15:36Z

Dbell5: /* Files to be modified */

This wiki page contains description of changes designed, and implemented for [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for CSC/ECE 517, Spring 2020.

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''Controllers'''
**''assignments_controller.rb'': To refresh the topics list when changing tabs
**''popup_controller.rb'': To add a potential error message to the rubric view scores popup

*'''Models'''
**''assignment.rb'': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
**''assignment_form.rb'': Add topic ids to created assignment questionnaires
**''assignment_questionnaire.rb'': Add topic id to assignment questionnaire model
**''review_response_map.rb'': To allow finding review questionnaires by topic id
**''sign_up_topic.rb'': To allow a topic to have many assignment questionnaires attach to it (via topic id)

*'''Views'''
**''edit.html.erb'': To move topic editing view to its own file that is rendered as part of assignment edit
**''_rubrics.html.erb'': Factor out common code into a function, update to use topic id
**''_topics.html.erb'': Topic editing view that was moved from assignment edit view
**''view_review_scores_popup.html.erb'': Assignments that vary by topic should not be displayed, instead getting error
**''_table_line.html.erb'': Add questionnaires to signup sheet table if assignment varies by topic

*'''Helpers'''
**''assignment_helper.rb'': To add a topic id to the searchable fields for a questionnaire

*DB Migrate
**''XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb'': Migration to add topic id to assignment questionnaire schema

*'''All of the related test files to accommodate the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''Controllers'''
**''assignments_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''grades_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''popup_controller.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''Models'''
**''assignment.rb'': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
**''assignment_form.rb'': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
**''assignment_participant.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''feedback_response_map.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''on_the_fly_calc.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''self_review_response_map.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''tag_prompt_deployment.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''Views'''
**''_rubrics.html.erb'': Modify to set assignment vary by round/topic fields instead of non persisted flags

*'''Helpers'''
**''assignment_helper.rb'': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
**''grades_helper.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**''summary_helper.rb'': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''DB Migrate'''
**''XXXXXXXXX_add_vary_by_topic_to_assignments.rb'': Migration to add “vary by topic” field to assignment
**''XXXXXXXXX_add_vary_by_round_to_assignments.rb'': Migration to add “vary by round” field to assignment

*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.
[[File:HW04_table.PNG]]

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==New Implementation==
While trying to integrate the previous team's implementation of this feature, we discovered that their method for querying AssignmentQuestionnaires (AQs) was flawed.

It would always query for the AQs by assignment id, current round number, and current topic id.

However, if the assignment did not have reviews that vary by round or topic, these values would be nil on the AQ, and the previous query would fail.

To fix this, we modified the ''assignment_questionnaire'' function in the AssignmentForm model to check the Assignment's ''vary_by_round'' and ''vary_by_topic'' flags, and then add the corresponding fields to the query.

This means that:
* If an assignment does not vary by round or by topic, the query for AQs will be by assignment id only.
* If the assignment varies by round but not topic, the query for AQs will be by assignment id and round number.

[[File:AQNewImpl.PNG]]

==Testing Plan==
As part of our implementation, we modified existing code as well as added new code. To ensure that existing functionality was not broken, and new functionality worked as expected, we used the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*The following existing RSpec test files have been modified and they pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced the following new RSpec test files. We have retained those file.
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*All these rspec tests passed.

===Manual Testing in Expertiza UI===
Here we describe manual UI Testing steps to edit an existing assignment to allow it have to specialized rubrics for different topic types. These steps are also shown in recorded demo video.
*Login to Expertiza using instructor account (For testing, username: '''instructor6''', password: '''password''')
*Click on Manage > Assignments
*Click on Edit option for any assignment, you should get following view. Make sure '''Has topics?''' box is checked.
[[File:Assignment edit 3.png]]
*Click on Rubrics tab. You will see 2 checkboxes ('''Review rubric varies by round?''', '''Review rubric varies by topic?''')
*Check the box for '''Review rubric varies by topic?'''
*Go to Topics tab and verify that there is dropdown menu beside each Topic.
*Select a rubric from dropdown menu, and click '''Save'''
[[File:Topics rubric 2.png]]
*Go back to Home, and select the same assignment to edit. When you click on Topics tab, you should see the rubric you had selected.

===Coverage===
All rspec tests part of [https://travis-ci.org/github/expertiza/expertiza/builds/679301186 Travis CI build] has passed. The [https://coveralls.io/jobs/61850924 code coverage] has increased by 10.9% to 41.425%.

== Future Work==
After merging the previous team's work with expertiza, several RSpec tests failed.
*This was due to how questionnaires were retrieved/found for an assignment. Staggered deadlines also rely on finding questionnaires using topic id, which isn't handled until after an assignment is made.
*'''Resolved'''
However, undoing filtering results in even more errors across models, controllers, and forms. This feature, however is a quality of life improvement and is less of a priority.
* '''Unresolved'''
Another quality of life improvement is that staggered deadlines and topics cannot be added to an assignment until after the assignment has been created and saved.
* This issue exists on expertiza's beta branch itself, separate from our changes.
*'''Unresolved'''

==Useful Links==
*'''Github''': https://github.com/abhishekupadhyaya/expertiza
*'''Pull Request''': https://github.com/expertiza/expertiza/pull/1720
*'''Demo Recording''': https://www.youtube.com/watch?v=3Ns3DYGtJdQ

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-05-04T20:07:01Z

Dbell5: /* Files to be modified */

This wiki page contains description of changes designed, and implemented for [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for CSC/ECE 517, Spring 2020.

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''Controllers'''
**assignments_controller.rb: To refresh the topics list when changing tabs
**popup_controller.rb: To add a potential error message to the rubric view scores popup

*'''Models'''
**assignment.rb: Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
**assignment_form.rb: Add topic ids to created assignment questionnaires
**assignment_questionnaire.rb: Add topic id to assignment questionnaire model
**review_response_map.rb: To allow finding review questionnaires by topic id
**sign_up_topic.rb: To allow a topic to have many assignment questionnaires attach to it (via topic id)

*'''Views'''
**edit.html.erb: To move topic editing view to its own file that is rendered as part of assignment edit
**_rubrics.html.erb: Factor out common code into a function, update to use topic id
**_topics.html.erb: Topic editing view that was moved from assignment edit view
**view_review_scores_popup.html.erb: Assignments that vary by topic should not be displayed, instead getting error
**_table_line.html.erb: Add questionnaires to signup sheet table if assignment varies by topic

*'''Helpers'''
**assignment_helper.rb: To add a topic id to the searchable fields for a questionnaire

*DB Migrate
**XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb: Migration to add topic id to assignment questionnaire schema

*'''All of the related test files to accommodate the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*Controllers
**'''app/controllers/assignments_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**'''app/controllers/grades_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**'''app/controllers/popup_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*Models
**'''app/models/assignment.rb''': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
**'''app/models/assignment_form.rb''': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
**'''app/models/assignment_participant.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**'''app/models/feedback_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**'''app/models/on_the_fly_calc.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**'''app/models/self_review_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**'''app/models/tag_prompt_deployment.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*Views
**'''app/views/assignments/edit/_rubrics.html.erb''': Modify to set assignment vary by round/topic fields instead of non persisted flags

*Helpers
**'''app/helpers/assignment_helper.rb''': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
**'''app/helpers/grades_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
**'''app/helpers/summary_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*DB Migrate
**'''db/migrate/XXXXXXXXX_add_vary_by_topic_to_assignments.rb''': Migration to add “vary by topic” field to assignment
**'''db/migrate/XXXXXXXXX_add_vary_by_round_to_assignments.rb''': Migration to add “vary by round” field to assignment

*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.
[[File:HW04_table.PNG]]

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==New Implementation==
While trying to integrate the previous team's implementation of this feature, we discovered that their method for querying AssignmentQuestionnaires (AQs) was flawed.

It would always query for the AQs by assignment id, current round number, and current topic id.

However, if the assignment did not have reviews that vary by round or topic, these values would be nil on the AQ, and the previous query would fail.

To fix this, we modified the ''assignment_questionnaire'' function in the AssignmentForm model to check the Assignment's ''vary_by_round'' and ''vary_by_topic'' flags, and then add the corresponding fields to the query.

This means that:
* If an assignment does not vary by round or by topic, the query for AQs will be by assignment id only.
* If the assignment varies by round but not topic, the query for AQs will be by assignment id and round number.

[[File:AQNewImpl.PNG]]

==Testing Plan==
As part of our implementation, we modified existing code as well as added new code. To ensure that existing functionality was not broken, and new functionality worked as expected, we used the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*The following existing RSpec test files have been modified and they pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced the following new RSpec test files. We have retained those file.
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*All these rspec tests passed.

===Manual Testing in Expertiza UI===
Here we describe manual UI Testing steps to edit an existing assignment to allow it have to specialized rubrics for different topic types. These steps are also shown in recorded demo video.
*Login to Expertiza using instructor account (For testing, username: '''instructor6''', password: '''password''')
*Click on Manage > Assignments
*Click on Edit option for any assignment, you should get following view. Make sure '''Has topics?''' box is checked.
[[File:Assignment edit 3.png]]
*Click on Rubrics tab. You will see 2 checkboxes ('''Review rubric varies by round?''', '''Review rubric varies by topic?''')
*Check the box for '''Review rubric varies by topic?'''
*Go to Topics tab and verify that there is dropdown menu beside each Topic.
*Select a rubric from dropdown menu, and click '''Save'''
[[File:Topics rubric 2.png]]
*Go back to Home, and select the same assignment to edit. When you click on Topics tab, you should see the rubric you had selected.

===Coverage===
All rspec tests part of [https://travis-ci.org/github/expertiza/expertiza/builds/679301186 Travis CI build] has passed. The [https://coveralls.io/jobs/61850924 code coverage] has increased by 10.9% to 41.425%.

== Future Work==
After merging the previous team's work with expertiza, several RSpec tests failed.
*This was due to how questionnaires were retrieved/found for an assignment. Staggered deadlines also rely on finding questionnaires using topic id, which isn't handled until after an assignment is made.
*'''Resolved'''
However, undoing filtering results in even more errors across models, controllers, and forms. This feature, however is a quality of life improvement and is less of a priority.
* '''Unresolved'''
Another quality of life improvement is that staggered deadlines and topics cannot be added to an assignment until after the assignment has been created and saved.
* This issue exists on expertiza's beta branch itself, separate from our changes.
*'''Unresolved'''

==Useful Links==
*'''Github''': https://github.com/abhishekupadhyaya/expertiza
*'''Pull Request''': https://github.com/expertiza/expertiza/pull/1720
*'''Demo Recording''': https://www.youtube.com/watch?v=3Ns3DYGtJdQ

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-04-25T04:33:33Z

Dbell5: Undo revision 134293 by Dbell5 (talk)

This wiki page contains description of changes designed, and implemented for [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for CSC/ECE 517, Spring 2020.

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''app/controllers/assignments_controller.rb''': To refresh the topics list when changing tabs
*'''app/controllers/popup_controller.rb''': To add a potential error message to the rubric view scores popup
*'''app/helpers/assignment_helper.rb''': To add a topic id to the searchable fields for a questionnaire
*'''app/models/assignment.rb''': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
*'''app/models/assignment_form.rb''': Add topic ids to created assignment questionnaires
*'''app/models/assignment_questionnaire.rb''': Add topic id to assignment questionnaire model
*'''app/models/review_response_map.rb''': To allow finding review questionnaires by topic id
*'''app/models/sign_up_topic.rb''': To allow a topic to have many assignment questionnaires attach to it (via topic id)
*'''app/views/assignments/edit.html.erb''': To move topic editing view to its own file that is rendered as part of assignment edit
*'''app/views/assignments/edit/_rubrics.html.erb''': Factor out common code into a function, update to use topic id
*'''app/views/assignments/edit/_topics.html.erb''': Topic editing view that was moved from assignment edit view
*'''app/views/popup/view_review_scores_popup.html.erb''': Assignments that vary by topic should not be displayed, instead getting error
*'''app/views/sign_up_sheet/_table_line.html.erb''': Add questionnaires to signup sheet table if assignment varies by topic
*'''db/migrate/XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb''': Migration to add topic id to assignment questionnaire schema
*'''...As well as many test files related to the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''app/models/assignment.rb''': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
*'''app/helpers/assignment_helper.rb''': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
*'''app/models/assignment_form.rb''': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
*'''app/views/assignments/edit/_rubrics.html.erb''': Modify to set assignment vary by round/topic fields instead of non persisted flags
*'''db/migrate/XXXXXXXXX_add_vary_by_topic_to_assignments.rb''': Migration to add “vary by topic” field to assignment
*'''db/migrate/XXXXXXXXX_add_vary_by_round_to_assignments.rb''': Migration to add “vary by round” field to assignment
*'''app/controllers/assignments_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/grades_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/popup_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/helpers/grades_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/helpers/summary_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/assignment_participant.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/feedback_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/on_the_fly_calc.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/self_review_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/tag_prompt_deployment.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.
[[File:HW04_table.PNG]]

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==New Implementation==
While trying to integrate the previous team's implementation of this feature, we discovered that their method for querying AssignmentQuestionnaires (AQs) was flawed.

It would always query for the AQs by assignment id, current round number, and current topic id.

However, if the assignment did not have reviews that vary by round or topic, these values would be nil on the AQ, and the previous query would fail.

To fix this, we modified the ''assignment_questionnaire'' function in the AssignmentForm model to check the Assignment's ''vary_by_round'' and ''vary_by_topic'' flags, and then add the corresponding fields to the query.

This means that:
* If an assignment does not vary by round or by topic, the query for AQs will be by assignment id only.
* If the assignment varies by round but not topic, the query for AQs will be by assignment id and round number.
* etc..

[[File:AQNewImpl.PNG]]

==Testing Plan==
As part of our implementation, we would need to modify existing code as well as add new code. To ensure that existing functionality is not broken, and new functionality works as expected, we will be using the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*The following existing RSpec test files will be modified and they should all pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced the following new RSpec test files. We plan to use these rspec files.
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*All these rspec tests should pass for us to complete the testing.

===Test the updated/new features on Expertiza UI instance===
*Since the changes in the project would be related to topics, and rubrics for assignment, those features will be tested in Expertiza UI. Current view of Topics and Rubrics tab looks as follows:
[[File:Edit assignment rubric.png]]
[[File:Edit assignment topic.png]]

Once the changes are implemented, additional check boxes, and dropdown fields will be added in both these tabs. For example: as shown in the following screenshot, we will have an additional check box '''"Review rubric varies by topic?"''' in the Rubrics tab.

[[File:Ui testing checkbox.png]]

When the check box '''"Review rubric varies by topic?"''' is checked, a new '''Questionnaire''' column with drop down menu will be added in the Topics tab as shown in the screenshot below taken from the previous team's implementation demo.

[[File:Ui testing checkbox selected topics.png]]

===Coverage===
As can be seen from [https://coveralls.io/jobs/61850438 Coverall], our changes have increased the code coverage by 21.1% to 51.615%.

== Future Work==
After merging the previous team's work with expertiza, several RSpec tests failed.
*This was due to how questionnaires were retrieved/found for an assignment. Staggered deadlines also rely on finding questionnaires using topic id, which isn't handled until after an assignment is made.
*'''Resolved'''
However, undoing filtering results in even more errors across models, controllers, and forms. This feature, however is a quality of life improvement and is less of a priority.
* '''Unresolved'''
Another quality of life improvement is that staggered deadlines and topics cannot be added to an assignment until after the assignment has been created and saved.
* This issue exists on expertiza's beta branch itself, separate from our changes.
*'''Unresolved'''

==Useful Links==
*'''Github''': https://github.com/abhishekupadhyaya/expertiza
*'''Pull Request''': https://github.com/expertiza/expertiza/pull/1720
*'''Demo Recording''': https://www.youtube.com/watch?v=3Ns3DYGtJdQ

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-04-25T04:32:24Z

Dbell5: /* Previous Implementation */

This wiki page contains description of changes designed, and implemented for [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for CSC/ECE 517, Spring 2020.

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://youtu.be/3Ns3DYGtJdQ

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''app/controllers/assignments_controller.rb''': To refresh the topics list when changing tabs
*'''app/controllers/popup_controller.rb''': To add a potential error message to the rubric view scores popup
*'''app/helpers/assignment_helper.rb''': To add a topic id to the searchable fields for a questionnaire
*'''app/models/assignment.rb''': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
*'''app/models/assignment_form.rb''': Add topic ids to created assignment questionnaires
*'''app/models/assignment_questionnaire.rb''': Add topic id to assignment questionnaire model
*'''app/models/review_response_map.rb''': To allow finding review questionnaires by topic id
*'''app/models/sign_up_topic.rb''': To allow a topic to have many assignment questionnaires attach to it (via topic id)
*'''app/views/assignments/edit.html.erb''': To move topic editing view to its own file that is rendered as part of assignment edit
*'''app/views/assignments/edit/_rubrics.html.erb''': Factor out common code into a function, update to use topic id
*'''app/views/assignments/edit/_topics.html.erb''': Topic editing view that was moved from assignment edit view
*'''app/views/popup/view_review_scores_popup.html.erb''': Assignments that vary by topic should not be displayed, instead getting error
*'''app/views/sign_up_sheet/_table_line.html.erb''': Add questionnaires to signup sheet table if assignment varies by topic
*'''db/migrate/XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb''': Migration to add topic id to assignment questionnaire schema
*'''...As well as many test files related to the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''app/models/assignment.rb''': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
*'''app/helpers/assignment_helper.rb''': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
*'''app/models/assignment_form.rb''': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
*'''app/views/assignments/edit/_rubrics.html.erb''': Modify to set assignment vary by round/topic fields instead of non persisted flags
*'''db/migrate/XXXXXXXXX_add_vary_by_topic_to_assignments.rb''': Migration to add “vary by topic” field to assignment
*'''db/migrate/XXXXXXXXX_add_vary_by_round_to_assignments.rb''': Migration to add “vary by round” field to assignment
*'''app/controllers/assignments_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/grades_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/popup_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/helpers/grades_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/helpers/summary_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/assignment_participant.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/feedback_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/on_the_fly_calc.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/self_review_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/tag_prompt_deployment.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.
[[File:HW04_table.PNG]]

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==New Implementation==
While trying to integrate the previous team's implementation of this feature, we discovered that their method for querying AssignmentQuestionnaires (AQs) was flawed.

It would always query for the AQs by assignment id, current round number, and current topic id.

However, if the assignment did not have reviews that vary by round or topic, these values would be nil on the AQ, and the previous query would fail.

To fix this, we modified the ''assignment_questionnaire'' function in the AssignmentForm model to check the Assignment's ''vary_by_round'' and ''vary_by_topic'' flags, and then add the corresponding fields to the query.

This means that:
* If an assignment does not vary by round or by topic, the query for AQs will be by assignment id only.
* If the assignment varies by round but not topic, the query for AQs will be by assignment id and round number.
* etc..

[[File:AQNewImpl.PNG]]

==Testing Plan==
As part of our implementation, we would need to modify existing code as well as add new code. To ensure that existing functionality is not broken, and new functionality works as expected, we will be using the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*The following existing RSpec test files will be modified and they should all pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced the following new RSpec test files. We plan to use these rspec files.
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*All these rspec tests should pass for us to complete the testing.

===Test the updated/new features on Expertiza UI instance===
*Since the changes in the project would be related to topics, and rubrics for assignment, those features will be tested in Expertiza UI. Current view of Topics and Rubrics tab looks as follows:
[[File:Edit assignment rubric.png]]
[[File:Edit assignment topic.png]]

Once the changes are implemented, additional check boxes, and dropdown fields will be added in both these tabs. For example: as shown in the following screenshot, we will have an additional check box '''"Review rubric varies by topic?"''' in the Rubrics tab.

[[File:Ui testing checkbox.png]]

When the check box '''"Review rubric varies by topic?"''' is checked, a new '''Questionnaire''' column with drop down menu will be added in the Topics tab as shown in the screenshot below taken from the previous team's implementation demo.

[[File:Ui testing checkbox selected topics.png]]

===Coverage===
As can be seen from [https://coveralls.io/jobs/61850438 Coverall], our changes have increased the code coverage by 21.1% to 51.615%.

== Future Work==
After merging the previous team's work with expertiza, several RSpec tests failed.
*This was due to how questionnaires were retrieved/found for an assignment. Staggered deadlines also rely on finding questionnaires using topic id, which isn't handled until after an assignment is made.
*'''Resolved'''
However, undoing filtering results in even more errors across models, controllers, and forms. This feature, however is a quality of life improvement and is less of a priority.
* '''Unresolved'''
Another quality of life improvement is that staggered deadlines and topics cannot be added to an assignment until after the assignment has been created and saved.
* This issue exists on expertiza's beta branch itself, separate from our changes.
*'''Unresolved'''

==Useful Links==
*'''Github''': https://github.com/abhishekupadhyaya/expertiza
*'''Pull Request''': https://github.com/expertiza/expertiza/pull/1720
*'''Demo Recording''': https://www.youtube.com/watch?v=LukuCG5XBl8

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-04-25T02:51:05Z

Dbell5:

This wiki page contains description of changes planned for [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for CSC/ECE 517, Spring 2020.

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''app/controllers/assignments_controller.rb''': To refresh the topics list when changing tabs
*'''app/controllers/popup_controller.rb''': To add a potential error message to the rubric view scores popup
*'''app/helpers/assignment_helper.rb''': To add a topic id to the searchable fields for a questionnaire
*'''app/models/assignment.rb''': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
*'''app/models/assignment_form.rb''': Add topic ids to created assignment questionnaires
*'''app/models/assignment_questionnaire.rb''': Add topic id to assignment questionnaire model
*'''app/models/review_response_map.rb''': To allow finding review questionnaires by topic id
*'''app/models/sign_up_topic.rb''': To allow a topic to have many assignment questionnaires attach to it (via topic id)
*'''app/views/assignments/edit.html.erb''': To move topic editing view to its own file that is rendered as part of assignment edit
*'''app/views/assignments/edit/_rubrics.html.erb''': Factor out common code into a function, update to use topic id
*'''app/views/assignments/edit/_topics.html.erb''': Topic editing view that was moved from assignment edit view
*'''app/views/popup/view_review_scores_popup.html.erb''': Assignments that vary by topic should not be displayed, instead getting error
*'''app/views/sign_up_sheet/_table_line.html.erb''': Add questionnaires to signup sheet table if assignment varies by topic
*'''db/migrate/XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb''': Migration to add topic id to assignment questionnaire schema
*'''...As well as many test files related to the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''app/models/assignment.rb''': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
*'''app/helpers/assignment_helper.rb''': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
*'''app/models/assignment_form.rb''': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
*'''app/views/assignments/edit/_rubrics.html.erb''': Modify to set assignment vary by round/topic fields instead of non persisted flags
*'''db/migrate/XXXXXXXXX_add_vary_by_topic_to_assignments.rb''': Migration to add “vary by topic” field to assignment
*'''db/migrate/XXXXXXXXX_add_vary_by_round_to_assignments.rb''': Migration to add “vary by round” field to assignment
*'''app/controllers/assignments_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/grades_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/popup_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/helpers/grades_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/helpers/summary_helper.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/assignment_participant.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/feedback_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/on_the_fly_calc.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/self_review_response_map.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/models/tag_prompt_deployment.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods

*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.
[[File:HW04_table.PNG]]

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==Testing Plan==
As part of our implementation, we would need to modify existing code as well as add new code. To ensure that existing functionality is not broken, and new functionality works as expected, we will be using the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*The following existing RSpec test files will be modified and they should all pass as part of testing:
**spec/controllers/assignments_controller_spec.rb
**spec/controllers/questionnaires_controller_spec.rb
**spec/controllers/response_controller_spec.rb
**spec/factories/factories.rb
**spec/features/assignment_creation_spec.rb
**spec/features/quiz_spec.rb
**spec/features/staggered_deadline_spec.rb
**spec/models/assignment_form_spec.rb
**spec/models/assignment_spec.rb
**spec/models/on_the_fly_calc_spec.rb
**spec/models/response_spec.rb
**spec/models/review_response_map_spec.rb

===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced the following new RSpec test files. We plan to use these rspec files.
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*All these rspec tests should pass for us to complete the testing.

===Test the updated/new features on Expertiza UI instance===
*Since the changes in the project would be related to topics, and rubrics for assignment, those features will be tested in Expertiza UI. Current view of Topics and Rubrics tab looks as follows:
[[File:Edit assignment rubric.png]]
[[File:Edit assignment topic.png]]

Once the changes are implemented, additional check boxes, and dropdown fields will be added in both these tabs. For example: as shown in the following screenshot, we will have an additional check box '''"Review rubric varies by topic?"''' in the Rubrics tab.

[[File:Ui testing checkbox.png]]

When the check box '''"Review rubric varies by topic?"''' is checked, a new '''Questionnaire''' column with drop down menu will be added in the Topics tab as shown in the screenshot below taken from the previous team's implementation demo.

[[File:Ui testing checkbox selected topics.png]]

== Obstacles ==
While resolving tests for the Travis build to pass, we finally reached:

This error does not appear on the local tests, so it is out of our control and related to Travis itself.

== Future Work==
After merging the previous team's work with expertiza, several RSpec tests failed.
*This was due to how questionnaires were retrieved/found for an assignment. Staggered deadlines also rely on finding questionnaires using topic id, which isn't handled until after an assignment is made.
*'''Resolved'''
However, undoing filtering results in even more errors across models, controllers, and forms. This feature, however is a quality of life improvement and is less of a priority.
* '''Unresolved'''
Another quality of life improvement is that staggered deadlines and topics cannot be added to an assignment until after the assignment has been created and saved.
* This issue exists on expertiza's beta branch itself, separate from our changes.
*'''Unresolved'''

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020

2020-04-09T01:35:03Z

Dbell5:

[[CSC/ECE_517_Spring_2020_-_E2000._Refactor_stage_deadlines_in_assignment.rb]]

[[CSC/ECE 517 Spring 2020 - E2014. Refactor datetimepicker.js]]

[[CSC/ECE_517_Spring_2020/E2012._refactor_lottery_controller.rb]]

[[CSC/ECE_517_Spring_2020/P2000.Refactor_pyrh_oauth_capabilities_to_a_separate_class]]

[[CSC/ECE 517 Spring 2020 - E2015. Conflict notification]]

[[CSC/ECE_517_Spring_2020/P2000.Refactor_pyrh_oauth_capabilities_to_use_marshmallow]]

[[CSC/ECE_517_Spring_2020_-_E2004._Refactor_assignments_controller]]

[[CSC/ECE_517_Spring_2020_-_E2026._Specialized_rubrics_for_different_topic_types]]

CSC/ECE 517 Spring 2020

2020-04-09T01:33:46Z

Dbell5:

[[CSC/ECE_517_Spring_2020_-_E2000._Refactor_stage_deadlines_in_assignment.rb]]

[[CSC/ECE 517 Spring 2020 - E2014. Refactor datetimepicker.js]]

[[CSC/ECE_517_Spring_2020/E2012._refactor_lottery_controller.rb]]

[[CSC/ECE_517_Spring_2020/P2000.Refactor_pyrh_oauth_capabilities_to_a_separate_class]]

[[CSC/ECE 517 Spring 2020 - E2015. Conflict notification]]

[[CSC/ECE_517_Spring_2020/P2000.Refactor_pyrh_oauth_capabilities_to_use_marshmallow]]
[[CSC/ECE_517_Spring_2020_-_E2026._Specialized_rubrics_for_different_topic_types]]

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-04-09T01:20:14Z

Dbell5: /* Background */

This wiki page contains description of changes made as part of [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for Spring 2020, CSC/ECE 517

== Purpose==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''app/controllers/assignments_controller.rb''': To refresh the topics list when changing tabs
*'''app/controllers/popup_controller.rb''': To add a potential error message to the rubric view scores popup
*'''app/helpers/assignment_helper.rb''': To add a topic id to the searchable fields for a questionnaire
*'''app/models/assignment.rb''': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
*'''app/models/assignment_form.rb''': Add topic ids to created assignment questionnaires
*'''app/models/assignment_questionnaire.rb''': Add topic id to assignment questionnaire model
*'''app/models/review_response_map.rb''': To allow finding review questionnaires by topic id
*'''app/models/sign_up_topic.rb''': To allow a topic to have many assignment questionnaires attach to it (via topic id)
*'''app/views/assignments/edit.html.erb''': To move topic editing view to its own file that is rendered as part of assignment edit
*'''app/views/assignments/edit/_rubrics.html.erb''': Factor out common code into a function, update to use topic id
*'''app/views/assignments/edit/_topics.html.erb''': Topic editing view that was moved from assignment edit view
*'''app/views/popup/view_review_scores_popup.html.erb''': Assignments that vary by topic should not be displayed, instead getting error
*'''app/views/sign_up_sheet/_table_line.html.erb''': Add questionnaires to signup sheet table if assignment varies by topic
*'''db/migrate/XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb''': Migration to add topic id to assignment questionnaire schema
*'''...As well as many test files related to the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''app/models/assignment.rb''': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
*'''app/helpers/assignment_helper.rb''': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
*'''app/models/assignment_form.rb''': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
*'''app/views/assignments/edit/_rubrics.html.erb''': Modify to set assignment vary by round/topic fields instead of non persisted flags
*'''db/migrate/XXXXXXXXX_add_vary_by_topic_to_assignments.rb''': Migration to add “vary by topic” field to assignment
*'''db/migrate/XXXXXXXXX_add_vary_by_round_to_assignments.rb''': Migration to add “vary by round” field to assignment
*'''app/controllers/assignments_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/grades_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/popup_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''...Several other files need the above refactor as well'''
*'''All of the related test files to accommodate the above changes'''

==Database Flow==
We will be re-adding the database flow that was added in the previous implementation, linking ''sign_up_topic'' to ''assignment_questionnaire'' via a '''topic_id''' field.

In addition to that, we will be adding two additional boolean fields to the ''assignment'' schema: '''vary_by_round''' and '''vary_by_topic'''. As discussed earlier, in the previous implementation, these were methods that were called to determine if an assignment varied by round/topic rather than a persisted value.

[[File:E2026DatabaseFlow.png]]

Only a subset of the fields for each table is shown in the diagram because most of the fields are not relevant to these changes and would only serve to distract from the relevant changes. Additions are shown in bold.

==Testing Plan==
As part of our implementation, we would need to modify existing code as well as add new code. To ensure that existing functionality is not broken, and new functionality works as expected, we will be using the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*All the existing RSpec tests related to the files that have to be modified should pass.
===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced following new RSpec test files:
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*We plan to use those rspec files as well as implement new ones that might be required once we start on our implementation.
*All these rspec tests should pass for us to complete the testing.
===Test the updated/new features on Expertiza UI instance===
*Since the changes in the project would be related to topics, and rubrics for assignment, those features will be tested in Expertiza UI. Current view of Topics and Rubrics tab looks as follows:
[[File:Edit assignment rubric.png]]
[[File:Edit assignment topic.png]]

Additional check boxes, and dropdown fields will be added in both these tabs once we implement the changes. Those changes in the view as well as their functionality should be tested.

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-04-09T01:13:38Z

Dbell5: /* Design Strategy */

This wiki page contains description of changes made as part of [https://docs.google.com/document/d/1PEackBQqIcNfvD5idGUtxA9PBqLD5zoaEViWhYcX7Oo/edit#heading=h.z86zstcoj6u7 E2026. Specialized rubrics for different topic types (e.g., Servo project, refactoring project)], a Final Project for Spring 2020, CSC/ECE 517

== Background ==
In CSC/ECE 517, there are Expertiza-based course projects, Mozilla-based course projects, etc. However, currently, we can only specify one kind of rubric for all kinds of course projects. This means that refactoring projects, testing projects, and Mozilla projects need to use the same rubric. We hope we could specify different rubrics to be used with different kinds of course projects.

This project was implemented by another team in Spring 2019. We will be implementing our project on top of what was implemented by the previous team, and fix the issues that were found in their implementation. Information on what the previous team had worked on is described in the '''Previous Implementation''' section.

== Previous Implementation==
All aspects of previous implementation were good, and their design was well appreciated. Their changes were also merged to '''expertiza:beta''' branch, but were later reverted since the specialized rubrics weren’t saved in the database. This [https://github.com/expertiza/expertiza/issues/1475 github issue] provides a detailed explanation to the problem.

For us to get started, we were provided with following links from previous implementation:
*http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics
*https://github.com/gabalmat/expertiza/tree/beta
*https://github.com/expertiza/expertiza/pull/1444
*https://www.youtube.com/watch?v=F7nQsIUspQM

==Design Strategy==
This feature was previously implemented and was detailed [http://wiki.expertiza.ncsu.edu/index.php/E1936_Specialized_Rubrics here]. The feature concluded with allowing 4 rubric scenarios for an assignment:
*Rubric does not vary by round or by topic.
*Rubric varies by round, but not by topic.
*Rubric varies by topic, but not by round.
*Rubric varies by both round and topic.
However, there were two issues with the implementation:
*Rubrics in the dropdown were only those created by the logged-in instructor, so the TA wouldn’t be able to see them
*Rubrics would not be saved after selecting them and saving.
In light of these issues, [https://github.com/gabalmat/expertiza/commit/23ed2cca2976f55515ea8980646e4432858bf446 changes] to expertiza were made as follows:
*Two additional columns are added into the Assignment table that determines whether Rubrics varies by either Round or Topic with default values False
*'''update_assignment_questionnaires''' method is re-implemented
**Having extra column in the assignment questionnaire table topic_id, no need for deleting all the data and re-writing it again every single time in the DB (this caused the previous implementation to have a delay when selecting **rubrics)
**The only varying value is questionnaire_id, the rest values may not change from Topics or Rubrics tabs, but can be added
*There are 4 (four) possible cases for saving and updating data:
**used_in_round = null and topic_id = null
**used_in_round = integer and topic_id = null
**used_in_round = null and topic_id = integer
**used_in_round = integer and topic_id = integer
This solved the issue of having rubrics save
===Problem===
The feature we have to implement was not fully committed due to the previously mentioned problems, so we have to reincorporate the missing code. However, a resulting issue is the inability for an instructor or TA to use rubrics that they did not create. The current problem can be broken down into the following parts:
# Integrate the changes made from the original implementation into the current version of Expertiza
# Allow an instructor/TA to chose rubrics that aren’t only theirs
# Create and update tests to reflect the changes as needed
===Proposed Solution===
Our proposed solution keeps the previous implementation, but changes rubric filtering to allow instructors/TAs to use filters that are not theirs. Alongside the changes in the original implementation we propose:
*Allow an instructor to choose different rubrics for different topics
**PROBLEM: The drop-downs for selecting rubrics show only those rubrics created by the currently-logged in instructor (per project mentor).
***SOLUTION: Change the filtering in the questionnaire_options method to reflect the desired filtering.
***FILE: app/helpers/assignment_helper.rb

==UML Diagram==
Following the [https://expertiza.csc.ncsu.edu/index.php/E1936_Specialized_Rubrics previous implementations's] footsteps, this diagram depicts the interactions between an instructor and an assignment. The instructor may edit, delete, copy, and other stuff (already existing in Expertiza). Alongside editing topics and due dates, the instructor can edit what rubrics are assigned to an assignment. The highlighted portions are of interest. The topics tab allows instructors to specify which rubric associates with each topic while the rubrics tab lets the instructor determine if the assignment will vary by topic or not.

[[File:Use_case_diagram_1936.png]]

==Files to be modified==

Since our project is to improve upon a previous implementation that was slightly flawed, we will be modifying all of the same files that they previously modified (even if we don’t choose to alter their implementation in that file).

The major modified files from the previous implementation include:
*'''app/controllers/assignments_controller.rb''': To refresh the topics list when changing tabs
*'''app/controllers/popup_controller.rb''': To add a potential error message to the rubric view scores popup
*'''app/helpers/assignment_helper.rb''': To add a topic id to the searchable fields for a questionnaire
*'''app/models/assignment.rb''': Add methods to determine if an assignment varies by rubric/topic. (will be refactored as part of our change, however)
*'''app/models/assignment_form.rb''': Add topic ids to created assignment questionnaires
*'''app/models/assignment_questionnaire.rb''': Add topic id to assignment questionnaire model
*'''app/models/review_response_map.rb''': To allow finding review questionnaires by topic id
*'''app/models/sign_up_topic.rb''': To allow a topic to have many assignment questionnaires attach to it (via topic id)
*'''app/views/assignments/edit.html.erb''': To move topic editing view to its own file that is rendered as part of assignment edit
*'''app/views/assignments/edit/_rubrics.html.erb''': Factor out common code into a function, update to use topic id
*'''app/views/assignments/edit/_topics.html.erb''': Topic editing view that was moved from assignment edit view
*'''app/views/popup/view_review_scores_popup.html.erb''': Assignments that vary by topic should not be displayed, instead getting error
*'''app/views/sign_up_sheet/_table_line.html.erb''': Add questionnaires to signup sheet table if assignment varies by topic
*'''db/migrate/XXXXXXXXXXX_add_topic_id_to_assignment_questionnaires.rb''': Migration to add topic id to assignment questionnaire schema
*'''...As well as many test files related to the above changes'''

To address the issues brought up with the previous implementation, we will also make the following major modifications:
*'''app/models/assignment.rb''': Add persisted fields to the assignment for varying by topic/round instead of using methods to determine it
*'''app/helpers/assignment_helper.rb''': Refactor by moving function to find questionnaire / assignment questionnaire to assignment_form.rb. Remove filters that only allow instructors to see rubrics.
*'''app/models/assignment_form.rb''': No longer delete all existing questionnaires on update, update them instead. Now find questionnaire by assignment questionnaire and type rather than assignment/type/round_number/topic_id.
*'''app/views/assignments/edit/_rubrics.html.erb''': Modify to set assignment vary by round/topic fields instead of non persisted flags
*'''db/migrate/XXXXXXXXX_add_vary_by_topic_to_assignments.rb''': Migration to add “vary by topic” field to assignment
*'''db/migrate/XXXXXXXXX_add_vary_by_round_to_assignments.rb''': Migration to add “vary by round” field to assignment
*'''app/controllers/assignments_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/grades_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''app/controllers/popup_controller.rb''': Refactor to use the persisted assignment fields for varying by topic/round instead of using methods
*'''...Several other files need the above refactor as well'''
*'''All of the related test files to accommodate the above changes'''

==Database Flow==

==Testing Plan==
As part of our implementation, we would need to modify existing code as well as add new code. To ensure that existing functionality is not broken, and new functionality works as expected, we will be using the following Test Strategy (which was also used by previous team):
===Run and pass existing RSpec Tests===
*All the existing RSpec tests related to the files that have to be modified should pass.
===Develop New RSpec Tests===
*As part of previous implementation, the team had introduced following new RSpec test files:
**spec/helpers/assignment_helper_spec.rb
**spec/models/self_review_response_map_spec.rb
*We plan to use those rspec files as well as implement new ones that might be required once we start on our implementation.
*All these rspec tests should pass for us to complete the testing.
===Test the updated/new features on Expertiza UI instance===
*Since the changes in the project would be related to topics, and rubrics for assignment, those features will be tested in Expertiza UI. Current view of Topics and Rubrics tab looks as follows:
[[File:Edit assignment rubric.png]]
[[File:Edit assignment topic.png]]

Additional check boxes, and dropdown fields will be added in both these tabs once we implement the changes. Those changes in the view as well as their functionality should be tested.

==Team Information==
*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor''': Anuja Kulkarni

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-04-09T01:05:00Z

Dbell5: /* Design Strategy */

CSC/ECE 517 Spring 2020 - E2026. Specialized rubrics for different topic types

2020-04-09T00:58:49Z

Dbell5: /* UML Diagram */

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-04-01T01:34:12Z

Dbell5: /* Additional Changes */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. This controller is responsible for creating and changing assignments. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''assignment_form_save_handler''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method from 33 to 15. Assignment Branch Condition size was also reduced from 56.37 to less than 15.

[[File:Create method11.png]]

Following methods were extracted from '''assignment_form_save_handler''':
* '''fix_assignment_missing_path''' - to handle non existent directory path
* '''update_assignment_form''' - to update assignment due dates, and questionnaire array in assignment form

[[File:Create helper m1.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit method.png]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.
Thus, the methods '''update_due_date''', '''update_assignment_badges''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* user_timezone_specified flashes an error if the user did not specify a preferred time zone for an assignment.

[[File:Edit helper1.png]]

[[File:Edit helper2.png]]

[[File:Edit helper3.png]]

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Copy method.png]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

[[File:Copy helper.png]]

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list11.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.

[[File:Update_feedback_assignment_form_attr1.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Additional Changes==
You may have noticed method names have been changed. This was done to clarify what the methods do, as there were no comments to explain the methods, and shorten the names.
Methods have also been given comments to describe their functionality.

==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller1.png|20px*20px]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

===Coverage===
As can be seen from [https://coveralls.io/jobs/60782737 Coverall], our changes have increased the coverage of assignments_controller.rb by 10.9%, from 30.5% up to 41.41%. While we did not make any changes to the tests, we did reduce the amount of testable code by reducing redundancy and extracting common code in the controller.

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'
* More thorough tests can be added to the assignments_controller spec to get the coverage closer to 100%.
==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-04-01T01:33:26Z

Dbell5:

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. This controller is responsible for creating and changing assignments. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''assignment_form_save_handler''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method from 33 to 15. Assignment Branch Condition size was also reduced from 56.37 to less than 15.

[[File:Create method11.png]]

Following methods were extracted from '''assignment_form_save_handler''':
* '''fix_assignment_missing_path''' - to handle non existent directory path
* '''update_assignment_form''' - to update assignment due dates, and questionnaire array in assignment form

[[File:Create helper m1.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit method.png]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.
Thus, the methods '''update_due_date''', '''update_assignment_badges''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* user_timezone_specified flashes an error if the user did not specify a preferred time zone for an assignment.

[[File:Edit helper1.png]]

[[File:Edit helper2.png]]

[[File:Edit helper3.png]]

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Copy method.png]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

[[File:Copy helper.png]]

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list11.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.

[[File:Update_feedback_assignment_form_attr1.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Additional Changes==
You may have noticed method names have been changed. This was done to clarify what the methods do, as there were no comments to explain the methods, and shorten the names.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

===Coverage===
As can be seen from [https://coveralls.io/jobs/60782737 Coverall], our changes have increased the coverage of assignments_controller.rb by 10.9%, from 30.5% up to 41.41%. While we did not make any changes to the tests, we did reduce the amount of testable code by reducing redundancy and extracting common code in the controller.

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'
* More thorough tests can be added to the assignments_controller spec to get the coverage closer to 100%.
==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-04-01T01:30:39Z

Dbell5: /* Background */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. This controller is responsible for creating and changing assignments. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''assignment_form_save_handler''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method from 33 to 15. Assignment Branch Condition size was also reduced from 56.37 to less than 15.

[[File:Create method11.png]]

Following methods were extracted from '''assignment_form_save_handler''':
* '''fix_assignment_missing_path''' - to handle non existent directory path
* '''update_assignment_form''' - to update assignment due dates, and questionnaire array in assignment form

[[File:Create helper m1.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit method.png]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.
Thus, the methods '''update_due_date''', '''update_assignment_badges''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* user_timezone_specified flashes an error if the user did not specify a preferred time zone for an assignment.

[[File:Edit helper1.png]]

[[File:Edit helper2.png]]

[[File:Edit helper3.png]]

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Copy method.png]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

[[File:Copy helper.png]]

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list11.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.

[[File:Update_feedback_assignment_form_attr1.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

===Coverage===
As can be seen from [https://coveralls.io/jobs/60782737 Coverall], our changes have increased the coverage of assignments_controller.rb by 10.9%, from 30.5% up to 41.41%. While we did not make any changes to the tests, we did reduce the amount of testable code by reducing redundancy and extracting common code in the controller.

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'
* More thorough tests can be added to the assignments_controller spec to get the coverage closer to 100%.
==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-04-01T01:27:37Z

Dbell5: /* The branch condition size for edit method is too high */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''assignment_form_save_handler''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method from 33 to 15. Assignment Branch Condition size was also reduced from 56.37 to less than 15.

[[File:Create method11.png]]

Following methods were extracted from '''assignment_form_save_handler''':
* '''fix_assignment_missing_path''' - to handle non existent directory path
* '''update_assignment_form''' - to update assignment due dates, and questionnaire array in assignment form

[[File:Create helper m1.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit method.png]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.
Thus, the methods '''update_due_date''', '''update_assignment_badges''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* user_timezone_specified flashes an error if the user did not specify a preferred time zone for an assignment.

[[File:Edit helper1.png]]

[[File:Edit helper2.png]]

[[File:Edit helper3.png]]

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Copy method.png]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

[[File:Copy helper.png]]

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list11.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.

[[File:Update_feedback_assignment_form_attr1.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

===Coverage===
As can be seen from [https://coveralls.io/jobs/60782737 Coverall], our changes have increased the coverage of assignments_controller.rb by 10.9%, from 30.5% up to 41.41%. While we did not make any changes to the tests, we did reduce the amount of testable code by reducing redundancy and extracting common code in the controller.

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'
* More thorough tests can be added to the assignments_controller spec to get the coverage closer to 100%.
==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-04-01T01:26:13Z

Dbell5: /* The branch condition size for edit method is too high */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''assignment_form_save_handler''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method from 33 to 15. Assignment Branch Condition size was also reduced from 56.37 to less than 15.

[[File:Create method11.png]]

Following methods were extracted from '''assignment_form_save_handler''':
* '''fix_assignment_missing_path''' - to handle non existent directory path
* '''update_assignment_form''' - to update assignment due dates, and questionnaire array in assignment form

[[File:Create helper m1.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit method.png]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.
Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone for an assignment.

[[File:Edit helper1.png]]

[[File:Edit helper2.png]]

[[File:Edit helper3.png]]

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Copy method.png]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

[[File:Copy helper.png]]

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list11.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.

[[File:Update_feedback_assignment_form_attr1.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

===Coverage===
As can be seen from [https://coveralls.io/jobs/60782737 Coverall], our changes have increased the coverage of assignments_controller.rb by 10.9%, from 30.5% up to 41.41%. While we did not make any changes to the tests, we did reduce the amount of testable code by reducing redundancy and extracting common code in the controller.

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'
* More thorough tests can be added to the assignments_controller spec to get the coverage closer to 100%.
==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-04-01T01:15:37Z

Dbell5: /* The branch condition size for edit method is too high */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''assignment_form_save_handler''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method from 33 to 15. Assignment Branch Condition size was also reduced from 56.37 to less than 15.

[[File:Create method11.png]]

Following methods were extracted from '''assignment_form_save_handler''':
* '''fix_assignment_missing_path''' - to handle non existent directory path
* '''update_assignment_form''' - to update assignment due dates, and questionnaire array in assignment form

[[File:Create helper m1.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit method.png]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.
Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

[[File:Edit helper1.png]]

[[File:Edit helper2.png]]

[[File:Edit helper3.png]]

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list11.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.

[[File:Update_feedback_assignment_form_attr1.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

===Coverage===
As can be seen from [https://coveralls.io/jobs/60782737 Coverall], our changes have increased the coverage of assignments_controller.rb by 10.9%, from 30.5% up to 41.41%. While we did not make any changes to the tests, we did reduce the amount of testable code by reducing redundancy and extracting common code in the controller.

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'
* More thorough tests can be added to the assignments_controller spec to get the coverage closer to 100%.
==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-04-01T01:14:36Z

Dbell5: /* The branch condition size for update_feedback_assignment_form_attributes method is too high */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''assignment_form_save_handler''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method from 33 to 15. Assignment Branch Condition size was also reduced from 56.37 to less than 15.

[[File:Create method11.png]]

Following methods were extracted from '''assignment_form_save_handler''':
* '''fix_assignment_missing_path''' - to handle non existent directory path
* '''update_assignment_form''' - to update assignment due dates, and questionnaire array in assignment form

[[File:Create helper m1.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit method.png]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.
Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

[[File:Edit helper1.png]]
[[File:Edit helper2.png]]
[[File:Edit helper3.png]]

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list11.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.

[[File:Update_feedback_assignment_form_attr1.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

===Coverage===
As can be seen from [https://coveralls.io/jobs/60782737 Coverall], our changes have increased the coverage of assignments_controller.rb by 10.9%, from 30.5% up to 41.41%. While we did not make any changes to the tests, we did reduce the amount of testable code by reducing redundancy and extracting common code in the controller.

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'
* More thorough tests can be added to the assignments_controller spec to get the coverage closer to 100%.
==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-31T21:00:55Z

Dbell5: /* Test Plan */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

'''Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail , but changes in the assignments_controller may not be the cause. Our build fails, but the fix is proposed [https://github.com/expertiza/expertiza/pull/1686/files# in another team's pull request, as they picked up where other developers left off for that code.]'''

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-31T00:30:17Z

Dbell5: /* Testing */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail (they currently do not, but did as the result of another team making changes elsewhere), but changes in the assignments_controller may not be the cause.

==Testing==

To test our changes, follow [https://docs.google.com/document/d/1tXmwju6R7KQbvycku-bdXxa6rXSUN4BMyvjY3ROmMSw/editlink these instructions] to set up the expertiza environment. If our repository produces errors during setup, verify if the issue is our repository or your setup by trying to setup the main expertiza repository.

'''In a terminal do the following:'''

* navigate to the experiza folder
* enter: rspec /app/controllers/assignments_controller
** for RSpec tests
* enter: rubocop /app/controllers/assignments_controller
** for Rubocop tests

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the tests passed with our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.

[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-30T22:29:42Z

Dbell5: /* Test Plan */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

Additional testing occurs in assignment_creation_spec, but that tests the integration of the assignments controller with other features. These may fail (they currently do not, but did as the result of another team making changes elsewhere), but changes in the assignments_controller may not be the cause.

==Testing==

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the existing tests passed with all our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.
[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-30T22:16:09Z

Dbell5: /* Test Plan */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
These tests were included and not added to because they cover all the instances that the methods would be used under. Our additions to the code, helper methods, do not have tests as they are used in the main methods and do not need their own tests. Otherwise, there would be redundant testing. The helper methods did not change any functionality.

==Testing==

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the existing tests passed with all our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.
[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-30T22:13:57Z

Dbell5: /* Future improvements */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
Helper methods do not have tests as the are used in the main methods. Otherwise, there would be redundant testing.

==Testing==

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the existing tests passed with all our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.
[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* There are other portions of the assignments_controller that can be refactored.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-30T22:12:19Z

Dbell5: /* Test Plan */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** ''creates a new AssignmentForm object and renders assignment#new page''
* '''create'''
** ''when assignment_form is saved successfully''
** ''when assignment_form is not saved''
* '''edit'''
** ''when assignment has staggered deadlines''
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* '''update'''
** ''when params does not have key :assignment_form''
*** ''when assignment is saved successfully''
*** ''when assignment is not saved successfully''
** ''when params has key :assignment_form''
*** ''when the timezone preference of current user is nil and assignment form updates attributes successfully''
*** ''when the timezone preference of current user is not nil and assignment form updates attributes successfully''
* '''show'''
** ''renders assignments#show page''
* '''copy'''
** ''when new assignment id fetches successfully''
** ''when new assignment directory is same as old''
** ''when new assignment id does not fetch successfully''
* '''delete'''
** ''when assignment is deleted successfully''
** ''when assignment is not deleted successfully''
Helper methods do not have tests as the are used in the main methods. Otherwise, there would be redundant testing.

==Testing==

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the existing tests passed with all our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.
[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* The rspec features fails for assignment creation. Tests need to be redone to properly represent the code that is present or deleted.
* The code needs more documentation. Nearly all of the functionality in assignments_controller is undocumented and can be unclear to new developers.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-30T22:05:10Z

Dbell5: /* Test Plan */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* '''action_allowed?'''
** ''when params action is edit or update''
*** ''when the role name of current user is super admin or admin''
*** ''when current user is the instructor of current assignment''
*** ''when current user is the ta of the course which current assignment belongs to''
*** ''when current user is a ta but not the ta of the course which current assignment belongs to''
*** ''when current user is the instructor of the course which current assignment belongs to''
*** ''when current user is an instructor but not the instructor of current course or current assignment''
** ''when params action is not edit and update''
** ''when the role current user is super admin/admin/instructor/ta''
** ''when the role current user is student''
* '''new'''
** creates a new AssignmentForm object and renders assignment#new page
* create
** when assignment_form is saved successfully
** when assignment_form is not saved
* edit
** when assignment has staggered deadlines
** As value errors should be covered in the ability to make assignments, the only test needed for this is when deadlines are staggered. A reminder for specifying rubrics is also tested.
* update
* show
* copy
* delete
Helper methods do not have tests as the are used in the main methods. Otherwise, there would be redundant testing.

==Testing==

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the existing tests passed with all our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.
[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* The rspec features fails for assignment creation. Tests need to be redone to properly represent the code that is present or deleted.
* The code needs more documentation. Nearly all of the functionality in assignments_controller is undocumented and can be unclear to new developers.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-30T21:51:31Z

Dbell5: /* Test Plan */

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

RSpect tests exist for the main methods used in the assignments controller:
* action_allowed?
** when params action is edit or update
*** when the role name of current user is super admin or admin
*** when current user is the instructor of current assignment
*** when current user is the ta of the course which current assignment belongs to
*** when current user is a ta but not the ta of the course which current assignment belongs to
*** when current user is the instructor of the course which current assignment belongs to
*** when current user is an instructor but not the instructor of current course or current assignment
** when params action is not edit and update
** when the role current user is super admin/admin/instructor/ta
** when the role current user is student
* new
* create
* edit
* update
* show
* copy
* delete
Helper methods do not have tests as the are used in the main methods. Otherwise, there would be redundant testing.

==Testing==

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the existing tests passed with all our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.
[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* The rspec features fails for assignment creation. Tests need to be redone to properly represent the code that is present or deleted.
* The code needs more documentation. Nearly all of the functionality in assignments_controller is undocumented and can be unclear to new developers.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-30T21:39:10Z

Dbell5:

This wiki page contains description of changes made as part of E2004, an OSS assignment for Spring 2020, CSC/ECE 517

==Background==
Expertiza allows the instructors and teaching assistants to store, retrieve, edit or delete assignments for a course that they are teaching. '''assignments_controller.rb''' is one of the biggest controllers that has a lot of scope for refactoring. It consists of the CRUD operations for an assignment along with small helper functions related to the deadlines, meta-reviews, team formation and so on. The primary issues with the code in this controller are listed as tasks.

== Tasks ==
=== The branch condition size for create method is too high ===

Current implementation of create method has too many assignments and multiple layers of if conditions. It also has too many lines of code. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] has identified following issues with the method:
Assignment Branch Condition size for create is too high. [56.37/15]
Method `create` has 33 lines of code (exceeds 25 allowed). Consider refactoring.

We extracted a method called '''handle_assignment_form_save''' from the code to handle the case when assignment_form is saved. This allowed in reducing the number of lines in the '''create''' method. Similarly, a helper method called '''handle_assignment_form_save''' was extracted out of the code from '''handle_assignment_form_save''' to make sure each method was only doing a single task. In order to reduce too many conditional statements, we also separated out code to handle for non existent assignment directory as '''handle_assignment_directory_path_nonexist''' method.

By doing all of this, we were able to reduce the number of lines in the '''create''' method from 33 to 14, and Assignment Branch Condition size from 56.37 to less than 15.

[[File:Create_file1.png]]
[[File:Create_file2.png]]
[[File:Create_file3.png]]

===The branch condition size for edit method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for edit is too high. [29.58/15]

[[File:Edit12.jpeg]]

This method had assignments and method calls that could be consolidated. By removing and grouping assignments and method calls into helper methods, we can still use them and reduce the branch condition size.

[[File:Reduce branch size of edit method1.jpeg]]

Thus, the methods '''update_due_date''', '''update_assignment_badges''', '''handle_missing_assignment_details''', and '''user_timezone_specified''' were created.

* update_due_date contains the loop that adjusts due dates for an assignment.
* update_assignment_badges updates instance variables
* handle_missing_assignment_details handles some incomplete assignment details
* user_timezone_specified flashes an error if the user did not specify a preferred time zone.

===The branch condition size for copy method is too high===
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Assignment Branch Condition size for copy is too high. [19.42/15]

[[File:Before reduce assignment copy branch size.JPG]]

The branch size of the copy method was inflated due to:
* Assigning (old_assign and new_assign) things that are only used once or not all
* Calling methods to assign values only used once

[[File:Reduce assignment copy branch size.JPG ]]

To reduce the branch size:
* Group assignments into a helper method which reduces the assignment and method call count

[[File:Assignment copy helper methods.JPG]]

We created helper methods '''update_copy_session''' and '''check_same_directory?'''
* update_copy_session contains assignments unused in the copy method, but relevant to the program
* check_same_directory compares directories for two assignment IDs, consolidating the conditional used in the copy method.

===The branch condition size for empty_rubrics_list method is too high===

The current implementation of the '''empty_rubrics_list''' method has many different branches of the code that could be executed. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] identified Assignment Branch Condition size to be too high.
Assignment Branch Condition size for empty_rubrics_list is too high. [18.47/15]
These include multiple '''if/unless''' statements as well as loops that may or may not be executed. We sought to reduce this problem by breaking up the sections of the code with different branches into their own functions, reducing the branch condition size for '''empty_rubrics_list'''.

We extracted code to remove questionnaire types from the rubric list that are already in existing assignment questionnaires into a method named '''remove_existing_questionnaire'''.

Likewise, we extracted code that conditionally removes questionnaire types from the rubric list (such as removing the teammate review questionnaire if the team only has a size of 1) into a method named '''remove_invalid_questionnaires'''.

[[File:Empty rubrics list1.png]]

===The branch condition size for update_feedback_assignment_form_attributes method is too high===
This implementation uses nested and sequential conditional statements. While it works to decide which flash message should appear, it can be simplified by removing the nested structure and using '''elsif''' statements.
[[File:Update_feedback_assignment_form_attributes.png]]

===The cognitive complexity of create method is too high===
The current implementation of the create method has a high cognitive complexity because there are nested iteration blocks. [https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] shows following issue:
Method `create` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There are two '''.each''' blocks, which are used to update '''assignment_questionnaire''' and '''due_date'''. We have resolved this issue by extracting those blocks into two separate methods named '''update_assignment_questionnaire''' and '''update_assignment_questionnaire'''.

===The cognitive complexity of empty_rubrics_list method is too high===

The current implementation of the empty_rubrics_list method has a high cognitive complexity due to there being nested iteration blocks.[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate] indicates following issue:

Method `empty_rubrics_list` has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.

There is a '''.each''' block on the assignment questionnaires and there is a '''.reject''' block on the rubric list entries nested within it. We sought to solve this issue by extracting the .reject block into a separate method named '''remove_existing_questionnaire'''.

===A line in the handle_current_user_timezonepref_nil method is too long===

The existing code had a line length of 180 characters.
Line is too long. [180/160]

[https://github.com/github/rubocop-github/blob/master/STYLEGUIDE.md Rubocop style guide] recommends line length to be less than 100 characters. We have used backslash (\) to split the line into two lines of 98, and 97 characters each.
==Test Plan==

in progress
==Testing==

The '''assignments_controller.rb''' file has a testing file called the '''assignment_controller_sepc.rb'''. We ran rspec tests, and made sure all the existing tests passed with all our changes.

[[File:Rspec assignment controller.png]]

We also ran rubocop for '''assignments_controller.rb''' file, and were able to eliminate the issues reported for methods we worked on.
[[File:Rubocop.png]]

Here is a [https://drive.google.com/file/d/1wmmk1jO_w6QKA6JfbGfm874tGIMvZ27o/view?usp=sharing link to a video] showing test results including rubocop results for '''assignments_controller.rb'''

'''Application Link''': http://152.46.18.46:8080

'''Username''': instructor6

'''Password''': password

==Future improvements==
* The rspec features fails for assignment creation. Tests need to be redone to properly represent the code that is present or deleted.
* The code needs more documentation. Nearly all of the functionality in assignments_controller is undocumented and can be unclear to new developers.
* There could be a collection of messages that are used in the application that can then be referenced in the code. This would have two benefits: allowing for a different message collection to be used for translations and also removing duplicate messages like are seen here:
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "Assignment #{assignment_form.assignment.id} was deleted.", request)
flash[:success] = 'The assignment was successfully deleted.'

==Team Information==

*Dave Bell (dbell5)
*Steven Green (stgreen)
*Abhishek Upadhyaya Ghimire (aupadhy3)
*'''Mentor:''' Anuja Kulkarni

==References==
[https://codeclimate.com/github/expertiza/expertiza/app/controllers/assignments_controller.rb/source#issue-c5fc0395d3a59720b512316fd0e23502 Code Climate Issues]

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-23T20:05:43Z

Dbell5: /* The branch condition size for copy method is too high */

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-23T20:05:18Z

Dbell5: /* The branch condition size for edit method is too high */

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-23T20:04:11Z

Dbell5: /* Future improvements */

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-23T19:59:42Z

Dbell5: /* The branch condition size for copy method is too high */

File:Reduce assignment copy branch size.JPG

2020-03-23T19:49:11Z

Dbell5:

File:Assignment copy helper methods.JPG

2020-03-23T19:49:00Z

Dbell5:

File:Before reduce assignment copy branch size.JPG

2020-03-23T19:48:46Z

Dbell5:

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-23T19:43:40Z

Dbell5: /* The branch condition size for edit method is too high */

File:Edit1.JPG

2020-03-23T19:43:09Z

Dbell5:

File:Reduce branch size of edit method.JPG

2020-03-23T19:41:57Z

Dbell5:

CSC/ECE 517 Spring 2020 - E2004. Refactor assignments controller

2020-03-23T19:39:08Z

Dbell5: /* The branch condition size for edit method is too high */