CSC/ECE 517 Fall 2017/E17A0 Team-based reviewing
Introduction
Motivation
Traditionally, only individuals have been able to perform assignment reviews in Expertiza. Over the years, it has been observed that it would be more desirable if assignment setters could define how a specific assignment could be reviewed. One of the ways will be by an individual, and a new way of reviews will be by teams. The same teams formed for the assignment would be able to participate in reviewing peer assignments.
The benefits of peers review are numerous, and there is an additional benefit of reviews by teams. In general, we may be getting to a point where traditional individual reviews are becoming ineffective, often missing nuances in an assignments details and concepts. It is a general belief that team-based reviews will address this, often pooling together resources from multiple reviewers working as a team to form a one cohesive and often comprehensive review. On the flip-side, it may be difficult sometimes for all team members to meet to perform reviews, and this may either delay reviews or curtail them. Ultimately, it is felt that the advantages of team-based reviews far outweigh any inherent shortcomings.
Changes to be Implemented
1. A new button to select Reviewers as a team will be provided at the Assignments Edit page in the Review Strategy tab for the instructors
2. Any member of a team should be able to select a review to be done by their team.
3. Team members should not be allowed to edit a review simultaneously.
4. The responses as well as the response_maps in the database should be deleted when the instructor decides to the change the 'Review Strategy' after the reviews have already started.
In summary, functionality should be provided to allow Team reviews by connecting everything as mentioned above.
Additional Changes
Other than the changes required to be done by us, during the course of the project we have implemented extra changes as suggested by the professor. These changes include:
1. Providing the delete review logic in case the instructor decides to change the review strategy.
2. To track 20 mins of inactivity to auto-save the review in case when the user starts the review and forgets to save it. A message will flash every 30 secs in the last 5 mins to inform the user that the review will be saved if there is no activity.
3. Initially, the e-mail to the reviewee, for the review was sent when a response was created (i.e when a response is saved). Because of this there is a chance that the reviewee won't find any review since the review has not been submitted only saved. Thus, we changed this so that the e-mail is sent only when a review is submitted.
Database Migrations
We will add the following fields in existing models of the Expertiza.
1. To assign team based reviewing attribute to the assignemnts a new boolean field reviewer_is_team was added to the with the default set to false.
2. To create a common response for all the team members we added a team_id field to the ResponseMap model. This identifies the reviewer team.
3. To prevent concurrent editing of the reviews by the team members we lock the response in the ResponseMap. This is check is based on the new is_locked field.
4. To identify the user who has locked the review we have also added a locked_by field in the ResponseMap which carries the current_user.id.
Approach
1. The first change is a simple view change. The checkbox will added in "assignments/edit/_review_strategy.html.erb". The checkbox value will be passed to the response_controller and depending on the value of the checkbox, the response_map will be created. The default value for the checkbox is "false". Thus, it will not break any existing functionality.
...
<!--#E17A0 Allow reviews to be set to be reviewed by a team -->
<tr id="reviewer_is_team">
<td>
<input name="assignment_form[assignment][reviewer_is_team]" type="hidden" value="false"/>
<%= check_box_tag('assignment_form[assignment][reviewer_is_team]', 'true', @assignment_form.assignment.reviewer_is_team) %>
<%= label_tag('assignment_form[assignment][reviewer_is_team]', 'Reviews are to be performed by teams.') %>
</td>
</tr>
...
2. For this feature to work we must create correct mappings. In Expertiza, all reviewees are AssignmentTeams, even in a non-team assignment where each team consists of a single participant. However, with this project, reviewers can be either participants or teams. In our design we are creating only one ResponseMap per team, i.e if the "reviewer_is_team" is set by the instructor, the ResponseMap created will have the reviewer_id as the participant_id of the requesting student and the team_id as the id of requesting students team in that assignment. Now to map this single response to all students in a team, we changed the strategy which returns response_mappings, in the "student_review_controller", for a particular reviewer. If the assignment has team_reviewers, then we find the responses by "team_id", and if there are no team reviewers for the assignment then we return responses by matching the "reviewer_id". Thus, if our mappings are created correctly, then the entire team will be directed to a single response_map and thus a single response irrespective of who requests the review. Thus, any of the team members will be able to request reviews as as the mappings will created with the team_id. The reason for us to not set the "reviewer_id" null or 0 for team reviewers is that, some of the functionalities like number of reviews, scores are all mapped to a single participant. Thus, if that is set that to any value, we will have to define all new methods for team reviewers. But, now we can use all those functions on our response and then make that same response object available to all members of the team. By doing this, we are neither creating redundant maps and nor are we defining new methods for the same functions. The code snippet below shows our new strategy to find the responses for a particular reviewer.
...
# E17A0 If an assignment is to be reviewed by a team, select it by team_id otherwise by reviewer_id
if @reviewer_team_info[:reviewer_is_team_member]
@review_mappings = ReviewResponseMap.where(team_id: @reviewer_team_info[:team_id])
@team = Team.find(@reviewer_team_info[:team_id])
else
@review_mappings = ReviewResponseMap.where(reviewer_id: @participant.id)
end
...
3. This is an important feature since if we allow teammates to edit reviews concurrently, each of them will overwrite each other's changes. We have handled this by using the is_locked and locked_by field in the response_map table. Using these fields, we will lock the response for the current_user. Thus, when one teammate is editing, the response is locked and that members id is stored in the locked_by field. Now for other users we will check locked_by field. If it does not match their id and they click on the edit button, then a error message will be displayed which will inform the user that his/her teammate is already editing the same review. This response is then unlocked when the current_user saves the response. The methods for locking and unlocking the response are defined in the
...
#E17A0 Lock response
def lock_response_map response_id
review_response_map = ReviewResponseMap.find_by_id(Response.find(response_id).map_id)
unless review_response_map.nil?
ReviewResponseMap.update(review_response_map.id, :is_locked => true, :locked_by => current_user.id)
end
end
#E17A0 Unlock Response
def unlock_response_map response_id
review_response_map = ReviewResponseMap.find_by_id(response_id)
unless review_response_map.nil?
ReviewResponseMap.update(review_response_map.id, :is_locked => false, :locked_by => current_user.id)
end
end
...
4/5. Changes 4 and 5 are both database migrations. Thus, we just add these fields in their respective tables.
6. This feature is provided to handle a rare edge case. Say that the instructor decides to change the review strategy after the reviews have already started. Now, the format of the response_maps already created will not match the response_maps that will be created after the strategy change. Thus, if the instructor wants to change the strategy, s/he can delete all reviews already done, and then instruct students to start over. The delete_reviews method is defined in the assignment model. Our method deletes all the response_maps as well as all the response objects created.
...
def delete_reviews
responsemap = ResponseMap.where(reviewed_object_id: self.id, type: 'ReviewResponseMap')
response = Response.where(map_id: responsemap.ids)
response.each { |r| Answer.destroy_all(response_id: r.id)}
Response.destroy_all(map_id: responsemap.ids)
ResponseMap.destroy_all(id: responsemap.ids)
end
...
Files Involved
| response_map.rb | assignment.rb | response_controller.rb | review_mapping_controller.rb |
| /edit/_review_strategy.html.erb | student_review/_responses.html.erb | review_mapping/select_reviewer.html.haml | response_controller_spec.rb |
| assignment_controller.rb | student_review_controller.rb | application_helper.rb | access_helper.rb |
| assignment_team.rb | /response/response.html.erb | /review_mapping/_list_review_mapping.html.erb | /student_review/_set_dynamic_review.html.erb |
| /student_review/list.html.erb | config/routes.rb | assignment_controller_spec.rb |
Design Principles Followed
1. MVC - The project is implemented in Ruby on Rails that uses MVC architecture. It separates an application’s data model, user interface, and control logic into three distinct components (model, view, and controller, respectively).
2. DRY Principle - We are trying to reuse the existing functionalities in Expertiza, thus avoiding code duplication. Whenever possible, code modification based on the existing classes, controllers, or tables will be done instead of creating the new one.
3. Polymorphism - We will use polymorphism to provide a single interface to entities of different types. In Expertiza, individual and team-based reviewers will share one single interface.
Use Case
1. Name: Teams submitting reviews for assignment or topic
2. Actor: Student (as member of a team)
3. Other Participants: Team Members
4. Precondition: Instructor has set up an assignment with review strategy as "Team Reviews". This assignment also has only teams. No students can participate as an individual. Thus, when a participant is added to the assignment, s/he will be added as a single member team. Thus, in the diagram above, the participant is a single member team.
5. Primary Sequence:
Edge Cases
Apart from the case explained above we have handled two edge cases. These cases are as explained below:
Case 1:
Scenario:
We are not allowing concurrent editing of reviews for teams. We are doing this by locking the response_map. Thus, when a team member is editing a review we lock the response using that team member's id. It can only be unlocked when the review has been saved. When locked the other team members can only view the review.
Case 2:
Scenario:
The beginning of a review is a special case since when the begin button is clicked a review form is only rendered. Nothing is saved. Since there is no response object linked to the review, we cannot lock anything. Thus, if this case is not handled all the teammates will be able to begin the review concurrently and the person who saves last will overwrite changes of all teammates. Thus, we lock the respnse_map. Now when one of the teammate begins the review ,and the other teammates try to begin too, they will get an error saying that the review is already being edited by someone else.
Case 3:
Scenario:
If a team member forgets to save or submit a review and has not closed the browser window, then we automatically save the review after an inactivity timeout of 20 mins thus unlocking it for the other team members to edit. After 15 mins of inactivity a message will be flashed every 30 secs, on the users screen informing him/her that after 5 more mins of inactivity the review will be auto-saved. The progress will not be lost. If the user wishes to continue editing, the counter can be reset with a simple movement of the cursor.
Case 4:
Scenario:
At a critical time, if a team is unable to contact the team member who has locked the review by closing the browser window without saving the review, the other team members can contact the instructor who has been provided an unlock button for such emergency situations. The instructor can now unlock that review and then the other team members can edit it. This button was added since, in the case when a user directly closes the window without saving, we cannot track inactivity. Thus, the 20 mins auto-save condition will not function. For this condition to work there is a prerequisite that the browser window must be open. Thus, to handle the case when the aforementioned prerequisite fails, we have provided the unlock button to the instructor.
Case 5:
Scenario:
An instructor can delete completed review if he/she decides to change the review strategy for an assignment between team reviews and individual reviews. When the instructor changes the strategy and clicks on the save button, there will be a pop-up confirming if the instructor wants to delete all the reviews. If a user is editing a review at the instant when instructor is deleting the reviews, upon pressing the save review or submit review button, the user is redirected to the student_review/list.html.erb page
UI Changes
1. Currently there is no team reviewing in Expertiza. Thus, we will provide a check-box, in the review strategy tab of edit assignments, for the instructor. Checking this box will set the reviewers to teams, rather than individual participants. As seen in the new implementation, the delete button has been removed from the review_strategy tab. Instead, now when the instructor decides to change the review policy, s/he will get a pop-up.
Current Implementation is as follows:
New Implementation will be as shown below:
2. By default, the reviewers will be individual students. When they request reviews, they request reviews for themselves. If the instructor changes the reviewers to teams, then each participant (who is member of some team), will request reviews on behalf of the entire team.
If the assignment has individual reviews:
If the assignment has team reviews:
When Instructor assigns reviews:
Test Plan
The Assignments functions; including creation, updating, saving and modifying methods have already been tested, and our test plan includes testing for the new team-based reviewing functionality that we have added. Our test plan includes the following test methods:
1. Test the delete_reviews method to check whether it displays a flash message when there are no reviews to be delete.
2. Test the delete_reviews method to check whether it displays the success flash message on deletion of reviews.
3. Test the action_allowed? method to check whether it denies certain action when the review is locked by a user other than current user.
4. Test the action_allowed? method to check whether it allows certain action when the review is locked by current user.
5. Test the reviewer_is_team_member method to check whether the current user is a member of the assignment review team.
Additional Links
Github Project Repo
Github Pull Request
Project Demo
Team Information
Aditya Shah
Amey Deshmukh
Deepak Kalro
Philip Musyoki