Add content_item to content_participations table

Add the content_item column to enable tracking the
read state of each submission item (grade, comment, rubric)

Ensure content_participation_counts doesn't allow negative `unread_count`

closes EVAL-2657
flag=visibility_feedback_student_grades_page

Test plan:
  - Existing test and new tests pass

Change-Id: Ia0a7aaadd944914de56e596736537d6946ce7d3c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/301671
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Kai Bjorkman <kbjorkman@instructure.com>
QA-Review: Cameron Ray <cameron.ray@instructure.com>
Product-Review: Cameron Ray <cameron.ray@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Migration-Review: Jeremy Stanley <jeremy@instructure.com>
This commit is contained in:
paulo.chaves 2022-09-22 18:37:01 -03:00 committed by Paulo Chaves
parent dd84f1faed
commit 35b93d1934
4 changed files with 505 additions and 22 deletions

View File

@ -21,21 +21,25 @@
class ContentParticipation < ActiveRecord::Base
include Workflow
ACCESSIBLE_ATTRIBUTES = %i[content user workflow_state].freeze
ACCESSIBLE_ATTRIBUTES = %i[content user workflow_state content_item].freeze
CONTENT_ITEMS = %w[grade comment rubric].freeze
belongs_to :content, polymorphic: [:submission]
belongs_to :user
before_create :set_root_account_id
after_save :update_participation_count
after_save :update_participation_count, if: :call_update_participation_count?
validates :content_type, :content_id, :user_id, :workflow_state, presence: true
validates :content_type, :content_id, :user_id, :workflow_state, :content_item, presence: true
validates :content_item, inclusion: { in: CONTENT_ITEMS }
workflow do
state :unread
state :read
end
attr_accessor :unread_count_offset
def self.create_or_update(opts = {})
opts = opts.with_indifferent_access
content = opts.delete(:content)
@ -52,18 +56,138 @@ class ContentParticipation < ActiveRecord::Base
participant
end
def call_update_participation_count?
return true unless Account.site_admin.feature_enabled?(:visibility_feedback_student_grades_page)
content.posted?
end
def update_participation_count
return unless saved_change_to_workflow_state?
offset = if Account.site_admin.feature_enabled?(:visibility_feedback_student_grades_page)
unread_count_offset
else
((workflow_state == "unread") ? 1 : -1)
end
ContentParticipationCount.create_or_update({
context: content.context,
user: user,
content_type: content_type,
offset: (workflow_state == "unread" ? 1 : -1),
offset: offset,
})
end
def set_root_account_id
self.root_account_id = content.assignment.root_account_id
end
def self.participate(content:, user:, workflow_state: "unread", content_item: "grade")
return unless Account.site_admin.feature_enabled?(:visibility_feedback_student_grades_page)
raise "cannot read user and content" unless content.is_a?(Submission) && user.is_a?(User)
participant = nil
unique_constraint_retry do
participations = content.content_participations.where(user_id: user)
participant = create_first_participation_item(participations, content, user, workflow_state, content_item)
participant ||= update_existing_participation_item(participations, workflow_state, content_item)
participant ||= add_participation_item(participations, content, user, workflow_state, content_item)
participant.save! if participant.new_record? || participant.changed?
end
participant
end
def self.create_first_participation_item(participations, content, user, workflow_state, content_item)
return if participations.any?
participant = build_item(content, user, workflow_state, content_item)
participant.unread_count_offset = workflow_state == "unread" ? 1 : 0
participant
end
private_class_method :create_first_participation_item
def self.update_existing_participation_item(participations, workflow_state, content_item)
participant = participations.find { |p| p.content_item == content_item }
return participant if participant.nil? || same_workflow_state?(participant, workflow_state)
participations -= [participant]
participant.unread_count_offset = if participations.empty? || all_read?(participations)
workflow_state == "unread" ? 1 : -1
else
0
end
participant.workflow_state = workflow_state
participant
end
private_class_method :update_existing_participation_item
def self.add_participation_item(participations, content, user, workflow_state, content_item)
participant = build_item(content, user, workflow_state, content_item)
participant.unread_count_offset = (all_read?(participations) && workflow_state == "unread") ? 1 : 0
participant
end
private_class_method :add_participation_item
def self.all_read?(items)
items.all? { |participant| participant.workflow_state == "read" }
end
private_class_method :all_read?
def self.build_item(content, user, workflow_state, content_item)
content.content_participations.build(user: user, workflow_state: workflow_state, content_item: content_item)
end
private_class_method :build_item
def self.same_workflow_state?(participant, workflow_state)
participant.present? && participant.workflow_state == workflow_state
end
private_class_method :same_workflow_state?
def self.submission_read?(content:, user:)
submission_read_state(content, user) != "unread"
end
def self.submission_item_read?(content:, user:, content_item:)
submission_read_state(content, user, content_item) != "unread"
end
def self.submission_read_state(content, user, content_item = nil)
raise "content is not a Submission" unless content.is_a?(Submission)
raise "#{content_item} is invalid" if content_item.present? && !CONTENT_ITEMS.include?(content_item)
states = if content_item.present?
ContentParticipation.where(content: content, user: user, content_item: content_item).pluck(:workflow_state)
else
ContentParticipation.where(content: content, user: user).pluck(:workflow_state)
end
return nil if states.empty?
return "unread" if states.any?("unread")
"read"
end
def self.already_read_count(ids = [], user)
ContentParticipation
.group(:content_id)
.where(
content_type: "Submission",
content_id: ids,
user_id: user
)
.having("sum(case workflow_state when 'unread' then 1 else 0 end) = 0")
.pluck(:content_id)
.count
end
end

