diff --git a/app/controllers/course/assessment/marketplace_listings_controller.rb b/app/controllers/course/assessment/marketplace_listings_controller.rb index 50e4a5e200..cdd1ca3fe0 100644 --- a/app/controllers/course/assessment/marketplace_listings_controller.rb +++ b/app/controllers/course/assessment/marketplace_listings_controller.rb @@ -3,17 +3,10 @@ class Course::Assessment::MarketplaceListingsController < Course::Assessment::Co before_action :authorize_publish_to_marketplace! def create - listing = Course::Assessment::Marketplace::Listing.find_or_initialize_by(assessment: @assessment) - now = Time.zone.now - listing.published = true - listing.first_published_at ||= now - listing.last_published_at = now - listing.publisher ||= current_user - if listing.save - render json: { published: true }, status: :ok - else - render json: { errors: listing.errors.full_messages }, status: :unprocessable_content - end + listing = Course::Assessment::Marketplace::PublishService.publish(@assessment, current_user) + render json: { published: listing.published }, status: :ok + rescue ActiveRecord::RecordInvalid => e + render json: { errors: e.record.errors.full_messages }, status: :unprocessable_content end def destroy diff --git a/app/models/course/assessment/marketplace.rb b/app/models/course/assessment/marketplace.rb index 235cbc69e9..35029b6dc1 100644 --- a/app/models/course/assessment/marketplace.rb +++ b/app/models/course/assessment/marketplace.rb @@ -1,6 +1,39 @@ # frozen_string_literal: true module Course::Assessment::Marketplace + CONTAINER_TITLE = 'Marketplace Version Store' + def self.table_name_prefix 'course_assessment_marketplace_' end + + # The single, hidden, system-wide container Course that stores every published + # version snapshot. Located via a settings pointer on the default instance (no + # `courses` column, per design V4). Find-or-heals: (re)creates and re-persists + # the pointer if it is unset or references a missing course. Runs without a + # tenant because callers may be scoped to any instance. + # + # @return [Course] + def self.container + ActsAsTenant.without_tenant do + instance = Instance.default + id = instance.settings(:marketplace).container_course_id + (id && Course.find_by(id: id)) || create_container!(instance) + end + end + + # @return [Course] + def self.create_container!(instance) + ActsAsTenant.with_tenant(instance) do + User.with_stamper(User.system) do + course = Course.create!(instance: instance, title: CONTAINER_TITLE, + description: 'System store for marketplace version snapshots.', + published: false, gamified: false, enrollable: false, + creator: User.system, updater: User.system) + instance.settings(:marketplace).container_course_id = course.id + instance.save! + course + end + end + end + private_class_method :create_container! end diff --git a/app/models/course/assessment/marketplace/listing.rb b/app/models/course/assessment/marketplace/listing.rb index 0722ac6492..383fc4b62b 100644 --- a/app/models/course/assessment/marketplace/listing.rb +++ b/app/models/course/assessment/marketplace/listing.rb @@ -2,8 +2,14 @@ class Course::Assessment::Marketplace::Listing < ApplicationRecord belongs_to :assessment, class_name: 'Course::Assessment', inverse_of: :marketplace_listing belongs_to :publisher, class_name: 'User', inverse_of: false + belongs_to :current_version, class_name: 'Course::Assessment::Marketplace::ListingVersion', + inverse_of: false, optional: true + belongs_to :source_course, class_name: 'Course', inverse_of: false, optional: true + belongs_to :fallback_maintainer, class_name: 'User', inverse_of: false, optional: true has_many :adoptions, class_name: 'Course::Assessment::Marketplace::Adoption', inverse_of: :listing, dependent: :destroy + has_many :versions, class_name: 'Course::Assessment::Marketplace::ListingVersion', + inverse_of: :listing, dependent: :destroy validates :assessment_id, uniqueness: true validates :publisher, presence: true diff --git a/app/models/course/assessment/marketplace/listing_version.rb b/app/models/course/assessment/marketplace/listing_version.rb new file mode 100644 index 0000000000..ffabdb2398 --- /dev/null +++ b/app/models/course/assessment/marketplace/listing_version.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true +class Course::Assessment::Marketplace::ListingVersion < ApplicationRecord + belongs_to :listing, class_name: 'Course::Assessment::Marketplace::Listing', + inverse_of: :versions + belongs_to :assessment, class_name: 'Course::Assessment', inverse_of: false + belongs_to :published_by, class_name: 'User', inverse_of: false + + validates :version, presence: true, + numericality: { only_integer: true, greater_than: 0 }, + uniqueness: { scope: :listing_id } + validates :assessment, presence: true + validates :published_by, presence: true + validates :creator, presence: true + validates :updater, presence: true + + scope :ordered, -> { order(version: :asc) } +end diff --git a/app/services/course/assessment/marketplace/publish_service.rb b/app/services/course/assessment/marketplace/publish_service.rb new file mode 100644 index 0000000000..dd696ecfb7 --- /dev/null +++ b/app/services/course/assessment/marketplace/publish_service.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +# Publishes an assessment to the marketplace and cuts version 1 (copy-on-publish, +# design V2/§5.1): (re)activate the listing, capture provenance, snapshot the source +# assessment into the hidden container course, and point `current_version` at the +# snapshot. This slice cuts ONLY the first version — deliberate "publish new version" +# is Slice 4. No rename, no read repoint here. +class Course::Assessment::Marketplace::PublishService + # @param [Course::Assessment] assessment the source assessment being published + # @param [User] publisher the user triggering the publish + # @return [Course::Assessment::Marketplace::Listing] + def self.publish(assessment, publisher) + new(assessment, publisher).publish + end + + # Idempotent single-listing version cut, reused by the backfill. No-op (returns the + # existing version) if the listing already has one. + # @param [Course::Assessment::Marketplace::Listing] listing an already-published listing + # @param [User] publisher + # @return [Course::Assessment::Marketplace::ListingVersion] + def self.ensure_first_version!(listing, publisher) + new(listing.assessment, publisher).ensure_first_version!(listing) + end + + # One-time backfill: version every published, version-less listing and stamp + # `adopted_version = 1` on its version-less adoptions. Idempotent. + # @return [void] + def self.backfill_all! + ActsAsTenant.without_tenant do + # Never-versioned published listings only. Keying idempotency on the absence of + # any version (not just a nil `current_version_id`) makes reruns safe and avoids + # re-cutting v1 for a listing that already has one — in production the two are + # equivalent, since a version is only ever created together with `current_version`. + Course::Assessment::Marketplace::Listing.published.where.missing(:versions).find_each do |listing| + ensure_first_version!(listing, listing.publisher) + listing.adoptions.where(adopted_version: nil).update_all(adopted_version: 1) + end + end + nil + end + + def initialize(assessment, publisher) + @assessment = assessment + @publisher = publisher + end + + # @return [Course::Assessment::Marketplace::Listing] + def publish + with_publish_context do + listing = activate_listing + cut_first_version!(listing) if listing.current_version_id.nil? + listing + end + end + + # @return [Course::Assessment::Marketplace::ListingVersion] + def ensure_first_version!(listing) + return listing.current_version if listing.current_version_id + + with_publish_context do + capture_provenance(listing) + listing.save! + cut_first_version!(listing) + end + listing.current_version + end + + private + + # Runs the publish body without a tenant (the container lives in the default + # instance; callers may be scoped to any instance) and with the stamper set so + # nested creator/updater resolve on the listing, version, and snapshot copy. + def with_publish_context(&block) + ActsAsTenant.without_tenant do + User.with_stamper(@publisher) do + Course::Assessment::Marketplace::Listing.transaction(&block) + end + end + end + + # @return [Course::Assessment::Marketplace::Listing] + def activate_listing + listing = Course::Assessment::Marketplace::Listing.find_or_initialize_by(assessment: @assessment) + now = Time.zone.now + listing.published = true + listing.first_published_at ||= now + listing.last_published_at = now + listing.publisher ||= @publisher + capture_provenance(listing) + listing.save! + listing + end + + # Denormalized so the identity survives origin-course deletion (design §3.2). + # Coursemology's Course models only a title (no course-code / academic-period + # concept), so `source_course_code` / `source_academic_period` are reserved-nil. + def capture_provenance(listing) + course = @assessment.course + listing.source_course ||= course + listing.source_course_name ||= course.title + listing.fallback_maintainer ||= course.course_users.find_by(role: :owner)&.user + end + + def cut_first_version!(listing) + snapshot = snapshot_into_container(listing.assessment) + version = listing.versions.create!(version: 1, assessment: snapshot, published_by: @publisher, + creator: @publisher, updater: @publisher) + listing.update!(current_version: version) + version + end + + # @return [Course::Assessment] the immutable snapshot living in the container course + def snapshot_into_container(assessment) + container = Course::Assessment::Marketplace.container + copy = Course::Duplication::ObjectDuplicationService.duplicate_objects( + assessment.course, container, assessment, current_user: @publisher + ) + reparent_into_container_tab(copy, container) + copy + end + + def reparent_into_container_tab(copy, container) + tab = container.assessment_categories.first.tabs.first + return if copy.tab_id == tab.id + + copy.tab = tab + copy.folder.parent = tab.category.folder + copy.save! + end +end diff --git a/db/migrate/20260722000001_add_marketplace_versioning.rb b/db/migrate/20260722000001_add_marketplace_versioning.rb new file mode 100644 index 0000000000..7b70263849 --- /dev/null +++ b/db/migrate/20260722000001_add_marketplace_versioning.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true +class AddMarketplaceVersioning < ActiveRecord::Migration[7.2] + def change + create_table :course_assessment_marketplace_listing_versions do |t| + t.references :listing, null: false, + foreign_key: { to_table: :course_assessment_marketplace_listings, + name: 'fk_camlv_listing_id', + on_delete: :cascade }, + index: { name: 'fk__camlv_listing_id' } + t.integer :version, null: false + t.references :assessment, null: false, + foreign_key: { to_table: :course_assessments, + name: 'fk_camlv_assessment_id' }, + index: { name: 'fk__camlv_assessment_id' } + t.references :published_by, null: false, + foreign_key: { to_table: :users, name: 'fk_camlv_published_by' }, + index: { name: 'fk__camlv_published_by' } + t.references :creator, null: false, + foreign_key: { to_table: :users, name: 'fk_camlv_creator_id' }, + index: { name: 'fk__camlv_creator_id' } + t.references :updater, null: false, + foreign_key: { to_table: :users, name: 'fk_camlv_updater_id' }, + index: { name: 'fk__camlv_updater_id' } + t.timestamps null: false + end + add_index :course_assessment_marketplace_listing_versions, [:listing_id, :version], + unique: true, name: 'index_camlv_on_listing_id_and_version' + + change_table :course_assessment_marketplace_listings, bulk: true do |t| + t.references :current_version, null: true, + foreign_key: { to_table: :course_assessment_marketplace_listing_versions, + name: 'fk_caml_current_version_id', + on_delete: :nullify }, + index: { name: 'fk__caml_current_version_id' } + t.references :source_course, null: true, + foreign_key: { to_table: :courses, + name: 'fk_caml_source_course_id', + on_delete: :nullify }, + index: { name: 'fk__caml_source_course_id' } + t.string :source_course_name + t.string :source_course_code + t.string :source_academic_period + t.references :fallback_maintainer, null: true, + foreign_key: { to_table: :users, + name: 'fk_caml_fallback_maintainer_id' }, + index: { name: 'fk__caml_fallback_maintainer_id' } + end + + change_table :course_assessment_marketplace_adoptions, bulk: true do |t| + t.integer :adopted_version + t.integer :dismissed_version + t.boolean :reminders_muted, null: false, default: false + end + end +end diff --git a/db/migrate/20260722000002_backfill_marketplace_versions.rb b/db/migrate/20260722000002_backfill_marketplace_versions.rb new file mode 100644 index 0000000000..a5faac83a5 --- /dev/null +++ b/db/migrate/20260722000002_backfill_marketplace_versions.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true +class BackfillMarketplaceVersions < ActiveRecord::Migration[7.2] + # Data-only. Versions every existing published listing as v1 (snapshotting into + # the container via the publish service) and stamps adopted_version = 1 on its + # adoptions. Idempotent — reruns skip already-versioned listings. + def up + Course::Assessment::Marketplace::PublishService.backfill_all! + end + + def down + # Irreversible: version snapshots are retained. + end +end diff --git a/db/schema.rb b/db/schema.rb index 0d63cf08be..d0f43982e5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2026_07_20_154800) do +ActiveRecord::Schema[7.2].define(version: 2026_07_22_000002) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -287,6 +287,9 @@ t.bigint "updater_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "adopted_version" + t.integer "dismissed_version" + t.boolean "reminders_muted", default: false, null: false t.index ["creator_id"], name: "fk__cama_creator_id" t.index ["destination_course_id"], name: "fk__cama_destination_course_id" t.index ["duplicated_assessment_id"], name: "fk__cama_duplicated_assessment_id", unique: true @@ -311,6 +314,23 @@ t.index ["user_id"], name: "index_marketplace_allowlist_rules_one_per_user", unique: true, where: "(rule_type = 0)" end + create_table "course_assessment_marketplace_listing_versions", force: :cascade do |t| + t.bigint "listing_id", null: false + t.integer "version", null: false + t.bigint "assessment_id", null: false + t.bigint "published_by_id", null: false + t.bigint "creator_id", null: false + t.bigint "updater_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["assessment_id"], name: "fk__camlv_assessment_id" + t.index ["creator_id"], name: "fk__camlv_creator_id" + t.index ["listing_id", "version"], name: "index_camlv_on_listing_id_and_version", unique: true + t.index ["listing_id"], name: "fk__camlv_listing_id" + t.index ["published_by_id"], name: "fk__camlv_published_by" + t.index ["updater_id"], name: "fk__camlv_updater_id" + end + create_table "course_assessment_marketplace_listings", force: :cascade do |t| t.bigint "assessment_id", null: false t.boolean "published", default: false, null: false @@ -321,10 +341,19 @@ t.bigint "updater_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.bigint "current_version_id" + t.bigint "source_course_id" + t.string "source_course_name" + t.string "source_course_code" + t.string "source_academic_period" + t.bigint "fallback_maintainer_id" t.index ["assessment_id"], name: "fk__course_assessment_marketplace_listings_assessment_id", unique: true t.index ["creator_id"], name: "fk__course_assessment_marketplace_listings_creator_id" + t.index ["current_version_id"], name: "fk__caml_current_version_id" + t.index ["fallback_maintainer_id"], name: "fk__caml_fallback_maintainer_id" t.index ["published"], name: "index_course_assessment_marketplace_listings_on_published" t.index ["publisher_id"], name: "fk__course_assessment_marketplace_listings_publisher_id" + t.index ["source_course_id"], name: "fk__caml_source_course_id" t.index ["updater_id"], name: "fk__course_assessment_marketplace_listings_updater_id" end @@ -2012,8 +2041,16 @@ add_foreign_key "course_assessment_marketplace_adoptions", "users", column: "updater_id", name: "fk_course_assessment_marketplace_adoptions_updater_id" add_foreign_key "course_assessment_marketplace_allowlist_rules", "instances" add_foreign_key "course_assessment_marketplace_allowlist_rules", "users" + add_foreign_key "course_assessment_marketplace_listing_versions", "course_assessment_marketplace_listings", column: "listing_id", name: "fk_camlv_listing_id", on_delete: :cascade + add_foreign_key "course_assessment_marketplace_listing_versions", "course_assessments", column: "assessment_id", name: "fk_camlv_assessment_id" + add_foreign_key "course_assessment_marketplace_listing_versions", "users", column: "creator_id", name: "fk_camlv_creator_id" + add_foreign_key "course_assessment_marketplace_listing_versions", "users", column: "published_by_id", name: "fk_camlv_published_by" + add_foreign_key "course_assessment_marketplace_listing_versions", "users", column: "updater_id", name: "fk_camlv_updater_id" + add_foreign_key "course_assessment_marketplace_listings", "course_assessment_marketplace_listing_versions", column: "current_version_id", name: "fk_caml_current_version_id", on_delete: :nullify add_foreign_key "course_assessment_marketplace_listings", "course_assessments", column: "assessment_id", name: "fk_course_assessment_marketplace_listings_assessment_id", on_delete: :cascade + add_foreign_key "course_assessment_marketplace_listings", "courses", column: "source_course_id", name: "fk_caml_source_course_id", on_delete: :nullify add_foreign_key "course_assessment_marketplace_listings", "users", column: "creator_id", name: "fk_course_assessment_marketplace_listings_creator_id" + add_foreign_key "course_assessment_marketplace_listings", "users", column: "fallback_maintainer_id", name: "fk_caml_fallback_maintainer_id" add_foreign_key "course_assessment_marketplace_listings", "users", column: "publisher_id", name: "fk_course_assessment_marketplace_listings_publisher_id" add_foreign_key "course_assessment_marketplace_listings", "users", column: "updater_id", name: "fk_course_assessment_marketplace_listings_updater_id" add_foreign_key "course_assessment_plagiarism_checks", "course_assessments", column: "assessment_id", name: "fk_course_assessment_plagiarism_checks_assessment_id" diff --git a/spec/controllers/course/assessment/marketplace_listings_controller_spec.rb b/spec/controllers/course/assessment/marketplace_listings_controller_spec.rb index ed42190bd3..b86f08c4b7 100644 --- a/spec/controllers/course/assessment/marketplace_listings_controller_spec.rb +++ b/spec/controllers/course/assessment/marketplace_listings_controller_spec.rb @@ -22,6 +22,13 @@ expect(listing.publisher).to eq(admin) end + it 'cuts version 1 through the publish seam' do + expect { subject }.to change { Course::Assessment::Marketplace::ListingVersion.count }.by(1) + listing = assessment.reload.marketplace_listing + expect(listing.current_version&.version).to eq(1) + expect(listing.current_version.published_by).to eq(admin) + end + context 'when the assessment was previously published then removed (re-publish)' do let!(:listing) do create(:course_assessment_marketplace_listing, assessment: assessment, published: false, diff --git a/spec/factories/course_assessment_marketplace_listing_versions.rb b/spec/factories/course_assessment_marketplace_listing_versions.rb new file mode 100644 index 0000000000..c63f1840e3 --- /dev/null +++ b/spec/factories/course_assessment_marketplace_listing_versions.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true +FactoryBot.define do + factory :course_assessment_marketplace_listing_version, + class: Course::Assessment::Marketplace::ListingVersion do + listing { association :course_assessment_marketplace_listing } + assessment + published_by { listing.publisher } + sequence(:version) { |n| n } + end +end diff --git a/spec/models/course/assessment/marketplace/listing_spec.rb b/spec/models/course/assessment/marketplace/listing_spec.rb index 7eeb9a26f3..e5ef369564 100644 --- a/spec/models/course/assessment/marketplace/listing_spec.rb +++ b/spec/models/course/assessment/marketplace/listing_spec.rb @@ -43,5 +43,55 @@ expect(subject.adoption_count).to eq(2) end end + + describe 'versioning associations (additive; all nullable)' do + let(:listing) { create(:course_assessment_marketplace_listing) } + + it 'is valid without any versioning fields set' do + expect(listing.current_version).to be_nil + expect(listing.source_course).to be_nil + expect(listing.fallback_maintainer).to be_nil + expect(listing).to be_valid + end + + it 'has many ordered versions and can point at a current version' do + v1 = create(:course_assessment_marketplace_listing_version, listing: listing, version: 1) + v2 = create(:course_assessment_marketplace_listing_version, listing: listing, version: 2) + listing.update!(current_version: v2) + expect(listing.versions).to contain_exactly(v1, v2) + expect(listing.current_version).to eq(v2) + end + + it 'destroys its versions when destroyed' do + create(:course_assessment_marketplace_listing_version, listing: listing, version: 1) + expect { listing.destroy }. + to change { Course::Assessment::Marketplace::ListingVersion.count }.by(-1) + end + + it 'does not destroy versions belonging to another listing' do + other_listing = create(:course_assessment_marketplace_listing) + other_version = create(:course_assessment_marketplace_listing_version, + listing: other_listing, version: 1) + create(:course_assessment_marketplace_listing_version, listing: listing, version: 1) + + expect { listing.destroy }. + to change { Course::Assessment::Marketplace::ListingVersion.count }.by(-1) + expect(other_version.reload).to be_persisted + end + + it 'optionally references a source course, fallback maintainer, and provenance' do + course = create(:course) + maintainer = create(:user) + listing.update!(source_course: course, source_course_name: 'Intro to AI', + source_course_code: 'CS2109S', + source_academic_period: 'AY2024/25 Sem 1', + fallback_maintainer: maintainer) + expect(listing.source_course).to eq(course) + expect(listing.fallback_maintainer).to eq(maintainer) + expect(listing.source_course_name).to eq('Intro to AI') + expect(listing.source_course_code).to eq('CS2109S') + expect(listing.source_academic_period).to eq('AY2024/25 Sem 1') + end + end end end diff --git a/spec/models/course/assessment/marketplace/listing_version_spec.rb b/spec/models/course/assessment/marketplace/listing_version_spec.rb new file mode 100644 index 0000000000..e667fb664b --- /dev/null +++ b/spec/models/course/assessment/marketplace/listing_version_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::Marketplace::ListingVersion, type: :model do + let(:instance) { Instance.default } + with_tenant(:instance) do + let(:listing) { create(:course_assessment_marketplace_listing) } + + describe 'validations' do + it 'is valid with the factory' do + expect(build(:course_assessment_marketplace_listing_version, listing: listing)).to be_valid + end + + it 'requires a positive integer version' do + version = build(:course_assessment_marketplace_listing_version, listing: listing, version: 0) + expect(version).not_to be_valid + expect(version.errors[:version]).to be_present + end + + it 'requires a version' do + version = build(:course_assessment_marketplace_listing_version, listing: listing, version: nil) + expect(version).not_to be_valid + expect(version.errors[:version]).to be_present + end + + it 'requires an integer version' do + version = build(:course_assessment_marketplace_listing_version, listing: listing, version: 1.5) + expect(version).not_to be_valid + expect(version.errors[:version]).to be_present + end + + it 'requires an assessment' do + version = build(:course_assessment_marketplace_listing_version, listing: listing, assessment: nil) + expect(version).not_to be_valid + expect(version.errors[:assessment]).to be_present + end + + it 'requires a publisher' do + version = build(:course_assessment_marketplace_listing_version, listing: listing, published_by: nil) + expect(version).not_to be_valid + expect(version.errors[:published_by]).to be_present + end + + it 'enforces version uniqueness scoped to the listing' do + create(:course_assessment_marketplace_listing_version, listing: listing, version: 1) + duplicate = build(:course_assessment_marketplace_listing_version, listing: listing, version: 1) + expect(duplicate).not_to be_valid + expect(duplicate.errors[:version]).to be_present + end + + it 'allows the same version number on a different listing' do + create(:course_assessment_marketplace_listing_version, listing: listing, version: 1) + other = build(:course_assessment_marketplace_listing_version, + listing: create(:course_assessment_marketplace_listing), version: 1) + expect(other).to be_valid + end + end + + describe 'associations' do + it 'belongs to a listing, snapshot assessment, and publisher' do + version = create(:course_assessment_marketplace_listing_version, listing: listing) + expect(version.listing).to eq(listing) + expect(version.assessment).to be_a(Course::Assessment) + expect(version.published_by).to be_a(User) + end + end + + describe '.ordered' do + it 'orders by ascending version' do + v2 = create(:course_assessment_marketplace_listing_version, listing: listing, version: 2) + v1 = create(:course_assessment_marketplace_listing_version, listing: listing, version: 1) + expect(listing.versions.ordered).to eq([v1, v2]) + end + end + end +end diff --git a/spec/models/course/assessment/marketplace_spec.rb b/spec/models/course/assessment/marketplace_spec.rb new file mode 100644 index 0000000000..927621a538 --- /dev/null +++ b/spec/models/course/assessment/marketplace_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::Marketplace, type: :model do + describe '.container' do + around do |example| + # This suite is non-transactional and commits permanently (app/CLAUDE.md), so a + # container course created by an example would otherwise leak and accumulate on + # every run. Start each example from a known state (nil pointer → find-or-heal), + # then destroy the container it created and restore the original pointer. + original_id = ActsAsTenant.without_tenant do + Instance.default.settings(:marketplace).container_course_id + end + ActsAsTenant.without_tenant do + Instance.default.settings(:marketplace).container_course_id = nil + Instance.default.save! + end + + example.run + + ActsAsTenant.without_tenant do + created_id = Instance.default.settings(:marketplace).container_course_id + Course.where(id: created_id).find_each(&:destroy) if created_id + Instance.default.settings(:marketplace).container_course_id = original_id + Instance.default.save! + end + end + + it 'creates a hidden, unpublished system container course in the default instance' do + container = described_class.container + expect(container).to be_a(Course) + expect(container.instance_id).to eq(Instance::DEFAULT_INSTANCE_ID) + expect(container.published).to be(false) + expect(container.title).to eq(Course::Assessment::Marketplace::CONTAINER_TITLE) + end + + it 'is created and updated by the system user' do + container = described_class.container + expect(container.creator).to eq(User.system) + expect(container.updater).to eq(User.system) + end + + it 'creates exactly one course when no pointer exists yet' do + expect { described_class.container }. + to change { ActsAsTenant.without_tenant { Course.count } }.by(1) + end + + it 'persists the pointer and returns the same course on subsequent calls' do + first = described_class.container + second = described_class.container + expect(second.id).to eq(first.id) + ActsAsTenant.without_tenant do + # Reload to bypass the memoized Instance.default and assert the pointer + # was actually persisted to the DB (not just held in memory) — this is + # what makes `save!` in create_container! observable. + expect(Instance.default.reload.settings(:marketplace).container_course_id).to eq(first.id) + end + end + + it 'does not create a new course on subsequent calls' do + described_class.container + expect { described_class.container }. + not_to(change { ActsAsTenant.without_tenant { Course.count } }) + end + + it 'enrolls only the system user as owner, so no other user ever sees it' do + container = described_class.container + # Enroll other_user in an unrelated real course so the negative assertion is + # non-vacuous: containing_user DOES surface a course they belong to, yet never + # the container. + other_course = ActsAsTenant.with_tenant(Instance.default) { create(:course) } + other_user = ActsAsTenant.with_tenant(Instance.default) do + create(:course_manager, course: other_course).user + end + ActsAsTenant.without_tenant do + expect(Course.containing_user(User.system)).to include(container) + expect(Course.containing_user(other_user)).to include(other_course) + expect(Course.containing_user(other_user)).not_to include(container) + end + end + + it 'is not publicly accessible' do + container = described_class.container + ActsAsTenant.without_tenant do + expect(Course.publicly_accessible).not_to include(container) + end + end + + it 're-heals a stale pointer: creates exactly one replacement course and persists it' do + ActsAsTenant.without_tenant do + Instance.default.settings(:marketplace).container_course_id = -999 + Instance.default.save! + end + + container = nil + expect { container = described_class.container }. + to change { ActsAsTenant.without_tenant { Course.count } }.by(1) + expect(container.id).not_to eq(-999) + ActsAsTenant.without_tenant do + expect(Instance.default.settings(:marketplace).container_course_id).to eq(container.id) + end + end + end +end diff --git a/spec/services/course/assessment/marketplace/publish_service_spec.rb b/spec/services/course/assessment/marketplace/publish_service_spec.rb new file mode 100644 index 0000000000..ec2b2d698a --- /dev/null +++ b/spec/services/course/assessment/marketplace/publish_service_spec.rb @@ -0,0 +1,123 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::Marketplace::PublishService, type: :service do + let(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:assessment) { create(:assessment, course: course) } + let(:publisher) { create(:user) } + + def container + ActsAsTenant.without_tenant { Course::Assessment::Marketplace.container } + end + + describe '.publish' do + it 'activates the listing and cuts version 1 published by the publisher' do + listing = described_class.publish(assessment, publisher) + expect(listing.published).to be(true) + expect(listing.current_version).to be_present + expect(listing.current_version.version).to eq(1) + expect(listing.current_version.published_by).to eq(publisher) + end + + it 'snapshots a distinct copy of the assessment into the container course' do + listing = described_class.publish(assessment, publisher) + snapshot = listing.current_version.assessment + ActsAsTenant.without_tenant do + expect(snapshot).not_to eq(assessment) + expect(snapshot.course).to eq(container) + end + end + + it 'creates exactly one version row' do + expect { described_class.publish(assessment, publisher) }. + to change { Course::Assessment::Marketplace::ListingVersion.count }.by(1) + end + + it 'captures denormalized provenance from the source course' do + listing = described_class.publish(assessment, publisher) + expect(listing.source_course).to eq(course) + expect(listing.source_course_name).to eq(course.title) + expect(listing.fallback_maintainer).to eq(course.course_users.find_by(role: :owner).user) + end + + it 'leaves source_course_code and source_academic_period nil (no source data in Coursemology)' do + listing = described_class.publish(assessment, publisher) + expect(listing.source_course_code).to be_nil + expect(listing.source_academic_period).to be_nil + end + + it 'does not cut a second version when re-published (first-publish only this slice)' do + described_class.publish(assessment, publisher) + expect { described_class.publish(assessment, publisher) }. + not_to(change { Course::Assessment::Marketplace::ListingVersion.count }) + end + + it 'preserves first_published_at and bumps last_published_at on re-publish' do + old = 3.days.ago + listing = create(:course_assessment_marketplace_listing, assessment: assessment, + published: false, + first_published_at: old, + last_published_at: old) + result = described_class.publish(assessment, publisher) + expect(result.id).to eq(listing.id) + expect(result.first_published_at).to be_within(1.second).of(old) + expect(result.last_published_at).to be > old + end + end + + describe '.ensure_first_version!' do + let(:listing) do + create(:course_assessment_marketplace_listing, assessment: assessment, published: true) + end + + it 'cuts version 1 for a published listing that has none' do + expect(listing.current_version).to be_nil + version = described_class.ensure_first_version!(listing, publisher) + expect(version.version).to eq(1) + expect(listing.reload.current_version).to eq(version) + end + + it 'is idempotent: a second call cuts no new version' do + described_class.ensure_first_version!(listing, publisher) + expect { described_class.ensure_first_version!(listing, publisher) }. + not_to(change { Course::Assessment::Marketplace::ListingVersion.count }) + end + + it 'captures provenance during the version cut' do + described_class.ensure_first_version!(listing, publisher) + expect(listing.reload.source_course).to eq(course) + expect(listing.source_course_name).to eq(course.title) + end + end + + describe '.backfill_all!' do + it 'snapshots each published, version-less listing as v1 and sets adopted_version' do + listing = create(:course_assessment_marketplace_listing, assessment: assessment, published: true) + adoption = create(:course_assessment_marketplace_adoption, listing: listing) + + described_class.backfill_all! + + expect(listing.reload.current_version&.version).to eq(1) + expect(adoption.reload.adopted_version).to eq(1) + end + + it 'leaves an already-versioned listing untouched' do + listing = create(:course_assessment_marketplace_listing, assessment: assessment, published: true) + described_class.ensure_first_version!(listing, publisher) + original_version_id = listing.reload.current_version_id + + described_class.backfill_all! + + expect(listing.reload.current_version_id).to eq(original_version_id) + end + + it 'ignores unpublished listings' do + unpublished = create(:course_assessment_marketplace_listing, assessment: assessment, published: false) + described_class.backfill_all! + expect(unpublished.reload.current_version).to be_nil + end + end + end +end