From c6cddcd6a419bb353f00160c2688fe2c756f08b3 Mon Sep 17 00:00:00 2001 From: Xander Moffatt Date: Fri, 19 Apr 2024 10:44:15 -0600 Subject: [PATCH] create Lti::RegistrationAccountBindings model why: * For the LIME project, we decided to rework the current data model for storing LTI related data. The current system is quite old and has accumulated a lot of tech debt from little patches over the years, so we hope to refine and simplify the data model as much as is reasonably possible. closes INTEROP-8500 flag=none test plan: * in a rails console, get an Account `a`, a User `u`, and an Lti::Registration `lr` * create a new RAB (Lti::RegistrationAccountBinding): ``` Lti::RegistrationAccountBinding.create! account: a, lti_registration: lr, created_by: u , updated_by: u ``` * the created RAB should have a workflow_state of "off" * check out the associated MRA commit * get a cross-shard User `xu`, like `User.find("2~1")` * bonus points if that user hasn't already been associated to something on shard 1 * update the RAB's updated_by to point to that user: ``` rab.updated_by = xu rab.save! ``` * that should succeed and `rab.updated_by` should correctly reference that cross-shard user * create another RAB with a cross-shard user for both created_by and updated_by - it should succeed Change-Id: I750a725ec0c82fa08de3922c76c855b0a0618116 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/345691 Migration-Review: Cody Cutrer Tested-by: Service Cloud Jenkins Reviewed-by: Ryan Hawkins QA-Review: Ryan Hawkins Product-Review: Xander Moffatt --- app/models/account.rb | 1 + app/models/developer_key_account_binding.rb | 1 + app/models/lti/registration.rb | 1 + .../lti/registration_account_binding.rb | 38 ++++++++++++++++++ app/models/user.rb | 8 ++++ ...reate_lti_registration_account_bindings.rb | 39 +++++++++++++++++++ ...ty_to_lti_registration_account_bindings.rb | 30 ++++++++++++++ spec/models/account_spec.rb | 1 + .../lti/registration_account_binding_spec.rb | 28 +++++++++++++ spec/models/lti/registration_spec.rb | 1 + 10 files changed, 148 insertions(+) create mode 100644 app/models/lti/registration_account_binding.rb create mode 100644 db/migrate/20240417211623_create_lti_registration_account_bindings.rb create mode 100644 db/migrate/20240419173548_add_replica_identity_to_lti_registration_account_bindings.rb create mode 100644 spec/models/lti/registration_account_binding_spec.rb diff --git a/app/models/account.rb b/app/models/account.rb index 7d3f35ef8817d..72614c2d914d9 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -77,6 +77,7 @@ class Account < ActiveRecord::Base has_many :active_folders, -> { where("folder.workflow_state<>'deleted'").order("folders.name") }, class_name: "Folder", as: :context, inverse_of: :context has_many :developer_keys has_many :developer_key_account_bindings, inverse_of: :account, dependent: :destroy + has_many :lti_registration_account_bindings, class_name: "Lti::RegistrationAccountBinding", inverse_of: :account, dependent: :destroy has_many :authentication_providers, -> { ordered }, inverse_of: :account, diff --git a/app/models/developer_key_account_binding.rb b/app/models/developer_key_account_binding.rb index b730dd05a3e67..91305d07f7703 100644 --- a/app/models/developer_key_account_binding.rb +++ b/app/models/developer_key_account_binding.rb @@ -29,6 +29,7 @@ class DeveloperKeyAccountBinding < ApplicationRecord belongs_to :account belongs_to :developer_key belongs_to :root_account, class_name: "Account" + has_one :lti_registration_account_binding, class_name: "Lti::RegistrationAccountBinding", inverse_of: :developer_key_account_binding validates :account, :developer_key, presence: true diff --git a/app/models/lti/registration.rb b/app/models/lti/registration.rb index 6dbc82e5a8ce9..b12cca45f8de3 100644 --- a/app/models/lti/registration.rb +++ b/app/models/lti/registration.rb @@ -26,6 +26,7 @@ class Lti::Registration < ActiveRecord::Base belongs_to :updated_by, class_name: "User", inverse_of: :updated_lti_registrations, optional: false has_one :ims_registration, class_name: "Lti::IMS::Registration", inverse_of: :lti_registration, foreign_key: :lti_registration_id has_one :developer_key, inverse_of: :lti_registration, foreign_key: :lti_registration_id + has_many :lti_registration_account_bindings, class_name: "Lti::RegistrationAccountBinding", dependent: :destroy, inverse_of: :registration validates :name, :admin_nickname, :vendor, length: { maximum: 255 } validates :name, presence: true diff --git a/app/models/lti/registration_account_binding.rb b/app/models/lti/registration_account_binding.rb new file mode 100644 index 0000000000000..3fe4d215fab44 --- /dev/null +++ b/app/models/lti/registration_account_binding.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# +# Copyright (C) 2024 - 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 . + +class Lti::RegistrationAccountBinding < ActiveRecord::Base + extend RootAccountResolver + + include Workflow + workflow do + state :off + state :on + state :allow + end + + belongs_to :account, inverse_of: :lti_registration_account_bindings, optional: false + belongs_to :registration, class_name: "Lti::Registration", inverse_of: :lti_registration_account_bindings, optional: false + belongs_to :root_account, class_name: "Account" + belongs_to :created_by, class_name: "User", inverse_of: :created_lti_registration_account_bindings + belongs_to :updated_by, class_name: "User", inverse_of: :updated_lti_registration_account_bindings + belongs_to :developer_key_account_binding, inverse_of: :lti_registration_account_binding + + resolves_root_account through: :account +end diff --git a/app/models/user.rb b/app/models/user.rb index 9debee407aa7f..11ad2c65954d2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -250,6 +250,14 @@ def self.enrollment_conditions(state) inverse_of: :user has_many :created_lti_registrations, class_name: "Lti::Registration", foreign_key: "created_by_id", inverse_of: :created_by has_many :updated_lti_registrations, class_name: "Lti::Registration", foreign_key: "updated_by_id", inverse_of: :updated_by + has_many :created_lti_registration_account_bindings, + class_name: "Lti::RegistrationAccountBinding", + foreign_key: :created_by_id, + inverse_of: :created_by + has_many :updated_lti_registration_account_bindings, + class_name: "Lti::RegistrationAccountBinding", + foreign_key: :updated_by_id, + inverse_of: :updated_by has_many :comment_bank_items, -> { where("workflow_state<>'deleted'") } has_many :microsoft_sync_partial_sync_changes, class_name: "MicrosoftSync::PartialSyncChange", dependent: :destroy, inverse_of: :user diff --git a/db/migrate/20240417211623_create_lti_registration_account_bindings.rb b/db/migrate/20240417211623_create_lti_registration_account_bindings.rb new file mode 100644 index 0000000000000..804734d4ab336 --- /dev/null +++ b/db/migrate/20240417211623_create_lti_registration_account_bindings.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# +# Copyright (C) 2024 - 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 . + +class CreateLtiRegistrationAccountBindings < ActiveRecord::Migration[7.0] + tag :predeploy + + def change + create_table :lti_registration_account_bindings do |t| + t.belongs_to :registration, null: false, foreign_key: { to_table: :lti_registrations } + t.belongs_to :account, null: false, foreign_key: true + t.belongs_to :created_by, foreign_key: { to_table: :users } + t.belongs_to :updated_by, foreign_key: { to_table: :users } + t.string :workflow_state, null: false, default: "off" + + t.references :root_account, foreign_key: { to_table: :accounts }, index: false, null: false + t.references :developer_key_account_binding, foreign_key: true, index: { name: "index_lrab_on_developer_key_account_binding_id" } + t.replica_identity_index + t.timestamps + + t.index %i[account_id registration_id], name: "index_lti_reg_bindings_on_account_id_and_registration_id", unique: true + end + end +end diff --git a/db/migrate/20240419173548_add_replica_identity_to_lti_registration_account_bindings.rb b/db/migrate/20240419173548_add_replica_identity_to_lti_registration_account_bindings.rb new file mode 100644 index 0000000000000..4851611691822 --- /dev/null +++ b/db/migrate/20240419173548_add_replica_identity_to_lti_registration_account_bindings.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# +# Copyright (C) 2024 - 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 . + +class AddReplicaIdentityToLtiRegistrationAccountBindings < ActiveRecord::Migration[7.0] + tag :predeploy + + def up + set_replica_identity :lti_registration_account_bindings + end + + def down + set_replica_identity :lti_registration_account_bindings, :default + end +end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 3d50267f0675d..80cb2fa2a7fed 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -26,6 +26,7 @@ it { is_expected.to have_one(:outcome_proficiency).dependent(:destroy) } it { is_expected.to have_many(:lti_resource_links).class_name("Lti::ResourceLink") } it { is_expected.to have_many(:lti_registrations).class_name("Lti::Registration").dependent(:destroy) } + it { is_expected.to have_many(:lti_registration_account_bindings).class_name("Lti::RegistrationAccountBinding").dependent(:destroy) } end describe "validations" do diff --git a/spec/models/lti/registration_account_binding_spec.rb b/spec/models/lti/registration_account_binding_spec.rb new file mode 100644 index 0000000000000..3317fe81d5a0c --- /dev/null +++ b/spec/models/lti/registration_account_binding_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# +# Copyright (C) 2024 - 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 . + +RSpec.describe Lti::RegistrationAccountBinding do + describe "validations" do + it { is_expected.to belong_to(:registration).class_name("Lti::Registration").optional(false).inverse_of(:lti_registration_account_bindings) } + it { is_expected.to belong_to(:account).class_name("Account").optional(false).inverse_of(:lti_registration_account_bindings) } + it { is_expected.to belong_to(:created_by).class_name("User").inverse_of(:created_lti_registration_account_bindings) } + it { is_expected.to belong_to(:updated_by).class_name("User").inverse_of(:updated_lti_registration_account_bindings) } + it { is_expected.to belong_to(:developer_key_account_binding).class_name("DeveloperKeyAccountBinding") } + end +end diff --git a/spec/models/lti/registration_spec.rb b/spec/models/lti/registration_spec.rb index 7e02c65329e7b..3bbd202dfd015 100644 --- a/spec/models/lti/registration_spec.rb +++ b/spec/models/lti/registration_spec.rb @@ -27,5 +27,6 @@ it { is_expected.to have_one(:developer_key).class_name("DeveloperKey").inverse_of(:lti_registration).with_foreign_key(:lti_registration_id) } it { is_expected.to belong_to(:created_by).class_name("User").optional(false) } it { is_expected.to belong_to(:updated_by).class_name("User").optional(false) } + it { is_expected.to have_many(:lti_registration_account_bindings).class_name("Lti::RegistrationAccountBinding").dependent(:destroy) } end end