fix(marketplace): track adoptions through a course roll-forward

Adoption rows were keyed off the SOURCE's own marketplace listing, so
they only ever fired for copies of the authoring assessment. A copy of
an adopted copy -- next semester's course, rolled forward -- carries no
listing of its own, so it dropped out of the listing's reach silently:
no adoption row, no "a newer version is available" reminder ever again,
and the count missed a course genuinely using the content.

Key the row off the source's own adoption row instead. The chain now
propagates through every generation, and each copy inherits the vintage
its source holds rather than what the listing currently serves, so a
rolled-forward copy is still told when it falls behind.

Copies of the assessment that AUTHORS a listing stop being recorded.
Those are the publisher's own -- their course rolled forward, or the
assessment handed to a colleague directly -- made without anyone
choosing the listing, and recording them let a listing with no adopters
at all show an adoption count that climbed every semester its author
re-ran the course.

Author: LWS49

app/models/course/assessment.rb

@@ -295,29 +295,40 @@ def csv_downloadable?
     questions.any?(&:csv_downloadable?)
   end
 
-  # Records +duplicate+, a copy of this assessment, as an adoption of this assessment's marketplace
-  # listing. Every copy of a listed assessment is an adoption, whichever duplication path produced
-  # it, so this is called by the duplication services rather than by the marketplace's own job.
+  # Records +duplicate+, a copy of this assessment, against the marketplace listing its content came
+  # from. Called by the duplication services rather than by the marketplace's own job, because a copy
+  # of an adopted assessment is made by ordinary duplication -- rolling a course forward for the new
+  # semester, copying a selection of objects across -- and a copy the listing cannot reach is a copy
+  # it can never send a version reminder to.
+  #
+  # Keyed off this assessment's own ADOPTION ROW, so only content that came through the marketplace
+  # propagates. An assessment that AUTHORS a listing is deliberately not a source here: copies of it
+  # are the publisher's own -- their course rolled forward, or the assessment handed to a colleague
+  # directly -- made without anyone choosing the listing, so counting them would let a listing nobody
+  # adopted show a rising adoption count.
   #
   # The listing itself is never carried over -- +initialize_duplicate+ below does not duplicate the
-  # +marketplace_listing+ association -- so a copy always starts out unlisted.
+  # +marketplace_listing+ association -- so a copy always starts out unlisted, which is exactly why a
+  # copy of a copy has to find its listing through the source's adoption row rather than its own.
   #
   # @param [Course::Assessment] duplicate The saved copy of this assessment.
   # @param [Course] destination_course The course the copy was duplicated into.
   # @param [User] current_user The user who triggered the duplication.
   def record_marketplace_adoption(duplicate, destination_course, current_user)
-    return unless marketplace_listing&.published?
     # Publishing duplicates the source INTO the container to cut a snapshot. That is the listing
     # growing a version, not a course adopting it, so the container is never an adopter.
     return if destination_course.preview?
 
+    source_adoption = Course::Assessment::Marketplace::Adoption.find_by(duplicated_assessment_id: id)
+    return if source_adoption.nil?
+
     Course::Assessment::Marketplace::Adoption.create!(
-      listing: marketplace_listing,
+      listing: source_adoption.listing,
       destination_course: destination_course,
       duplicated_assessment: duplicate,
-      # Stamped here rather than at the call site: this is the single writer of adoption rows, and the
-      # adopter's "your copy is behind" banner has nothing to compare against without it.
-      adopted_version_at: marketplace_listing.current_version&.published_at,
+      # The vintage the SOURCE holds, not what the listing currently serves: crediting a rolled-forward
+      # copy with the latest version would silently mark stale content as up to date.
+      adopted_version_at: source_adoption.adopted_version_at,
       creator: current_user,
       updater: current_user
     )

app/services/course/duplication/base_service.rb

@@ -32,8 +32,9 @@ def initialize_duplicator(*)
 
   # Hands every duplicated assessment its own copy so it can record a marketplace adoption. Copies
   # made outside +Course::Assessment::Marketplace::DuplicationJob+ -- selected object duplications
-  # and full course duplications that happen to carry a listed assessment along -- are adoptions
-  # too, and the listing has to know about them to reach every course holding a copy.
+  # and full course duplications that happen to carry an ADOPTED assessment along, most often a
+  # course rolled forward for a new batch of students -- are adoptions too, and the listing has to know
+  # about them to reach every course holding a copy.
   #
   # This sweep lives in the duplication service rather than in a model's +after_duplicate_save+
   # hook because that hook only runs for the top-level objects of an object duplication, and never

