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

Revert "Handle line item downstream changes in BP syncs" 

This reverts commit cbbdf5a9ce

Reason for revert: Failed post-merge building

[fsc-max-nodes=18] [fsc-timeout=40]

Change-Id: I914fbe367796a86c8d26c0b56627322353c5267f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/317927
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Luis Oliveira <luis.oliveira@instructure.com>
Product-Review: Luis Oliveira <luis.oliveira@instructure.com>
This commit is contained in:
Matheus 2023-05-10 21:19:31 -03:00 committed by Luis Oliveira
parent d136523768
commit 8320ab07c3
5 changed files with 10 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
app/models/importers/assignment_importer.rb
View File

@ -507,11 +507,6 @@ module Importers
params[:client_id] = li[:client_id] unless tool
if Account.site_admin.feature_enabled?(:blueprint_line_item_support) && primary_line_item
params = clear_params_before_overwriting_child_li(params, primary_line_item, migration)
primary_line_item.mark_as_importing! migration
end
if primary_line_item&.coupled && (li[:coupled] || !any_coupled_line_items)
# Modify the default coupled line item if:
# * We are processing a coupled line item (need to replace properties
@ -549,24 +544,5 @@ module Importers
attachment.move_to_bottom if attachment.saved_change_to_folder_id?
assignment.annotatable_attachment = attachment
end
def self.clear_params_before_overwriting_child_li(params, primary_line_item, migration)
return params unless (child_tag = migration.master_course_subscription.content_tag_for(primary_line_item.assignment))
return params unless child_tag.downstream_changes.present?
primary_line_item.class.base_class.restricted_column_settings.each do |type, columns|
changed_columns = params.keys.map(&:to_s) & columns if child_tag.downstream_changes & ["lti_line_items_#{type}"] # changed restricted types
if changed_columns.any?
if primary_line_item.assignment.child_content_restrictions[type] # don't overwrite downstream changes _unless_ it's locked
child_tag.downstream_changes -= "lti_line_items_#{type}" # remove them from the downstream changes since we're going to overwrite
child_tag.save!
else
changed_columns.each { |cc| params.delete(cc.to_sym) } # if not locked then we should ignore the params in the category (content or settings)
end
end
end
params
end
end
end

4
app/models/lti/line_item.rb
View File

@ -51,10 +51,6 @@ class Lti::LineItem < ApplicationRecord
AGS_EXT_SUBMISSION_TYPE = "#{AGS_EXT_PREFIX}submission_type"
AGS_EXT_LAUNCH_URL = "#{AGS_EXT_PREFIX}launch_url"
include MasterCourses::CollectionRestrictor
self.collection_owner_association = :assignment
restrict_columns :content, [:resource_id]
def assignment_line_item?
assignment.line_items.order(:created_at).first.id == id
end

2
app/models/master_courses/collection_restrictor.rb
View File

@ -31,7 +31,7 @@ module MasterCourses::CollectionRestrictor
klass.cattr_accessor :collection_owner_association # this is the association to find the quiz
klass.after_update :mark_downstream_changes, if: -> { klass != Lti::LineItem || Account.site_admin.feature_enabled?(:blueprint_line_item_support) }
klass.after_update :mark_downstream_changes
end
module ClassMethods

5
config/feature_flags/learning_structures_release_flags.yml
View File

@ -125,8 +125,3 @@ auto_subscribe_account_calendars:
state: allowed_on
development:
state: allowed_on
blueprint_line_item_support:
state: hidden
display_name: 'Blueprint Resource ID Support'
description: Allows blueprint courses to keep track of assignment's resource id changes.
applies_to: SiteAdmin

72
spec/models/master_courses/master_migration_spec.rb
View File