View File

@ -46,19 +46,27 @@ class ContentParticipationCount < ActiveRecord::Base
end
participant.attributes = opts.slice(*ACCESSIBLE_ATTRIBUTES)
# if the participant was just created, the count will already be correct
if opts[:offset].present? && !participant.new_record?
current_unread_count = participant.unread_count(refresh: false)
unless current_unread_count == 0 && opts[:offset] == -1
participant.unread_count = current_unread_count + opts[:offset]
end
end
set_unread_count(participant, opts)
participant.save if participant.new_record? || participant.changed?
end
end
participant
end
def self.set_unread_count(participant, opts = {})
offset = opts.delete(:offset)
unread_count = opts.delete(:unread_count)
# allow setting the unread_count and when not present increment|decrement using an offset
unless unread_count.is_a?(Integer)
unread_count = participant.unread_count(refresh: false) + offset.to_i
end
participant.unread_count = unread_count > 0 ? unread_count : 0
end
private_class_method :set_unread_count
def self.unread_count_for(type, context, user)
return 0 unless user.present? && context.present?
@ -100,17 +108,22 @@ class ContentParticipationCount < ActiveRecord::Base
SQL
(subs_with_grades + subs_with_comments).uniq
end
already_read_count = if potential_ids.any?
ContentParticipation.where(
content_type: "Submission",
content_id: potential_ids,
user_id: user,
workflow_state: "read"
).count
else
0
end
potential_ids.size - already_read_count
potential_ids.size - already_read_count(potential_ids, user)
end
end
def self.already_read_count(ids = [], user)
return 0 if ids.empty?
if Account.site_admin.feature_enabled?(:visibility_feedback_student_grades_page)
ContentParticipation.already_read_count(ids, user)
else
ContentParticipation.where(
content_type: "Submission",
content_id: ids,
user_id: user,
workflow_state: "read"
).count
end
end

View File

@ -0,0 +1,42 @@
# frozen_string_literal: true
#
# Copyright (C) 2022 - present Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class AddContentItemToContentParticipations < ActiveRecord::Migration[6.1]
tag :predeploy
disable_ddl_transaction!
def change
add_column :content_participations, :content_item, :string, null: false,
default: "grade",
if_not_exists: true
add_index :content_participations, %w[content_id content_type user_id content_item],
name: "index_content_participations_by_type_uniquely",
unique: true,
algorithm: :concurrently,
if_not_exists: true
remove_index :content_participations, column: %w[content_id content_type user_id],
name: "index_content_participations_uniquely",
algorithm: :concurrently,
if_exists: true
end
end

