From d4cbf3eca0cdbd62149b33dad665bb1180fa64b5 Mon Sep 17 00:00:00 2001 From: Jason Gillett Date: Wed, 18 Sep 2024 13:42:57 -0600 Subject: [PATCH] Display checkpointed discussion in assign to tray MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes VICE-4299 flag=discussion_checkpoint Note: Updating checkpointed dates from the tray will not work until VICE-4300 Test Plan 1. Create a checkpointed discussion with various states of override due dates and availability 2. Compare the discussion edit page with the assignTo tray cards 3. Should display all override due dates corectly 3a. assign to tray locations discussion show, discussion index, assignment index Change-Id: Ia7125e027370ee7e96cfd8482e5592961316e59a Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/357825 Reviewed-by: Omar Soto-Fortuño Tested-by: Service Cloud Jenkins Reviewed-by: Juan Chavez QA-Review: Juan Chavez Product-Review: Jason Gillett --- .../learning_object_dates_controller.rb | 3 +- app/models/assignment_override.rb | 9 +++ lib/api/v1/assignment_override.rb | 9 ++- spec/models/assignment_override_spec.rb | 24 ++++++ .../discussions/discussion_topic_show_spec.rb | 50 +++++++++++- spec/selenium/helpers/items_assign_to_tray.rb | 78 +++++++++++++++++++ .../react/Item/ItemAssignToTrayContent.tsx | 27 ++++++- .../react/Item/utils.ts | 31 ++++++++ 8 files changed, 221 insertions(+), 10 deletions(-) diff --git a/app/controllers/learning_object_dates_controller.rb b/app/controllers/learning_object_dates_controller.rb index ea3aac80f30..328f8fd5e2d 100644 --- a/app/controllers/learning_object_dates_controller.rb +++ b/app/controllers/learning_object_dates_controller.rb @@ -116,7 +116,8 @@ class LearningObjectDatesController < ApplicationController Api.paginate(section_visibilities, self, route) end - all_overrides = assignment_overrides_json(overrides, @current_user, include_names: true) + include_child_override_due_dates = Account.site_admin.feature_enabled?(:discussion_checkpoints) + all_overrides = assignment_overrides_json(overrides, @current_user, include_names: true, include_child_override_due_dates:) all_overrides += section_visibility_to_override_json(section_visibilities, overridable) if visibilities_to_override render json: { diff --git a/app/models/assignment_override.rb b/app/models/assignment_override.rb index 631051c8a6e..878218bd7a7 100644 --- a/app/models/assignment_override.rb +++ b/app/models/assignment_override.rb @@ -557,4 +557,13 @@ class AssignmentOverride < ActiveRecord::Base def set_root_account_id write_attribute(:root_account_id, root_account_id) unless read_attribute(:root_account_id) end + + def sub_assignment_due_dates + child_overrides.active.preload(:assignment).map do |child| + { + sub_assignment_tag: child.assignment&.sub_assignment_tag, + due_at: child.due_at + } + end + end end diff --git a/lib/api/v1/assignment_override.rb b/lib/api/v1/assignment_override.rb index 4ea8363113c..36ca08e7b7f 100644 --- a/lib/api/v1/assignment_override.rb +++ b/lib/api/v1/assignment_override.rb @@ -23,7 +23,7 @@ module Api::V1::AssignmentOverride OVERRIDABLE_ID_FIELDS = %i[assignment_id quiz_id context_module_id discussion_topic_id wiki_page_id attachment_id].freeze - def assignment_override_json(override, visible_users = nil, student_names: nil, module_names: nil) + def assignment_override_json(override, visible_users = nil, student_names: nil, module_names: nil, include_child_override_due_dates: nil) fields = %i[id title unassign_item] OVERRIDABLE_ID_FIELDS.each { |f| fields << f if override.send(f).present? } fields.push(:due_at, :all_day, :all_day_date) if override.due_at_overridden @@ -52,6 +52,9 @@ module Api::V1::AssignmentOverride when "Noop" json[:noop_id] = override.set_id end + if include_child_override_due_dates + json[:sub_assignment_due_dates] = override.sub_assignment_due_dates + end end end @@ -70,7 +73,7 @@ module Api::V1::AssignmentOverride end end - def assignment_overrides_json(overrides, user = nil, include_names: false) + def assignment_overrides_json(overrides, user = nil, include_names: false, include_child_override_due_dates: false) visible_users_ids = ::AssignmentOverride.visible_enrollments_for(overrides.compact, user).select(:user_id) # we most likely already have the student_ids preloaded here because of overridden_for, but just in case if overrides.any? { |ov| ov.present? && ov.set_type == "ADHOC" && !ov.preloaded_student_ids } @@ -82,7 +85,7 @@ module Api::V1::AssignmentOverride module_ids = overrides.select { |ov| ov.present? && ov.context_module_id.present? }.map(&:context_module_id).uniq module_names = ContextModule.where(id: module_ids).pluck(:id, :name).to_h end - overrides.map { |override| assignment_override_json(override, visible_users_ids, student_names:, module_names:) if override } + overrides.map { |override| assignment_override_json(override, visible_users_ids, student_names:, module_names:, include_child_override_due_dates:) if override } end def assignment_override_collection(learning_object, include_students: false) diff --git a/spec/models/assignment_override_spec.rb b/spec/models/assignment_override_spec.rb index 0cb94fff7fe..7a3ceefefba 100644 --- a/spec/models/assignment_override_spec.rb +++ b/spec/models/assignment_override_spec.rb @@ -515,6 +515,30 @@ describe AssignmentOverride do expect(child_override.reload.workflow_state).to eq "deleted" expect(parent_override.reload.workflow_state).to eq "deleted" end + + describe "#sub_assignment_due_dates" do + before :once do + @parent_assignment = @course.assignments.create! + @parent_override = AssignmentOverride.create!(title: "Parent Override", set_type: "ADHOC", assignment: @parent_assignment) + @child_assignment1 = parent_assignment.sub_assignments.create!(context: @course, sub_assignment_tag: CheckpointLabels::REPLY_TO_TOPIC) + @child_assignment2 = parent_assignment.sub_assignments.create!(context: @course, sub_assignment_tag: CheckpointLabels::REPLY_TO_ENTRY) + @child_override1 = AssignmentOverride.create!(title: "Child Override 1", set_type: "ADHOC", assignment: @child_assignment1, due_at: 1.day.from_now, parent_override: @parent_override) + @child_override2 = AssignmentOverride.create!(title: "Child Override 2", set_type: "ADHOC", assignment: @child_assignment2, due_at: 2.days.from_now, parent_override: @parent_override) + end + + it "returns the correct sub_assignment_due_dates" do + expected_result = [ + { sub_assignment_tag: CheckpointLabels::REPLY_TO_TOPIC, due_at: @child_override1.due_at }, + { sub_assignment_tag: CheckpointLabels::REPLY_TO_ENTRY, due_at: @child_override2.due_at } + ] + expect(@parent_override.sub_assignment_due_dates).to eq(expected_result) + end + + it "returns an empty array when there are no child overrides" do + @parent_override.child_overrides.destroy_all + expect(@parent_override.sub_assignment_due_dates).to eq([]) + end + end end end diff --git a/spec/selenium/discussions/discussion_topic_show_spec.rb b/spec/selenium/discussions/discussion_topic_show_spec.rb index 2ecfdb37fc9..eff67747f1e 100644 --- a/spec/selenium/discussions/discussion_topic_show_spec.rb +++ b/spec/selenium/discussions/discussion_topic_show_spec.rb @@ -420,16 +420,37 @@ describe "Discussion Topic Show" do assignment: ) + group = dt.course.groups.create! + dt.update!(group_category: group.group_category) + student_in_group = student_in_course(course: dt.course, active_all: true).user + group.group_memberships.create!(user: student_in_group) + + new_section = @topic.course.course_sections.create! + + everyone_due_at = 3.days.from_now + student_due_at = 4.days.from_now + group_due_at = 5.days.from_now + section_due_at = 6.days.from_now + + student_lock_at = 11.days.from_now + student_unlock_at = 1.day.from_now + Checkpoints::DiscussionCheckpointCreatorService.call( discussion_topic: dt, checkpoint_label: CheckpointLabels::REPLY_TO_TOPIC, - dates: [{ type: "everyone", due_at: 2.days.from_now }], + dates: [{ type: "everyone", due_at: everyone_due_at }, + { type: "override", set_type: "ADHOC", student_ids: [@student.id], due_at: student_due_at, lock_at: student_lock_at, unlock_at: student_unlock_at }, + { type: "override", set_type: "CourseSection", set_id: new_section.id, due_at: section_due_at }, + { type: "override", set_type: "Group", set_id: group.id, due_at: group_due_at }], points_possible: 6 ) Checkpoints::DiscussionCheckpointCreatorService.call( discussion_topic: dt, checkpoint_label: CheckpointLabels::REPLY_TO_ENTRY, - dates: [{ type: "everyone", due_at: 3.days.from_now }], + dates: [{ type: "everyone", due_at: everyone_due_at }, + { type: "override", set_type: "ADHOC", student_ids: [@student.id], due_at: student_due_at, lock_at: student_lock_at, unlock_at: student_unlock_at }, + { type: "override", set_type: "CourseSection", set_id: new_section.id, due_at: section_due_at }, + { type: "override", set_type: "Group", set_id: group.id, due_at: group_due_at }], points_possible: 7, replies_required: 2 ) @@ -441,6 +462,31 @@ describe "Discussion Topic Show" do expect(module_item_assign_to_card.first).not_to contain_css(due_date_input_selector) expect(module_item_assign_to_card.first).to contain_css(reply_to_topic_due_date_input_selector) expect(module_item_assign_to_card.first).to contain_css(required_replies_due_date_input_selector) + all_dates = get_all_dates_for_all_cards + + # Everyone Card + expect(format_date_for_view(all_dates[0][:reply_to_topic], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(everyone_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(format_date_for_view(all_dates[0][:required_replies], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(everyone_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(all_dates[0][:available_from]).to eq("") + expect(all_dates[0][:until]).to eq("") + + # Student Card + expect(format_date_for_view(all_dates[1][:reply_to_topic], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(student_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(format_date_for_view(all_dates[1][:required_replies], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(student_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(format_date_for_view(all_dates[1][:available_from], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(student_unlock_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(format_date_for_view(all_dates[1][:until], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(student_lock_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + + # Section Card + expect(format_date_for_view(all_dates[2][:reply_to_topic], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(section_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(format_date_for_view(all_dates[2][:required_replies], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(section_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(all_dates[2][:available_from]).to eq("") + expect(all_dates[2][:until]).to eq("") + + # Group Card + expect(format_date_for_view(all_dates[3][:reply_to_topic], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(group_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(format_date_for_view(all_dates[3][:required_replies], "%b %d, %Y %I:%M %p")).to eq(format_date_for_view(group_due_at.in_time_zone("UTC"), "%b %d, %Y %I:%M %p")) + expect(all_dates[3][:available_from]).to eq("") + expect(all_dates[3][:until]).to eq("") end it "does not show the button when the user does not have the moderate_forum permission" do diff --git a/spec/selenium/helpers/items_assign_to_tray.rb b/spec/selenium/helpers/items_assign_to_tray.rb index dff4f2651a2..4afe1a7ef3d 100644 --- a/spec/selenium/helpers/items_assign_to_tray.rb +++ b/spec/selenium/helpers/items_assign_to_tray.rb @@ -137,6 +137,22 @@ module ItemsAssignToTray "[data-testid = 'lock_at_input']" end + def reply_to_topic_datetime_selector + "[data-testid='reply_to_topic_due_at_input'] input[type='text']" + end + + def required_replies_datetime_selector + "[data-testid='required_replies_due_at_input'] input[type='text']" + end + + def available_from_datetime_selector + "[data-testid='unlock_at_input'] input[type='text']" + end + + def until_datetime_selector + "[data-testid='lock_at_input'] input[type='text']" + end + #------------------------------ Elements ------------------------------ def add_assign_to_card f(add_assign_to_card_selector) @@ -302,6 +318,22 @@ module ItemsAssignToTray f(tray_header_selector) end + def reply_to_topic_datetime_inputs + ff(reply_to_topic_datetime_selector) + end + + def required_replies_datetime_inputs + ff(required_replies_datetime_selector) + end + + def available_from_datetime_inputs + ff(available_from_datetime_selector) + end + + def until_datetime_inputs + ff(until_datetime_selector) + end + #------------------------------ Actions ------------------------------ def click_add_assign_to_card @@ -389,4 +421,50 @@ module ItemsAssignToTray end wait_for_ajaximations end + + def combine_date_and_time(date_input, time_input) + date = date_input.attribute("value") + time = time_input.attribute("value") + return "" if date.empty? && time.empty? + + "#{date} #{time}".strip + end + + def get_reply_to_topic_datetime(card_index) + combine_date_and_time(reply_to_topic_datetime_inputs[card_index * 2], reply_to_topic_datetime_inputs[(card_index * 2) + 1]) + end + + def get_required_replies_datetime(card_index) + combine_date_and_time(required_replies_datetime_inputs[card_index * 2], required_replies_datetime_inputs[(card_index * 2) + 1]) + end + + def get_available_from_datetime(card_index) + combine_date_and_time(available_from_datetime_inputs[card_index * 2], available_from_datetime_inputs[(card_index * 2) + 1]) + end + + def get_until_datetime(card_index) + combine_date_and_time(until_datetime_inputs[card_index * 2], until_datetime_inputs[(card_index * 2) + 1]) + end + + def get_all_dates_for_card(card_index) + { + reply_to_topic: get_reply_to_topic_datetime(card_index), + required_replies: get_required_replies_datetime(card_index), + available_from: get_available_from_datetime(card_index), + until: get_until_datetime(card_index) + } + end + + def get_all_dates_for_all_cards + card_count = [ + reply_to_topic_datetime_inputs.length, + required_replies_datetime_inputs.length, + available_from_datetime_inputs.length, + until_datetime_inputs.length + ].max / 2 # Divide by 2 because we have separate inputs for date and time + + (0...card_count).map do |card_index| + get_all_dates_for_card(card_index) + end + end end diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/ItemAssignToTrayContent.tsx b/ui/shared/context-modules/differentiated-modules/react/Item/ItemAssignToTrayContent.tsx index d51632bb8b8..4b35df9b7aa 100644 --- a/ui/shared/context-modules/differentiated-modules/react/Item/ItemAssignToTrayContent.tsx +++ b/ui/shared/context-modules/differentiated-modules/react/Item/ItemAssignToTrayContent.tsx @@ -52,6 +52,7 @@ import ItemAssignToCard, { } from './ItemAssignToCard' import {getOverriddenAssignees, itemTypeToApiURL} from '../../utils/assignToHelper' import {getEveryoneOption, type ItemAssignToTrayProps} from './ItemAssignToTray' +import {getDueAtForCheckpointTag} from './utils' const I18n = useI18nScope('differentiated_modules') @@ -84,6 +85,8 @@ export interface ItemAssignToTrayContentProps } const MAX_PAGES = 10 +const REPLY_TO_TOPIC = 'reply_to_topic' +const REPLY_TO_ENTRY = 'reply_to_entry' function makeCardId(): string { return uid('assign-to-card', 12) @@ -338,6 +341,19 @@ const ItemAssignToTrayContent = ({ const overriddenTargets = getOverriddenAssignees(overrides) delete dateDetailsApiResponse.overrides const baseDates: BaseDateDetails = dateDetailsApiResponse + if ( + dateDetailsApiResponse.checkpoints && + Array.isArray(dateDetailsApiResponse.checkpoints) + ) { + dateDetailsApiResponse.checkpoints.forEach((checkpoint: any) => { + if (checkpoint.tag === REPLY_TO_ENTRY) { + baseDates.required_replies_due_at = checkpoint.due_at + } else if (checkpoint.tag === REPLY_TO_TOPIC) { + baseDates.reply_to_topic_due_at = checkpoint.due_at + } + }) + } + const onlyOverrides = !dateDetailsApiResponse.visible_to_everyone const allModuleAssignees: string[] = [] const hasModuleOverride = overrides?.some(override => override.context_module_id) @@ -354,8 +370,8 @@ const ItemAssignToTrayContent = ({ isValid: true, hasAssignees: true, due_at: baseDates.due_at, - reply_to_topic_due_at: null, - required_replies_due_at: null, + reply_to_topic_due_at: baseDates.reply_to_topic_due_at, + required_replies_due_at: baseDates.required_replies_due_at, original_due_at: baseDates.due_at, unlock_at: baseDates.unlock_at, lock_at: baseDates.lock_at, @@ -412,13 +428,16 @@ const ItemAssignToTrayContent = ({ return } const cardId = makeCardId() + const reply_to_topic_due_at = getDueAtForCheckpointTag(override, REPLY_TO_TOPIC) + const required_replies_due_at = getDueAtForCheckpointTag(override, REPLY_TO_ENTRY) + cards.push({ key: cardId, isValid: true, hasAssignees: true, due_at: override.due_at, - reply_to_topic_due_at: null, - required_replies_due_at: null, + reply_to_topic_due_at, + required_replies_due_at, original_due_at: override.due_at, unlock_at: override.unlock_at, lock_at: override.lock_at, diff --git a/ui/shared/context-modules/differentiated-modules/react/Item/utils.ts b/ui/shared/context-modules/differentiated-modules/react/Item/utils.ts index 0cfce21507a..b29a15ec963 100644 --- a/ui/shared/context-modules/differentiated-modules/react/Item/utils.ts +++ b/ui/shared/context-modules/differentiated-modules/react/Item/utils.ts @@ -36,6 +36,30 @@ type UseDatesHookArgs = { cardId: string onCardDatesChange?: (cardId: string, dateAttribute: string, dateValue: string | null) => void } +type Override = { + id: string + assignment_id: string + title: string + due_at: string | null + all_day: boolean + all_day_date: string | null + unlock_at: string + lock_at: string + unassign_item: boolean + student_ids: string[] + students: Student[] + sub_assignment_due_dates: SubAssignmentDueDate[] +} + +type SubAssignmentDueDate = { + sub_assignment_tag: string + due_at: string | null +} + +type Student = { + id: string + name: string +} type UseDatesHookResult = [ // requiredRepliesDueDate @@ -442,3 +466,10 @@ export const generateCardActionLabels = (selected: string[]) => { } } } + +export const getDueAtForCheckpointTag = (override: Override, checkpointTag: String) => { + return override.sub_assignment_due_dates + ? override.sub_assignment_due_dates.find(item => item.sub_assignment_tag === checkpointTag) + ?.due_at || null + : null +}