spec/jobs/course/assessment/marketplace/duplication_job_spec.rb

@@ -362,19 +362,6 @@ def backdate_current_version(record = listing)
       end
     end
 
-    # Grandchildren-excluded: an adoption is written for copies of a *listed* assessment, and a copy
-    # is never itself listed, so duplicating an already-adopted copy writes no second-generation row.
-    it 'does not write an adoption for an ordinary ObjectDuplicationService copy' do
-      run
-      copy = destination_course.assessments.order(:created_at).last
-      third_course = create(:course)
-      expect do
-        Course::Duplication::ObjectDuplicationService.duplicate_objects(
-          destination_course, third_course, copy, current_user: user
-        )
-      end.not_to change(Course::Assessment::Marketplace::Adoption, :count)
-    end
-
     # The sidebar entry point sends no tab, and a tab from another course can be sent by an
     # out-of-date URL. Neither may leave the redirect pointing at a tab the user cannot open.
     describe 'when the requested tab is absent or foreign' do
@@ -429,11 +416,11 @@ def run_with_tab(tab_id)
       end
     end
 
-    # A listed assessment can leave its course by paths that do not go through this job: an
-    # instructor duplicating selected objects, or a full course duplication that carries the
-    # listed assessment along. Those copies must obey the same two rules as the job's copies --
-    # the listing stays singular, and the destination course is recorded as an adopter.
-    describe 'manual duplication of a listed assessment' do
+    # Marketplace content leaves a course by paths that do not go through this job: an instructor
+    # duplicating selected objects, or a full course duplication carrying the assessment along. Both
+    # must keep the listing singular, and both must record the destination as an adopter -- but only
+    # for content that came THROUGH the marketplace, which is what separates these two describes.
+    describe 'manual duplication of the assessment that authors a listing' do
       let(:manual_destination) { create(:course) }
 
       before { listing }
@@ -460,14 +447,11 @@ def duplicate_whole_course
           expect(copy.marketplace_listing).to be_nil
         end
 
-        it 'records the destination course as an adopter' do
-          copy = nil
-          expect { copy = duplicate_selected_objects }.
-            to change(Course::Assessment::Marketplace::Adoption, :count).by(1)
-          adoption = Course::Assessment::Marketplace::Adoption.order(:id).last
-          expect(adoption.listing).to eq(listing)
-          expect(adoption.destination_course).to eq(manual_destination)
-          expect(adoption.duplicated_assessment).to eq(copy)
+        # The publisher handing their own assessment to somebody directly bypassed the marketplace
+        # entirely, so the marketplace has no adoption to record.
+        it 'records no adoption' do
+          expect { duplicate_selected_objects }.
+            not_to change(Course::Assessment::Marketplace::Adoption, :count)
         end
       end
 
@@ -481,13 +465,65 @@ def duplicate_whole_course
           expect(new_course.assessments.map(&:marketplace_listing)).to all(be_nil)
         end
 
-        it 'records the new course as an adopter' do
+        # The publisher rolling their own course forward. Recorded, this would let a listing nobody
+        # has adopted show an adoption count that climbs by one every semester its author re-runs.
+        it 'records no adoption' do
+          expect { duplicate_whole_course }.
+            not_to change(Course::Assessment::Marketplace::Adoption, :count)
+        end
+      end
+    end
+
+    # The other half: an ADOPTED copy carried along by ordinary duplication. This is the semester
+    # roll-forward, and the copy it makes both counts as a course using the listing and stays
+    # reachable by the listing's version reminders.
+    describe 'manual duplication of an adopted copy' do
+      let(:adopting_course) { create(:course) }
+
+      # The real import path, so the copy carries a genuine adoption row rather than a hand-built one.
+      def adopt
+        described_class.perform_now([listing.id], adopting_course,
+                                    adopting_course.assessment_categories.first.tabs.first.id,
+                                    current_user: user)
+        adopting_course.assessments.order(:created_at).last
+      end
+
+      context 'when duplicating selected objects' do
+        it 'records the destination course as an adopter of the same listing' do
+          adopted = adopt
+          onward_destination = create(:course)
+          copy = nil
+
+          expect do
+            copy = Course::Duplication::ObjectDuplicationService.duplicate_objects(
+              adopting_course, onward_destination, adopted, current_user: user
+            )
+          end.to change { listing.reload.adoption_count }.from(1).to(2)
+
+          adoption = Course::Assessment::Marketplace::Adoption.
+                     find_by(duplicated_assessment_id: copy.id)
+          expect(adoption.listing).to eq(listing)
+          expect(adoption.destination_course).to eq(onward_destination)
+          # The vintage its source held, so the copy is told it is behind once a newer version lands.
+          expect(adoption.adopted_version_at).
+            to be_within(1.second).of(listing.current_version.published_at)
+        end
+      end
+
+      context 'when duplicating the whole course' do
+        it 'records the new course as an adopter of the same listing' do
+          adopt
+
           new_course = nil
