CSC/ECE 517 Fall 2021 - E2165. Fix teammate-review view: Difference between revisions
Line 72: | Line 72: | ||
[[File:Logic e2165.png|border|center|alt=Flow diagram of proposed changes for showing team review data to instructor]] | [[File:Logic e2165.png|border|center|alt=Flow diagram of proposed changes for showing team review data to instructor]] | ||
===Changes made to the code are shown below. Every change is shown and explained separately=== | |||
====A. Added the following to app/assets/stylesheets/grades.scss:==== | |||
<pre> | <pre> | ||
.teammate_table.hidden { | .teammate_table.hidden { | ||
Line 82: | Line 82: | ||
====B. In app/controllers/grades_controller.rb: ==== | |||
B.1: | '''B.1:''' | ||
Old code | |||
@vmlist << populate_view_model(questionnaire) | @vmlist << populate_view_model(questionnaire) | ||
New code | |||
if questionnaire.type == "TeammateReviewQuestionnaire" | if questionnaire.type == "TeammateReviewQuestionnaire" | ||
# as teammate review will be different for each participant | # as teammate review will be different for each participant | ||
Line 103: | Line 102: | ||
end | end | ||
B.2: | '''B.2: ''' | ||
Old code | |||
def populate_view_model(questionnaire) | def populate_view_model(questionnaire) | ||
vm = VmQuestionResponse.new(questionnaire, @assignment, @round) | vm = VmQuestionResponse.new(questionnaire, @assignment, @round) | ||
Line 115: | Line 115: | ||
end | end | ||
New code | |||
def populate_view_model(participant, questionnaire, team) | def populate_view_model(participant, questionnaire, team) | ||
vm = VmQuestionResponse.new(questionnaire, @assignment, @round) | vm = VmQuestionResponse.new(questionnaire, @assignment, @round) | ||
Line 129: | Line 128: | ||
====C. In app/models/vm_question_response.rb:==== | |||
C.1: Added the reviewee_id method | '''C.1: '''Added the reviewee_id method | ||
def reviewee_id | def reviewee_id | ||
Line 137: | Line 136: | ||
end | end | ||
C.2: Added the reviewee_name method | '''C.2: '''Added the reviewee_name method | ||
def reviewee_name(ip_address = nil) | def reviewee_name(ip_address = nil) | ||
Line 144: | Line 143: | ||
end | end | ||
C.3: | '''C.3: ''' | ||
Old code | |||
elsif @questionnaire_type == "TeammateReviewQuestionnaire" | elsif @questionnaire_type == "TeammateReviewQuestionnaire" | ||
reviews = participant.teammate_reviews | reviews = participant.teammate_reviews | ||
Line 152: | Line 152: | ||
participant = Participant.find(review_mapping.reviewer_id) | participant = Participant.find(review_mapping.reviewer_id) | ||
New code | |||
elsif @questionnaire_type == "TeammateReviewQuestionnaire" | elsif @questionnaire_type == "TeammateReviewQuestionnaire" | ||
reviews = participant.teammate_reviews | reviews = participant.teammate_reviews | ||
Line 162: | Line 160: | ||
participant = Participant.find(review_mapping.reviewer_id) | participant = Participant.find(review_mapping.reviewer_id) | ||
C.4: | '''C.4: ''' | ||
Old code | |||
row.score_row.push(VmQuestionResponseScoreCell.new(answer.answer, color_code, answer.comments, vm_tag_prompts)) | row.score_row.push(VmQuestionResponseScoreCell.new(answer.answer, color_code, answer.comments, vm_tag_prompts)) | ||
New code | |||
row.score_row.push(VmQuestionResponseScoreCell.new(answer.answer, color_code, answer.comments, vm_tag_prompts, answer.response_id)) | row.score_row.push(VmQuestionResponseScoreCell.new(answer.answer, color_code, answer.comments, vm_tag_prompts, answer.response_id)) | ||
Line 172: | Line 171: | ||
====D. In app/models/vm_question_response_score_cell.rb:==== | |||
D.1: | '''D.1: '''In the method initialize of class VmQuestionResponseScoreCell | ||
Old code | |||
def initialize(score_value, color_code, comments, vmprompts = nil) | def initialize(score_value, color_code, comments, vmprompts = nil) | ||
@score_value = score_value | @score_value = score_value | ||
Line 183: | Line 183: | ||
end | end | ||
New code | |||
def initialize(score_value, color_code, comments, vmprompts = nil, response_id) | def initialize(score_value, color_code, comments, vmprompts = nil, response_id) | ||
Line 193: | Line 193: | ||
end | end | ||
D.2: Added a method reviewer_id to the class VmQuestionResponseScoreCell | '''D.2: '''Added a method reviewer_id to the class VmQuestionResponseScoreCell | ||
def reviewer_id | def reviewer_id | ||
Line 203: | Line 203: | ||
====E. In app/views/grades/_add_icon_to_name.html.erb:==== | |||
E.1 Added the following code to the file: | '''E.1 '''Added the following code to the file: | ||
<%# Determine the content of the reviewed by text %> | <%# Determine the content of the reviewed by text %> | ||
Line 220: | Line 220: | ||
<% end%> | <% end%> | ||
E.2: | '''E.2: ''' | ||
Old code | |||
<% if review.done_by_staff_participant? %> | <% if review.done_by_staff_participant? %> | ||
Line 229: | Line 230: | ||
<% end %> | <% end %> | ||
New code | |||
<% if review.done_by_staff_participant? %> | <% if review.done_by_staff_participant? %> | ||
Line 265: | Line 265: | ||
====F.In app/views/grades/view_team.html.erb:==== | |||
F.1: Added function teammateSelectOnChange: | '''F.1: '''Added function teammateSelectOnChange: | ||
<pre> | <pre> | ||
Line 291: | Line 291: | ||
</pre> | </pre> | ||
F.2: Old code | '''F.2: ''' | ||
Old code | |||
<pre> | <pre> | ||
<% @vmlist.each do |vm| %> | <% @vmlist.each do |vm| %> | ||
Line 307: | Line 309: | ||
</pre> | </pre> | ||
New code | |||
<pre> | <pre> | ||
<%# This boolean is used to render a dropdown just before the first teammate review heat grid %> | <%# This boolean is used to render a dropdown just before the first teammate review heat grid %> | ||
Line 337: | Line 338: | ||
</pre> | </pre> | ||
F.3 | '''F.3 ''' | ||
Old code | |||
<pre> | <pre> | ||
<!-- display the list of questions for this questionnaire --> | <!-- display the list of questions for this questionnaire --> | ||
Line 348: | Line 350: | ||
</pre> | </pre> | ||
New code | |||
<pre> | <pre> | ||
<!-- display the list of questions for this questionnaire --> | <!-- display the list of questions for this questionnaire --> | ||
Line 361: | Line 362: | ||
</pre> | </pre> | ||
F.4 | '''F.4 ''' | ||
Old code | |||
<pre> | <pre> | ||
<% vm.list_of_reviews.each do |review| %> | <% vm.list_of_reviews.each do |review| %> | ||
Line 385: | Line 387: | ||
</pre> | </pre> | ||
New code | |||
<pre> | <pre> | ||
<% vm.list_of_reviews.each do |review| %> | <% vm.list_of_reviews.each do |review| %> | ||
Line 406: | Line 407: | ||
</pre> | </pre> | ||
F.5 | '''F.5 ''' | ||
Old code | |||
<pre> | <pre> | ||
<% vm.list_of_rows.each do |row| %> | <% vm.list_of_rows.each do |row| %> | ||
Line 423: | Line 425: | ||
</pre> | </pre> | ||
New code | |||
<pre> | <pre> | ||
<% vm.list_of_rows.each do |row| %> | <% vm.list_of_rows.each do |row| %> | ||
Line 451: | Line 452: | ||
</pre> | </pre> | ||
F.6 Removed the following code | '''F.6 '''Removed the following code. | ||
<pre> | <pre> | ||
Line 459: | Line 460: | ||
</pre> | </pre> | ||
F.7 | '''F.7 ''' | ||
Old code | |||
<pre> | <pre> | ||
<!--loop that creates the collapsed-by-default row, which lists all comments. --> | <!--loop that creates the collapsed-by-default row, which lists all comments. --> | ||
Line 475: | Line 477: | ||
</pre> | </pre> | ||
New code | |||
<pre> | <pre> | ||
<!--loop that creates the collapsed-by-default row, which lists all comments. --> | <!--loop that creates the collapsed-by-default row, which lists all comments. --> | ||
Line 503: | Line 504: | ||
====G. In spec/controllers/grades_controller_spec.rb:==== | |||
G.1 | '''G.1 ''' | ||
Old code | |||
<pre> | <pre> | ||
before(:each) do | before(:each) do | ||
Line 518: | Line 520: | ||
</pre> | </pre> | ||
New code | |||
<pre> | <pre> | ||
before(:each) do | before(:each) do | ||
Line 531: | Line 532: | ||
G.2 Remove xdescribe '#view_team' and change describe '#view_team' as follows. | '''G.2 '''Remove xdescribe '#view_team' and change describe '#view_team' as follows. | ||
Old code | |||
<pre> | <pre> | ||
describe '#view_team' do | describe '#view_team' do | ||
Line 550: | Line 552: | ||
</pre> | </pre> | ||
New code | |||
<pre> | <pre> | ||
describe '#view_team' do | describe '#view_team' do | ||
Line 656: | Line 657: | ||
====H. In spec/factories/factories.rb:==== | |||
H.1 Add the following factory: | '''H.1 '''Add the following factory: | ||
<pre> | <pre> | ||
Line 664: | Line 665: | ||
</pre> | </pre> | ||
H.2 Add another factory | '''H.2 '''Add another factory | ||
<pre> | <pre> |
Revision as of 05:36, 28 November 2021
Team Introduction
Dr. Edward Gehringer (instructor), Parimal Mehta (mentor)
1. Anjali Garg (unity ID : agarg25)
2. Arvasu Chikara (unity ID: achikar2)
3. Lalit Bangad (unity ID: llbangad)
4. Vinay Deshmukh (unity ID: vdeshmu)
Problems to be addressed
After a project is submitted and reviewed, both the team members (students) and instructors should be able to view the teammate reviews. Students should be able to view the reviews by going to a particular assignment's -> your scores, at the bottom of the page. An instructor can view the teammate reviews by going to assignments -> assignment -> particular team's submission -> view submission -> assign grades -> view scores.
In the case of the instructor, a single heat grid is shown whereas in the case of a team member, we cant view the scores. In the heat grid, it is not clear if the scores visible are for the reviews that the student has *written* for their teammates or the scores that the student has *received* from their teammates. Additionally, for instructors can choose from a list of the students on the team, but they have no way to tell whether the heat grid shows the reviews done *by* that student or done *for* that student.
Methodology Overview
We have divided our work in the following tasks to address the above problems:
1. Distinguish reviews done for a student by rendering separates heatgrids for each student
2. For instructors, the table will have sections based on the students
3. Include a checkbox on the edit assignment page that enables/disables visibility of heat grid to students
4. Fix the average column for a table (which breaks for some reviews)
5. Show a composite score derived from all reviews. Make this code generalized, so other kinds of heat grid views could use it.
6. Changes should work even in an anonymized view
Motivations
1. Enable users to clearly tell which reviews are done by them, and which reviews are done for them.
2. Code should be generic so it can be used for different types of heat grids as well.
Control Flow to access the concerned screens
1. For instructor (Log in as an instructor):-
Screen after logging in by the instructor
Got to assignments on the Manage menu
Go to assignments
Go to submissions for any assignment
Go to Assign Grade
You will land on the page where the changes are made and can be reviewed
2. Student:- (Logged in as an instructor)
After landing on the assignments page, select any student from an assignment as shown below
Go to/Select any assignment
Go to your scores
You will land on the page where the changes are made and can be reviewed
Models used / Explanation of fixes
The below diagram illustrates the proposed code change for showing all team members with the teammate review data to an instructor. The existing code only iterated on the `Participant` whose ID is present in the URL of the page. To fix this, we will iterate over all teammates of the given participant so that all teammate reviews for all team members are shown to the instructor.
Changes made to the code are shown below. Every change is shown and explained separately
A. Added the following to app/assets/stylesheets/grades.scss:
.teammate_table.hidden { display: none; }
B. In app/controllers/grades_controller.rb:
B.1:
Old code
@vmlist << populate_view_model(questionnaire)
New code
if questionnaire.type == "TeammateReviewQuestionnaire" # as teammate review will be different for each participant # we need to create a separate table for each teammate @participant.team.participants.each do |team_participant| @vmlist << populate_view_model(team_participant, questionnaire, @team) end else # only one participant needs iterating, as apart from teammatereview # all other reviews are at "team" level @vmlist << populate_view_model(@participant, questionnaire, @team) end
B.2:
Old code
def populate_view_model(questionnaire) vm = VmQuestionResponse.new(questionnaire, @assignment, @round) vmquestions = questionnaire.questions vm.add_questions(vmquestions) vm.add_team_members(@team) vm.add_reviews(@participant, @team, @assignment.vary_by_round) vm.number_of_comments_greater_than_10_words vm end
New code
def populate_view_model(participant, questionnaire, team) vm = VmQuestionResponse.new(questionnaire, @assignment, @round) vmquestions = questionnaire.questions vm.add_questions(vmquestions) vm.add_team_members(team) vm.add_reviews(participant, team, @assignment.vary_by_round) vm.number_of_comments_greater_than_10_words vm end
C. In app/models/vm_question_response.rb:
C.1: Added the reviewee_id method
def reviewee_id @participant_who_is_reviewee.user_id end
C.2: Added the reviewee_name method
def reviewee_name(ip_address = nil) part = @participant_who_is_reviewee User.find_by(id: part.user_id).fullname(ip_address) end
C.3:
Old code
elsif @questionnaire_type == "TeammateReviewQuestionnaire" reviews = participant.teammate_reviews reviews.each do |review| review_mapping = TeammateReviewResponseMap.find_by(id: review.map_id) participant = Participant.find(review_mapping.reviewer_id)
New code
elsif @questionnaire_type == "TeammateReviewQuestionnaire" reviews = participant.teammate_reviews @participant_who_is_reviewee = participant reviews.each do |review| review_mapping = TeammateReviewResponseMap.find_by(id: review.map_id) participant = Participant.find(review_mapping.reviewer_id)
C.4:
Old code
row.score_row.push(VmQuestionResponseScoreCell.new(answer.answer, color_code, answer.comments, vm_tag_prompts))
New code
row.score_row.push(VmQuestionResponseScoreCell.new(answer.answer, color_code, answer.comments, vm_tag_prompts, answer.response_id))
D. In app/models/vm_question_response_score_cell.rb:
D.1: In the method initialize of class VmQuestionResponseScoreCell
Old code
def initialize(score_value, color_code, comments, vmprompts = nil) @score_value = score_value @color_code = color_code @comment = comments @vm_prompts = vmprompts end
New code
def initialize(score_value, color_code, comments, vmprompts = nil, response_id) @score_value = score_value @color_code = color_code @comment = comments @vm_prompts = vmprompts @response_id = response_id end
D.2: Added a method reviewer_id to the class VmQuestionResponseScoreCell
def reviewer_id resp = Response.find(@response_id) map = ResponseMap.find(resp.map_id) map.reviewer_id end
E. In app/views/grades/_add_icon_to_name.html.erb:
E.1 Added the following code to the file:
<%# Determine the content of the reviewed by text %> <% review_text = "" %> <% if @current_role_name.eql?'Student' or questionnaire_type != "TeammateReviewQuestionnaire" %> <%# Use Review Template:Index format for student view, and when questionnaire_type is not teammatereview %> <% review_text = "Review #{i + 1}" %> <% else %> <% resp = Response.find(review.id) %> <% map = ResponseMap.find(resp.map_id) %> <% reviewer_id = map.reviewer_id %> <% fullname = Participant.find_by(id: reviewer_id).fullname(session[:ip]) %> <% review_text = "Review by #{fullname}" %> <% end%>
E.2:
Old code
<% if review.done_by_staff_participant? %>
<img data-toggle="tooltip" data-placement="right" title="Icon indicates this review was done by course staff" class="img_icon" src="/assets/staff.png"> <a target="_blank" href="../response/view?id=<%= review.id.to_s %>" data-toggle="tooltip" data-placement="right" title="Click to see details"><%= "Review " + i.to_s %></a>
<% else %>
<a target="_blank" href="../response/view?id=<%= review.id.to_s %>" data-toggle="tooltip" data-placement="right" title="Click to see details"><%= "Review " + i.to_s %></a>
<% end %>
New code
<% if review.done_by_staff_participant? %>
<img data-toggle="tooltip" data-placement="right" title="Icon indicates this review was done by course staff" class="img_icon" src="/assets/staff.png" > <a target="_blank" href="../response/view?id=<%= review.id.to_s %>" data-toggle="tooltip" data-placement="right" title="Click to see details"> <%= review_text %> </a>
<% else %>
<a target="_blank" href="../response/view?id=<%= review.id.to_s %>" data-toggle="tooltip" data-placement="right" title="Click to see details" > <%= review_text %> </a>
<% end %>
F.In app/views/grades/view_team.html.erb:
F.1: Added function teammateSelectOnChange:
function teammateSelectOnChange(selectNode) { const revieweeIdToShow= selectNode.value; const tableSelector = ".teammate_table"; if(revieweeIdToShow == "all") { document.querySelectorAll(tableSelector).forEach((tableNode) => { tableNode.classList.remove("hidden"); }); return; } document.querySelectorAll(tableSelector).forEach((tableNode) => { const tableRevieweeId = tableNode.getAttribute('data-reviewee_id'); if(tableRevieweeId == revieweeIdToShow) { tableNode.classList.remove("hidden"); } else { tableNode.classList.add("hidden"); } }); }
F.2:
Old code
<% @vmlist.each do |vm| %> <% if (vm.questionnaire_display_type == "Metareview" or vm.questionnaire_display_type == "Author Feedback" or vm.questionnaire_display_type == "Teammate Review") and @current_role_name.eql?'Student' %> <% else %> <% if vm.list_of_reviewers.length > 0 %> <!-- display the list of questions for this questionnaire --> @@ -87,6 +131,8 @@ <h4 style="display:inline-block;"><%= vm.questionnaire_display_type %> <% if vm.questionnaire_type == "ReviewQuestionnaire" %> (Round: <%= vm.round.to_s %> of <%= vm.rounds.to_s %>) <%end %>
New code
<%# This boolean is used to render a dropdown just before the first teammate review heat grid %> <% first_teammate_review_appeared = false %> <% @vmlist.each do |vm| %> <% if not @current_role_name.eql?'Student' and not first_teammate_review_appeared and vm.questionnaire_type == "TeammateReviewQuestionnaire" %> <div> <select onchange="teammateSelectOnChange(this)"> <option value="all">All</option> <% @vmlist.select { |tm_vm| tm_vm.questionnaire_type == "TeammateReviewQuestionnaire" }.each do |tm_vm| %> <option value="<%= tm_vm.reviewee_id %>"> <%= tm_vm.reviewee_name(session[:ip]) %> </option> <% end %> </select> </div> <% first_teammate_review_appeared = true %> <% end %> <% if (vm.questionnaire_display_type == "Metareview" or vm.questionnaire_display_type == "Author Feedback") and @current_role_name.eql?'Student' %> <% elsif (vm.questionnaire_type == "TeammateReviewQuestionnaire" and @current_role_name.eql?'Student' and !@assignment.show_teammate_reviews)%> <% elsif (vm.questionnaire_type == "TeammateReviewQuestionnaire" and @current_role_name.eql?'Student' and @assignment.show_teammate_reviews and current_user.id != vm.reviewee_id )%> <% else %> <% if vm.questionnaire_type == "TeammateReviewQuestionnaire" %> <div class="teammate_table" data-reviewee_id="<%= vm.reviewee_id %>"> <% else %> <div> <% end %>
F.3
Old code
<!-- display the list of questions for this questionnaire --> @@ -87,6 +131,8 @@ <h4 style="display:inline-block;"><%= vm.questionnaire_display_type %> <% if vm.questionnaire_type == "ReviewQuestionnaire" %> (Round: <%= vm.round.to_s %> of <%= vm.rounds.to_s %>) <%end %>
New code
<!-- display the list of questions for this questionnaire --> @@ -87,6 +131,8 @@ <h4 style="display:inline-block;"><%= vm.questionnaire_display_type %> <% if vm.questionnaire_type == "ReviewQuestionnaire" %> (Round: <%= vm.round.to_s %> of <%= vm.rounds.to_s %>) <% elsif vm.questionnaire_type == "TeammateReviewQuestionnaire" %> (For: <%= vm.reviewee_name(session[:ip]) %>) <%end %>
F.4
Old code
<% vm.list_of_reviews.each do |review| %> <!-- instructors (or higher level users) see reviewer name, students see anonymized string --> <% if (['Student'].include? @current_role_name) && @assignment.is_anonymous%> <%= render :partial => 'add_icon_to_name', :locals => {review: review, i: i} %> <% else %> <% user_name = User.find(Participant.find(ResponseMap.find(Response.find(review.id).map_id).reviewer_id).user_id).fullname(session[:ip]).to_s %> <%= render :partial => 'add_icon_to_name', :locals => {review: review, i: i} %> <% end %> <% i += 1 %> <% end %> <th class="sorter-true" align="right" > Avg<span></span> </th> <th class="sorter-false"> <span data-toggle="tooltip" data-placement="right" title="A count of comments, for the respective question, which have word count > 10. The purpose of this metric is to represent how many comments for the question are of a substantial length to provide quality feedback.">metric-1</span> </th> </tr> </thead>
New code
<% vm.list_of_reviews.each do |review| %> <!-- instructors (or higher level users) see reviewer name, students see anonymized string --> <% if (['Student'].include? @current_role_name) && @assignment.is_anonymous%> <%= render :partial => 'add_icon_to_name', :locals => {review: review, i: i, questionnaire_type: vm.questionnaire_type } %> <% else %> <% user_name = User.find(Participant.find(ResponseMap.find(Response.find(review.id).map_id).reviewer_id).user_id).fullname(session[:ip]).to_s %> <%= render :partial => 'add_icon_to_name', :locals => {review: review, i: i, questionnaire_type: vm.questionnaire_type } %> <% end %> <% i += 1 %> <% end %> <th class="sorter-true" align="right" > Avg<span></span> </th> </tr> </thead>
F.5
Old code
<% vm.list_of_rows.each do |row| %> <!-- notice the data-target. because we're toggling via looped code, we need to dynamically generate the identifier. --> <tr data-toggle="collapse" class="accordion-toggle" data-target="#hid<%= row.question_id.to_s + vm.round.to_s %>"> <td class = 'cf' data-toggle="tooltip" title="<%= row.question_text %>"> <div style='float: left'><%= j.to_s %></div><div style='float: right'> &#<%= type_and_max(row) %>;</div> </td> <!-- actual cells with scores and colored background. --> <% row.score_row.each do |score| %> <% score_comment = score.comment.nil? ? '' : score.comment %> <!-- changing nil to null string to avoid Null Pointer Exception --> <td class="<%= score.color_code %>" align="center" data-toggle="tooltip" title="<%= strip_tags(score.comment).html_safe%>"> <span class="<%= underlined?(score) %>"><%= score.score_value.to_s %></span> </td> <% end %>
New code
<% vm.list_of_rows.each do |row| %> <% target_id = "hid__#{row.question_id}__#{vm.round}" %> <% if vm.questionnaire_type == "TeammateReviewQuestionnaire" %> <%# Since we render different heatgrids with the same questions for each teammember, we need to distinguish the ID, based on the user for whom, the reviews have been created. %> <% target_id += "__#{vm.reviewee_id}" %> <% end%> <!-- notice the data-target. because we're toggling via looped code, we need to dynamically generate the identifier. --> <tr data-toggle="collapse" class="accordion-toggle" data-target="<%= "#" + target_id %>"> <td class = 'cf' data-toggle="tooltip" title="<%= row.question_text %>"> <div style='float: left'><%= j.to_s %></div><div style='float: right'> &#<%= type_and_max(row) %>;</div> </td> <!-- actual cells with scores and colored background. --> <% row.score_row.each do |score| %> <% score_comment = score.comment.nil? ? '' : score.comment %> <!-- changing nil to null string to avoid Null Pointer Exception --> <td class="<%= score.color_code %>" align="center" data-toggle="tooltip" title="<%= strip_tags(score.comment).html_safe%>"> <span class="<%= underlined?(score) %>"> <%= score.score_value.to_s %> </span> </td> <% end %>
F.6 Removed the following code.
<td class = 'cf' align="center"> <%= row.countofcomments.to_s %> </td>
F.7
Old code
<!--loop that creates the collapsed-by-default row, which lists all comments. --> <tr class="tablesorter-childRow"> <td colspan="<%= (i+1).to_s %>" class="hiddenRow" ><div id="hid<%= row.question_id.to_s + vm.round.to_s %>" class="accordion-body collapse" style="margin-left:10px;"> <div style="padding-top: 10px; padding-bottom: 10px;"><%= sanitize row.question_text %></div> <div> <table class="table tbl_questlist"> @@ -181,7 +233,17 @@ </thead> <% for index in 0 .. row.score_row.length - 1 %> <tr> <td>Review <%=index + 1%></td>
New code
<!--loop that creates the collapsed-by-default row, which lists all comments. --> <tr class="tablesorter-childRow"> <td colspan="<%= (i+1).to_s %>" class="hiddenRow" ><div id="<%= target_id %>" class="accordion-body collapse" style="margin-left:10px;"> <div style="padding-top: 10px; padding-bottom: 10px;"><%= sanitize row.question_text %></div> <div> <table class="table tbl_questlist"> @@ -181,7 +233,17 @@ </thead> <% for index in 0 .. row.score_row.length - 1 %> <tr> <% if @current_role_name.eql?'Student' or vm.questionnaire_type != "TeammateReviewQuestionnaire" %> <%# Use Review {{index}} format for student view, and when questionnaire_type is not teammatereview %> <td>Review <%=index + 1%></td> <% else %> <td> <% reviewer_id = row.score_row[index].reviewer_id %> <% fullname = Participant.find_by(id: reviewer_id).fullname(session[:ip]) %> Review by <%= fullname %> </td> <% end%>
G. In spec/controllers/grades_controller_spec.rb:
G.1
Old code
before(:each) do allow(AssignmentParticipant).to receive(:find).with('1').and_return(participant) allow(participant).to receive(:team).and_return(team) stub_current_user(instructor, instructor.role.name, instructor.role) allow(Assignment).to receive(:find).with('1').and_return(assignment) @@ -91,29 +90,106 @@ end end
New code
before(:each) do allow(participant).to receive(:team).and_return(team) stub_current_user(instructor, instructor.role.name, instructor.role) allow(Assignment).to receive(:find).with('1').and_return(assignment) @@ -91,29 +90,106 @@ end end
G.2 Remove xdescribe '#view_team' and change describe '#view_team' as follows.
Old code
describe '#view_team' do render_views context 'when view_team page is viewed by a student who is also a TA for another course' do it 'renders grades#view_team page' do allow(participant).to receive(:team).and_return(team) allow(AssignmentQuestionnaire).to receive(:find_by).with(assignment_id: 1, questionnaire_id: 1).and_return(assignment_questionnaire) allow(AssignmentQuestionnaire).to receive(:where).with(any_args).and_return([assignment_questionnaire]) allow(assignment).to receive(:late_policy_id).and_return(false) allow(assignment).to receive(:calculate_penalty).and_return(false) allow(assignment).to receive(:compute_total_score).with(any_args).and_return(100) allow(review_questionnaire).to receive(:get_assessments_round_for).with(participant, 1).and_return([review_response]) allow(Answer).to receive(:compute_scores).with([review_response], [question]).and_return(max: 95, min: 88, avg: 90) params = {id: 1} allow(TaMapping).to receive(:exists?).with(ta_id: 1, course_id: 1).and_return(true)
New code
describe '#view_team' do render_views # All stubs and factories are instantiated within this scope so as to not # clash with other broken test. # The names of the factories created for testing view_team # have `_vt` suffix ```where necessary``` to differentiate them from # the above declared factories let(:student1_vt) { create(:student, name: "barry", id: 12) } let(:student2_vt) { create(:student, name: "iris", id: 13) } let(:tm_questionnaire) { build( :teammate_review_questionnaire, id: 12, questions: [question], max_question_score: 5 ) } let(:assignment_vt) { create(:assignment, id: 6, max_team_size: 2, questionnaires: [tm_questionnaire]) } let(:assignment_questionnaire_vt) { create(:tm_assignment_questionnaire, id: 12, used_in_round: nil, assignment: assignment_vt, questionnaire: tm_questionnaire) } let(:team_vt) { create(:assignment_team, id: 12, assignment: assignment_vt) } let(:participant_vt) { create(:participant, id: 12, assignment: assignment_vt, user_id: student1_vt.id) } let(:participant2_vt) { create(:participant, id: 13, assignment: assignment_vt, user_id: student2_vt.id) } before(:each) do # Need to stub this method so the factory instance with # the stubbed :team method is returned by :find # instead of a separate instance with the same data allow(AssignmentParticipant) .to receive(:find) .with(participant_vt.id.to_s) .and_return(participant_vt) allow(participant_vt).to receive(:team).and_return(team_vt) allow(team_vt).to receive(:participants).and_return([participant_vt, participant2_vt]) allow(AssignmentQuestionnaire) .to receive(:find_by) .with(assignment_id: assignment_vt.id, questionnaire_id: tm_questionnaire.id) .and_return(assignment_questionnaire_vt) allow(AssignmentQuestionnaire) .to receive(:find_by) .with(assignment_id: assignment.id, questionnaire_id: review_questionnaire.id) .and_return(assignment_questionnaire_vt) allow(AssignmentQuestionnaire) .to receive(:where) .with(any_args) .and_return([assignment_questionnaire_vt]) allow(assignment_vt) .to receive(:late_policy_id) .and_return(false) allow(assignment_vt) .to receive(:calculate_penalty) .and_return(false) allow(assignment_vt) .to receive(:compute_total_score) .with(any_args) .and_return(100) allow(review_questionnaire).to receive(:get_assessments_round_for).with(participant, 1).and_return([review_response]) allow(Answer).to receive(:compute_scores).with([review_response], [question]).and_return(max: 95, min: 88, avg: 90) end it 'renders grades#view_team page' do params = {id: participant_vt.id} get :view_team, params expect(response).to render_template(:view_team) end context 'when view_team page is opened by student' do it 'dropdown is not rendered' do session = { user: student1_vt } params = {id: participant_vt.id} stub_current_user(student1_vt, student1_vt.role.name, student1_vt.role) get :view_team, params expect(response.body).to_not have_selector('select') end end context 'when view_team page is opened by instructor' do it 'dropdown is rendered' do session = { user: instructor } params = {id: participant_vt.id} get :view_team, params expect(response.body).to have_selector('select') end end context 'when view_team page is viewed by a student who is also a TA for another course' do it 'renders grades#view_team page' do params = { id: participant_vt.id } allow(TaMapping).to receive(:exists?).with(ta_id: participant_vt.user_id, course_id: 1).and_return(true)
H. In spec/factories/factories.rb:
H.1 Add the following factory:
factory :teammate_review_questionnaire, class: TeammateReviewQuestionnaire do
H.2 Add another factory
factory :tm_assignment_questionnaire, class: AssignmentQuestionnaire do user_id 1 questionnaire { association(:teammate_review_questionnaire) } questionnaire_weight 100 used_in_round nil topic_id nil dropdown 1 end
Present Status of the application
Image 1. Instructor's view of teammate review: This is the current screen given to the instructor. Here we see that the instructor can not determine who has given which review to which student in a team.
Image 2. Student's view of teammate review: Currently, a student is not able to see what grades were given by their teammates. We need to implement that functionality so that the student is able to view those grades.
Image 3. UML Diagram: We are planning to add two variables namely showTeammateReview and hideReviewerName. showTeammateReview will decide whether the students would be able to view the reviews given to them by their teammates. hideReviewerName will decide whether the student is able to view the reviewee's name or not.
Proposed changes
1) Proposed UI for instructor : The instructor is able to see the score for all teammates. In the existing code, only one teammate's scores are being shown. But, we are planning to change the way the (VmQuestionResponse array is populated)[1], so that the scores for all teammates are appended to it so it correctly shows what should have already been shown in this screen for the instructor.
Note: When a student visits this screen, they will only be shown the reviews received by them, assuming the instructor has allowed visibility of teammate reviews to students.
2) Error in existing UI for student : In the existing UI for a student, the highlighted block should not be rendered. This part of the screen is meant for the instructors/TAs. This portion of the screen being rendered to a student is a mistake, and we will remove this part of the screen from the student's screen as part of our proposed changes.
3) Show teammate review to student UI : The existing code doesn't render a student's teammate review scores at all. We are planning to render the scores to a student (as shown below) when they visit this screen. However, the visibility of this screen is contingent on whether the instructor has allowed visibility of the teammate review screen to students. If the instructor has disabled this feature, then the heatgrid will not be displayed at all.
Files to be modified
In the current design plan, the following files are planned to be edited:
1. app/controllers/grades_controller.rb - Controller file which will change how the view model is populated
2. app/views/grades/view_team.html.erb - View file, which will conditionally render the teammate name, for a teammate review heatgrid
3. spec/controllers/grades_controller_spec.rb - For tests verifying the proposed changes
4. app/models/vm_question_response.rb - We need to modify the view-model so it can tell us the name of the person being reviewed for the current heatgrid
Apart from these, there will be a schema change for the `Assignment` model, as we need to add the appropriate boolean flags
Design Patterns
Existing code does not use any design patterns, apart from if-else. It uses if-else conditional statements to check what type of questionnaire is present in the viewmodel `VmQuestionResponse`, and based on that, it renders the appropriate content.
We can use decorator pattern, for handling the visibility of the heatgrid when rendering heatgrids for a student login.
User Stories
1. As an instructor, I can view what reviews one teammate has given to other teammates.
2. As a student, I can view what reviews my teammates have given to me.
3. As an instructor, I can select if a student's reviews are anonymously shown to their teammates.
Testing
CURRENT STATUS
1. Presently, the tests only cover if the view_team is page is rendered when it is asked for.
OUR PROPOSAL
- When show_team is set as false by the professor, the student should not be able to see his scores.
- When show_team is set as true by the professor, the student should be able to see his.
- An instructor can select which student's scores he/she wants to see. If no student's name is selected then the professor can see the scores of all the students. - The instructor can decide whether to show the names of the reviewer to the reviewer in teammate reviews, by selecting a checkbox for the same.