add outcome calculation method model

closes OUT-3759
flag=account_level_mastery_scales

Test plan:
- run migrations
- in the canvas ui or rails console, create subaccounts
- in the rails console, add outcome calculation
  methods to the root account and some of the subaccounts:
  > ap Account.all.pluck(:id, :name)
  > OutcomeCalculationMethod.create! context: Account.find(id),
      calculation_method: :latest, calculation_int: nil
- verify that calculation methods are found through
  `account.resolved_outcome_calculation_method`

Change-Id: Iba4b8db2e1c78b7f7c5964eec270ed75c4f94d66
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/242745
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Reviewed-by: Augusto Callejas <acallejas@instructure.com>
QA-Review: Augusto Callejas <acallejas@instructure.com>
Product-Review: Augusto Callejas <acallejas@instructure.com>
This commit is contained in:
Michael Brewer-Davis 2020-07-14 10:05:40 -05:00 committed by Michael Brewer-Davis
parent e773d4e05f
commit cd9eb4fab7
10 changed files with 282 additions and 29 deletions

View File

@ -80,6 +80,7 @@ class Account < ActiveRecord::Base
has_many :content_migrations, :as => :context, :inverse_of => :context
has_many :sis_batch_errors, foreign_key: :root_account_id, inverse_of: :root_account
has_one :outcome_proficiency, dependent: :destroy
has_one :outcome_calculation_method, as: :context, inverse_of: :context, dependent: :destroy
has_many :auditor_authentication_records,
class_name: "Auditors::ActiveRecord::AuthenticationRecord",
@ -185,6 +186,10 @@ class Account < ActiveRecord::Base
outcome_proficiency || parent_account&.resolved_outcome_proficiency
end
def resolved_outcome_calculation_method
outcome_calculation_method || parent_account&.resolved_outcome_calculation_method
end
include ::Account::Settings
include ::Csp::AccountHelper

View File

@ -216,7 +216,7 @@ class Course < ActiveRecord::Base
dependent: :destroy
has_many :conditional_release_rules, inverse_of: :course, class_name: "ConditionalRelease::Rule", dependent: :destroy
has_one :outcome_calculation_method, as: :context, inverse_of: :context, dependent: :destroy
prepend Profile::Association
@ -3553,6 +3553,10 @@ class Course < ActiveRecord::Base
end
end
def resolved_outcome_calculation_method
outcome_calculation_method || account&.resolved_outcome_calculation_method
end
private
def effective_due_dates

View File

