Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T18:38:21Z

Lmcampb5: /* Resources */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]]
 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]]
 
* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found [https://youtu.be/RIsVKTDc5J0 here] 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T15:05:23Z

Lmcampb5: /* Files Requiring Modification */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]]
 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]]
 
* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T15:03:19Z

Lmcampb5: /* Files Requiring Modification */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]]
 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]]
 
* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T15:02:59Z

Lmcampb5: /* Files Requiring Modification */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]]
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]]
* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T15:02:24Z

Lmcampb5: /* Files Requiring Modification */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
 
 
 

**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 
 
 
 
* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T15:01:31Z

Lmcampb5: /* Files Requiring Modification */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:59:32Z

Lmcampb5: /* Files Requiring Modification */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 
 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:58:45Z

Lmcampb5: /* Files Requiring Modification */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 
 
* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:58:13Z

Lmcampb5: /* Conclusion */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

The calibration tab, in the assignment edit view, appears identical to the copied assignment.

*Future Considerations
** submitter_count, a field in the [https://expertiza.csc.ncsu.edu/index.php/Assignments assignments table] is not dealt with in our implementation. Interestingly, the existing 'Design exercise' assignment used for testing had a submitter_count of 0 the database.
** Directory paths for submissions are still not explicitly considered.
** Additional rspec tests should be created. An easy test would be to check response_maps for the new assignment after calling the copy method and verifying the same number of records exist.
** Several database queries are performed in each of our methods. Performance improvements could likely be made by considering another strategy.
** Our copy naming methods are identical, the only difference being the assignment field used to check for name uniqueness (assignment name vs directory path). We couldn't figure out how to consolidate this to a single method (how can you pass a attribute/field type to a method?)
** More defensive programming could be done, i.e. when we copy records, we don't check whether saving the new record is successful. If we did check and a copy failed to save, what should be done?

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:46:00Z

Lmcampb5: /* Conclusion */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Calibration_copy_testing.PNG]]

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

File:Calibration copy testing.PNG

2022-04-30T14:45:52Z

Lmcampb5:

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:41:09Z

Lmcampb5: /* Conclusion */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Test_expertiza_calibration_copy.PNG]]

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

File:Test expertiza calibration copy.PNG

2022-04-30T14:40:37Z

Lmcampb5:

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:39:52Z

Lmcampb5: /* Conclusion */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==
Our testing was all manual testing. Like the old team, we used the assignment titled "Design exercise" from the expertiza_development database, while logged in as instructor6 to verify our implementation.

[[File:Example.jpg]]

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:34:39Z

Lmcampb5: /* Final Implementation */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews' is called in the existing assignment_form class method 'copy', if the copied assignment is able to be saved (we don't want to create all of these new records pointlessly).

We also added methods for copy naming schemes. This is needed because the assignment model validates uniqueness of both the assignment name and directory_path.

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:29:47Z