-          expect { new_course = duplicate_whole_course }.
-            to change(Course::Assessment::Marketplace::Adoption, :count).by(1)
-          adoption = Course::Assessment::Marketplace::Adoption.order(:id).last
+          expect do
+            new_course = Course::Duplication::CourseDuplicationService.duplicate_course(
+              adopting_course, current_user: user, new_title: "#{adopting_course.title} copy"
+            )
+          end.to change { listing.reload.adoption_count }.from(1).to(2)
+
+          adoption = Course::Assessment::Marketplace::Adoption.
+                     find_by(destination_course_id: new_course.id)
           expect(adoption.listing).to eq(listing)
-          expect(adoption.destination_course).to eq(new_course)
         end
       end
     end

spec/models/course/assessment_spec.rb

@@ -547,6 +547,79 @@
       end
     end
 
+    # Only content that came THROUGH the marketplace propagates: a copy descended from an adoption
+    # stays reachable by version reminders and counts as a course using the listing, while copies of
+    # the publisher's own authoring assessment are not adoptions at all.
+    describe '#record_marketplace_adoption' do
+      let(:destination_course) { create(:course) }
+      let(:copy) { create(:assessment, course: destination_course) }
+      let(:duplicating_user) { create(:user) }
+      let(:listing) { create(:course_assessment_marketplace_listing, :versioned, course: course) }
+
+      def record(source, destination = destination_course)
+        source.record_marketplace_adoption(copy, destination, duplicating_user)
+        Course::Assessment::Marketplace::Adoption.find_by(duplicated_assessment_id: copy.id)
+      end
+
+      # Nobody chose the listing here — the publisher is duplicating their own assessment, whether
+      # into next semester's course or a colleague's. Recording it would let a listing with no
+      # adopters at all show an adoption count that climbs every term.
+      it 'records nothing for a copy of the assessment that authors the listing' do
+        expect(record(listing.authoring_assessment)).to be_nil
+      end
+
+      it 'records nothing for an assessment with neither a listing nor an adoption' do
+        expect(record(create(:assessment, course: course))).to be_nil
+      end
+
+      # The roll-forward case: next semester's course carries a copy of a copy. That source holds no
+      # listing of its own, so the chain runs through its adoption row -- without it the new copy
+      # drops out of the listing's reach and never sees a version reminder again.
+      context 'when the source is itself an adopted copy' do
+        let(:adopted) { create(:assessment, course: create(:course)) }
+        let!(:source_adoption) do
+          create(:course_assessment_marketplace_adoption,
+                 listing: listing, destination_course: adopted.course,
+                 duplicated_assessment: adopted, adopted_version_at: 30.days.ago.change(usec: 0))
+        end
+
+        it 'records the copy against the same listing' do
+          adoption = record(adopted)
+
+          expect(adoption).to be_present
+          expect(adoption.listing).to eq(listing)
+          expect(adoption.destination_course).to eq(destination_course)
+        end
+
+        # The copy holds whatever vintage its source held, NOT the listing's latest -- crediting it
+        # with the served version would silently mark a stale copy as up to date.
+        it 'inherits the vintage its source holds rather than the served one' do
+          adoption = record(adopted)
+
+          expect(adoption.adopted_version_at).
+            to be_within(1.second).of(source_adoption.adopted_version_at)
+        end
+
+        # Unlisting is a visibility decision. Severing the chain there would strand copies that
+        # already exist and can still be updated.
+        it 'records the row even once the listing is off the marketplace' do
+          listing.update!(published: false)
+
+          expect(record(adopted)).to be_present
+        end
+
+        # Publishing duplicates the source INTO the container to cut a snapshot. That is the listing
+        # growing a version, not a course adopting it.
+        it 'records nothing when the copy lands in the marketplace container' do
+          container = ActsAsTenant.without_tenant do
+            Course::Assessment::Marketplace::PreviewContainerService.container_course
+          end
+
+          expect(record(adopted, container)).to be_nil
+        end
+      end
+    end
+
     describe 'in-transaction marketplace authoring re-point' do
       # The re-point enqueues nothing, but the env default is `:background_thread` — a real thread
       # sharing this example's connection — and these examples assert on row counts in the container.