ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

show an indicator for unread rubric comments 

test plan:
 - enable the "Submission feedback indicators" feature
 - have an assignment with a rubric that uses
   free-form comments
 - as a student, submit the assignment
 - as a teacher, grade the assignment while leaving
   comments in the rubric in speedgrader
 - as a student, click the dashboard notification about
   the graded assignment to go to the submission details
   page
 - there should be an unread indicator next to the
   "Show Rubric" link
 - click "Show Rubric" and that indicator should go away
 - refresh the page and the indicator should still be gone
 - the "Show Rubric" dot should look the same with K-5 mode
   on or off
 - VO should tell you if unread comments exist while the
   "Show Rubric" or "View Feedback" link is focused

flag=submission_feedback_indicators

closes LS-2670

Change-Id: I4b8b977eb9478342c1d2fc08b9910f90cdeed317
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/274396
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Robin Kuss <rkuss@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Peyton Craighill <pcraighill@instructure.com>
This commit is contained in:
Jeremy Stanley 2021-09-24 10:40:18 -06:00
parent 91a2939e27
commit a3bb59b199
14 changed files with 211 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
app/controllers/submissions_api_controller.rb
View File

@ -209,7 +209,9 @@ class SubmissionsApiController < ApplicationController
batch_jobs_in_actions :only => [:update], :batch => { :priority => Delayed::LOW_PRIORITY }
before_action :ensure_submission, :only => [:show,
:document_annotations_read_state,
:mark_document_annotations_read]
:mark_document_annotations_read,
:rubric_comments_read_state,
:mark_rubric_comments_read]
include Api::V1::Progress
include Api::V1::Submission
include Submissions::ShowHelper
@ -1141,6 +1143,52 @@ class SubmissionsApiController < ApplicationController
change_topic_read_state("unread")
end
# @API Get rubric comments read state
#
# Return whether new rubric comments made on a submission have been seen by the student being assessed.
#
# @example_request
#
# curl 'https://<canvas>/api/v1/courses/<course_id>/assignments/<assignment_id>/submissions/<user_id>/rubric_comments/read' \
# -H "Authorization: Bearer <token>"
#
# @example_response
# {
# "read": false
# }
#
def rubric_comments_read_state
if authorized_action(@submission, @current_user, :read)
render json: { read: !@user.unread_rubric_comments?(@submission) }
end
end
# @API Mark rubric comments as read
#
# Indicate that rubric comments made on a submission have been read by the student being assessed.
# Only the student who owns the submission can use this endpoint.
#
# NOTE: Rubric comments will be marked as read automatically when they are viewed in Canvas web.
#
# @example_request
#
# curl 'https://<canvas>/api/v1/courses/<course_id>/assignments/<assignment_id>/submissions/<user_id>/rubric_comments/read' \
# -X PUT \
# -H "Authorization: Bearer <token>" \
# -H "Content-Length: 0"
#
# @example_response
# {
# "read": true
# }
#
def mark_rubric_comments_read
return render_unauthorized_action unless @user == @current_user
@user.mark_rubric_comments_read!(@submission)
render json: { read: true }
end
# @API Get document annotations read state
#
# Return whether annotations made on a submitted document have been read by the student

12
app/models/rubric_assessment.rb
View File

@ -41,6 +41,7 @@ class RubricAssessment < ActiveRecord::Base
before_save :update_artifact_parameters
before_save :htmlify_rating_comments
before_save :mark_unread_comments
before_create :set_root_account_id
after_save :update_assessment_requests, :update_artifact
after_save :track_outcomes
@ -143,6 +144,17 @@ class RubricAssessment < ActiveRecord::Base
true
end
def mark_unread_comments
return unless artifact.is_a?(Submission)
return unless data_changed? && data.present?
if data.any? { |rating| rating.is_a?(Hash) && rating[:comments].present? }
user.mark_rubric_comments_unread!(artifact)
end
true
end
def update_assessment_requests
requests = self.assessment_requests
if active_rubric_association?

13
app/models/user.rb
View File

@ -1696,6 +1696,19 @@ class User < ActiveRecord::Base
set_preference(:unread_submission_annotations, submission.global_id, nil)
end
def unread_rubric_comments?(submission)
!!get_preference(:unread_rubric_comments, submission.global_id)
end
def mark_rubric_comments_unread!(submission)
set_preference(:unread_rubric_comments, submission.global_id, true)
end
def mark_rubric_comments_read!(submission)
# this will delete the user_preference_value
set_preference(:unread_rubric_comments, submission.global_id, nil)
end
def prefers_high_contrast?
!!feature_enabled?(:high_contrast)
end