@ -169,26 +169,23 @@ describe MasterCourses::MasterMigration do
describe "Assignment's external tools migration" do
before :once do
developer_key = DeveloperKey.create!(account: Account.default)
tool = external_tool_model(context: Account.default, opts: { use_1_3: true, developer_key: developer_key })
tag = ContentTag.new(content: tool, url: "http://example.com/original", context: @original_assignment)
account_admin_user(active_all: true)
@copy_from = @course
@copy_to = course_factory
@sub = @template.add_child_course!(@copy_to)
@original_assignment = @copy_from.assignments.create!(title: "some assignment", submission_types: "external_tool", points_possible: 10)
@original_assignment.update!(external_tool_tag: tag)
@original_line_item = @original_assignment.line_items.first
@original_line_item.update!(resource_id: "some_resource_id")
end
before do
@copy_to = course_factory
@sub = @template.add_child_course!(@copy_to)
@original_assignment = @copy_from.assignments.create!(title: "some assignment", submission_types: "external_tool")
@original_assignment.build_external_tool_tag(url: "http://example.com/original", new_tab: true)
@original_assignment.save!
run_master_migration
@assignment_copy = @copy_to.assignments.where(migration_id: mig_id(@original_assignment)).first
@line_item_copy = @assignment_copy.line_items.last
end
it "copies external tool tag over" do
@assignment_copy = @copy_to.assignments.where(migration_id: mig_id(@original_assignment)).first
expect(@assignment_copy.reload.external_tool_tag).to be_truthy
expect(@assignment_copy.reload.external_tool_tag.url).to eq "http://example.com/original"
end
@ -202,63 +199,12 @@ describe MasterCourses::MasterMigration do
end
it "does not update associated course's external tool tag on blueprint update if the associated course had an independent update" do
@assignment_copy = @copy_to.assignments.where(migration_id: mig_id(@original_assignment)).first
@assignment_copy.external_tool_tag.update!(url: "http://example.com/associated_updated", new_tab: true)
@original_assignment.touch
run_master_migration
expect(@assignment_copy.reload.external_tool_tag.url).to eq "http://example.com/associated_updated"
end
context "with blueprint_line_item_support ON" do
it "respects line item downstream editing and assignment locking" do
Account.site_admin.enable_feature! :blueprint_line_item_support
@original_line_item.update!(resource_id: "updated_resource_id")
@original_assignment.update!(title: "updated assignment title")
run_master_migration
expect(@line_item_copy.reload.resource_id).to eq("updated_resource_id")
@line_item_copy.update! resource_id: "downstream_resource_id"
@original_line_item.update!(resource_id: "updated_resource_id AGAIN")
@original_assignment.update!(title: "updated assignment title AGAIN")
@original_assignment.touch
run_master_migration
# The one line item downstream change stops assignment synch as a whole
expect(@assignment_copy.reload.title).to eq("updated assignment title")
expect(@line_item_copy.reload.label).to eq("updated assignment title")
expect(@line_item_copy.reload.resource_id).to eq("downstream_resource_id")
@template.content_tag_for(@original_assignment).update_attribute(:restrictions, { content: true })
run_master_migration
expect(@assignment_copy.reload.title).to eq("updated assignment title AGAIN")
expect(@line_item_copy.reload.label).to eq("updated assignment title AGAIN")
expect(@line_item_copy.reload.resource_id).to eq("updated_resource_id AGAIN")
end
end
context "with blueprint_line_item_support OFF" do
it "respects line item downstream editing and assignment locking" do
Account.site_admin.disable_feature! :blueprint_line_item_support
expect(@original_line_item.resource_id).to eq("some_resource_id")
@original_line_item.update!(resource_id: "updated_resource_id")
@original_assignment.update!(title: "updated assignment title")
run_master_migration
expect(@line_item_copy.reload.resource_id).to eq("updated_resource_id")
@line_item_copy.update! resource_id: "downstream_resource_id"
@original_line_item.update!(resource_id: "updated_resource_id AGAIN")
@original_assignment.update!(title: "updated assignment title AGAIN")
@original_assignment.touch
run_master_migration
expect(@assignment_copy.reload.title).to eq("updated assignment title AGAIN")
expect(@line_item_copy.reload.label).to eq("updated assignment title AGAIN")
expect(@line_item_copy.reload.resource_id).to eq("updated_resource_id AGAIN")
end
end
end
describe "Course pace migration" do