Lmcampb5: /* Final Implementation */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method, if the original assignment was calibrated, calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews. If the original assignment is not calibrated, this method does nothing.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]
* We copy all teams with a parent_id matching the original assignment ID. See [https://expertiza.csc.ncsu.edu/index.php/Teams teams database documentation]
* We recreate mappings needed to associate participants with teams and review_response maps.
** Our teams and participants copy methods return hash maps that are used to lookup previous mappings. For each new team and participant, we can lookup how the original records were mapped
*** Create new TeamsUsers. This associates participants with teams. See [https://expertiza.csc.ncsu.edu/index.php/Teams_users teams users database documentation]
*** Create new ReviewResponseMap. This is a type of response map that associates reviewers and reviewees, linked to an assignment, together. See [https://expertiza.csc.ncsu.edu/index.php/Response_maps response map database documentation]
* We copy all review_responses, using the original review response map as a reference. See [https://expertiza.csc.ncsu.edu/index.php/Responses response database documentation]

Our new 'copy_calibrated_reviews

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:21:26Z

Lmcampb5: /* Final Implementation */

== '''Introduction''' ==
A “calibration assignment” is an assignment where the students are asked to review work that has also been reviewed by a member of the course staff. If the student’s review “resembles” the staff-member’s review, then the student is presumed to be a competent reviewer. Here is a further description of calibration assignments. To set up calibration, the instructor (or TA) adds a few extra participants to the assignment. The instructor (or TA) then impersonates the extra participants, and submits work on behalf of each of the extra participants.

== '''Problem Statement''' ==
Having the instructor (or TA) impersonate the extra participants, and submit work on behalf of each of the extra participants is extra trouble. It would be nice if an instructor didn’t have to resubmit the same calibration submissions every semester. Copying the extra participants along with their teams, submissions, and responses when copying an assignment makes things much easier.

== '''Previous Implementation''' ==
The previous project did fully implement code that copies over the calibration submissions whenever a calibrated assignment is submitted. It was done by ensuring that when a assignment is copied that 1) the same extra participants as the assignment you are copying are used 2) the URLs and/or files that were submitted to the previous assignment are also copied and 3) the URLs and files are submitted to the assignment as well

 
Shown below is a diagram of how the current system works (copying the extra participants/reviews):
[[File:Calibration2.png|600px|left]]
 
 
 

The previous implementation needs to be revisited and made more elegant. A description of the needed refactoring and changes is below.

=== '''What needs to be done''' ===
*Ensure what gets copied (when and how) is done properly
**Reviews done by the creator of the assignment are copied, but only reviews that have been started --> how do you recognize a calibration review?: Can check 'calibrate_to' field in response maps where 'reviewed_object_id' equals the assignment ID being copied
**If an author did any non-calibration reviews in the assignment being copied, those reviews would also be treated as calibration reviews
**Submitted files are copied too, but if the calibration submissions are done as part of the COPYING of the assignment, they will necessarily use directories 0, 1,
*Handling the case of students submitting assignments before calibration assignments are done
**Project assumes that calibrations start at directory 0
**Not the case if any students submit before the calibration submissions are done
*Submitter_count needs to be set to the largest directory number that was used for calibration
**First student to submit will have their submission show up in the same directory with the first calibration submission
*Several class methods in assignment_form.rb should be moved to different classes as instance methods in those classes
**Add better comments - eg. a comment for a method that copies an assignment should not reveal details about calibrations. Currently, only 1/4 methods in this class have comments.
**Currently, for assignment_form.rb we change the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc. We will improve this by making it "Copy of <name> <copy number>" and putting this function in a new method.
**[[File:wiki1.png|800px|left]] 
 
 
 
 
 
 
*Clean up participant.rb
**Look for a more elegant way of (self.create_participant) copying/creating of participants → DRY violation
**Write the create method in assignment_participant instead of directly in participant.rb
*Clean up team.rb
**Create a copy method in team that copies the field used by all teams
**Rest of the stuff is just used by assignment teams (submitted hyperlinks etc.)
**Create a copy method and override in assignment_team
*Reevaluate instance variables. Are these needed, or could they be local variables instead?
*Look at editing out 9min video due to long waits for responses during copying
*Verify our work using not only user 'Instructor6' but also Professor Jennifer Kidd's account - 'Instructor 2026'

== '''Design''' ==

==='''Files Requiring Modification'''===
* assignment_form.rb
**Intead of setting the name of the copied assignments to “copy of copy” and "copy of copy of copy" etc, We will change it to name "Copy of <name> <copy number>" and put this function in a new method. This code currently resides in the self.copy method (which is too long and does too much for one method), so we will move it to a new method called copy_name.
**[[File:wiki1.png|800px|left]] 
**Implement new method for copying objects needed to recreate calibration reviews

* participant.rb
** Write the create method in assignment_participant instead of directly in participant.rb
** Here is the current create method in participant.rb:
** [[File:wiki2.png|600px|left]] 

* response.rb
** Implement method for copying previous responses

* response_map.rb
** Implement method that creates ReviewResponse mapping, based on mapping of copied assignment

* submission_record.rb
** Implement method for copying submission records of an assignment

* team.rb
** Create a copy method in team.rb that copies all teams of a previous assignment

* teams_user.rb
** Create method for creating new teams_users so that new participants and teams are associated the same way they were for the previous assignment

* assignment_participant.rb
** Method for copying extra participants should be here, we wouldn't need to copy participants with type 'CourseParticipant'

== '''Final Implementation''' ==
We implemented a new class method, 'copy_calibrated_reviews', in the assignment_form model. This method calls methods in models listed in previous section to copy all objects needed to recreate calibrated reviews.
* We copy all submission records with assignment_id matching the original assignment. See [https://expertiza.csc.ncsu.edu/index.php/Submission_records submission record database documentation]
* We copy all participants with parent_id matching the original assignment ID. We did this in the participant model but should have moved it to assignment_participant since we are not interested in course participants. See [https://expertiza.csc.ncsu.edu/index.php/Participants participant database documentation]

== '''Test Plan''' ==

=== '''Testing Goals''' ===

The main goal of our testing is to ensure that when a new calibration assignment is added, all of the qualifying previous calibration submissions
and reviews are successfully copied over without overwriting any existing submissions.

=== '''Automated Unit Tests''' ===

Using RSpec, we have to test that the following unit tests pass:

1. When the TA adds a calibration assignment, extra participants are added to the assignment.

2. When extra participants are added to an assignment, calibration submissions are copied over.

3. When calibration submissions are copied over, the submitter_count value increases by the number of calibration submissions, so that previous submissions are
not overwritten.

4. After calibration assignments are copied over, only calibration reviews from the creator of the assignment are copied over.

5. After calibration assignments are copied over, only calibration reviews that have been started are copied over.

6. Calling specific methods in assignment.rb return a NoMethodError to indicate that these methods are no longer class methods and have been moved elsewhere as
instance methods.

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to our pull request can be found [https://github.com/expertiza/expertiza/pull/2403 here] 
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub pull request can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:13:17Z

Lmcampb5: /* Files Requiring Modification */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:12:03Z

Lmcampb5: /* Files Requiring Modification */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:11:00Z

Lmcampb5: /* Files Requiring Modification */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T14:03:46Z

Lmcampb5: /* What needs to be done */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T13:55:04Z

Lmcampb5: /* Problem Statement */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T13:41:42Z

Lmcampb5: /* Resources */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-30T13:41:17Z

Lmcampb5: /* Resources */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-07T00:26:27Z

Lmcampb5: /* What needs to be done */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-07T00:20:03Z

Lmcampb5: /* Files Requiring Modification */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-07T00:10:15Z

Lmcampb5: /* What needs to be done */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-07T00:06:43Z

Lmcampb5:

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-06T23:59:44Z

Lmcampb5: /* Resources */

== '''Introduction''' ==
Add description for E2234

== '''Problem Statement''' ==
Add description

== '''Previous Implementation''' ==
Add description
=== '''What needs to be done''' ===
Detail notes from meeting with mentor

== '''Design''' ==

==='''Files Requiring Modification'''===
More detail

== '''Final Implementation''' ==

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM here] 
A link to the previous GitHub Repo can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-06T23:59:02Z

Lmcampb5: /* Resources */

== '''Introduction''' ==
Add description for E2234

== '''Problem Statement''' ==
Add description

== '''Previous Implementation''' ==
Add description
=== '''What needs to be done''' ===
Detail notes from meeting with mentor

== '''Design''' ==

==='''Files Requiring Modification'''===
More detail

== '''Final Implementation''' ==

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to the Expertiza GitHub can be found [https://github.com/expertiza/expertiza here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM
here] 
A link to the previous GitHub Repo can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-06T23:58:22Z

Lmcampb5: /* Resources */

== '''Introduction''' ==
Add description for E2234

== '''Problem Statement''' ==
Add description

== '''Previous Implementation''' ==
Add description
=== '''What needs to be done''' ===
Detail notes from meeting with mentor

== '''Design''' ==

==='''Files Requiring Modification'''===
More detail

== '''Final Implementation''' ==

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
A link to the Expertiza GitHub can be found [ here] 
A link to our repository can be found [https://github.com/njdowdle/expertiza/tree/beta here] 
Our implementation video can be found here 
A link to the previous Wiki can be found [https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2020_-_E2075._calibration_submissions_should_be_copied_along_with_calibration_assignments#Project_Overview here] 
The previous implementation video can be found [https://youtu.be/cyhy2mVXcPM
here] 
A link to the previous GitHub Repo can be found [https://github.com/expertiza/expertiza/pull/1856 here]

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:36:51Z

Lmcampb5: /* Team Information */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:36:35Z

Lmcampb5:

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:35:36Z

Lmcampb5: /* Files Requiring Modification */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:34:41Z

Lmcampb5: /* Previous Implementation */

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:34:09Z

Lmcampb5: /* Problem Statement */

== '''Introduction''' ==
Add description for E2234

== '''Problem Statement''' ==
Add description

== '''Previous Implementation''' ==

=== '''What needs to be done''' ===

== '''Design''' ==

==='''Files Requiring Modification'''===

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
Add Expertiza GitHub Link 
Add Our Repository Link 
Add Our New Video Link 
Add Previous Wiki 
Add Previous Video 
Add Previous GitHub Repo Link

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:33:53Z

Lmcampb5: /* Introduction */

== '''Introduction''' ==
Add description for E2234

== '''Problem Statement''' ==

== '''Previous Implementation''' ==

=== '''What needs to be done''' ===

== '''Design''' ==

==='''Files Requiring Modification'''===

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
Add Expertiza GitHub Link 
Add Our Repository Link 
Add Our New Video Link 
Add Previous Wiki 
Add Previous Video 
Add Previous GitHub Repo Link

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:33:20Z

Lmcampb5: /* Resources */

== '''Introduction''' ==
E2234

== '''Problem Statement''' ==

== '''Previous Implementation''' ==

=== '''What needs to be done''' ===

== '''Design''' ==

==='''Files Requiring Modification'''===

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
Add Expertiza GitHub Link 
Add Our Repository Link 
Add Our New Video Link 
Add Previous Wiki 
Add Previous Video 
Add Previous GitHub Repo Link

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:31:16Z

Lmcampb5:

== '''Introduction''' ==
E2234

== '''Problem Statement''' ==

== '''Previous Implementation''' ==

=== '''What needs to be done''' ===

== '''Design''' ==

==='''Files Requiring Modification'''===

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Conclusion ''' ==

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
Add Expertiza GitHub Link
Add Our Repository Link
Add Our New Video Link
Add Previous Wiki
Add Previous Video
Add Previous GitHub Repo Link

CSC/ECE 517 Spring 2022 - E2234. Calibration submissions should be copied along with calibration assignments

2022-04-04T23:26:24Z

Lmcampb5: Created page with "== '''Introduction''' == E2234 == '''Problem Statement''' == == '''Previous Implementation''' == === '''What needs to be done''' === == '''Design''' == ==='''Files Requi..."

== '''Introduction''' ==
E2234

== '''Problem Statement''' ==

== '''Previous Implementation''' ==

=== '''What needs to be done''' ===

== '''Design''' ==

==='''Files Requiring Modification'''===

== '''Test Plan''' ==

=== '''UI Testing''' ===

=== '''Automated Testing''' ===

== '''Team Information''' ==

Mentor: Dr. Ed Gehringer (efg@ncsu.edu) 
Nolan Dowdle (njdowdle@ncsu.edu) 
Laura Fox (lmcampb5@ncsu.edu) 
Soeun Jo (sjo@ncsu.edu) 
Harshil Shah (hshah6@ncsu.edu) 

== '''Resources''' ==
Add Expertiza GitHub Link
Add Our Repository Link
Add Our New Video Link
Add Previous Wiki
Add Previous Video
Add Previous GitHub Repo Link

CSC/ECE 517 Spring 2022

2022-04-04T23:15:03Z

Lmcampb5: /* Final Projects */

== 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 - 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 - E2228: Refactor JavaScript]]
* [[CSC/ECE 517 Spring 2022 - E2239: Further refactoring and improvement of review mapping helper.rb ]]

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-04T01:52:48Z

Lmcampb5: /* view */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously had 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do implicity get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller testing can be found [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink
*folder_action

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
The function tests whether a user is allowed to perform edit actions, as dictated by the current_user_id matching the @participant.user_id and ensuring they have the proper edit rights. When successful, the call should render the ":edit" template.

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
The function first verifies if the user has rights to view the current contents by validating with current_user_id matching the @participant.user_id. If this is valid, then you are redirected to the edit action, except in the 'view: true' format.

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that it flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
We finished with 27 implemented tests in 9 contexts, and 9 pending tests that could be implemented in the future.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage. The coveralls bot on GitHub reported 51% on our 2nd pull request. We believe this number may be lower because private methods in the controller weren't explicitly tested.
[[File:E2207_Coverage.png]]

The controller could use refactoring. Specifically, it appears the remove_hyperlink method is not currently used. It should be used if verified to work or removed.

Testing of the folder_action method should be done, but we were unable to get it working with the file fixtures approach.

Two pull requests were created for Expertiza (see related links). Our first pull request was accepted and merged into the beta branch. Our 2nd pull request is pending review. We are not aware of any automated build errors and the expertiza-travisci-bot commented that the changes look good on our latest pull request.

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A 2nd pull request made, after the first pull request was accepted can be found [https://github.com/expertiza/expertiza/pull/2363/ here]

A video of all tests running can be seen [https://youtu.be/V4Yo3vp9qME here].

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-04T01:52:30Z

Lmcampb5: /* edit */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously had 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do implicity get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller testing can be found [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink
*folder_action

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
The function tests whether a user is allowed to perform edit actions, as dictated by the current_user_id matching the @participant.user_id and ensuring they have the proper edit rights. When successful, the call should render the ":edit" template.

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
Add Description

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that it flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
We finished with 27 implemented tests in 9 contexts, and 9 pending tests that could be implemented in the future.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage. The coveralls bot on GitHub reported 51% on our 2nd pull request. We believe this number may be lower because private methods in the controller weren't explicitly tested.
[[File:E2207_Coverage.png]]

The controller could use refactoring. Specifically, it appears the remove_hyperlink method is not currently used. It should be used if verified to work or removed.

Testing of the folder_action method should be done, but we were unable to get it working with the file fixtures approach.

Two pull requests were created for Expertiza (see related links). Our first pull request was accepted and merged into the beta branch. Our 2nd pull request is pending review. We are not aware of any automated build errors and the expertiza-travisci-bot commented that the changes look good on our latest pull request.

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A 2nd pull request made, after the first pull request was accepted can be found [https://github.com/expertiza/expertiza/pull/2363/ here]

A video of all tests running can be seen [https://youtu.be/V4Yo3vp9qME here].

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-03T21:45:53Z

Lmcampb5: /* Conclusion */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously had 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do implicity get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller testing can be found [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink
*folder_action

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
Add Description

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
Add Description

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that it flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
We finished with 27 implemented tests in 9 contexts, and 9 pending tests that could be implemented in the future.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage. The coveralls bot on GitHub reported 51% on our 2nd pull request. We believe this number may be lower because private methods in the controller weren't explicitly tested.
[[File:E2207_Coverage.png]]

The controller could use refactoring. Specifically, it appears the remove_hyperlink method is not currently used. It should be used if verified to work or removed.

Testing of the folder_action method should be done, but we were unable to get it working with the file fixtures approach.

Two pull requests were created for Expertiza (see related links). Our first pull request was accepted and merged into the beta branch. Our 2nd pull request is pending review. We are not aware of any automated build errors and the expertiza-travisci-bot commented that the changes look good on our latest pull request.

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A 2nd pull request made, after the first pull request was accepted can be found [https://github.com/expertiza/expertiza/pull/2363/ here]

A video of all tests running can be seen [https://youtu.be/V4Yo3vp9qME here].

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-03T21:23:59Z

Lmcampb5: /* submit hyperlink */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously had 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do implicity get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller testing can be found [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink
*folder_action

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
Add Description

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
Add Description

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that it flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
Add description. Include test coverage info, any automatic build errors, how we could enhance testing, how many contexts were used, etc. Talk about need for refactoring of the controller.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage
[[File:E2207_Coverage.png]]

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A 2nd pull request made, after the first pull request was accepted can be found [https://github.com/expertiza/expertiza/pull/2363/ here]

A video of all tests running can be seen [https://youtu.be/V4Yo3vp9qME here].

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-03T21:22:33Z

Lmcampb5: /* Submitted Content Controller Methods */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously had 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do implicity get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller testing can be found [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink
*folder_action

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
Add Description

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
Add Description

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that is flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
Add description. Include test coverage info, any automatic build errors, how we could enhance testing, how many contexts were used, etc. Talk about need for refactoring of the controller.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage
[[File:E2207_Coverage.png]]

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A 2nd pull request made, after the first pull request was accepted can be found [https://github.com/expertiza/expertiza/pull/2363/ here]

A video of all tests running can be seen [https://youtu.be/V4Yo3vp9qME here].

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-03T21:21:30Z

Lmcampb5: /* Test Plan */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously had 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do implicity get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller can be found below [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
Add Description

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
Add Description

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that is flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
Add description. Include test coverage info, any automatic build errors, how we could enhance testing, how many contexts were used, etc. Talk about need for refactoring of the controller.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage
[[File:E2207_Coverage.png]]

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A 2nd pull request made, after the first pull request was accepted can be found [https://github.com/expertiza/expertiza/pull/2363/ here]

A video of all tests running can be seen [https://youtu.be/V4Yo3vp9qME here].

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-03T21:19:03Z

Lmcampb5: /* Related Links */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously has 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do inevitably get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller can be found below [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
Add Description

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
Add Description

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that is flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
Add description. Include test coverage info, any automatic build errors, how we could enhance testing, how many contexts were used, etc. Talk about need for refactoring of the controller.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage
[[File:E2207_Coverage.png]]

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A 2nd pull request made, after the first pull request was accepted can be found [https://github.com/expertiza/expertiza/pull/2363/ here]

A video of all tests running can be seen [https://youtu.be/V4Yo3vp9qME here].

CSC/ECE 517 Spring 2022 - E2207: Testing for submitted content controller.rb

2022-04-03T20:54:23Z

Lmcampb5: /* Conclusion */

== About Expertiza ==

[http://expertiza.ncsu.edu/ Expertiza] is the software benefits for both instructors and students by supporting various types of submissions and providing reusable objects for peer review. It is an open-source project based on [http://rubyonrails.org/ Ruby on Rails] framework. It allows the instructors not only to create and customize new or existing assignments but also to create a list of topics the students can sign up for. Students can form teams to work on various projects and assignments. Expertiza also lets students peer-review other students' submissions, enabling them to work together to improve others' learning experiences.

== Current Project Description ==

This Expertiza OSS project "E2207: Testing for submitted_content_controller" is focused on adding unit tests for submitted_content_controller.rb.

=== Files Involved ===

* submitted_content_controller.rb
* submitted_content_controller_spec.rb
* test files (helloworld.c and the-rspec-book_p2_1.pdf) located [https://github.com/LFox95/expertiza/tree/beta/spec/fixtures/files here]

The test files are used to test functionality in the submit_file method.

=== Running Tests ===
Ensure the test files are located in /expertizabeta/spec/fixtures/files

rspec ./spec/controllers/submitted_content_controller_spec.rb

== Test Plan ==

The submitted_content_controller previously has 1 public method (submit_file) tested which was very minimal. Additionally, the controller has 8 public methods that had no previous testing, and 9 private methods.

This test plan only covers testing for the public methods. RSpec is a Behavior Driven Design (BDD) focused test language where code interface and behavior is tested, not implementation. Additionally in Sandi Metz's talk on The Magic Tricks of Testing, she warns against testing private methods as testing them binds you to the current implementation of the code. Most private methods do inevitably get tested while testing the public methods.

The RSpec unit tests added for these methods in the spec file 'submitted_content_controller_spec.rb' are discussed below.

===Submitted Content Controller Methods===
The code of the controller can be found below [https://github.com/LFox95/expertiza/blob/beta/spec/controllers/submitted_content_controller_spec.rb here]

The public methods which are not tested are:
*remove_hyperlink

The private methods which are tested in a public method are:
*check_content_type_integrity - tested in submit_file public method
*check_file_size - tested in submit_file public method

The private methods which are not explicitly tested are:
*file_type
*move_selected_file
*rename_selected_file
*delete_selected_file
*copy_selected_file
*create_new_folder
*one_team_can_submit_work?

===Test Code===

==== action_allowed? ====
The action_allowed? method checks that the current user is a student with required authorizations for the edit, submit_file, and submit_hyperlink methods. The test approach was borrowed from E2205 and expanded on. It tests the return value of action_allowed? in 7 scenarios

Code snippet:
<pre>
describe '#action_allowed?' do
context 'current user is not authorized' do
it 'does not allow action for no user' do
expect(controller.send(:action_allowed?)).to be false
end
it 'does not allow action for student without authorizations' do
allow(controller).to receive(:current_user).and_return(build(:student))
expect(controller.send(:action_allowed?)).to be false
end
end
context 'current user has needed privileges' do
it 'allows edit action for student with needed authorizations' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:are_needed_authorizations_present?).and_return(true)
controller.params = {action: 'edit'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit file action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_file'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows submit hyperlink action for students with team that can submit' do
stub_current_user(student1, student1.role.name, student1.role)
allow(controller).to receive(:one_team_can_submit_work?).and_return(true)
controller.params = {action: 'submit_hyperlink'}
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for admin' do
stub_current_user(admin, admin.role.name, admin.role)
expect(controller.send(:action_allowed?)).to be true
end
it 'allows action for super admin' do
stub_current_user(super_admin, super_admin.role.name, super_admin.role)
expect(controller.send(:action_allowed?)).to be true
end
end
end
</pre>

==== controller_locale ====
We verify the controller_locale method returns the expected locale_for_student: I18n.default_locale

Code snippet:
<pre>
describe '#controller_locale' do
it 'should return I18n.default_locale' do
user = student1
stub_current_user(user, user.role.name, user.role)
expect(controller.send(:controller_locale)).to eq(I18n.default_locale)
end
end
</pre>

==== edit ====
Add Description

Code snippet:
<pre>
describe 'student#edit' do
it 'student#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'instructor#edit' do
it 'instructor#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
describe 'superadmin#edit' do
it 'superadmin#edit it' do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
allow(Participant).to receive(:find_by).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :edit, params: params
expect(response).to render_template(:edit)
end
end
</pre>

==== view ====
Add Description

Code snippet:
<pre>
describe 'student#view' do
it 'student#view it' do
#puts('-----STUDENT#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'instructor#view' do
it 'instructor#view it' do
#puts('-----instructor#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(instructor1, instructor1.role.name, instructor1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
#puts('-------inst a-----------')
response = get :view, params: params
#puts('-------inst b-----------')
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
describe 'superadmin#view' do
it 'superadmin#view it' do
#puts('-----superadmin#view--------')
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(superadmin1, superadmin1.role.name, superadmin1.role)
allow(participant).to receive(:name).and_return('Name')
params = { id: 21 }
response = get :view, params: params
expect(response).to redirect_to(action: :edit, view: true, id: 21)
end
end
</pre>

==== submit hyperlink ====
We test the submit_hyperlink error checking by verifying that is flashes an error for duplicate hyperlinks and invalid URLs

Code snippet:
<pre>
describe '#submit_hyperlink' do
context 'current user is participant and submits hyperlink' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
allow(participant).to receive(:team).and_return(team)
allow(participant).to receive(:name).and_return('Name')
end
it 'flashes error if a duplicate hyperlink is submitted' do
allow(team).to receive(:hyperlinks).and_return(['google.com'])
params = {submission: "google.com", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'You or your teammate(s) have already submitted the same hyperlink.'
end
it 'flashes error if url is invalid' do
allow(team).to receive(:hyperlinks).and_return([])
params = {submission: "abc123", id: 21}
response = get :submit_hyperlink, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses #{$ERROR_INFO}
end
end
end
</pre>

==== remove hyperlink ====
We created 2 pending tests for this method, it doesn't appear to currently be used in the submitted_content_controller and may need work.

Code snippet:
<pre>
describe '#remove_hyperlink' do
#NOTE - this method is not currently used, the below context is a start
# at proposed tests that may be useful in the future
context 'current user is participant' do
before(:each) do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(student1, student1.role.name, student1.role)
#allow(participant).to receive(:team).and_return(team)
#allow(team).to receive(:hyperlinks).and_return(['google.com'])
end
it 'redirects to edit if submissions are allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'redirects to view if submissions are not allowed' #do
#params = {id: 1}
#allow(assignment).to receive(:submission_allowed).and_return(true)
#response = get :remove_hyperlink, params
#expect(response).to redirect_to(action: :view, id: 1)
#end
end
end
</pre>

==== submit_file ====
Submit_file had one test prior to this project, however this test appears to have broken after the Expertiza Rails 5.1 migration. We added tests for error checking, verifying that an error is flashed if a file exceeding the size limit or a file with an unaccepted type is submitted. We did this using file fixtures and added two new files for test to \spec\fixtures\files. We chose to use file fixtures to keep these tests fast (F.I.R.S.T) as opposed to creating test files on the fly

Code snippet:
<pre>
describe '#submit_file' do
context 'current user does not match up with the participant' do
# this test has problems after rails 5.1 migration, getting 'undefined method 'size' for nil:NilClass' error
# when 'check_content_size' method is called in submitted_content_controller, doesn't seem to be a problem in
# other submit_file tests that should call the same method though...
it 'renders edit template' #do
#allow(AssignmentParticipant).to receive(:find).and_return(participant)
#stub_current_user(instructor1, instructor1.role.name, instructor1.role)
#request_params = { id: 1 }
#response = get :submit_file, params: request_params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
end
context 'user that is participant uploads a file' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'flashes error for file exceeding size limit' do
allow(controller).to receive(:check_content_size).and_return(false)
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/the-rspec-book_p2_1.pdf")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to be_present # not checking message content since it uses variable size limit
end
it 'flashes error for file of unexpected type' do
allow(SubmittedContentController).to receive(:check_content_type_integrity).and_return(false)
allow(MimeMagic).to receive(:by_magic).and_return("not valid")
allow_any_instance_of(Rack::Test::UploadedFile::String).to receive(:read).and_return("")
file = Rack::Test::UploadedFile.new("#{Rails.root}/spec/fixtures/files/helloworld.c")
params = {uploaded_file: file,
id: 1}
response = get :submit_file, params: params
expect(response).to redirect_to(action: :edit, id: 1)
expect(flash[:error]).to eq 'File type error'
end
# we could test that file is written and submission record is created, but we could have to
# make assumptions about how path is formed and user input for path/filename is sanitized. We don't want
# this test coupled to the existing implementation
end
end
</pre>

==== folder_action ====
We've added 6 pending tests for folder_action. The folder_action method actually manipulates files on the server so testing these methods may require some creativity. Using file fixtures (similar to testing done for submit_file method) may be a way to implement the pending tests.

Code snippet:
<pre>
describe '#folder_action' do
context 'current user does not match up with the participant' do
#method just returns in this context, how do we test that?
end
context 'current user is participant performing folder action' do
before(:each) do
allow(AssignmentParticipant).to receive(:find).and_return(participant)
stub_current_user(student1, student1.role.name, student1.role)
end
it 'redirects to edit' #do
#params = {id: 1, faction: nil}
#response = get :folder_action, params
#expect(response).to redirect_to(action: :edit, id: 1)
#end
it 'delete action deletes selected files'
it 'rename action renames selected file'
it 'move action moves selected file'
it 'copy action copies selected file'
it 'create folder action creates new directory'
end
end
</pre>

==== download ====
We created 5 tests (one of them pending) to verify that the download method flashes errors for invalid path names and non-existant files

Code snippet:
<pre>
describe '#download' do
context 'user downloads file' do
it 'flashes error for nil folder name' do
params = {folder_name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error for nil file name' do
params = {name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if attempt is to download entire folder' do
params = {folder_name: 'test_directory', name: nil}
response = get :download, params: params
expect(flash[:error]).to be_present # not checking message content since it uses exception message
end
it 'flashes error if file does not exist' do
params = {folder_name: 'unlikely_dir_name', name: 'nonexistantfile.no'}
response = get :download, params: params
expect(flash[:error]).to be_present #not checking message content since it uses exception message
end
it 'calls send for a valid file download' do
# still figuring this one out...
#params = {folder_name: 'test_dir', name: 'test.txt'}
#File.stub(:exist?).and_return(true)
#response = get :download, params
#expect(download).to receive(:send_file)
end
end
end
</pre>

== Conclusion ==
Add description. Include test coverage info, any automatic build errors, how we could enhance testing, how many contexts were used, etc. Talk about need for refactoring of the controller.

Coverall coverage report (expertiza/coverage/index.html) reported 44.32% coverage
[[File:E2207_Coverage.png]]

== Related Links ==

The main repository can be found [https://github.com/expertiza/expertiza/tree/beta here].

The forked git repository for this project can be found [https://github.com/LFox95/expertiza here].

The pull request can be found [https://github.com/expertiza/expertiza/pull/2349 here].

A video of all tests running can be seen [here].

File:E2207 Coverage.png

2022-04-03T20:53:38Z

Lmcampb5: