Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more entities in which duplication is traceable #5786

Closed
wants to merge 9 commits into from
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ class Course::Assessment::Question::ProgrammingImportJob < ApplicationJob
# @param [Course::Assessment::Question::Programming] question The programming question to
# import the package to.
# @param [Attachment] attachment The attachment containing the package.
def perform_tracked(question, attachment)
def perform_tracked(question, attachment, course = nil)
question.course = course if course
ActsAsTenant.without_tenant { perform_import(question, attachment) }
end

Expand Down
4 changes: 0 additions & 4 deletions app/models/course/assessment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ class Course::Assessment < ApplicationRecord
through: :question_bundles
has_many :question_bundle_assignments, class_name: Course::Assessment::QuestionBundleAssignment.name,
inverse_of: :assessment, dependent: :destroy
has_one :duplication_traceable, class_name: DuplicationTraceable::Assessment.name,
inverse_of: :assessment, dependent: :destroy

validate :tab_in_same_course
validate :selected_test_type_for_grading
Expand Down Expand Up @@ -130,8 +128,6 @@ class Course::Assessment < ApplicationRecord
joins(lesson_plan_item: :default_reference_time)
end)

delegate :source, :source=, to: :duplication_traceable, allow_nil: true

def self.use_relative_model_naming?
true
end
Expand Down
3 changes: 3 additions & 0 deletions app/models/course/lesson_plan/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,13 @@ class Course::LessonPlan::Item < ApplicationRecord

belongs_to :course, inverse_of: :lesson_plan_items
has_many :todos, class_name: Course::LessonPlan::Todo.name, inverse_of: :item, dependent: :destroy
has_one :duplication_traceable, class_name: DuplicationTraceable::LessonPlanItem.name,
inverse_of: :lesson_plan_item, dependent: :destroy, foreign_key: :lesson_plan_item_id

delegate :start_at, :start_at=, :start_at_changed?, :bonus_end_at, :bonus_end_at=, :bonus_end_at_changed?,
:end_at, :end_at=, :end_at_changed?,
to: :default_reference_time
delegate :source, :source=, to: :duplication_traceable, allow_nil: true
before_validation :link_default_reference_time

# Returns a frozen CourseReferenceTime or CoursePersonalTime.
Expand Down
16 changes: 0 additions & 16 deletions app/models/duplication_traceable/assessment.rb

This file was deleted.

16 changes: 16 additions & 0 deletions app/models/duplication_traceable/lesson_plan_item.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true
class DuplicationTraceable::LessonPlanItem < ApplicationRecord
acts_as_duplication_traceable

validates :lesson_plan_item, presence: true
belongs_to :lesson_plan_item, class_name: Course::LessonPlan::Item.name, inverse_of: :duplication_traceable

# Class that the duplication traceable depends on.
def self.dependent_class
Course::LessonPlan::Item.name
end

def self.initialize_with_dest(dest, **options)
new(lesson_plan_item: dest, **options)
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
class RenameAndChangeAssessmentIdIntoLessonPlanItemsId < ActiveRecord::Migration[6.0]
def up
add_column :duplication_traceable_assessments, :lesson_plan_item_id, :integer

# try writing in ActiveRecord instead of SQL
# assign proper lesson plan items id to each of the assessment id
exec_query(<<-SQL)
UPDATE duplication_traceable_assessments AS dta
SET lesson_plan_item_id = clpi.lesson_plan_item_id
FROM (
SELECT dta1.id as id, clpi1.id as lesson_plan_item_id FROM (
(
SELECT id, assessment_id
FROM duplication_traceable_assessments
) AS dta1
JOIN course_lesson_plan_items AS clpi1
ON dta1.assessment_id = clpi1.actable_id
)
) AS clpi
SQL

# remove the column assessment_id since it's no longer needed at this point
remove_column :duplication_traceable_assessments, :assessment_id
rename_table :duplication_traceable_assessments, :duplication_traceable_lesson_plan_items

add_foreign_key :duplication_traceable_lesson_plan_items, :course_lesson_plan_items, column: :lesson_plan_item_id, primary_key: :id
end

def down
add_column :duplication_traceable_lesson_plan_items, :assessment_id, :integer

# assign back the assessment id to each of the rows
exec_query(<<-SQL)
UPDATE duplication_traceable_lesson_plan_items AS dtl
SET assessment_id = clpi.assessment_id
FROM (
SELECT dtl1.id as id, clpi1.actable_id AS assessment_id FROM (
(
SELECT id, lesson_plan_item_id
FROM duplication_traceable_lesson_plan_items
) as dtl1
JOIN course_lesson_plan_items AS clpi1
ON dtl1.lesson_plan_item_id = clpi1.id
)
) AS clpi
SQL
remove_column :duplication_traceable_lesson_plan_items, :lesson_plan_item_id
rename_table :duplication_traceable_lesson_plan_items, :duplication_traceable_assessments

add_foreign_key :duplication_traceable_assessments, :course_assessments, column: :assessment_id, primary_key: :id
end
end
13 changes: 6 additions & 7 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2023_01_15_054448) do
ActiveRecord::Schema.define(version: 2023_02_24_075953) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -1180,16 +1180,15 @@
t.index ["updater_id"], name: "fk__courses_updater_id"
end

create_table "duplication_traceable_assessments", force: :cascade do |t|
t.bigint "assessment_id", null: false
t.index ["assessment_id"], name: "fk__duplication_traceable_assessments_assessment_id"
end

create_table "duplication_traceable_courses", force: :cascade do |t|
t.bigint "course_id", null: false
t.index ["course_id"], name: "fk__duplication_traceable_courses_course_id"
end

create_table "duplication_traceable_lesson_plan_items", force: :cascade do |t|
t.integer "lesson_plan_item_id"
end

create_table "duplication_traceables", force: :cascade do |t|
t.string "actable_type"
t.bigint "actable_id"
Expand Down Expand Up @@ -1557,8 +1556,8 @@
add_foreign_key "courses", "instances", name: "fk_courses_instance_id"
add_foreign_key "courses", "users", column: "creator_id", name: "fk_courses_creator_id"
add_foreign_key "courses", "users", column: "updater_id", name: "fk_courses_updater_id"
add_foreign_key "duplication_traceable_assessments", "course_assessments", column: "assessment_id"
add_foreign_key "duplication_traceable_courses", "courses"
add_foreign_key "duplication_traceable_lesson_plan_items", "course_lesson_plan_items", column: "lesson_plan_item_id"
add_foreign_key "duplication_traceables", "users", column: "creator_id"
add_foreign_key "duplication_traceables", "users", column: "updater_id"
add_foreign_key "generic_announcements", "instances", name: "fk_generic_announcements_instance_id"
Expand Down
9 changes: 8 additions & 1 deletion lib/autoload/duplicator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,20 @@ def duplicate_object(source_object) # rubocop:disable Metrics/AbcSize
@duplicated_objects[key] = duplicate
duplicate.initialize_duplicate(self, key)

# Set duplication source, if it's being tracked for this class.
# Set duplication source, if it's being tracked for this class or has its actable class being tracked
if duplicate.class.method_defined?(:duplication_traceable)
traceable = duplicate.class.reflect_on_association(:duplication_traceable).options[:class_name].constantize
duplicate.duplication_traceable = traceable.initialize_with_dest(duplicate,
source_id: source_object.id,
creator: @options[:current_user],
updater: @options[:current_user])
elsif duplicate.respond_to?(:acting_as) && duplicate.acting_as.class.method_defined?(:duplication_traceable)
dup_parent = duplicate.acting_as
traceable = dup_parent.class.reflect_on_association(:duplication_traceable).options[:class_name].constantize
dup_parent.duplication_traceable = traceable.initialize_with_dest(dup_parent,
source_id: source_object.acting_as.id,
creator: @options[:current_user],
updater: @options[:current_user])
end
end
end
Expand Down
7 changes: 0 additions & 7 deletions spec/factories/duplication_traceable_assessments.rb

This file was deleted.

28 changes: 28 additions & 0 deletions spec/factories/duplication_traceable_lesson_plan_items.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true
FactoryBot.define do
factory :lesson_plan_item, class: Course::LessonPlan::Item.name do
course
base_exp { rand(1..10) * 100 }
time_bonus_exp { rand(1..10) * 100 }
start_at { 1.day.ago }
bonus_end_at { 1.day.from_now }
end_at { nil }
sequence(:title) { |n| "Example Lesson Plan Item #{n}" }
published { false }
has_personal_times { true }
affects_personal_times { true }

trait :with_bonus_end_time do
bonus_end_at { 2.days.from_now }
end

trait :with_end_time do
end_at { 3.days.from_now }
end
end

factory :duplication_traceable_lesson_plan_item, class: DuplicationTraceable::LessonPlanItem.name do
source { build(:lesson_plan_item) }
lesson_plan_item
end
end
4 changes: 0 additions & 4 deletions spec/models/course/assessment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@
it { is_expected.to have_many(:submissions).dependent(:destroy) }
it { is_expected.to have_many(:conditions) }
it { is_expected.to have_many(:assessment_conditions).dependent(:destroy) }
it { is_expected.to have_one(:duplication_traceable).dependent(:destroy) }

it { should delegate_method(:source).to(:duplication_traceable).allow_nil }
it { should delegate_method(:source=).to(:duplication_traceable).with_arguments(nil).allow_nil }

let(:instance) { Instance.default }
with_tenant(:instance) do
Expand Down
4 changes: 4 additions & 0 deletions spec/models/course/lesson_plan/lesson_plan_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
without_validating_presence
end
it { is_expected.to have_many(:todos).inverse_of(:item).dependent(:destroy) }
it { is_expected.to have_one(:duplication_traceable).dependent(:destroy) }

it { should delegate_method(:source).to(:duplication_traceable).allow_nil }
it { should delegate_method(:source=).to(:duplication_traceable).with_arguments(nil).allow_nil }

let!(:instance) { Instance.default }
with_tenant(:instance) do
Expand Down
Original file line number Diff line number Diff line change
@@ -1,48 +1,48 @@
# frozen_string_literal: true
require 'rails_helper'

RSpec.describe DuplicationTraceable::Assessment, type: :model do
RSpec.describe DuplicationTraceable::LessonPlanItem, type: :model do
it { is_expected.to act_as(DuplicationTraceable) }

let!(:instance) { Instance.default }
with_tenant(:instance) do
let(:course) { create(:course) }
let(:user) { create(:course_manager, course: course).user }
let(:assessment) { create(:assessment, :published_with_mcq_question, course: course) }
let(:lesson_plan_item) { create(:course_lesson_plan_item, course: course) }

subject do
build(:duplication_traceable_assessment, assessment: nil, source: nil)
build(:duplication_traceable_lesson_plan_item, lesson_plan_item: nil, source: nil)
end

describe 'validations' do
it 'validates the presence of a destination assessment' do
expect(subject).to_not be_valid
expect(subject.errors[:assessment]).not_to be_empty
expect(subject.errors[:lesson_plan_item]).not_to be_empty
end
end

describe '#source and #source=' do
it 'correctly reads and updates the source' do
expect(subject.source).to be(nil)
subject.source = assessment
expect(subject.source).to eq(assessment)
subject.source = lesson_plan_item
expect(subject.source).to eq(lesson_plan_item)
end
end

describe '.dependent_class' do
it 'returns Course::Assessment' do
expect(DuplicationTraceable::Assessment.dependent_class).to eq(Course::Assessment.name)
it 'returns Course::LessonPlan::Item' do
expect(DuplicationTraceable::LessonPlanItem.dependent_class).to eq(Course::LessonPlan::Item.name)
end
end

describe '.initialize_with_dest' do
it 'creates an instance with the dest initialized' do
traceable = DuplicationTraceable::Assessment.initialize_with_dest(assessment)
expect(traceable.assessment).to eq(assessment)
traceable = DuplicationTraceable::LessonPlanItem.initialize_with_dest(lesson_plan_item)
expect(traceable.lesson_plan_item).to eq(lesson_plan_item)
end

it 'passes in the other options correctly' do
traceable = DuplicationTraceable::Assessment.initialize_with_dest(assessment, creator: user)
traceable = DuplicationTraceable::LessonPlanItem.initialize_with_dest(lesson_plan_item, creator: user)
expect(traceable.creator).to eq(user)
end
end
Expand Down