Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-29T02:12:58Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. Meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''Approach taken to solve the issues''' ==
* We started by looking at issues happening for meta reviews in beta branch
* Next we tried to understand difference between meta-review and review.
* Further we tried to explore the possibility of combining meta-reviews and review to follow DRY principle in code.
* But we observed that since meta review is advanced stage in review cycle and each meta-review is associated with some previous review and reviewee, there is no way to have single model and controller.
* So we scrapped the idea of changing the design at root level.
* The best approach here to handle current meta-review issues is to fix them individually.
* The details on how each issue will be approached and fixed is given below.
====* None of the issues need any change of workflow as per preliminary analysis, thus no UML diagram is added.====

== '''[https://github.com/expertiza/expertiza/issues/198 #198 - Begin link not working correctly]''' ==

===Issue Description===

In beta branch begin is not working correctly. The page crashes sometime:
Observations:
* While selecting "Metareview" rubric in local db, the begin link crashed.
* Following the code we observed it was returning questionnaire list, where only object was expected.
* Fixing it, we were able to see page redirecting successfully.
[[File: Meta review rubric.png | 400px]]
[[File: Request button disappear.png | 400px]]
[[File: Meta begin crash.png | 400px]]
==== Plan:====
* We will be looking into this issue further, if the questionnaire was supposed to be the way it is?
* Further the loaded page doesn't have any portion of view for meta-review, instead it shows just plain old review. We will be checking the history of this change and try to find at what time partial view file was removed from code.

== '''Submit button isn't available for meta-reviews''' ==

=== Issue Description===
* The issue at present is there is no submit button for meta-reviews.
* Users should see save and submit buttons as part of the meta-review form similar to the behavior as the review form.
==== Plan:====
* We would be looking into the response.html.erb file to fix the issue.

The bug is found and fixed and can be checked here:
https://github.com/amundra02/expertiza/commit/d0cd6b2c3816115360e798f8962a2b8f5e64db2a

== '''Hide meta-review deadline under review strategy when use meta review deadline checkbox is unchecked under due date ''' ==

=== Issue Description===
* The issue at present is that when the meta-review deadline checkbox under due-date has no impact on meta-review deadline under review strategy.
* Users should see the meta-review deadline under review strategy only when meta-review deadline checkbox is checked.
==== Plan:====
* We would be looking into the review_stragtegy.html.erb file to fix the issue.
==== Changes:====
* We have added inline style to the section in meta-review under review strategy to hide/show the section based on a ruby variable.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===
There was an error tag in view, which is corrected:
https://github.com/amundra02/expertiza/commit/3e535992876291738334e2504bc5ef22bd8610d0

== '''"Numbers of Meta-reviews left:” does not update after a user requests a review.''' ==
===Issue Description===
* Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.
==== Plan:====
* This isn't an issue and is the expected behavior. The same logic is for reviews also. Number of left reviews should only be reduced once any review is submitted.

== '''"Request a new meta-review" button bugs''' ==
===Issue Description===
Currently after the "Request a new meta-review" button is clicked the button disappears and no new meta-review can be requested. The "Request new a meta-review" button should be hidden only when:

* The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
* The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.

[[File: Request meta.png | 600px]]
[[File: Request button disappear.png | 600px]]

==== Plan:====
* This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.
The code changes are done to fix this issue are availabel here: https://github.com/amundra02/expertiza/commit/d319dbf02100501c657c1257917c325607192a1f

[[File:Request meta review code.png|1200px]]

== '''Request for meta review is breaking the website when there are no review''' ==

=== Issue Description===
* When there are no review available the request for meta-review deadline is crashing the site.
==== Plan:====
* Fixing it, we were able to show a small error message instead of allowing the site to crash
Code changes: https://github.com/amundra02/expertiza/commit/de9366941a7932af1cd9c0bf0c946f2ac05638c0

== '''Testing Plan''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
1. We will target to get all existing test passed, to make sure no existing functionality for other features break.

2. Also, we will be adding few test cases for the issues fixed in this project and hoping to increase coverage.

=====New test case added:=====
Objective: To find the behavior of questionnaire function in metareview_response_map.rb when there is no questionnaire in assignment.
Expected behavior: It should return nil object, rather than some garbage object.

The test case added increased test coverage by small margin.

Code changes: https://github.com/amundra02/expertiza/commit/bca6534e78f1edb46dc22e907c23af6bcbef952b

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-29T02:12:34Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. Meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''Approach taken to solve the issues''' ==
* We started by looking at issues happening for meta reviews in beta branch
* Next we tried to understand difference between meta-review and review.
* Further we tried to explore the possibility of combining meta-reviews and review to follow DRY principle in code.
* But we observed that since meta review is advanced stage in review cycle and each meta-review is associated with some previous review and reviewee, there is no way to have single model and controller.
* So we scrapped the idea of changing the design at root level.
* The best approach here to handle current meta-review issues is to fix them individually.
* The details on how each issue will be approached and fixed is given below.
====* None of the issues need any change of workflow as per preliminary analysis, thus no UML diagram is added.====

== '''[https://github.com/expertiza/expertiza/issues/198 #198 - Begin link not working correctly]''' ==

===Issue Description===

In beta branch begin is not working correctly. The page crashes sometime:
Observations:
* While selecting "Metareview" rubric in local db, the begin link crashed.
* Following the code we observed it was returning questionnaire list, where only object was expected.
* Fixing it, we were able to see page redirecting successfully.
[[File: Meta review rubric.png | 400px]]
[[File: Request button disappear.png | 400px]]
[[File: Meta begin crash.png | 400px]]
==== Plan:====
* We will be looking into this issue further, if the questionnaire was supposed to be the way it is?
* Further the loaded page doesn't have any portion of view for meta-review, instead it shows just plain old review. We will be checking the history of this change and try to find at what time partial view file was removed from code.

== '''Submit button isn't available for meta-reviews''' ==

=== Issue Description===
* The issue at present is there is no submit button for meta-reviews.
* Users should see save and submit buttons as part of the meta-review form similar to the behavior as the review form.
==== Plan:====
* We would be looking into the response.html.erb file to fix the issue.

The bug is found and fixed and can be checked here:
https://github.com/amundra02/expertiza/commit/d0cd6b2c3816115360e798f8962a2b8f5e64db2a

== '''Hide meta-review deadline under review strategy when use meta review deadline checkbox is unchecked under due date ''' ==

=== Issue Description===
* The issue at present is that when the meta-review deadline checkbox under due-date has no impact on meta-review deadline under review strategy.
* Users should see the meta-review deadline under review strategy only when meta-review deadline checkbox is checked.
==== Plan:====
* We would be looking into the review_stragtegy.html.erb file to fix the issue.
==== Changes:====
* We have added inline style to the section in meta-review under review strategy to hide/show the section based on a ruby variable.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===
There was an error tag in view, which is corrected:
https://github.com/amundra02/expertiza/commit/3e535992876291738334e2504bc5ef22bd8610d0

== '''"Numbers of Meta-reviews left:” does not update after a user requests a review.''' ==
===Issue Description===
* Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.
==== Plan:====
* This isn't an issue and is the expected behavior. The same logic is for reviews also. Number of left reviews should only be reduced once any review is submitted.

== '''"Request a new meta-review" button bugs''' ==
===Issue Description===
Currently after the "Request a new meta-review" button is clicked the button disappears and no new meta-review can be requested. The "Request new a meta-review" button should be hidden only when:

* The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
* The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.

[[File: Request meta.png | 600px]]
[[File: Request button disappear.png | 600px]]

==== Plan:====
* This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.
The code changes are done to fix this issue are availabel here: https://github.com/amundra02/expertiza/commit/d319dbf02100501c657c1257917c325607192a1f

[[File:Request meta review code.png|1200px]]

== '''Request for meta review is breaking the website when there are no review''' ==

=== Issue Description===
* When there are no review available the request for meta-review deadline is crashing the site.
==== Plan:====
* Fixing it, we were able to show a small error message instead of allowing the site to crash
Code changes: https://github.com/amundra02/expertiza/commit/de9366941a7932af1cd9c0bf0c946f2ac05638c0

== '''Testing Plan''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
1. We will target to get all existing test passed, to make sure no existing functionality for other features break.

2. Also, we will be adding few test cases for the issues fixed in this project and hoping to increase coverage.

=====New test case added:=====
Objective: To find the behavior of questionnaire function in metareview_response_map.rb when there is no questionnaire in assignment.
Expected behavior: It should return nil object, rather than some garbage object.

The test case added test coverage by small margin.
Code changes: https://github.com/amundra02/expertiza/commit/bca6534e78f1edb46dc22e907c23af6bcbef952b

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-29T02:10:37Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-25T22:29:03Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. Meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''Approach taken to solve the issues''' ==
* We started by looking at issues happening for meta reviews in beta branch
* Next we tried to understand difference between meta-review and review.
* Further we tried to explore the possibility of combining meta-reviews and review to follow DRY principle in code.
* But we observed that since meta review is advanced stage in review cycle and each meta-review is associated with some previous review and reviewee, there is no way to have single model and controller.
* So we scrapped the idea of changing the design at root level.
* The best approach here to handle current meta-review issues is to fix them individually.
* The details on how each issue will be approached and fixed is given below.
====* None of the issues need any change of workflow as per preliminary analysis, thus no UML diagram is added.====

== '''[https://github.com/expertiza/expertiza/issues/198 #198 - Begin link not working correctly]''' ==

===Issue Description===

In beta branch begin is not working correctly. The page crashes sometime:
Observations:
* While selecting "Metareview" rubric in local db, the begin link crashed.
* Following the code we observed it was returning questionnaire list, where only object was expected.
* Fixing it, we were able to see page redirecting successfully.
[[File: Meta review rubric.png | 400px]]
[[File: Request button disappear.png | 400px]]
[[File: Meta begin crash.png | 400px]]
==== Plan:====
* We will be looking into this issue further, if the questionnaire was supposed to be the way it is?
* Further the loaded page doesn't have any portion of view for meta-review, instead it shows just plain old review. We will be checking the history of this change and try to find at what time partial view file was removed from code.

== '''Submit button isn't available for meta-reviews''' ==

=== Issue Description===
* The issue at present is there is no submit button for meta-reviews.
* Users should see save and submit buttons as part of the meta-review form similar to the behavior as the review form.
==== Plan:====
* We would be looking into the response.html.erb file to fix the issue.

The bug is found and fixed and can be checked here:
https://github.com/amundra02/expertiza/commit/d0cd6b2c3816115360e798f8962a2b8f5e64db2a

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===
There was an error tag in view, which is corrected:
https://github.com/amundra02/expertiza/commit/3e535992876291738334e2504bc5ef22bd8610d0

== '''"Numbers of Meta-reviews left:” does not update after a user requests a review.''' ==
===Issue Description===
* Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.
==== Plan:====
* This isn't an issue and is the expected behavior. The same logic is for reviews also. Number of left reviews should only be reduced once any review is submitted.

== '''"Request a new meta-review" button bugs''' ==
===Issue Description===
Currently after the "Request a new meta-review" button is clicked the button disappears and no new meta-review can be requested. The "Request new a meta-review" button should be hidden only when:

* The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
* The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.

[[File: Request meta.png | 600px]]
[[File: Request button disappear.png | 600px]]

==== Plan:====
* This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.
The code changes are done to fix this issue are availabel here: https://github.com/amundra02/expertiza/commit/d319dbf02100501c657c1257917c325607192a1f

[[File:Request meta review code.png|1200px]]

== '''Request for meta review is breaking the website when there are no review''' ==

=== Issue Description===
* When there are no review available the request for meta-review deadline is crashing the site.
==== Plan:====
* Fixing it, we were able to show a small error message instead of allowing the site to crash
Code changes: https://github.com/amundra02/expertiza/commit/de9366941a7932af1cd9c0bf0c946f2ac05638c0

== '''Testing Plan''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
1. We will target to get all existing test passed, to make sure no existing functionality for other features break.

2. Also, we will be adding few test cases for the issues fixed in this project and hoping to increase coverage.

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-25T22:27:48Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. Meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''Approach taken to solve the issues''' ==
* We started by looking at issues happening for meta reviews in beta branch
* Next we tried to understand difference between meta-review and review.
* Further we tried to explore the possibility of combining meta-reviews and review to follow DRY principle in code.
* But we observed that since meta review is advanced stage in review cycle and each meta-review is associated with some previous review and reviewee, there is no way to have single model and controller.
* So we scrapped the idea of changing the design at root level.
* The best approach here to handle current meta-review issues is to fix them individually.
* The details on how each issue will be approached and fixed is given below.
====* None of the issues need any change of workflow as per preliminary analysis, thus no UML diagram is added.====

== '''[https://github.com/expertiza/expertiza/issues/198 #198 - Begin link not working correctly]''' ==

===Issue Description===

In beta branch begin is not working correctly. The page crashes sometime:
Observations:
* While selecting "Metareview" rubric in local db, the begin link crashed.
* Following the code we observed it was returning questionnaire list, where only object was expected.
* Fixing it, we were able to see page redirecting successfully.
[[File: Meta review rubric.png | 400px]]
[[File: Request button disappear.png | 400px]]
[[File: Meta begin crash.png | 400px]]
==== Plan:====
* We will be looking into this issue further, if the questionnaire was supposed to be the way it is?
* Further the loaded page doesn't have any portion of view for meta-review, instead it shows just plain old review. We will be checking the history of this change and try to find at what time partial view file was removed from code.

== '''Submit button isn't available for meta-reviews''' ==

=== Issue Description===
* The issue at present is there is no submit button for meta-reviews.
* Users should see save and submit buttons as part of the meta-review form similar to the behavior as the review form.
==== Plan:====
* We would be looking into the response.html.erb file to fix the issue.

The bug is found and fixed and can be checked here:
https://github.com/amundra02/expertiza/commit/d0cd6b2c3816115360e798f8962a2b8f5e64db2a

= '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===
There was an error tag in view, which is corrected:
https://github.com/amundra02/expertiza/commit/3e535992876291738334e2504bc5ef22bd8610d0

== '''"Numbers of Meta-reviews left:” does not update after a user requests a review.''' ==
===Issue Description===
* Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.
==== Plan:====
* This isn't an issue and is the expected behavior. The same logic is for reviews also. Number of left reviews should only be reduced once any review is submitted.

== '''"Request a new meta-review" button bugs''' ==
===Issue Description===
Currently after the "Request a new meta-review" button is clicked the button disappears and no new meta-review can be requested. The "Request new a meta-review" button should be hidden only when:

* The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
* The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.

[[File: Request meta.png | 600px]]
[[File: Request button disappear.png | 600px]]

==== Plan:====
* This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.
The code changes are done to fix this issue are availabel here: https://github.com/amundra02/expertiza/commit/d319dbf02100501c657c1257917c325607192a1f

[[File:Request meta review code.png|1200px]]

== '''Request for meta review is breaking the website when there are no review''' ==

=== Issue Description===
* When there are no review available the request for meta-review deadline is crashing the site.
==== Plan:====
* Fixing it, we were able to show a small error message instead of allowing the site to crash
Code changes: https://github.com/amundra02/expertiza/commit/de9366941a7932af1cd9c0bf0c946f2ac05638c0

== '''Testing Plan''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
1. We will target to get all existing test passed, to make sure no existing functionality for other features break.

2. Also, we will be adding few test cases for the issues fixed in this project and hoping to increase coverage.

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-08T03:11:51Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. Meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''Approach taken to solve the issues''' ==
* We started by looking at issues happening for meta reviews in beta branch
* Next we tried to understand difference between meta-review and review.
* Further we tried to explore the possibility of combining meta-reviews and review to follow DRY principle in code.
* But we observed that since meta review is advanced stage in review cycle and each meta-review is associated with some previous review and reviewee, there is no way to have single model and controller.
* So we scrapped the idea of changing the design at root level.
* The best approach here to handle current meta-review issues is to fix them individually.
* The details on how each issue will be approached and fixed is given below.
====* None of the issues need any change of workflow as per preliminary analysis, thus no UML diagram is added.====

== '''[https://github.com/expertiza/expertiza/issues/198 #198 - Begin link not working correctly]''' ==

===Issue Description===

In beta branch begin is not working correctly. The page crashes sometime:
Observations:
* While selecting "Metareview" rubric in local db, the begin link crashed.
* Following the code we observed it was returning questionnaire list, where only object was expected.
* Fixing it, we were able to see page redirecting successfully.
[[File: Meta review rubric.png | 400px]]
[[File: Request button disappear.png | 400px]]
[[File: Meta begin crash.png | 400px]]
==== Plan:====
* We will be looking into this issue further, if the questionnaire was supposed to be the way it is?
* Further the loaded page doesn't have any portion of view for meta-review, instead it shows just plain old review. We will be checking the history of this change and try to find at what time partial view file was removed from code.

== '''Submit button isn't available for meta-reviews''' ==

=== Issue Description===
* The issue at present is there is no submit button for meta-reviews.
* Users should see save and submit buttons as part of the meta-review form similar to the behavior as the review form.
==== Plan:====
* We would be looking into the response.html.erb file to fix the issue.

== '''"Numbers of Meta-reviews left:” does not update after a user requests a review.''' ==
===Issue Description===
* Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.
==== Plan:====
* This isn't an issue and is the expected behavior. The same logic is for reviews also. Number of left reviews should only be reduced once any review is submitted.

== '''"Request a new meta-review" button bugs''' ==
===Issue Description===
Currently after the "Request a new meta-review" button is clicked the button disappears and no new meta-review can be requested. The "Request new a meta-review" button should be hidden only when:

* The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
* The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.

[[File: Request meta.png | 600px]]
[[File: Request button disappear.png | 600px]]

==== Plan:====
* This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

[[File:Request meta review code.png|1200px]]

== '''Request for meta review is breaking the website when there are no review''' ==

=== Issue Description===
* When there are no review available the request for meta-review deadline is crashing the site.
==== Plan:====
* Fixing it, we were able to show a small error message instead of allowing the site to crash

== '''Request for meta review is shown even before the review deadline is not passed''' ==

=== Issue Description===
* When the review deadline is not passed the request for meta review is getting displayed.
==== Plan:====
* For fixing this we are planning to add a simple if check

== '''When there are no review questions assigned to a review upon opening it the site crashes''' ==

=== Issue Description===
* When there are no review questions if I click on begin button the site is crashing
==== Plan:====
* For fixing this we are planning to add a simple if check on the number of questions.

== '''Testing Plan''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
1. We will target to get all existing test passed, to make sure no existing functionality for other features break.

2. Also, we will be adding few test cases for the issues fixed in this project and hoping to increase coverage.

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T20:37:44Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T20:24:14Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T20:21:30Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T20:18:00Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T20:16:18Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T20:13:37Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T20:12:26Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:31:50Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:23:34Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== Approach took to problem ==
* We started by looking at issues happening for meta reviews in beta branch
* Next we tried to understand difference between meta-review and review.
* Further we tried to explore the possibility of combining meta-reviews and review to follow DRY principle in code.
* But we observed that since meta review is advanced stage in review cycle and each meta-review is associated with some previous review and reviewee, there is no way to have single model and controller.
* So we scrapped the idea of changing the design at root level.
* The best approach here to handle current meta-review issues is to fix them individually.
* The details on how each issue will be approached and fixed is given below.

== '''[https://github.com/expertiza/expertiza/issues/198 #198 - Begin not working correctly]''' ==

===Issue Description===

In beta branch begin is not working correctly. The page crashes sometime.

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:16:26Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''[https://github.com/expertiza/expertiza/issues/198 #198 - Begin not working correctly]''' ==

===Issue Description===

In beta branch begin is not working correctly. The page crashes sometime.

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:13:05Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E2025 which was assigned to students last semester, Spring 2020.

Details of the previous implementation can be found here:
[CSC/ECE 517 Spring 2020 - E2025: Issues Related to Meta - Reviewing]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:12:11Z

Pgarg5:

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* When a user requests a new meta-review, (s)he is unable to get Begin (refer to above image) link to start a new meta-review. (Issue #198)
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E1997 which was assigned to students last semester, Fall 2019.

Details of the previous implementation can be found here:
[http://https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1997._Issues_related_to_meta-reviewing E1997]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

E2235. Issues related to meta-reviewing

2022-04-06T17:10:33Z

Pgarg5: Pgarg5 moved page E2235. Issues related to meta-reviewing to CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

#REDIRECT [[CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing]]

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:10:33Z

Pgarg5: Pgarg5 moved page E2235. Issues related to meta-reviewing to CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* An assignment is deemed to have meta-reviews if and only if there is a meta-review deadline for this assignment in the `assignments_questionnaires` table.
* If there is no meta-review deadline, then don’t show the question about whether there is a max meta-review limit.
* In the DB the default number of meta-reviews allowed and required should be null.
* As soon as someone checks the Has meta-review Limit box on the Review Strategy tab, the UI should fill in 3 and 3 as the required and allowed number.
* Student pages should show the list of reviews that have been performed.
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E1997 which was assigned to students last semester, Fall 2019.

Details of the previous implementation can be found here:
[http://https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1997._Issues_related_to_meta-reviewing E1997]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

CSC/ECE 517 Spring 2022

2022-04-06T17:09:56Z

Pgarg5:

== OSS Projects ==

* [[CSC/ECE 517 Spring 2022 - E2200: Testing advice_controller]]
* [[CSC/ECE 517 Spring 2022 - E2203: Adding tests for courses_controller, eula_controller]]
* [[CSC/ECE 517 Spring 2022 - E2204: Adding tests for markup_styles_controller, lock_controller]]
* [[CSC/ECE 517 Spring 2022 - E2208: Testing for submission_records_controller, profile_controller]]
* [[CSC/ECE 517 Spring 2022 - E2218: Refactor response_controller]]
* [[CSC/ECE 517 Spring 2022 - E2217: Refactor questionnaires_controller]]
* [[CSC/ECE 517 Spring 2022 - E2216: Refactor late_policies_controller]]
* [[CSC/ECE 517 Spring 2022 - E2215: Refactor student_quizzes_controller]]
* [[CSC/ECE 517 Spring 2022 - E2214: Refactor teams_controller]]
* [[CSC/ECE 517 Spring 2022 - E2219: Improve assessment360_controller.rb]]
* [[CSC/ECE 517 Spring 2022 - E2211: Testing for summary_helper]]
* [[CSC/ECE 517 Spring 2022 - E2225: Refactor review_mapping_helper.rb]]
* [[CSC/ECE 517 Spring 2022 - E2221: Refactor submitted content controller.rb]]
* [[CSC/ECE 517 Spring 2022 - E2206: Testing for users_controller]]
* [[CSC/ECE 517 Spring 2022 - E2207: Testing for submitted_content_controller.rb]]
* [[CSC/ECE 517 Spring 2022 - E2205: Testing for participants_controller, versions_controller]]
* [[CSC/ECE 517 Spring 2022 - E2212: Testing for hamer.rb]]
* [[CSC/ECE 517 Spring 2022 - S2200: Refactoring evaluation of SQL queries (Java/JUnit)]]
* [[CSC/ECE 517 Spring 2022 - S2222: Refactor impersonate_controller]]
* [[CSC/ECE 517 Spring 2022 - E2202- Testing for badges_controller, publishing_controller]]
*[[CSC/ECE 517 Spring 2022 - E2220: Refactor reputation_web_service_controller]]
* [[CSC/ECE 517 Spring 2022 - E2224: Refactor review_mapping_controller]]
* [[CSC/ECE 517 Spring 2022 - E2209: Testing for analytic helper.rb, join team requests helper.rb]]
* [[CSC/ECE 517 Spring 2022 - E2227: SQL Injection Fix]]
* [[CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb]]
* [[CSC/ECE 517 Spring 2022 - E2201: Testing for assignment_questionnaire_controller.rb]]
* [[CSC/ECE 517 Spring 2022 - E2243. Refactor student_teams functionality]]

== Final Projects ==
* [[CSC/ECE 517 Spring 2022 - E2245: View for results of bidding]]
* [[CSC/ECE 517 Spring 2022 - E2241: Heatgrid fixes and improvements]]
* [[CSC/ECE 517 Spring 2022 - E2237: Grading audit trail]]
* [[CSC/ECE 517 Spring 2022 - S2201: Improving User Experience for SQLFE]]
* [[CSC/ECE 517 Spring 2022 - E2236: Fix issues related to deadlines and late policies]]
* [[CSC/ECE 517 Spring 2022 - E2231: Allow reviewers to bid on what to review]]
* [[CSC/ECE 517 Spring 2022 - E2233. Improving search facility in Expertiza]]
* [[CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments]]
* [[CSC/ECE 517 Spring 2022 - E2244. Support for saying your team pair-programmed]]
* [[CSC/ECE 517 Spring 2022 - E2246: Email your authors/reviewers, with complete tests]]
* [[CSC/ECE 517 Spring 2022 - E2229: Track the time students look at other submissions]]
* [[CSC/ECE 517 Spring 2022 - E2239: Further refactoring and improvement of review mapping helper.rb ]]
* [[CSC/ECE 517 Spring 2022 - E2240. Re-write waitlist functionality ]]
* [[CSC/ECE 517 Spring 2022 - E2242. Fix teammate-review view ]]
* [[CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing]]

Expertiza Wiki:Pgarg5

2022-04-06T17:08:41Z

Pgarg5: Pgarg5 moved page PG Wiki:Pgarg5 to E2235. Issues related to meta-reviewing

#REDIRECT [[E2235. Issues related to meta-reviewing]]

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:08:41Z

Pgarg5: Pgarg5 moved page PG Wiki:Pgarg5 to E2235. Issues related to meta-reviewing

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* An assignment is deemed to have meta-reviews if and only if there is a meta-review deadline for this assignment in the `assignments_questionnaires` table.
* If there is no meta-review deadline, then don’t show the question about whether there is a max meta-review limit.
* In the DB the default number of meta-reviews allowed and required should be null.
* As soon as someone checks the Has meta-review Limit box on the Review Strategy tab, the UI should fill in 3 and 3 as the required and allowed number.
* Student pages should show the list of reviews that have been performed.
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E1997 which was assigned to students last semester, Fall 2019.

Details of the previous implementation can be found here:
[http://https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1997._Issues_related_to_meta-reviewing E1997]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

User talk:Pgarg5

2022-04-06T17:03:58Z

Pgarg5: Pgarg5 moved page User talk:Pgarg5 to PG Wiki:E2235. Issues related to meta-reviewing

#REDIRECT [[PG Wiki:E2235. Issues related to meta-reviewing]]

Expertiza Wiki:E2235. Issues related to meta-reviewing

2022-04-06T17:03:58Z

Pgarg5: Pgarg5 moved page User talk:Pgarg5 to PG Wiki:E2235. Issues related to meta-reviewing

#REDIRECT [[PG Wiki:Pgarg5]]

Expertiza Wiki:E2235. Issues related to meta-reviewing

2022-04-06T17:03:48Z

Pgarg5: Pgarg5 moved page User talk:Pgarg5 to PG Wiki:Pgarg5

#REDIRECT [[PG Wiki:Pgarg5]]

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:03:47Z

Pgarg5: Pgarg5 moved page User talk:Pgarg5 to PG Wiki:Pgarg5

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* An assignment is deemed to have meta-reviews if and only if there is a meta-review deadline for this assignment in the `assignments_questionnaires` table.
* If there is no meta-review deadline, then don’t show the question about whether there is a max meta-review limit.
* In the DB the default number of meta-reviews allowed and required should be null.
* As soon as someone checks the Has meta-review Limit box on the Review Strategy tab, the UI should fill in 3 and 3 as the required and allowed number.
* Student pages should show the list of reviews that have been performed.
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E1997 which was assigned to students last semester, Fall 2019.

Details of the previous implementation can be found here:
[http://https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1997._Issues_related_to_meta-reviewing E1997]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

CSC/ECE 517 Spring 2022 - E2235. Issues related to meta-reviewing

2022-04-06T17:02:04Z

Pgarg5: E2235. Issues related to meta-reviewing

== '''Overview of Project''' ==

In Expertiza, meta-review is a feature that enables a third party to evaluate the quality of feedback that a reviewer gives a reviewee. meta-review is an important feature that can help students become better reviewers for their peers by providing detailed feedback that is actionable and positively formulated. Unfortunately, this feature is broken and the following issues were identified. The task is to fix these issues so that this feature can be used again.

* An assignment is deemed to have meta-reviews if and only if there is a meta-review deadline for this assignment in the `assignments_questionnaires` table.
* If there is no meta-review deadline, then don’t show the question about whether there is a max meta-review limit.
* In the DB the default number of meta-reviews allowed and required should be null.
* As soon as someone checks the Has meta-review Limit box on the Review Strategy tab, the UI should fill in 3 and 3 as the required and allowed number.
* Student pages should show the list of reviews that have been performed.
* When a user requests a new meta-review, the field which says the number of meta-reviews left ( refer to above image “Numbers of Meta-reviews left:”) does not change and the button to request a new meta-review disappears.

===Previous Work===

This assignment is a repeat of E1997 which was assigned to students last semester, Fall 2019.

Details of the previous implementation can be found here:
[http://https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1997._Issues_related_to_meta-reviewing E1997]

Our goals are to update the logic, refactor non-DRY code, and fix bugs.

== '''[https://github.com/RPHolloway/expertiza/issues/32 #32 - Remove all debug comments in the UI]''' ==

===Issue Description===

It was noted in the previous team's implementation that multiple debugging comments were showing up during their video testing.

Debug messages shown when viewing a completed meta review:
[[File:Debug_comments.png]]

===Changes===

There was a mistake in <code>app/views/response/_review.html.erb</code> where "<<nowiki/>br/><<nowiki/>hr/><<nowiki/>br/>" was being displayed under the review being meta-reviewed. This was due to being included as a string constant rather than html code.

[[File:E2025_debug_comments_response_review.PNG]]

There was an instance in <code>app/views/response/view.html.erb</code> where ruby code responsible for getting a review to be displayed was being interpreted and displayed, even though the response was being displayed as html in the line before. This resulted in the response details being listed in the UI erroneously.

[[File:E2025_debug_response_view.PNG]]

== '''[https://github.com/RPHolloway/expertiza/issues/49 #49 - "Request a new meta-review" button crashes when there are no available reviews]''' ==

===Issue Description===

When there are no valid metareviews to perform (either there are no reivews performed on the assignment, the only valid reviews concern your own work or own reviews, you have completed all allowed metareviews, or you have reviewed all valid reviews already) then the "Request a New Review" button still is visible and displays the following error (correctly) when pressed.

===Reproducing Existing Issue===

1. Log in as instructor6, click on mange...-> Assignments

2. Click on edit assignment, navigate to Due Dates and check "Use meta-review deadline" checkbox

3. Select "Yes" for all submission allowed, Review allowed and Meta-review allowed in all of the deadline types

4. Navigate back to Assignment Tree View and click on "Add participant"

5. Add couple of students(say student1 and student2) to the assignment

6. Impersonate/login as student1 and make a submission

7. Log in as instructor6 as previously described and change the submission due date to be passed so we are currently in the review stage

8. Impersonate/login as student2 and Request for a new review under others work, submit the review

9. Log in as instructor6 as previously described and change the review due date to be passed so we are currently in the meta-review stage

10. Impersonate/login back as student1 (the one with the submission) and Request for a new meta review

[[File:E2025_outofreviews.png]]

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

Here we conditionally hide the button based on whether or not the participant has reviews available to perform. These checks are located in assignment.rb which is the model for an Assignment. The following method was added to assignment.rb and returns true when the participant has a valid meta-review available.

# Returns true if there is a meta-review task available for the user
def metareview_available?(user_id)
metareviewer = AssignmentParticipant.where(user_id: user_id, parent_id: self.id).first
begin
self.response_map_to_metareview(metareviewer)
true
rescue
false
end
end

== '''[https://github.com/RPHolloway/expertiza/issues/27 #27 - Defaulting Required and Allowed Meta-reviews to NULL in Assignment Database]''' ==

===Issue Description===

Currently, the number of required and allowed meta-reviews is always being set to 3 in the database. Per the specs of the feature, it should default the database values to NULL. When checkbox "Use meta-review deadline", in the "Due Dates" tab, is selected then the view should populate required and allowed meta-reviews to 3.

===Changes===

We created a migration that defaulted the metareviews_allowed and metareviews_required in the Assignment table to nil instead of what the previous team had as 3.

[[File:E2025_13.png]]

Since the number of meta-reviews allowed and required are recorded using the values in the text fields, changes were made to <code>app/views/assignment/edit/_review_strategy.html.erb</code> to set the values and display them as expected.

When the checkbox for setting meta-review limits is unchecked, the values of number of meta-reviews allowed and required should be null. When the checkbox is checked, the values should be set to 3 and 3 by default, or to the values already defined before (in case the assignment was saved with the values set already). On page load, if the checkbox is enabled, then the inputs should be shown with the default or pre-set values, but if the checkbox is disabled then they should be hidden and set to null.

[[File:E2025_metareview_limit_1.PNG]]

[[File:E2025_metareview_limit_2.PNG]]

Changes had to be made to <code>app/views/student_review/list.html.erb</code> to account for the values being <code>null</code>.

[[File:E2025_metareview_limit_3.PNG]]

===Testing===

Tests have been created that test if a metareview can have nil limits. In this case, no limit will be shown to the user. Before this change that was impossible

== '''Issue 5 - Meta Review Limit is Not Functioning Correctly in Assignment Creation''' ==

===Issue Description===

The "has meta review limit" should determine whether a limit or requirement is placed on the student to complete meta reviews. If disabled, then they should be able to complete as many as they want. This is independent of the Meta Review STAGE that is enabled/disabled in the due dates tab. They should not impact each other and currently do within the assignment creation.

===Reproducing Existing Issue===

1. If the "Use meta-review deadline" is disabled, then the Meta Review Limit panel disappears

2. "Has meta-review limit?" is hidden on edit

3. Possibility the "Has meta-review limit?" not being saved on assignment creation?

4. If the "Use meta-review deadline" is toggled (disabled, enabled), then the "Has meta-review limit?" resets to disabled

===Proposed Changes===

The code in _due_dates.html.erb and the _review_strategy.html.erb within the /app/app/views/assignments/edit folder is lazily written and does not really adhere to the project specifications of how metareviews should work on assignment creation. These views need to be edited to work correctly and also respond to the changes in issue 5.

===Changes===

The changes were too long to screenshot, so here is the specific diff section:

https://github.com/expertiza/expertiza/pull/1715/files#diff-540fd2bd6c34b48213716b1dd713733a
https://github.com/expertiza/expertiza/pull/1715/files#diff-80f39fa5f62c73ff8741b02d3b11656c

== '''Discovered Bugs:''' ==

== '''[https://github.com/RPHolloway/expertiza/issues/45 #45 - "Request a new meta-review" button bugs]''' ==

===Issue Description===

The "Request new a meta-review" button should be hidden when:
# The number of meta-reviews in-progress is greater than the maximum allowed outstanding reviews.
# The number of reviews allowed is limited AND the total number of meta-reviews completed or in-progress is greater than the number allowed.
# There are no valid reviews available for the participant.

===Changes===

This issue deals with the view "list.html.erb" under /app/views/student_review. The code below shows the conditional used to display the "Request new meta-review" button.

<% if @num_metareviews_in_progress >= Assignment.max_outstanding_reviews %>
Note: You can't have more than 2 outstanding meta-reviews. You must complete one of your
outstanding meta-reviews before selecting another.
<% elsif @assignment.num_metareviews_allowed && @num_metareviews_total == @assignment.num_metareviews_allowed %>
Note: You can not do more than <%=@assignment.num_metareviews_allowed%> meta-reviews according
to assignment policy.
<% elsif (!@assignment.metareview_available?(session[:user].id)) %>
Meta-reviews cannot be performed at this time.
<% else %>
<%= render :partial => 'set_dynamic_metareview', :locals => {:assignment => @assignment} %>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 0 reviewers. The user should NOT be able to see the 'Request a new meta-review to perform' button when there are no reviews available.
** with 1 reviewer.
*** A student should NOT be able to request a meta-review on their own review.
*** When the meta-review limit on the assignment is set to 1 then a student should NOT be able to request a second meta-review.
*** The user should not be able to see the 'Request a new meta-review to perform' button when they have performed all valid reviews.
** with 4 reviewers.
*** The user should be able to click the 'Begin' button and still see the 'Request a new meta-review to perform' button.
*** The user should NOT be able to see the 'Request a new meta-review to perform' button when they have reached the meta-review limit.
*** If the user requests two meta-reviews but has not submitted, then they should not be able to request a new meta-review.
*** The user should be able to request a meta-review if they are above their required but below the allowed number of meta-reviews.

== '''[https://github.com/RPHolloway/expertiza/issues/30 #30 - “Numbers of Meta-reviews left:” does not update after a user requests a review.]''' ==
===Issue Description===
Number of meta-reviews left should decrement when a user requests a meta-review. Current behavior only decremented when a review was submitted.

===Changes===

Previous behavior:

Number of Meta-Reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_completed %>

Fixed behavior:

Number of meta-reviews left: <%= @assignment.num_metareviews_allowed - @num_metareviews_total %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user should see the number of meta-reviews left decrement after a new meta-review is requested.
*** The user should see the number of meta-reviews left decrement after a new meta-review is completed.

== '''[https://github.com/RPHolloway/expertiza/issues/43 #43 - "Review Allowed" flag is required to be set on a due_date for a user to perform a meta-review.]''' ==
===Issue Description===
When an assignment with a meta-review is created the round that meta-reviews will be performed in should have the "Meta-review allowed" option set to yes. This should be the only requirement for making meta-reviews available in that round. The problem with the current implementation is that meta-reviews can only be performed in rounds where the "Review allowed" option is set.

[[File:E2025 Issue43.png]]

===Changes===
Code changes were applied to app/views/student_review/_responses.html.erb. Since this partial is shared by both reviews and meta-reviews in list.html.erb the title variable was used to determine if the begin option should be active or not.

Previous behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" && @assignment.can_review(topic_id) %>
Current behavior:
<% elsif @assignment.get_current_stage(topic_id) != "Complete" &&
((@assignment.can_review(topic_id) && title == "Review") ||
(@assignment.metareview_allowed(topic_id) && title == "Meta-review")) %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. All meta-review tests make meta-reviews allowed only during the meta-review round. This means that if the simple meta-review test is passing then this feature is working as intended.

== '''[https://github.com/RPHolloway/expertiza/issues/50 #50 - The submit button is not available for meta-reviews.]''' ==
===Issue Description===
Users should see save and submit buttons as part of the meta-review form. These buttons should have the same behavior as the review form. The problem right now is there is not submit button for meta-reviews.
===Changes===
Code changes were made adding 'Metareview' to the conditional that determines if the submit button should be shown or not.

Previous behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review')) %>
New behavior:
<% if @title && ((@title.eql? 'Review') || (@title.eql? 'Self Review') || (@title.eql? 'Metareview'))%>
<%= submit_tag "Submit "+@title, :name => "Submit", :id => "Submit" %>
<img src="/assets/info.png" title='- SAVE will only store the peer-review as draft and the peer-review will not be displayed to the reviewee. After clicking SUBMIT, the peer-review will be displayed to reviewee, and you will not be able to update it any more.'>
<% end %>

===Testing===
Functional tests from a student perspective are in /spec/features/meta_review_spec.rb. The tests assume the allowed number of meta-reviews is 5 and the required number of meta-reviews is 3. Tests for this feature are explained in summary below:
* For assignments that have a meta-review deadline...
** with 1 reviewer.
*** The user is able to SUBMIT a simple meta-review.

== '''Testing''' ==

=== Manual Testing ===

1. Log in as super_administrator2. Password is "password". Click on mange...-> Assignments.

2. Create a new Assignment by clicking the "+" button. Give the assignment these fields: Name, Course and Submission Directory.

[[File:New_assignment_general.png]]

3. Go to "Due Dates" tab and check "Use meta-review deadline" and then set the due dates 1 into the FUTURE for each review round and meta-review. Leave everything else as default as shown below:

[[File:New_assignment_duedates.png]]

4. Go to the "Rubrics" tab and populate as shown below:

[[File:New_assignment_rubrics.png]]

5. OPTIONAL: Go to "Review strategy page and select "Has meta-review limit?" and set the number of allowed and required meta-reviews.

6. Once done, "Create" the assignment.

7. Go back to Assignments. manage...-> Assignments.

8. Find the newly created assignment and click on "Add participant". Add the students below:

[[File:add_participants.png]]

9. Go to manage...Impersonate User and impersonate "Student485"

10. Go to the newly created assignment and submit a link for the assignment.

11. Revert back to super_administrator2 and change the submission due date to a date in the PAST so we are currently in the review stage.

12. Impersonate student486 and request for a new review under others work, submit the review.

13. Revert back to super_administrator2 as previously described and change the review due date to be passed so we are currently in the meta-review stage.

14. Impersonate as student485 and Request for a new meta review.

15. Press "Begin" and complete the meta-review

16. When the review is complete, the "Begin" should disappear and be replaced with "View" and "Edit"

===Automated Test Plan===
==== Context 1: Meta-review tests that have a meta-review deadline with 0 reviewers. ====
1. User should NOT be able to see 'Request a new meta-review to perform' button when there are no reviews available. When there are reviews that don't belong to the meta-reviewer submitted, meta-reviews can be performed.

==== Context 2: Meta-review tests that have a meta-review deadline with 1 reviewers. ====
1. User is able to SAVE a simple meta-review.

2. User is able to go BACK during a simple meta-review since this functionality is available for reviews.

3. When the limit and required number of meta-reviews on the assignment are equal, then a student will see they need to submit exactly that number of meta-reviews.

4. When there are more meta-reviews allowed than required, then a student will see they need to submit between the two numbers of meta-reviews

5. When the meta-review limits for an assignment are unset then a student will see that the number of meta-reviews aren't limited

6. A student should not see a number of required or allowed meta-reviews when there are no limits on meta-reviews

7. A student should see the number of meta-reviews decrement after a review is requested

8. A student should see the number of meta-reviews decrement after they complete a review

9. A student should not be able to request a meta-review on their own reviews

10. If the meta-review limit on the assignment is set to 1 then a student should not be able to request a second meta review

11. User should not be able to see 'Request a new meta-review to perform' button when they have reviewed all valid reviews already

==== Context 3: Meta-review tests that have a meta-review deadline with 4 reviewers. ====
1. User should be able to request more than one meta-review before completing any

2. User should not be able to see 'Request a new meta-review to perform' button when they have reached the meta-review limit

3. User should not be able to request more than two meta-reviews without completing at least 1

4. A student should be able to request a meta-review if they are above their required but below their allowed meta-reviews

[[File:E2025_tests_passing.PNG]]

===Coverage===
Current test coverage is 41.348%. Code coverage will increase based on the test plan above and then if the failing test case described below would be fixed.

Travis CI is currently failing because of:
Failures:
1) assignment function general tab should edit quiz number available to students
Failure/Error:
child_nodes.each do |node|
initialize_fnode_update_children(params, node, tmp_res)
end
NoMethodError:
undefined method `each' for "":String
# ./app/controllers/tree_display_controller.rb:209:in `children_node_ng'
# ./config/diagnostic.rb:11:in `call'
# ------------------
# --- Caused by: ---
# Capybara::CapybaraError:
# Your application server raised an error - It has been raised in your test code because Capybara.raise_server_errors == true
# /home/travis/build/expertiza/expertiza/vendor/bundle/ruby/2.2.0/gems/capybara-2.17.0/lib/capybara/session.rb:145:in `raise_server_error!'

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T23:56:00Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T23:55:42Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T23:55:06Z

Pgarg5:

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T22:50:24Z

Pgarg5:

This wiki page is for the information regarding the changes made for the E2223 OSS assignment for Spring 2022, CSC/ECE 517.

== Introduction ==
Expertiza is a open source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.

== Peer Review Information ==

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5899, password -> password

== About Controller ==
The sign up sheet controller
* Allows an instructor to add/remove topics
* Allows an instructor to assign/remove students to topics
* Allows an student to see the list of available topics which can be bid on for given OSS assignment.

== Issues ==
1. Naming is inconsistent. When used as a verb, “sign up” is two words. When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.” Cf. “sign_up_as_instructor”. Please make the naming consistent. Of course, this will result in changes in calling methods as well.

2. Add_signup_topics_staggered does not do anything different from add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set. [Assignment 1042 has staggered deadlines]

3. Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)

4. What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed. Provide comments as to what each method does.

5. The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.

6. Signup_as_instructor_action has if-else ladder. It can be made more elegant.

7. Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.

== Solutions ==
1. Changed the name of the controller from sign_up_sheet_controller.rb --> signup_sheet_controller.rb. Also changed several method names that had 'sign_up' being used as an adjective or a noun to 'signup'.

2. Changed the create method to only create a new topic if that topic doesn't already exist. If it does exist, the program flashes an error message and backs out.

3. Used a single update_attributes method in place of the many instance variables.

'''Before'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
@topic.topic_identifier = params[:topic][:topic_identifier]
update_max_choosers @topic
@topic.category = params[:topic][:category]
@topic.topic_name = params[:topic][:topic_name]
@topic.micropayment = params[:topic][:micropayment]
@topic.description = params[:topic][:description]
@topic.link = params[:topic][:link]
@topic.save
params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

'''After'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
update_max_choosers @topic
updated_max_choosers = @topic.max_choosers
@topic.update_attributes(topic_identifier: params[:topic][:topic_identifier], max_choosers: updated_max_choosers, category: params[:topic][:category], topic_name: params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

4. The requirement that deadlines be submitted is no longer implemented and therefore a separate function for "staggered" is no longer needed.

5. Ad_info was renamed to ads_for_partners_info to give better picture of what function does.
https://github.com/palvitgarg99/expertiza/commit/6507f15f6d23f44900465536a91cc6c80f90ad75

6. signup_as_instructor is not needed and has been removed.

7. The list method is broken into 2 functions. The small part of computing signup topics is created. Also comments has been added to understand the functio better.
https://github.com/palvitgarg99/expertiza/commit/4b7bb6113dfb8984d7aa4dca467b55f67de3c93a
https://github.com/palvitgarg99/expertiza/commit/bd86f5a4ec8aa02c27a0cd34c85bd21776feca2c
https://github.com/palvitgarg99/expertiza/commit/56be9eed8ac89632041608947108f87dfbcb28ab

8. Refactored the participants variable and improved readability.

'''Before'''
<pre>
@participants = SignedUpTeam.find_team_participants(assignment_id, session[:ip])
</pre>

'''After'''
<pre>
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
</pre>

9. Previously signup_as_instructor_action provided unhelpful feedback to the instructor by only stating if the instructor could/couldn't assign a topic for a student. The refactored code provides the instructor with the student id of the person that they are assigning, the team that the student is on, and the topic that the instructor is assigning to the team.

'''Before'''
<pre>
def signup_as_instructor_action
user = User.find_by(name: params[:username])
if user.nil? # validate invalid user
flash[:error] = "That student does not exist!"
else
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up the student for the topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up student for topic: ' + params[:topic_id].to_s)
else
flash[:error] = "The student has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a student who already has a topic')
end
else
flash[:error] = "The student is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'The student is not registered for the assignment: ' << user.id)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

'''After'''
<pre>
def signup_as_instructor_action
#put name of student and team in log
# flash name of student and team
user = User.find_by(name: params[:username])
team = Team.find_team_for_assignment_and_user(params[:assignment_id], user.id).first
assignment = Assignment.find(params[:assignment_id])
if user.nil? # validate invalid user
flash[:error] = user.name + " does not exist!"
else #If the user is not null and the student exists then sign up student for topic
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up " + user.name + " on " + team.name + " for the topic " + params[:topic_id].to_s
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up ' + user.name + ' on team ' + team.name + ' for topic: ' + params[:topic_id].to_s)
else #If the students team already has a topic then cancel the sign up
flash[:error] = user.name + " on " + team.name + " has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a ' + user.name + 'on ' + team.name + ' who already has a topic')
end
else #If the student isn't a part of the assignment then cancel the sign up
flash[:error] = user.name + " is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', user.name + ' is not registered for the assignment: ' + assignment.name)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

10. delete_signup and delete_signup_as_instructor violated the DRY principle due to both functions using identical if/else statements. To solve this issue, delete_signup_heleper function was created.

'''Before'''
<pre>
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "You have already submitted your work, so you are not allowed to drop your topic."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop your topic after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], session[:user].id)
flash[:success] = "You have successfully dropped your topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "The student has already submitted their work, so you are not allowed to remove them."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for already submitted work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop a student after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], participant.user_id)
flash[:success] = "You have successfully dropped the student from the topic!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
end
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

'''After'''
<pre>
def delete_signup_helper(isInstructor, participant, assignment, drop_topic_deadline)
messages = ["You have already submitted your work, so you are not allowed to drop your topic.",
"You cannot drop your topic after the drop topic deadline!",
"You have successfully dropped your topic!",
"The team has already submitted their work, so you are not allowed to remove them.",
"You cannot drop a student after the drop topic deadline!",
"You have successfully dropped the student from the topic!"
]
if isInstructor == false
flash1 = messages[0]
flash2 = messages[1]
flash3 = messages[2]
id_param = session[:user]
else
flash1 = messages[3]
flash2 = messages[4]
flash3 = messages[5]
id_param = participant
end

if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = flash1
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = flash2
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], id_param.id)
flash[:success] = flash3
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
end

# this function is used to delete a previous signup
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
delete_signup_helper(false, participant, assignment, drop_topic_deadline)
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
delete_signup_helper(true, participant, assignment, drop_topic_deadline)
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

== Testing ==
; RSpec
: signup_as_instructor_action is the only function updated. All other functions has remained the same. To run the spec file, run:
: rspec spec/controllers/signup_sheet_controller_spec.rb
; Physical Testing
:
# Login as instructor
# Hover on Manage Tab
# Click on Assignments Tab
# Click on Edit Button for "OSS project & documentation" assignment
# Select the Topic Tab
# Click the Green Check for any topic
# Enter a student UnityID and click submit

# Login as student
# Click on OSS project/Writing assignment 2
# Click on Signup sheet
# The list must be visible, which indiactes functionality is as before after refactoring list fucntion.

CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb

2022-03-21T22:18:53Z

Pgarg5: Pgarg5 moved page CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb to CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

#REDIRECT [[CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb]]

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T22:18:53Z

Pgarg5: Pgarg5 moved page CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb to CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

This wiki page is for the information regarding the changes made for the E2223 OSS assignment for Spring 2022, CSC/ECE 517.

== Introduction ==
Expertiza is a open source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.

== Peer Review Information ==

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5899, password -> password

== About Controller ==
The sign up sheet controller
* Allows an instructor to add/remove topics
* Allows an instructor to assign/remove students to topics
* Allows an student to see the list of available topics which can be bid on for given OSS assignment.

== Issues ==
1. Naming is inconsistent. When used as a verb, “sign up” is two words. When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.” Cf. “sign_up_as_instructor”. Please make the naming consistent. Of course, this will result in changes in calling methods as well.

2. Create method has and if-else condition determining if create or update should be called. Create method should not be responsible for calling update. Identify why the if-else condition exists and rectify it. [This may have already been fixed.]

3. Update method has a plethora of instance variables defined before updating. These might not be necessary (For e.g., look at update method of bookmarks_controller)Decide whether so many instance variables are really needed. Refactor the variables not needed out. [This may have already been fixed, but save_topic_deadline also has these problems.]

4. Add_signup_topics_staggered does not do anything different from add add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set.

5. Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)

6. What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed. Provide comments as to what each method does.

7. The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.

8. Refactor participants variable in load_add_signup_topics

9. Signup_as_instructor_action has if-else ladder. It can be made more elegant.

10. Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.

== Solutions ==
1. Changed the name of the controller from sign_up_sheet_controller.rb --> signup_sheet_controller.rb. Also changed several method names that had 'sign_up' being used as an adjective or a noun to 'signup'.

2. Changed the create method to only create a new topic if that topic doesn't already exist. If it does exist, the program flashes an error message and backs out.

3. Used a single update_attributes method in place of the many instance variables.

'''Before'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
@topic.topic_identifier = params[:topic][:topic_identifier]
update_max_choosers @topic
@topic.category = params[:topic][:category]
@topic.topic_name = params[:topic][:topic_name]
@topic.micropayment = params[:topic][:micropayment]
@topic.description = params[:topic][:description]
@topic.link = params[:topic][:link]
@topic.save
params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

'''After'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
update_max_choosers @topic
updated_max_choosers = @topic.max_choosers
@topic.update_attributes(topic_identifier: params[:topic][:topic_identifier], max_choosers: updated_max_choosers, category: params[:topic][:category], topic_name: params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

4. The requirement that deadlines be submitted is no longer implemented and therefore a separate function for "staggered" is no longer needed.

5. Load_add_signup_topics was removed and combined with add_signup_topics. Ad_info was renamed to ads_for_topic and refactored to remove instance variables.

'''Before'''
<pre>
def add_signup_topics
load_add_signup_topics(params[:id])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

# retrieves all the data associated with the given assignment. Includes all topics,
def load_add_signup_topics(assignment_id)
@id = assignment_id
@sign_up_topics = SignUpTopic.where('assignment_id = ?', assignment_id)
@slots_filled = SignUpTopic.find_slots_filled(assignment_id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(assignment_id)

@assignment = Assignment.find(assignment_id)
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ad_info(assignment.id, topic.id)
.........

def ad_info(_assignment_id, topic_id)
# List that contains individual result object
@result_list = []
# Get the results
@results = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
@results.each do |result|
team = result.team
topic = result.topic
resultMap = {}
resultMap[:team_id] = team.id
resultMap[:comments_for_advertisement] = team.comments_for_advertisement
resultMap[:name] = team.name
resultMap[:assignment_id] = topic.assignment_id
resultMap[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
@result_list.append(resultMap)
end
@result_list
end
</pre>

'''After'''
<pre>
def add_signup_topics
/ load_add_signup_topics(params[:id])/
# retrieves all the data associated with the given assignment. Includes all topics,
@id = params[:id]
@sign_up_topics = SignUpTopic.where('assignment_id = ?', params[:id])
@slots_filled = SignUpTopic.find_slots_filled(params[:id])
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(params[:id])

@assignment = Assignment.find(params[:id])
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
@participants = SignedUpTeam.find_team_participants(params[:id], session[:ip])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ads_for_topic(assignment.id, topic.id)
.........

def ads_for_topic(_assignment_id, topic_id)
# List that contains individual result object
ads = []
# Get the results
signed_up_teams = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
signed_up_teams.each do |result|
team = result.team
topic = result.topic
new_ad = {}
new_ad[:team_id] = team.id
new_ad[:comments_for_advertisement] = team.comments_for_advertisement
new_ad[:name] = team.name
new_ad[:assignment_id] = topic.assignment_id
new_ad[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
ads.append(new_ad)
end
ads
end
</pre>

6. signup_as_instructor is not needed and has been removed.

7. Comments on the list method have been provided, but the code is best left in the list method.

8. Refactored the participants variable and improved readability.

'''Before'''
<pre>
@participants = SignedUpTeam.find_team_participants(assignment_id, session[:ip])
</pre>

'''After'''
<pre>
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
</pre>

9. Previously signup_as_instructor_action provided unhelpful feedback to the instructor by only stating if the instructor could/couldn't assign a topic for a student. The refactored code provides the instructor with the student id of the person that they are assigning, the team that the student is on, and the topic that the instructor is assigning to the team.

'''Before'''
<pre>
def signup_as_instructor_action
user = User.find_by(name: params[:username])
if user.nil? # validate invalid user
flash[:error] = "That student does not exist!"
else
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up the student for the topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up student for topic: ' + params[:topic_id].to_s)
else
flash[:error] = "The student has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a student who already has a topic')
end
else
flash[:error] = "The student is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'The student is not registered for the assignment: ' << user.id)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

'''After'''
<pre>
def signup_as_instructor_action
#put name of student and team in log
# flash name of student and team
user = User.find_by(name: params[:username])
team = Team.find_team_for_assignment_and_user(params[:assignment_id], user.id).first
assignment = Assignment.find(params[:assignment_id])
if user.nil? # validate invalid user
flash[:error] = user.name + " does not exist!"
else #If the user is not null and the student exists then sign up student for topic
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up " + user.name + " on " + team.name + " for the topic " + params[:topic_id].to_s
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up ' + user.name + ' on team ' + team.name + ' for topic: ' + params[:topic_id].to_s)
else #If the students team already has a topic then cancel the sign up
flash[:error] = user.name + " on " + team.name + " has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a ' + user.name + 'on ' + team.name + ' who already has a topic')
end
else #If the student isn't a part of the assignment then cancel the sign up
flash[:error] = user.name + " is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', user.name + ' is not registered for the assignment: ' + assignment.name)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

10. delete_signup and delete_signup_as_instructor violated the DRY principle due to both functions using identical if/else statements. To solve this issue, delete_signup_heleper function was created.

'''Before'''
<pre>
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "You have already submitted your work, so you are not allowed to drop your topic."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop your topic after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], session[:user].id)
flash[:success] = "You have successfully dropped your topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "The student has already submitted their work, so you are not allowed to remove them."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for already submitted work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop a student after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], participant.user_id)
flash[:success] = "You have successfully dropped the student from the topic!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
end
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

'''After'''
<pre>
def delete_signup_helper(isInstructor, participant, assignment, drop_topic_deadline)
messages = ["You have already submitted your work, so you are not allowed to drop your topic.",
"You cannot drop your topic after the drop topic deadline!",
"You have successfully dropped your topic!",
"The team has already submitted their work, so you are not allowed to remove them.",
"You cannot drop a student after the drop topic deadline!",
"You have successfully dropped the student from the topic!"
]
if isInstructor == false
flash1 = messages[0]
flash2 = messages[1]
flash3 = messages[2]
id_param = session[:user]
else
flash1 = messages[3]
flash2 = messages[4]
flash3 = messages[5]
id_param = participant
end

if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = flash1
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = flash2
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], id_param.id)
flash[:success] = flash3
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
end

# this function is used to delete a previous signup
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
delete_signup_helper(false, participant, assignment, drop_topic_deadline)
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
delete_signup_helper(true, participant, assignment, drop_topic_deadline)
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

== Testing ==
; RSpec
: signup_as_instructor_action is the only function updated. All other functions has remained the same. To run the spec file, run:
: rspec spec/controllers/signup_sheet_controller_spec.rb
; Physical Testing
:
# Login as instructor
# Hover on Manage Tab
# Click on Assignments Tab
# Click on Edit Button for "OSS project & documentation" assignment
# Select the Topic Tab
# Click the Green Check for any topic
# Enter a student UnityID and click submit

# Login as student
# Click on OSS project/Writing assignment 2
# Click on Signup sheet
# The list must be visible, which indiactes functionality is as before after refactoring list fucntion.

CSC/ECE 517 Spring 2022

2022-03-21T22:16:16Z

Pgarg5:

Expertiza Wiki:CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb

2022-03-21T22:15:03Z

Pgarg5: Pgarg5 moved page PG Wiki:CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb to CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb

#REDIRECT [[CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb]]

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T22:15:03Z

Pgarg5: Pgarg5 moved page PG Wiki:CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb to CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb

This wiki page is for the information regarding the changes made for the E2223 OSS assignment for Spring 2022, CSC/ECE 517.

== Introduction ==
Expertiza is a open source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.

== Peer Review Information ==

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5899, password -> password

== About Controller ==
The sign up sheet controller
* Allows an instructor to add/remove topics
* Allows an instructor to assign/remove students to topics
* Allows an student to see the list of available topics which can be bid on for given OSS assignment.

== Issues ==
1. Naming is inconsistent. When used as a verb, “sign up” is two words. When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.” Cf. “sign_up_as_instructor”. Please make the naming consistent. Of course, this will result in changes in calling methods as well.

2. Create method has and if-else condition determining if create or update should be called. Create method should not be responsible for calling update. Identify why the if-else condition exists and rectify it. [This may have already been fixed.]

3. Update method has a plethora of instance variables defined before updating. These might not be necessary (For e.g., look at update method of bookmarks_controller)Decide whether so many instance variables are really needed. Refactor the variables not needed out. [This may have already been fixed, but save_topic_deadline also has these problems.]

4. Add_signup_topics_staggered does not do anything different from add add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set.

5. Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)

6. What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed. Provide comments as to what each method does.

7. The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.

8. Refactor participants variable in load_add_signup_topics

9. Signup_as_instructor_action has if-else ladder. It can be made more elegant.

10. Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.

== Solutions ==
1. Changed the name of the controller from sign_up_sheet_controller.rb --> signup_sheet_controller.rb. Also changed several method names that had 'sign_up' being used as an adjective or a noun to 'signup'.

2. Changed the create method to only create a new topic if that topic doesn't already exist. If it does exist, the program flashes an error message and backs out.

3. Used a single update_attributes method in place of the many instance variables.

'''Before'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
@topic.topic_identifier = params[:topic][:topic_identifier]
update_max_choosers @topic
@topic.category = params[:topic][:category]
@topic.topic_name = params[:topic][:topic_name]
@topic.micropayment = params[:topic][:micropayment]
@topic.description = params[:topic][:description]
@topic.link = params[:topic][:link]
@topic.save
params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

'''After'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
update_max_choosers @topic
updated_max_choosers = @topic.max_choosers
@topic.update_attributes(topic_identifier: params[:topic][:topic_identifier], max_choosers: updated_max_choosers, category: params[:topic][:category], topic_name: params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

4. The requirement that deadlines be submitted is no longer implemented and therefore a separate function for "staggered" is no longer needed.

5. Load_add_signup_topics was removed and combined with add_signup_topics. Ad_info was renamed to ads_for_topic and refactored to remove instance variables.

'''Before'''
<pre>
def add_signup_topics
load_add_signup_topics(params[:id])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

# retrieves all the data associated with the given assignment. Includes all topics,
def load_add_signup_topics(assignment_id)
@id = assignment_id
@sign_up_topics = SignUpTopic.where('assignment_id = ?', assignment_id)
@slots_filled = SignUpTopic.find_slots_filled(assignment_id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(assignment_id)

@assignment = Assignment.find(assignment_id)
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ad_info(assignment.id, topic.id)
.........

def ad_info(_assignment_id, topic_id)
# List that contains individual result object
@result_list = []
# Get the results
@results = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
@results.each do |result|
team = result.team
topic = result.topic
resultMap = {}
resultMap[:team_id] = team.id
resultMap[:comments_for_advertisement] = team.comments_for_advertisement
resultMap[:name] = team.name
resultMap[:assignment_id] = topic.assignment_id
resultMap[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
@result_list.append(resultMap)
end
@result_list
end
</pre>

'''After'''
<pre>
def add_signup_topics
/ load_add_signup_topics(params[:id])/
# retrieves all the data associated with the given assignment. Includes all topics,
@id = params[:id]
@sign_up_topics = SignUpTopic.where('assignment_id = ?', params[:id])
@slots_filled = SignUpTopic.find_slots_filled(params[:id])
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(params[:id])

@assignment = Assignment.find(params[:id])
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
@participants = SignedUpTeam.find_team_participants(params[:id], session[:ip])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ads_for_topic(assignment.id, topic.id)
.........

def ads_for_topic(_assignment_id, topic_id)
# List that contains individual result object
ads = []
# Get the results
signed_up_teams = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
signed_up_teams.each do |result|
team = result.team
topic = result.topic
new_ad = {}
new_ad[:team_id] = team.id
new_ad[:comments_for_advertisement] = team.comments_for_advertisement
new_ad[:name] = team.name
new_ad[:assignment_id] = topic.assignment_id
new_ad[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
ads.append(new_ad)
end
ads
end
</pre>

6. signup_as_instructor is not needed and has been removed.

7. Comments on the list method have been provided, but the code is best left in the list method.

8. Refactored the participants variable and improved readability.

'''Before'''
<pre>
@participants = SignedUpTeam.find_team_participants(assignment_id, session[:ip])
</pre>

'''After'''
<pre>
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
</pre>

9. Previously signup_as_instructor_action provided unhelpful feedback to the instructor by only stating if the instructor could/couldn't assign a topic for a student. The refactored code provides the instructor with the student id of the person that they are assigning, the team that the student is on, and the topic that the instructor is assigning to the team.

'''Before'''
<pre>
def signup_as_instructor_action
user = User.find_by(name: params[:username])
if user.nil? # validate invalid user
flash[:error] = "That student does not exist!"
else
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up the student for the topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up student for topic: ' + params[:topic_id].to_s)
else
flash[:error] = "The student has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a student who already has a topic')
end
else
flash[:error] = "The student is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'The student is not registered for the assignment: ' << user.id)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

'''After'''
<pre>
def signup_as_instructor_action
#put name of student and team in log
# flash name of student and team
user = User.find_by(name: params[:username])
team = Team.find_team_for_assignment_and_user(params[:assignment_id], user.id).first
assignment = Assignment.find(params[:assignment_id])
if user.nil? # validate invalid user
flash[:error] = user.name + " does not exist!"
else #If the user is not null and the student exists then sign up student for topic
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up " + user.name + " on " + team.name + " for the topic " + params[:topic_id].to_s
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up ' + user.name + ' on team ' + team.name + ' for topic: ' + params[:topic_id].to_s)
else #If the students team already has a topic then cancel the sign up
flash[:error] = user.name + " on " + team.name + " has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a ' + user.name + 'on ' + team.name + ' who already has a topic')
end
else #If the student isn't a part of the assignment then cancel the sign up
flash[:error] = user.name + " is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', user.name + ' is not registered for the assignment: ' + assignment.name)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

10. delete_signup and delete_signup_as_instructor violated the DRY principle due to both functions using identical if/else statements. To solve this issue, delete_signup_heleper function was created.

'''Before'''
<pre>
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "You have already submitted your work, so you are not allowed to drop your topic."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop your topic after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], session[:user].id)
flash[:success] = "You have successfully dropped your topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "The student has already submitted their work, so you are not allowed to remove them."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for already submitted work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop a student after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], participant.user_id)
flash[:success] = "You have successfully dropped the student from the topic!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
end
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

'''After'''
<pre>
def delete_signup_helper(isInstructor, participant, assignment, drop_topic_deadline)
messages = ["You have already submitted your work, so you are not allowed to drop your topic.",
"You cannot drop your topic after the drop topic deadline!",
"You have successfully dropped your topic!",
"The team has already submitted their work, so you are not allowed to remove them.",
"You cannot drop a student after the drop topic deadline!",
"You have successfully dropped the student from the topic!"
]
if isInstructor == false
flash1 = messages[0]
flash2 = messages[1]
flash3 = messages[2]
id_param = session[:user]
else
flash1 = messages[3]
flash2 = messages[4]
flash3 = messages[5]
id_param = participant
end

if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = flash1
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = flash2
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], id_param.id)
flash[:success] = flash3
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
end

# this function is used to delete a previous signup
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
delete_signup_helper(false, participant, assignment, drop_topic_deadline)
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
delete_signup_helper(true, participant, assignment, drop_topic_deadline)
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

== Testing ==
; RSpec
: signup_as_instructor_action is the only function updated. All other functions has remained the same. To run the spec file, run:
: rspec spec/controllers/signup_sheet_controller_spec.rb
; Physical Testing
:
# Login as instructor
# Hover on Manage Tab
# Click on Assignments Tab
# Click on Edit Button for "OSS project & documentation" assignment
# Select the Topic Tab
# Click the Green Check for any topic
# Enter a student UnityID and click submit

# Login as student
# Click on OSS project/Writing assignment 2
# Click on Signup sheet
# The list must be visible, which indiactes functionality is as before after refactoring list fucntion.

User:Pgarg5

2022-03-21T22:14:31Z

Pgarg5: Pgarg5 moved page User:Pgarg5 to PG Wiki:CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb

#REDIRECT [[PG Wiki:CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb]]

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T22:14:31Z

Pgarg5: Pgarg5 moved page User:Pgarg5 to PG Wiki:CSC/ECE 517 Fall 2021 - E2223. Refactor sign up sheet controller.rb

This wiki page is for the information regarding the changes made for the E2223 OSS assignment for Spring 2022, CSC/ECE 517.

== Introduction ==
Expertiza is a open source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.

== Peer Review Information ==

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5899, password -> password

== About Controller ==
The sign up sheet controller
* Allows an instructor to add/remove topics
* Allows an instructor to assign/remove students to topics
* Allows an student to see the list of available topics which can be bid on for given OSS assignment.

== Issues ==
1. Naming is inconsistent. When used as a verb, “sign up” is two words. When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.” Cf. “sign_up_as_instructor”. Please make the naming consistent. Of course, this will result in changes in calling methods as well.

2. Create method has and if-else condition determining if create or update should be called. Create method should not be responsible for calling update. Identify why the if-else condition exists and rectify it. [This may have already been fixed.]

3. Update method has a plethora of instance variables defined before updating. These might not be necessary (For e.g., look at update method of bookmarks_controller)Decide whether so many instance variables are really needed. Refactor the variables not needed out. [This may have already been fixed, but save_topic_deadline also has these problems.]

4. Add_signup_topics_staggered does not do anything different from add add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set.

5. Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)

6. What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed. Provide comments as to what each method does.

7. The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.

8. Refactor participants variable in load_add_signup_topics

9. Signup_as_instructor_action has if-else ladder. It can be made more elegant.

10. Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.

== Solutions ==
1. Changed the name of the controller from sign_up_sheet_controller.rb --> signup_sheet_controller.rb. Also changed several method names that had 'sign_up' being used as an adjective or a noun to 'signup'.

2. Changed the create method to only create a new topic if that topic doesn't already exist. If it does exist, the program flashes an error message and backs out.

3. Used a single update_attributes method in place of the many instance variables.

'''Before'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
@topic.topic_identifier = params[:topic][:topic_identifier]
update_max_choosers @topic
@topic.category = params[:topic][:category]
@topic.topic_name = params[:topic][:topic_name]
@topic.micropayment = params[:topic][:micropayment]
@topic.description = params[:topic][:description]
@topic.link = params[:topic][:link]
@topic.save
params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

'''After'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
update_max_choosers @topic
updated_max_choosers = @topic.max_choosers
@topic.update_attributes(topic_identifier: params[:topic][:topic_identifier], max_choosers: updated_max_choosers, category: params[:topic][:category], topic_name: params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

4. The requirement that deadlines be submitted is no longer implemented and therefore a separate function for "staggered" is no longer needed.

5. Load_add_signup_topics was removed and combined with add_signup_topics. Ad_info was renamed to ads_for_topic and refactored to remove instance variables.

'''Before'''
<pre>
def add_signup_topics
load_add_signup_topics(params[:id])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

# retrieves all the data associated with the given assignment. Includes all topics,
def load_add_signup_topics(assignment_id)
@id = assignment_id
@sign_up_topics = SignUpTopic.where('assignment_id = ?', assignment_id)
@slots_filled = SignUpTopic.find_slots_filled(assignment_id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(assignment_id)

@assignment = Assignment.find(assignment_id)
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ad_info(assignment.id, topic.id)
.........

def ad_info(_assignment_id, topic_id)
# List that contains individual result object
@result_list = []
# Get the results
@results = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
@results.each do |result|
team = result.team
topic = result.topic
resultMap = {}
resultMap[:team_id] = team.id
resultMap[:comments_for_advertisement] = team.comments_for_advertisement
resultMap[:name] = team.name
resultMap[:assignment_id] = topic.assignment_id
resultMap[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
@result_list.append(resultMap)
end
@result_list
end
</pre>

'''After'''
<pre>
def add_signup_topics
/ load_add_signup_topics(params[:id])/
# retrieves all the data associated with the given assignment. Includes all topics,
@id = params[:id]
@sign_up_topics = SignUpTopic.where('assignment_id = ?', params[:id])
@slots_filled = SignUpTopic.find_slots_filled(params[:id])
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(params[:id])

@assignment = Assignment.find(params[:id])
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
@participants = SignedUpTeam.find_team_participants(params[:id], session[:ip])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ads_for_topic(assignment.id, topic.id)
.........

def ads_for_topic(_assignment_id, topic_id)
# List that contains individual result object
ads = []
# Get the results
signed_up_teams = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
signed_up_teams.each do |result|
team = result.team
topic = result.topic
new_ad = {}
new_ad[:team_id] = team.id
new_ad[:comments_for_advertisement] = team.comments_for_advertisement
new_ad[:name] = team.name
new_ad[:assignment_id] = topic.assignment_id
new_ad[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
ads.append(new_ad)
end
ads
end
</pre>

6. signup_as_instructor is not needed and has been removed.

7. Comments on the list method have been provided, but the code is best left in the list method.

8. Refactored the participants variable and improved readability.

'''Before'''
<pre>
@participants = SignedUpTeam.find_team_participants(assignment_id, session[:ip])
</pre>

'''After'''
<pre>
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
</pre>

9. Previously signup_as_instructor_action provided unhelpful feedback to the instructor by only stating if the instructor could/couldn't assign a topic for a student. The refactored code provides the instructor with the student id of the person that they are assigning, the team that the student is on, and the topic that the instructor is assigning to the team.

'''Before'''
<pre>
def signup_as_instructor_action
user = User.find_by(name: params[:username])
if user.nil? # validate invalid user
flash[:error] = "That student does not exist!"
else
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up the student for the topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up student for topic: ' + params[:topic_id].to_s)
else
flash[:error] = "The student has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a student who already has a topic')
end
else
flash[:error] = "The student is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'The student is not registered for the assignment: ' << user.id)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

'''After'''
<pre>
def signup_as_instructor_action
#put name of student and team in log
# flash name of student and team
user = User.find_by(name: params[:username])
team = Team.find_team_for_assignment_and_user(params[:assignment_id], user.id).first
assignment = Assignment.find(params[:assignment_id])
if user.nil? # validate invalid user
flash[:error] = user.name + " does not exist!"
else #If the user is not null and the student exists then sign up student for topic
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up " + user.name + " on " + team.name + " for the topic " + params[:topic_id].to_s
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up ' + user.name + ' on team ' + team.name + ' for topic: ' + params[:topic_id].to_s)
else #If the students team already has a topic then cancel the sign up
flash[:error] = user.name + " on " + team.name + " has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a ' + user.name + 'on ' + team.name + ' who already has a topic')
end
else #If the student isn't a part of the assignment then cancel the sign up
flash[:error] = user.name + " is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', user.name + ' is not registered for the assignment: ' + assignment.name)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

10. delete_signup and delete_signup_as_instructor violated the DRY principle due to both functions using identical if/else statements. To solve this issue, delete_signup_heleper function was created.

'''Before'''
<pre>
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "You have already submitted your work, so you are not allowed to drop your topic."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop your topic after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], session[:user].id)
flash[:success] = "You have successfully dropped your topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "The student has already submitted their work, so you are not allowed to remove them."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for already submitted work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop a student after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], participant.user_id)
flash[:success] = "You have successfully dropped the student from the topic!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
end
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

'''After'''
<pre>
def delete_signup_helper(isInstructor, participant, assignment, drop_topic_deadline)
messages = ["You have already submitted your work, so you are not allowed to drop your topic.",
"You cannot drop your topic after the drop topic deadline!",
"You have successfully dropped your topic!",
"The team has already submitted their work, so you are not allowed to remove them.",
"You cannot drop a student after the drop topic deadline!",
"You have successfully dropped the student from the topic!"
]
if isInstructor == false
flash1 = messages[0]
flash2 = messages[1]
flash3 = messages[2]
id_param = session[:user]
else
flash1 = messages[3]
flash2 = messages[4]
flash3 = messages[5]
id_param = participant
end

if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = flash1
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = flash2
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], id_param.id)
flash[:success] = flash3
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
end

# this function is used to delete a previous signup
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
delete_signup_helper(false, participant, assignment, drop_topic_deadline)
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
delete_signup_helper(true, participant, assignment, drop_topic_deadline)
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

== Testing ==
; RSpec
: signup_as_instructor_action is the only function updated. All other functions has remained the same. To run the spec file, run:
: rspec spec/controllers/signup_sheet_controller_spec.rb
; Physical Testing
:
# Login as instructor
# Hover on Manage Tab
# Click on Assignments Tab
# Click on Edit Button for "OSS project & documentation" assignment
# Select the Topic Tab
# Click the Green Check for any topic
# Enter a student UnityID and click submit

# Login as student
# Click on OSS project/Writing assignment 2
# Click on Signup sheet
# The list must be visible, which indiactes functionality is as before after refactoring list fucntion.

CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

2022-03-21T21:07:47Z

Pgarg5: CSC/ECE 517 Spring 2022 - E2223. Refactor sign up sheet controller.rb

This wiki page is for the information regarding the changes made for the E2223 OSS assignment for Spring 2022, CSC/ECE 517.

== Introduction ==
Expertiza is a open source project developed on Ruby on Rails. This web application is maintained by the student and faculty at NC State. This application gives complete control to the instructor to maintain the assignments in their class. With multiple functionalities such as adding topics, creating groups, and peer reviews, Expertiza is a well developed application that can handle all types of assignments. To learn more about the full functionality Expertiza has to offer, visit the Expertiza wiki.

== Peer Review Information ==

For users intending to view the deployed Expertiza associated with this assignment, the credentials are below:
* Instructor login: username -> instructor6, password -> password
* Student login: username -> student5899, password -> password

== About Controller ==
The sign up sheet controller
* Allows an instructor to add/remove topics
* Allows an instructor to assign/remove students to topics
* Allows an student to see the list of available topics which can be bid on for given OSS assignment.

== Issues ==
1. Naming is inconsistent. When used as a verb, “sign up” is two words. When an adjective or a noun, it should be one word, e.g., “signup_sheet_controller.” Cf. “sign_up_as_instructor”. Please make the naming consistent. Of course, this will result in changes in calling methods as well.

2. Create method has and if-else condition determining if create or update should be called. Create method should not be responsible for calling update. Identify why the if-else condition exists and rectify it. [This may have already been fixed.]

3. Update method has a plethora of instance variables defined before updating. These might not be necessary (For e.g., look at update method of bookmarks_controller)Decide whether so many instance variables are really needed. Refactor the variables not needed out. [This may have already been fixed, but save_topic_deadline also has these problems.]

4. Add_signup_topics_staggered does not do anything different from add add_signup_topics. Separate functions are needed, because add_signup_topics_staggered needs to make sure that the deadlines are set.

5. Several method names can be improved (including: load_add_signup_topics, list, ad_info etc.)

6. What are differences between signup_as_instructor and signup_as_instructor_action methods? Investigate if they are needed and improve the method names if both are needed. Provide comments as to what each method does.

7. The list method is too long and is sparsely commented. Provide comments and identify if the method can be split or made cleaner by moving code to models or helper methods.

8. Refactor participants variable in load_add_signup_topics

9. Signup_as_instructor_action has if-else ladder. It can be made more elegant.

10. Delete_signup and delete_signup_as_instructor have much in common and violate the DRY principle. Refactor.

== Solutions ==
1. Changed the name of the controller from sign_up_sheet_controller.rb --> signup_sheet_controller.rb. Also changed several method names that had 'sign_up' being used as an adjective or a noun to 'signup'.

2. Changed the create method to only create a new topic if that topic doesn't already exist. If it does exist, the program flashes an error message and backs out.

3. Used a single update_attributes method in place of the many instance variables.

'''Before'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
@topic.topic_identifier = params[:topic][:topic_identifier]
update_max_choosers @topic
@topic.category = params[:topic][:category]
@topic.topic_name = params[:topic][:topic_name]
@topic.micropayment = params[:topic][:micropayment]
@topic.description = params[:topic][:description]
@topic.link = params[:topic][:link]
@topic.save
params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

'''After'''
<pre>
def update
@topic = SignUpTopic.find(params[:id])
if @topic
update_max_choosers @topic
updated_max_choosers = @topic.max_choosers
@topic.update_attributes(topic_identifier: params[:topic][:topic_identifier], max_choosers: updated_max_choosers, category: params[:topic][:category], topic_name: params[:topic][:topic_name], micropayment: params[:topic][:micropayment], description: params[:topic][:description], link: params[:topic][:link])
undo_link("The topic: \"#{@topic.topic_name}\" has been successfully updated. ")
else
flash[:error] = "The topic could not be updated."
end
# Akshay - correctly changing the redirection url to topics tab in edit assignment view.
redirect_to edit_assignment_path(params[:assignment_id]) + "#tabs-2"
end
</pre>

4. The requirement that deadlines be submitted is no longer implemented and therefore a separate function for "staggered" is no longer needed.

5. Load_add_signup_topics was removed and combined with add_signup_topics. Ad_info was renamed to ads_for_topic and refactored to remove instance variables.

'''Before'''
<pre>
def add_signup_topics
load_add_signup_topics(params[:id])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

# retrieves all the data associated with the given assignment. Includes all topics,
def load_add_signup_topics(assignment_id)
@id = assignment_id
@sign_up_topics = SignUpTopic.where('assignment_id = ?', assignment_id)
@slots_filled = SignUpTopic.find_slots_filled(assignment_id)
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(assignment_id)

@assignment = Assignment.find(assignment_id)
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ad_info(assignment.id, topic.id)
.........

def ad_info(_assignment_id, topic_id)
# List that contains individual result object
@result_list = []
# Get the results
@results = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
@results.each do |result|
team = result.team
topic = result.topic
resultMap = {}
resultMap[:team_id] = team.id
resultMap[:comments_for_advertisement] = team.comments_for_advertisement
resultMap[:name] = team.name
resultMap[:assignment_id] = topic.assignment_id
resultMap[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
@result_list.append(resultMap)
end
@result_list
end
</pre>

'''After'''
<pre>
def add_signup_topics
/ load_add_signup_topics(params[:id])/
# retrieves all the data associated with the given assignment. Includes all topics,
@id = params[:id]
@sign_up_topics = SignUpTopic.where('assignment_id = ?', params[:id])
@slots_filled = SignUpTopic.find_slots_filled(params[:id])
@slots_waitlisted = SignUpTopic.find_slots_waitlisted(params[:id])

@assignment = Assignment.find(params[:id])
# ACS Removed the if condition (and corresponding else) which differentiate assignments as team and individual assignments
# to treat all assignments as team assignments
# Though called participants, @participants are actually records in signed_up_teams table, which
# is a mapping table between teams and topics (waitlisted recored are also counted)
@participants = SignedUpTeam.find_team_participants(params[:id], session[:ip])
SignUpSheet.add_signup_topic(params[:id])
end

def add_signup_topics_staggered
add_signup_topics
end

def show_team
if !(assignment = Assignment.find(params[:assignment_id])).nil? and !(topic = SignUpTopic.find(params[:id])).nil?
@results = ads_for_topic(assignment.id, topic.id)
.........

def ads_for_topic(_assignment_id, topic_id)
# List that contains individual result object
ads = []
# Get the results
signed_up_teams = SignedUpTeam.where("topic_id = ?", topic_id.to_s)
# Iterate through the results of the query and get the required attributes
signed_up_teams.each do |result|
team = result.team
topic = result.topic
new_ad = {}
new_ad[:team_id] = team.id
new_ad[:comments_for_advertisement] = team.comments_for_advertisement
new_ad[:name] = team.name
new_ad[:assignment_id] = topic.assignment_id
new_ad[:advertise_for_partner] = team.advertise_for_partner

# Append to the list
ads.append(new_ad)
end
ads
end
</pre>

6. signup_as_instructor is not needed and has been removed.

7. Comments on the list method have been provided, but the code is best left in the list method.

8. Refactored the participants variable and improved readability.

'''Before'''
<pre>
@participants = SignedUpTeam.find_team_participants(assignment_id, session[:ip])
</pre>

'''After'''
<pre>
ip = session[:ip]
@participants = SignedUpTeam.find_team_participants(@id, ip)
</pre>

9. Previously signup_as_instructor_action provided unhelpful feedback to the instructor by only stating if the instructor could/couldn't assign a topic for a student. The refactored code provides the instructor with the student id of the person that they are assigning, the team that the student is on, and the topic that the instructor is assigning to the team.

'''Before'''
<pre>
def signup_as_instructor_action
user = User.find_by(name: params[:username])
if user.nil? # validate invalid user
flash[:error] = "That student does not exist!"
else
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up the student for the topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up student for topic: ' + params[:topic_id].to_s)
else
flash[:error] = "The student has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a student who already has a topic')
end
else
flash[:error] = "The student is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'The student is not registered for the assignment: ' << user.id)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

'''After'''
<pre>
def signup_as_instructor_action
#put name of student and team in log
# flash name of student and team
user = User.find_by(name: params[:username])
team = Team.find_team_for_assignment_and_user(params[:assignment_id], user.id).first
assignment = Assignment.find(params[:assignment_id])
if user.nil? # validate invalid user
flash[:error] = user.name + " does not exist!"
else #If the user is not null and the student exists then sign up student for topic
if AssignmentParticipant.exists? user_id: user.id, parent_id: params[:assignment_id]
if SignUpSheet.signup_team(params[:assignment_id], user.id, params[:topic_id])
flash[:success] = "You have successfully signed up " + user.name + " on " + team.name + " for the topic " + params[:topic_id].to_s
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor signed up ' + user.name + ' on team ' + team.name + ' for topic: ' + params[:topic_id].to_s)
else #If the students team already has a topic then cancel the sign up
flash[:error] = user.name + " on " + team.name + " has already signed up for a topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', 'Instructor is signing up a ' + user.name + 'on ' + team.name + ' who already has a topic')
end
else #If the student isn't a part of the assignment then cancel the sign up
flash[:error] = user.name + " is not registered for the assignment!"
ExpertizaLogger.info LoggerMessage.new(controller_name, '', user.name + ' is not registered for the assignment: ' + assignment.name)
end
end
redirect_to controller: 'assignments', action: 'edit', id: params[:assignment_id]
end
</pre>

10. delete_signup and delete_signup_as_instructor violated the DRY principle due to both functions using identical if/else statements. To solve this issue, delete_signup_heleper function was created.

'''Before'''
<pre>
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "You have already submitted your work, so you are not allowed to drop your topic."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop your topic after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping topic for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], session[:user].id)
flash[:success] = "You have successfully dropped your topic!"
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = "The student has already submitted their work, so you are not allowed to remove them."
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for already submitted work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = "You cannot drop a student after the drop topic deadline!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Drop failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], participant.user_id)
flash[:success] = "You have successfully dropped the student from the topic!"
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Student has been dropped from the topic: ' + params[:topic_id].to_s)
end
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

'''After'''
<pre>
def delete_signup_helper(isInstructor, participant, assignment, drop_topic_deadline)
messages = ["You have already submitted your work, so you are not allowed to drop your topic.",
"You cannot drop your topic after the drop topic deadline!",
"You have successfully dropped your topic!",
"The team has already submitted their work, so you are not allowed to remove them.",
"You cannot drop a student after the drop topic deadline!",
"You have successfully dropped the student from the topic!"
]
if isInstructor == false
flash1 = messages[0]
flash2 = messages[1]
flash3 = messages[2]
id_param = session[:user]
else
flash1 = messages[3]
flash2 = messages[4]
flash3 = messages[5]
id_param = participant
end

if !participant.team.submitted_files.empty? or !participant.team.hyperlinks.empty?
flash[:error] = flash1
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for already submitted a work: ' + params[:topic_id].to_s)
elsif !drop_topic_deadline.nil? and Time.now > drop_topic_deadline.due_at
flash[:error] = flash2
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].id, 'Dropping failed for ended work: ' + params[:topic_id].to_s)
else
delete_signup_for_topic(assignment.id, params[:topic_id], id_param.id)
flash[:success] = flash3
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].id, 'Student has dropped the topic: ' + params[:topic_id].to_s)
end
end

# this function is used to delete a previous signup
def delete_signup
participant = AssignmentParticipant.find(params[:id])
assignment = participant.assignment
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
# A student who has already submitted work should not be allowed to drop his/her topic!
# (A student/team has submitted if participant directory_num is non-null or submitted_hyperlinks is non-null.)
# If there is no drop topic deadline, student can drop topic at any time (if all the submissions are deleted)
# If there is a drop topic deadline, student cannot drop topic after this deadline.
delete_signup_helper(false, participant, assignment, drop_topic_deadline)
redirect_to action: 'list', id: params[:id]
end

def delete_signup_as_instructor
# find participant using assignment using team and topic ids
team = Team.find(params[:id])
assignment = Assignment.find(team.parent_id)
user = TeamsUser.find_by(team_id: team.id).user
participant = AssignmentParticipant.find_by(user_id: user.id, parent_id: assignment.id)
drop_topic_deadline = assignment.due_dates.find_by(deadline_type_id: 6)
delete_signup_helper(true, participant, assignment, drop_topic_deadline)
redirect_to controller: 'assignments', action: 'edit', id: assignment.id
end
</pre>

== Testing ==
; RSpec
: signup_as_instructor_action is the only function updated. All other functions has remained the same. To run the spec file, run:
: rspec spec/controllers/signup_sheet_controller_spec.rb
; Physical Testing
:
# Login as instructor
# Hover on Manage Tab
# Click on Assignments Tab
# Click on Edit Button for "OSS project & documentation" assignment
# Select the Topic Tab
# Click the Green Check for any topic
# Enter a student UnityID and click submit

# Login as student
# Click on OSS project/Writing assignment 2
# Click on Signup sheet
# The list must be visible, which indiactes functionality is as before after refactoring list fucntion.