1
app/models/user_preference_value.rb
View File

@ -54,6 +54,7 @@ class UserPreferenceValue < ActiveRecord::Base
add_user_preference :selected_calendar_contexts
add_user_preference :send_scores_in_emails_override, use_sub_keys: true
add_user_preference :unread_submission_annotations, use_sub_keys: true
add_user_preference :unread_rubric_comments, use_sub_keys: true
def self.settings
@preference_settings ||= {}

12
app/stylesheets/bundles/submission.scss
View File

@ -117,14 +117,20 @@
}
}
.submission_annotation_unread_indicator {
color: $fire;
.unread_indicator {
color: var(--ic-brand-button--primary-bgd);
font-size: x-large;
margin-#{direction(left)}: -4px;
vertical-align: 1px;
&::after {
font-family: 'Lato Extended';
content: "\2022";
}
&.submission_annotation {
margin-#{direction(left)}: -4px;
}
&.rubric_comment {
margin-#{direction(left)}: -13px;
}
}
.file-upload-submission {

9
app/views/submissions/show.html.erb
View File

@ -182,10 +182,17 @@
can_do(@assignment.rubric, @current_user, :read)
)
%>
<% unread_comments =
@context.root_account.feature_enabled?(:submission_feedback_indicators) &&
@current_user.unread_rubric_comments?(@submission) %>
<div class="submission-details-header__rubric <% if can_do(@assignment, @current_user, :grade) %>submission-details-header__rubric--can-grade<% end %>">
<a href="#" class="assess_submission_link Button Button--small Button--link" tabindex='0'>
<a href="#" class="assess_submission_link Button Button--small Button--link" tabindex='0' title="<%= unread_comments ? t('There are unread rubric comments.') : '' %>">
<i class="icon-rubric" aria-hidden="true"></i><%= t('show_rubric', 'Show Rubric') %>
</a>
<% if unread_comments %>
<span class="rubric_comment unread_indicator" aria-hidden="true"></span>
<% js_env mark_rubric_comments_read_url: api_v1_course_submission_rubric_comments_mark_read_url(@context.id, @assignment.id, @submission.user.id) %>
<% end %>
</div>
<% end %>
</div>

14
app/views/submissions/show_preview.html.erb
View File

@ -198,8 +198,13 @@
</div>
<div class="file-upload-submission-attachment">
<% unread_annotations =
@context.root_account.feature_enabled?(:submission_feedback_indicators) &&
@current_user&.unread_submission_annotations?(@submission) %>
<% if attachment.crocodoc_available? || attachment.canvadoc.try(:has_annotations?) %>
<% preview_document = t('preview_crocodoc_document', 'Preview your submission and view teacher feedback, if available')
<% preview_document = unread_annotations ?
t('Preview your submission and view teacher feedback. There are unread comments.') :
t('preview_crocodoc_document', 'Preview your submission and view teacher feedback, if available')
button_text = t('view_feedback_button', 'View Feedback')
%>
<% elsif attachment.canvadocable? %>
@ -220,11 +225,8 @@
>
<%= button_text %>
</a>
<% if @context.root_account.feature_enabled?(:submission_feedback_indicators) &&
@current_user&.unread_submission_annotations?(@submission) %>
<span class="submission_annotation_unread_indicator"
data-tooltip title="<%= I18n.t('There is unread feedback') %>">
</span>
<% if unread_annotations %>
<span class="submission_annotation unread_indicator" aria-hidden="true"></span>
<% end %>
<% end %>
</div>

2
config/routes.rb
View File

@ -1198,6 +1198,8 @@ CanvasRails::Application.routes.draw do
delete "#{context.pluralize}/:#{context}_id/assignments/:assignment_id/submissions/:user_id/read", action: :mark_submission_unread, as: "#{context}_submission_mark_unread"
get "#{context.pluralize}/:#{context}_id/assignments/:assignment_id/submissions/:user_id/document_annotations/read", action: :document_annotations_read_state, as: "#{path_prefix}_submission_document_annotations_read_state"
put "#{context.pluralize}/:#{context}_id/assignments/:assignment_id/submissions/:user_id/document_annotations/read", action: :mark_document_annotations_read, as: "#{path_prefix}_submission_document_annotations_mark_read"
get "#{context.pluralize}/:#{context}_id/assignments/:assignment_id/submissions/:user_id/rubric_comments/read", action: :rubric_comments_read_state, as: "#{path_prefix}_submission_rubric_comments_read_state"
put "#{context.pluralize}/:#{context}_id/assignments/:assignment_id/submissions/:user_id/rubric_comments/read", action: :mark_rubric_comments_read, as: "#{path_prefix}_submission_rubric_comments_mark_read"
get "#{context.pluralize}/:#{context}_id/assignments/:assignment_id/submissions", action: :index, as: "#{path_prefix}_assignment_submissions"
get "#{context.pluralize}/:#{context}_id/students/submissions", controller: :submissions_api, action: :for_students, as: "#{path_prefix}_student_submissions"
get "#{context.pluralize}/:#{context}_id/assignments/:assignment_id/submissions/:user_id", action: :show, as: "#{path_prefix}_assignment_submission"

46
spec/apis/v1/submissions_api_spec.rb
View File

@ -4800,6 +4800,52 @@ describe 'Submissions API', type: :request do
end
end
context 'rubric comments read state' do
before :once do
course_with_student_and_submitted_homework
end
it 'retrieves rubric comments read state' do
@student.mark_rubric_comments_unread!(@submission)
json = api_call_as_user(@student, :get, "/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}/rubric_comments/read",
{ course_id: @course.to_param, assignment_id: @assignment.to_param, user_id: @student.to_param,
action: 'rubric_comments_read_state', controller: 'submissions_api', format: 'json' })
expect(json).to eq({ 'read' => false })
@student.mark_rubric_comments_read!(@submission)
json = api_call_as_user(@student, :get, "/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}/rubric_comments/read",
{ course_id: @course.to_param, assignment_id: @assignment.to_param, user_id: @student.to_param,
action: 'rubric_comments_read_state', controller: 'submissions_api', format: 'json' })
expect(json).to eq({ 'read' => true })
end
it "requires read permission on the submission" do
temp = @student
other_student = student_in_course(active_all: true).user
@student = temp
api_call_as_user(other_student, :get, "/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}/rubric_comments/read",
{ course_id: @course.to_param, assignment_id: @assignment.to_param, user_id: @student.to_param,
action: 'rubric_comments_read_state', controller: 'submissions_api', format: 'json' },
{}, {}, { expected_status: 401 })
end
it 'marks rubric comments read' do
@student.mark_rubric_comments_unread!(@submission)
api_call_as_user(@student, :put, "/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}/rubric_comments/read",
{ course_id: @course.to_param, assignment_id: @assignment.to_param, user_id: @student.to_param,
action: 'mark_rubric_comments_read', controller: 'submissions_api', format: 'json' })
expect(@user.reload.unread_rubric_comments?(@submission)).to eq false
end
it "doesn't allow you to mark someone else's rubric comments read" do
api_call_as_user(@teacher, :put, "/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{@student.id}/rubric_comments/read",
{ course_id: @course.to_param, assignment_id: @assignment.to_param, user_id: @student.to_param,
action: 'mark_rubric_comments_read', controller: 'submissions_api', format: 'json' }, {},
{}, { expected_status: 401 })
end
end
context 'bulk update' do
before :each do
@student1 = user_factory(active_all: true)

1
spec/models/rubric_assessment_spec.rb
View File

@ -168,6 +168,7 @@ describe RubricAssessment do
t.extend HtmlTextHelper
expected = t.format_message(comment).first
expect(assessment.data.first[:comments_html]).to eq expected
expect(@student.reload.unread_rubric_comments?(@assignment.submission_for_student(@student))).to eq true
end
context "grading" do

24
spec/views/submissions/show.html.erb_spec.rb
View File

@ -511,6 +511,30 @@ describe "/submissions/show" do
classes = html.css('div.rubric_container').attribute('class').value.split(' ')
expect(classes).not_to include('assessing')
end
context "submission_feedback_indicators" do
before :once do
@course.root_account.enable_feature! :submission_feedback_indicators
end
it 'adds an indicator if unread comments are present' do
view_context(@course, @student)
@student.mark_rubric_comments_unread!(@submission)
assign(:assignment, @assignment)
assign(:submission, @submission)
render 'submissions/show'
expect(response.body).to include %{<span class="rubric_comment unread_indicator"}
end
it "does not show the indicator if unread comments aren't present" do
view_context(@course, @student)
@student.mark_rubric_comments_read!(@submission)
assign(:assignment, @assignment)
assign(:submission, @submission)
render 'submissions/show'
expect(response.body).not_to include %{<span class="rubric_comment unread_indicator"}
end
end
end
context 'when current_user is teacher' do

2
spec/views/submissions/show_preview.html.erb_spec.rb
View File

@ -93,7 +93,7 @@ describe "/submissions/show_preview" do
assign(:submission, submission)
@student.mark_submission_annotations_unread!(submission)
render template: "submissions/show_preview", locals: { anonymize_students: assignment.anonymize_students? }
expect(response.body).to include %{<span class="submission_annotation_unread_indicator"}
expect(response.body).to include %{<span class="submission_annotation unread_indicator"}
end
it "renders an iframe with a src to canvadoc sessions controller when assignment is a student annotation" do

88
ui/features/submissions/jquery/index.js vendored
View File

@ -55,31 +55,22 @@ function submissionLoaded(data) {
if ($('#submission_comment_' + comment.id).length > 0) {
continue
}
const $comment = $('#comment_blank')
.clone(true)
.removeAttr('id')
const $comment = $('#comment_blank').clone(true).removeAttr('id')
comment.posted_at = $.datetimeString(comment.created_at)
$comment.fillTemplateData({
data: comment,
id: 'submission_comment_' + comment.id
})
if (comment.media_comment_id) {
const $media_comment_link = $('#comment_media_blank')
.clone(true)
.removeAttr('id')
const $media_comment_link = $('#comment_media_blank').clone(true).removeAttr('id')
$media_comment_link.fillTemplateData({
data: comment
})
$comment
.find('.comment')
.empty()
.append($media_comment_link.show())
$comment.find('.comment').empty().append($media_comment_link.show())
} else {
for (var jdx in comment.attachments) {
const attachment = comment.attachments[jdx].attachment
const $attachment = $('#comment_attachment_blank')
.clone(true)
.removeAttr('id')
const $attachment = $('#comment_attachment_blank').clone(true).removeAttr('id')
attachment.comment_id = comment.id
$attachment.fillTemplateData({
data: attachment,
@ -88,9 +79,7 @@ function submissionLoaded(data) {
$comment.find('.comment_attachments').append($attachment.show())
}
}
$('.comments .comment_list')
.append($comment.show())
.scrollTop(10000)
$('.comments .comment_list').append($comment.show()).scrollTop(10000)
if ($('.grading_comment').val() === comment.comment) {
$('.grading_comment').val('')
}
@ -144,19 +133,19 @@ function makeRubricAccessible($rubric) {
13: 'enter',
27: 'esc'
}
$('.hide_rubric_link').keydown(function(e) {
$('.hide_rubric_link').keydown(function (e) {
if (keyCodes[e.which] == 'enter') {
e.preventDefault()
$(this).click()
}
})
$tabs.each(function() {
$tabs.each(function () {
$(this).bind('keydown', e => {
if (keyCodes[e.which] == 'esc') $('.hide_rubric_link').click()
})
})
$(tabBounds).each(function(e) {
$(this).bind('keydown', function(e) {
$(tabBounds).each(function (e) {
$(this).bind('keydown', function (e) {
if (keyCodes[e.which] == 'tab') {
const isLeavingHolder = $(this).is($(tabBounds).first()) ? e.shiftKey : !e.shiftKey
if (isLeavingHolder) {
@ -180,22 +169,18 @@ function makeRubricAccessible($rubric) {
}
function toggleRubric($rubric) {
const ariaSetting = $rubric.is(':visible')
$('#application')
.find('[data-hide_from_rubric]')
.attr('aria-hidden', ariaSetting)
$('#application').find('[data-hide_from_rubric]').attr('aria-hidden', ariaSetting)
}
function closeRubric() {
$('#rubric_holder').fadeOut(function() {
$('#rubric_holder').fadeOut(function () {
toggleRubric($(this))
$('.assess_submission_link').focus()
})
}
function openRubric() {
$('#rubric_holder').fadeIn(function() {
$('#rubric_holder').fadeIn(function () {
toggleRubric($(this))
$(this)
.find('.hide_rubric_link')
.focus()
$(this).find('.hide_rubric_link').focus()
})
}
function windowResize() {
@ -211,18 +196,14 @@ function windowResize() {
// submissions.coffee requires this file and then immediately triggers it,
// while submissionsSpec.jsx triggers it after setup is complete.
export function setup() {
$(document).ready(function() {
$(document).ready(function () {
$('.comments .comment_list .play_comment_link').mediaCommentThumbnail('small')
$(window)
.bind('resize', windowResize)
.triggerHandler('resize')
$(window).bind('resize', windowResize).triggerHandler('resize')
$('.comments_link').click(event => {
event.preventDefault()
$('.comments').slideToggle(() => {
$('.comments .media_comment_content').empty()
$('.comments textarea:visible')
.focus()
.select()
$('.comments textarea:visible').focus().select()
})
})
$('.save_comment_button').click(event => {
@ -304,24 +285,20 @@ export function setup() {
})
$('.attach_comment_file_link').click(event => {
event.preventDefault()
const $attachment = $('#comment_attachment_input_blank')
.clone(true)
.removeAttr('id')
const $attachment = $('#comment_attachment_input_blank').clone(true).removeAttr('id')
$attachment.find('input').attr('name', 'attachments[' + fileIndex++ + '][uploaded_data]')
$('#add_comment_form .comment_attachments').append($attachment.slideDown())
})
$('.delete_comment_attachment_link').click(function(event) {
$('.delete_comment_attachment_link').click(function (event) {
event.preventDefault()
$(this)
.parents('.comment_attachment_input')
.slideUp(function() {
.slideUp(function () {
$(this).remove()
})
})
$('.save_rubric_button').click(function() {
const $rubric = $(this)
.parents('#rubric_holder')
.find('.rubric')
$('.save_rubric_button').click(function () {
const $rubric = $(this).parents('#rubric_holder').find('.rubric')
const submitted_data = rubricAssessment.assessmentData($rubric)
const url = $('.update_rubric_assessment_url').attr('href')
const method = 'POST'
@ -351,9 +328,7 @@ export function setup() {
.val(assessment.id)
.text(assessment.assessor_name)
.attr('id', 'rubric_assessment_option_' + assessment.id)
$('#rubric_assessments_select')
.prepend($option)
.val(assessment.id)
$('#rubric_assessments_select').prepend($option).val(assessment.id)
}
$('#rubric_assessment_option_' + assessment.id).text(assessment.assessor_name)
$('#new_rubric_assessment_option').remove()
@ -385,10 +360,15 @@ export function setup() {
$('.assess_submission_link').click(event => {
event.preventDefault()
$('#rubric_assessments_select').change()
if (ENV.mark_rubric_comments_read_url) {
$.ajaxJSON(ENV.mark_rubric_comments_read_url, 'PUT', {}, () => {})
$('.rubric_comment.unread_indicator').hide()
$('.submission-details-header__rubric .assess_submission_link').attr('title', '')
}
openRubric()
})
$('#rubric_assessments_select')
.change(function() {
.change(function () {
const id = $(this).val()
let found = null
for (const idx in rubricAssessments) {
@ -413,9 +393,7 @@ export function setup() {
'create',
'any',
(id, type) => {
$('#media_media_recording')
.data('comment_id', id)
.data('comment_type', type)
$('#media_media_recording').data('comment_id', id).data('comment_type', type)
$(document).triggerHandler('comment_change')
$('#add_comment_form').show()
$('#media_media_recording').hide()
@ -432,7 +410,7 @@ export function setup() {
$('#media_media_recording').hide()
})
$('.comments .comment_list')
.delegate('.play_comment_link', 'click', function(event) {
.delegate('.play_comment_link', 'click', function (event) {
event.preventDefault()
const comment_id = $(this)
.parents('.comment_media')
@ -476,12 +454,10 @@ $(document).fragmentChange((event, hash) => {
if (params && params.comment) {
$('.grading_comment').val(params.comment)
}
$('.grading_comment')
.focus()
.select()
$('.grading_comment').focus().select()
}
})
INST.refreshGrades = function() {
INST.refreshGrades = function () {
const url = $('.submission_data_url').attr('href')
setTimeout(() => {
$.ajaxJSON(url, 'GET', {}, submissionLoaded)

6
ui/features/submissions_show_preview_upload/index.js
View File

@ -49,7 +49,11 @@ $(document).ready(() => {
height: $(document).height() * 0.75
})
.loadDocPreview($.extend({height: '100%'}, $(this).data()))
$('.submission_annotation_unread_indicator').hide()
$('.submission_annotation.unread_indicator').hide()
$('.file-upload-submission-attachment .modal_preview_link').attr(
'title',
I18n.t('Preview your submission and view teacher feedback, if available')
)
return false
})
})