View File

@ -57,6 +57,152 @@ describe ContentParticipation do
cp = ContentParticipation.where(user_id: @student).first
expect(cp.workflow_state).to eq "unread"
end
context "when feeback visibility ff is on" do
before do
Account.site_admin.enable_feature!(:visibility_feedback_student_grades_page)
end
it "creates 'grade' participation if content_item not given" do
expect do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
end.to change(ContentParticipation, :count).by 1
cp = ContentParticipation.where(user_id: @student).first
expect(cp.content_item).to eq "grade"
end
it "create a participation item as read" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
cp = ContentParticipation.where(user_id: @student).first
expect(cp.workflow_state).to eq "read"
end
it "creates all possible content items for a submission" do
expect do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread", content_item: "comment")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread", content_item: "rubric")
end.to change(ContentParticipation, :count).by 3
end
it "doesn't allow invalid content_item" do
expect do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread", content_item: "_ruby")
end.to raise_error(ActiveRecord::RecordInvalid)
end
it "doesn't duplicate content_item if submission is the same" do
expect do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
end.to change(ContentParticipation, :count).by 1
end
it "changes a content_item state from unread to read" do
expect do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
end.to change(ContentParticipation, :count).by 1
cp = ContentParticipation.where(user_id: @student).first
expect(cp.workflow_state).to eq "read"
end
it "changes a content_item state from read to unread" do
expect do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
end.to change(ContentParticipation, :count).by 1
cp = ContentParticipation.where(user_id: @student).first
expect(cp.workflow_state).to eq "unread"
end
context "when multiple submissions exist" do
before do
temp_assignment = @assignment
@assignment2 = assignment_model(course: @course)
@content2 = @assignment2.submit_homework(@student)
@assignment = temp_assignment
end
it "create two participation if same item but different submissions" do
expect do
ContentParticipation.participate(content: @content, user: @student)
ContentParticipation.participate(content: @content2, user: @student)
end.to change(ContentParticipation, :count).by 2
end
end
end
end
describe "submission_read?" do
context "when feeback visibility ff is on" do
before do
Account.site_admin.enable_feature!(:visibility_feedback_student_grades_page)
end
it "is read if no participation exist" do
expect(ContentParticipation.count).to eq 0
expect(ContentParticipation.submission_read?(content: @content, user: @student)).to be_truthy
end
it "is not read if existing item is unread" do
ContentParticipation.participate(content: @content, user: @student)
expect(ContentParticipation.submission_read?(content: @content, user: @student)).to be_falsey
end
it "is read if one content_item is present and state changes to read" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
expect(ContentParticipation.submission_read?(content: @content, user: @student)).to be_truthy
end
it "is read when all items are read" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, content_item: "comment", workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, content_item: "rubric", workflow_state: "read")
expect(ContentParticipation.submission_read?(content: @content, user: @student)).to be_truthy
end
it "is unread if at least one item is unread" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, content_item: "comment", workflow_state: "unread")
ContentParticipation.participate(content: @content, user: @student, content_item: "rubric", workflow_state: "read")
expect(ContentParticipation.submission_read?(content: @content, user: @student)).to be_falsey
end
end
end
describe "submission_item_read?" do
context "when feeback visibility ff is on" do
before do
Account.site_admin.enable_feature!(:visibility_feedback_student_grades_page)
end
it "grade is unread if workflow_state is not given" do
ContentParticipation.participate(content: @content, user: @student)
expect(ContentParticipation.submission_item_read?(
content: @content,
user: @student,
content_item: "grade"
)).to be_falsey
end
it "changes submisison grade from unread to read" do
ContentParticipation.participate(content: @content, user: @student)
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
expect(ContentParticipation.submission_item_read?(
content: @content,
user: @student,
content_item: "grade"
)).to be_truthy
end
end
end
describe "update_participation_count" do
@ -113,6 +259,164 @@ describe ContentParticipation do
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 0
end
context "when feeback visibility ff is on" do
before do
Account.site_admin.enable_feature!(:visibility_feedback_student_grades_page)
@content.update_columns(posted_at: Time.now.utc, workflow_state: "graded", score: 10)
end
it "unread_count is 1 when at least one submission participation item is unread" do
expect do
ContentParticipation.participate(content: @content, user: @student)
ContentParticipation.participate(content: @content, user: @student, content_item: "comment", workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, content_item: "rubric", workflow_state: "read")
end.to change(ContentParticipationCount, :count).by 1
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 1
end
it "unread_count is 0 when all submission participation items are read" do
expect do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read", content_item: "comment")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read", content_item: "rubric")
end.to change(ContentParticipationCount, :count).by 1
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 0
end
it "unread_count is 1 when all items are unread and one is set to read" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, content_item: "comment")
ContentParticipation.participate(content: @content, user: @student, content_item: "rubric")
expect do
ContentParticipation.participate(content: @content, user: @student, content_item: "rubric", workflow_state: "read")
end.to change(ContentParticipationCount, :count).by 0
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 1
end
it "unread_count is 1 when all items are read and one is set to unread" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read", content_item: "comment")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read", content_item: "rubric")
expect(ContentParticipationCount.count).to eq 1
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread", content_item: "comment")
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.reload.unread_count).to eq 1
end
it "unread_count is 0 when the last unread item is set to read" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread", content_item: "comment")
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read", content_item: "rubric")
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 1
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read", content_item: "comment")
expect(cpc.reload.unread_count).to eq 0
end
it "unread_count is 1 the only existing item is changed to unread" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 0
ContentParticipation.participate(content: @content, user: @student, workflow_state: "unread")
expect(cpc.reload.unread_count).to eq 1
end
it "unread_count is 0 the only existing item is changed to read" do
ContentParticipation.participate(content: @content, user: @student)
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 1
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
expect(cpc.reload.unread_count).to eq 0
end
context "when multiple submissions exist" do
before do
temp_assignment = @assignment
@assignment2 = assignment_model(course: @course)
@content2 = @assignment2.submit_homework(@student)
@assignment = temp_assignment
@content2.update_columns(posted_at: Time.now.utc, workflow_state: "graded", score: 10)
end
it "unread_count is 2 when submissions are unread" do
ContentParticipation.participate(content: @content, user: @student)
ContentParticipation.participate(content: @content2, user: @student)
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 2
end
it "unread_count is 0 when submissions are read" do
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content2, user: @student, workflow_state: "read")
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 0
end
it "unread_count changes from 2 to 0 after submissions are read" do
ContentParticipation.participate(content: @content, user: @student)
ContentParticipation.participate(content: @content2, user: @student)
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 2
ContentParticipation.participate(content: @content, user: @student, workflow_state: "read")
ContentParticipation.participate(content: @content2, user: @student, workflow_state: "read")
expect(cpc.reload.unread_count).to eq 0
end
it "unread_count is 1 when one submission is read and another is unread" do
ContentParticipation.participate(content: @content, user: @student)
ContentParticipation.participate(content: @content2, user: @student, workflow_state: "read")
cpc = ContentParticipationCount.where(user_id: @student).first
expect(cpc.unread_count).to eq 1
end
end
context "when multiple courses exist" do
before do
@course2 = Course.create!
@course2_student = student_in_course(course: @course2, active_all: true).user
@course2_assignment = assignment_model(course: @course2, due_at: 2.days, points_possible: 10)
@course2_content = @course2_assignment.submit_homework(@course2_student)
@course2_content.update_columns(posted_at: Time.now.utc, workflow_state: "graded", score: 10)
end
it "unread_count is 1 for each course" do
expect do
ContentParticipation.participate(content: @content, user: @student)
ContentParticipation.participate(content: @course2_content, user: @course2_student)
end.to change(ContentParticipationCount, :count).by 2
cpc1 = @course.content_participation_counts.where(user_id: @student).first
expect(cpc1.unread_count).to eq 1
cpc2 = @course2.content_participation_counts.where(user_id: @course2_student).first
expect(cpc2.unread_count).to eq 1
end
end
end
end
describe "create" do