@ -33,28 +33,18 @@ class LearningOutcome < ActiveRecord::Base
before_save :infer_root_account_ids
after_save :propagate_changes_to_rubrics
CALCULATION_METHODS = {
'decaying_average' => "Decaying Average",
'n_mastery' => "n Number of Times",
'highest' => "Highest Score",
'latest' => "Most Recent Score",
}.freeze
VALID_CALCULATION_INTS = {
"decaying_average" => (1..99),
"n_mastery" => (1..5),
"highest" => [].freeze,
"latest" => [].freeze,
}.freeze
validates :description, length: { maximum: maximum_text_length, allow_nil: true, allow_blank: true }
validates :short_description, length: { maximum: maximum_string_length }
validates :vendor_guid, length: { maximum: maximum_string_length, allow_nil: true }
validates :display_name, length: { maximum: maximum_string_length, allow_nil: true, allow_blank: true }
validates :calculation_method, inclusion: { in: CALCULATION_METHODS.keys,
message: -> { t(
"calculation_method must be one of the following: %{calc_methods}",
:calc_methods => CALCULATION_METHODS.keys.to_s
) }
validates :calculation_method, inclusion: {
in: OutcomeCalculationMethod::CALCULATION_METHODS,
message: -> {
t(
"calculation_method must be one of the following: %{calc_methods}",
:calc_methods => OutcomeCalculationMethod::CALCULATION_METHODS.to_s
)
}
}
validates :short_description, :workflow_state, presence: true
sanitize_field :description, CanvasSanitize::SANITIZE
@ -138,11 +128,11 @@ class LearningOutcome < ActiveRecord::Base
end
def self.valid_calculation_method?(method)
CALCULATION_METHODS.keys.include?(method)
OutcomeCalculationMethod::CALCULATION_METHODS.include?(method)
end
def self.valid_calculation_ints(method)
VALID_CALCULATION_INTS[method]
OutcomeCalculationMethod::VALID_CALCULATION_INTS[method]
end
def self.valid_calculation_int?(int, method)

View File

@ -0,0 +1,55 @@
#
# Copyright (C) 2020 - 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 OutcomeCalculationMethod < ApplicationRecord
include Canvas::SoftDeletable
extend RootAccountResolver
CALCULATION_METHODS = [
'decaying_average',
'n_mastery',
'highest',
'latest'
].freeze
VALID_CALCULATION_INTS = {
"decaying_average" => (1..99),
"n_mastery" => (1..5),
"highest" => [].freeze,
"latest" => [].freeze,
}.freeze
belongs_to :context, polymorphic: [:account, :course], required: true
resolves_root_account through: :context
validates :context, presence: true
validates :context_id, uniqueness: { scope: :context_type }
validates :calculation_method, inclusion: {
in: CALCULATION_METHODS,
message: "calculation_method must be one of #{CALCULATION_METHODS}"
}
validates :calculation_int, inclusion: {
in: ->(model) {
VALID_CALCULATION_INTS[model.calculation_method].presence || [nil] # if valid ints == [], value must be nil
},
if: ->(model) {
CALCULATION_METHODS.include?(model.calculation_method)
},
message: "invalid calculation_int for this calculation_method"
}
end

View File

@ -17,10 +17,12 @@
#
class OutcomeProficiency < ApplicationRecord
extend RootAccountResolver
has_many :outcome_proficiency_ratings, -> { order 'points DESC, id ASC' },
dependent: :destroy, inverse_of: :outcome_proficiency, autosave: true
belongs_to :account, inverse_of: :outcome_proficiency
belongs_to :root_account, class_name: 'Account', inverse_of: :outcome_proficiency
belongs_to :root_account, class_name: 'Account'
validates :account, uniqueness: true, presence: true
validates :outcome_proficiency_ratings, presence: { message: t('Missing required ratings') }
@ -28,7 +30,8 @@ class OutcomeProficiency < ApplicationRecord
validate :strictly_decreasing_points
validates :context_id, :context_type, presence: true
before_validation :ensure_context
before_save :set_root_account_id
resolves_root_account through: :account
def as_json(_options={})
{
@ -57,11 +60,6 @@ class OutcomeProficiency < ApplicationRecord
end
end
def set_root_account_id
return if self.root_account_id.present?
self.root_account_id = self.account.resolved_root_account_id
end
# TODO: get rid of once all existing proficiencies have their context populated and creation methods can be rewritten
def ensure_context
return if self.context_type.present? && self.context_id.present?

View File

@ -0,0 +1,35 @@
#
# Copyright (C) 2020 - 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 CreateOutcomeCalculationMethods < ActiveRecord::Migration[5.2]
tag :predeploy
def change
create_table :outcome_calculation_methods do |t|
t.string :context_type, null: false, limit: 255
t.integer :context_id, null: false, limit: 8
t.integer :calculation_int, limit: 2
t.string :calculation_method, null: false, limit: 255
t.string :workflow_state, null: false, default: 'active'
t.references :root_account, index: true, foreign_key: { to_table: :accounts }
t.timestamps null: false
end
add_index :outcome_calculation_methods, [:context_type, :context_id], unique: true, name: 'index_outcome_calculation_methods_on_context'
end
end

View File

@ -166,7 +166,7 @@ describe Outcomes::CsvImporter do
by_method = LearningOutcome.all.to_a.group_by(&:calculation_method)
methods = LearningOutcome::CALCULATION_METHODS.keys.sort
methods = OutcomeCalculationMethod::CALCULATION_METHODS.sort
expect(by_method.keys.sort).to eq(methods)
expect(by_method['decaying_average'].map(&:calculation_int)).to include(40)

View File

@ -33,6 +33,38 @@ describe Account do
expect(subaccount.resolved_outcome_proficiency).to eq proficiency
end
context 'resolved_outcome_calculation_method' do
it "retrieves parent account's outcome calculation method" do
root_account = Account.create!
method = OutcomeCalculationMethod.create! context: root_account, calculation_method: :highest
subaccount = root_account.sub_accounts.create!
expect(root_account.outcome_calculation_method).to eq method
expect(subaccount.outcome_calculation_method).to eq nil
expect(root_account.resolved_outcome_calculation_method).to eq method
expect(subaccount.resolved_outcome_calculation_method).to eq method
end
it "can override parent account's outcome calculation method" do
root_account = Account.create!
method = OutcomeCalculationMethod.create! context: root_account, calculation_method: :highest
subaccount = root_account.sub_accounts.create!
submethod = OutcomeCalculationMethod.create! context: subaccount, calculation_method: :latest
expect(root_account.outcome_calculation_method).to eq method
expect(subaccount.outcome_calculation_method).to eq submethod
expect(root_account.resolved_outcome_calculation_method).to eq method
expect(subaccount.resolved_outcome_calculation_method).to eq submethod
end
it "can be nil" do
root_account = Account.create!
subaccount = root_account.sub_accounts.create!
expect(root_account.outcome_calculation_method).to eq nil
expect(subaccount.outcome_calculation_method).to eq nil
expect(root_account.resolved_outcome_calculation_method).to eq nil
expect(subaccount.resolved_outcome_calculation_method).to eq nil
end
end
it "should provide a list of courses" do
expect{ Account.new.courses }.not_to raise_error
end

View File

@ -1566,6 +1566,41 @@ describe Course do
expect(@course.course_section_visibility(worthless_loser)).to eq []
end
end
context 'resolved_outcome_calculation_method' do
it "retrieves account's outcome calculation method" do
root_account = Account.create!
method = OutcomeCalculationMethod.create! context: root_account, calculation_method: :highest
course = course_model(account: root_account)
expect(course.outcome_calculation_method).to eq nil
expect(course.resolved_outcome_calculation_method).to eq method
end
it "can retrieve ancestor account's outcome calculation method" do
root_account = Account.create!
subaccount = root_account.sub_accounts.create!
method = OutcomeCalculationMethod.create! context: root_account, calculation_method: :highest
course = course_model(account: subaccount)
expect(course.outcome_calculation_method).to eq nil
expect(course.resolved_outcome_calculation_method).to eq method
end
it "can retrieve own outcome calculation method" do
root_account = Account.create!
OutcomeCalculationMethod.create! context: root_account, calculation_method: :highest
course = course_model(account: root_account)
course_method = OutcomeCalculationMethod.create! context: course, calculation_method: :latest
expect(course.outcome_calculation_method).to eq course_method
expect(course.resolved_outcome_calculation_method).to eq course_method
end
it "can be nil" do
root_account = Account.create!
course = course_model(account: root_account)
expect(course.outcome_calculation_method).to eq nil
expect(course.resolved_outcome_calculation_method).to eq nil
end
end
end

View File

@ -0,0 +1,99 @@
#
# Copyright (C) 2020 - 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/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
describe OutcomeCalculationMethod, type: :model do
subject { OutcomeCalculationMethod.create!(creation_params) }
let_once(:account) { account_model }
let(:calculation_method) { 'latest' }
let(:calculation_int) { nil }
let(:creation_params) { { context: account, calculation_method: calculation_method, calculation_int: calculation_int } }
describe 'validations' do
it { is_expected.to validate_presence_of :context }
it { is_expected.to validate_uniqueness_of(:context_id).scoped_to(:context_type) }
it { is_expected.to validate_inclusion_of(:calculation_method).in_array(OutcomeCalculationMethod::CALCULATION_METHODS) }
context 'calculation_int' do
context 'decaying_average' do
let(:calculation_method) { 'decaying_average' }
let(:calculation_int) { 3 }
it do
is_expected.to allow_values(
1,
20,
99
).for(:calculation_int)
end
it do
is_expected.not_to allow_values(
-1,
0,
100,
1000,
nil
).for(:calculation_int)
end
end
context 'n_mastery' do
let(:calculation_method) { 'n_mastery' }
let(:calculation_int) { 3 }
it do
is_expected.to allow_values(
1,
4,
5
).for(:calculation_int)
end
it do
is_expected.not_to allow_values(
-1,
0,
6,
10,
nil
).for(:calculation_int)
end
end
context 'highest' do
let(:calculation_method) { 'highest' }
it { is_expected.to allow_value(nil).for(:calculation_int) }
it { is_expected.not_to allow_values(1, 10, 100).for(:calculation_int) }
end
context 'latest' do
it { is_expected.to allow_value(nil).for(:calculation_int) }
it { is_expected.not_to allow_values(1, 10, 100).for(:calculation_int) }
end
end
end
it_behaves_like "soft deletion" do
subject { OutcomeCalculationMethod }
let(:creation_arguments) { [creation_params, creation_params.merge(context: course_model)] }
end
end