Skip to content

feat(marketplace): duplicate listings into current course - #8479

Open
LWS49 wants to merge 3 commits into
lws49/feat-marketplace-pr1-foundationfrom
lws49/feat-marketplace-pr4-duplicate
Open

feat(marketplace): duplicate listings into current course#8479
LWS49 wants to merge 3 commits into
lws49/feat-marketplace-pr1-foundationfrom
lws49/feat-marketplace-pr4-duplicate

Conversation

@LWS49

@LWS49 LWS49 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator
  • add DuplicationJob: copies listings into a course tab, writes adoption
  • add bulk duplicate endpoint enqueuing the job for selected listings
  • add DuplicateConfirmation modal with row + bulk triggers, job polling
  • add MarketplaceAPI.duplicate and duplicateListings poll operation
  • serialize and assert live distinct-course adoption count in index

@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr4-duplicate branch from 0b7f44e to d84d375 Compare July 8, 2026 04:52
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr3-page branch from 052b93c to 9169fa1 Compare July 17, 2026 08:09
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr4-duplicate branch from d84d375 to 284c2bb Compare July 17, 2026 08:10
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr3-page branch from 9169fa1 to e532cc2 Compare July 29, 2026 10:16
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr4-duplicate branch from 284c2bb to 0ac76f3 Compare July 29, 2026 10:17
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr3-page branch from e532cc2 to 97c5401 Compare July 29, 2026 10:28
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr4-duplicate branch from 0ac76f3 to 56b15aa Compare July 29, 2026 10:29
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr3-page branch from 97c5401 to 2e480cd Compare July 29, 2026 10:49
destination_course, third_course, copy, current_user: user
)
end.not_to change(Course::Assessment::Marketplace::Adoption, :count)
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also add tests confirming the following logic:

  • once duplication of a marketplace-listed-assessment is done, the listing itself does not get duplicated (so the total number of listings should not change as adoptions take place)

  • if marketplace-listed-assessment is duplicated manually (e.g. as part of full / partial course duplication), it should follow the same logic steps (listing should not duplicate, adoption should be tracked)

@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr3-page branch 2 times, most recently from 9eccba7 to c1588e3 Compare July 29, 2026 11:40
Base automatically changed from lws49/feat-marketplace-pr3-page to lws49/feat-marketplace-pr1-foundation July 29, 2026 12:06
- add DuplicationJob: copies listings into a course tab, writes adoption
- add bulk duplicate endpoint enqueuing the job for selected listings
- add DuplicateConfirmation modal with row + bulk triggers, job polling
- add MarketplaceAPI.duplicate and duplicateListings poll operation
- serialize and assert live distinct-course adoption count in index
@LWS49
LWS49 force-pushed the lws49/feat-marketplace-pr4-duplicate branch from 56b15aa to f25dc28 Compare July 29, 2026 12:08
A listed assessment can leave its course without going through
Marketplace::DuplicationJob -- an instructor duplicating selected objects, or
a full course duplication carrying the listed assessment along. Those copies
were previously untracked, so a listing could not reach every course holding
one of its copies.

- move adoption writing out of DuplicationJob and into
  Course::Duplication::BaseService#record_marketplace_adoptions
- call it from both object and full course duplication, leaving a single
  adoption writer instead of two
- expose Duplicator#duplicated_objects so the service can walk the
  source-to-duplicate pairs a duplication produced
- assert the listing itself is never duplicated, and that manual duplication
  records the destination course as an adopter

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end “duplicate from marketplace into the current course” functionality: a backend job + endpoint to duplicate selected marketplace listings into a destination course/tab while recording marketplace adoptions, and a frontend confirmation modal that triggers duplication and polls job completion.

Changes:

  • Introduces Course::Assessment::Marketplace::DuplicationJob and a POST #duplicate endpoint to enqueue duplication for selected published listings.
  • Records marketplace adoptions during object/course duplication flows and reports distinct-course adoption counts in marketplace listing index responses.
  • Adds a frontend confirmation modal + row-level trigger, API method, and operation to start duplication and poll job status.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
spec/jobs/course/assessment/marketplace/duplication_job_spec.rb Adds job specs covering duplication behavior, tab placement, and adoption recording.
spec/controllers/course/assessment/marketplace/listings_controller_spec.rb Adds controller specs for live adoption counts and duplicate endpoint enqueue/authorization.
lib/autoload/duplicator.rb Documents and exposes duplicated_objects for duplication bookkeeping.
client/locales/zh.json Adds i18n strings for duplication confirmation and status toasts.
client/locales/ko.json Adds i18n strings for duplication confirmation and status toasts.
client/locales/en.json Adds i18n strings for duplication confirmation and status toasts.
client/app/bundles/course/marketplace/translations.ts Registers new duplication-related message IDs/default messages.
client/app/bundles/course/marketplace/pages/MarketplaceIndex/MarketplaceTable.tsx Adds a row-level Duplicate button to open the confirmation flow.
client/app/bundles/course/marketplace/pages/MarketplaceIndex/index.tsx Wires the duplicate confirmation modal and reads from_tab from URL params.
client/app/bundles/course/marketplace/operations.ts Adds duplicateListings operation (enqueue + job polling).
client/app/bundles/course/marketplace/components/DuplicateConfirmation.tsx Adds confirmation modal that submits duplication and shows toast feedback.
client/app/bundles/course/marketplace/components/test/DuplicationConfirmation.test.tsx Adds frontend test coverage for posting duplication request payload.
client/app/api/course/Marketplace.ts Adds MarketplaceAPI.duplicate POST method.
app/services/course/duplication/object_duplication_service.rb Records marketplace adoptions after object duplication completes.
app/services/course/duplication/course_duplication_service.rb Records marketplace adoptions after course duplication completes.
app/services/course/duplication/base_service.rb Implements record_marketplace_adoptions based on duplicator.duplicated_objects.
app/jobs/course/assessment/marketplace/duplication_job.rb Implements duplication job and redirect behavior after duplication.
app/controllers/course/assessment/marketplace/listings_controller.rb Adds duplicate action with authorization and job submission.
Comments suppressed due to low confidence (1)

client/app/bundles/course/marketplace/components/DuplicateConfirmation.tsx:52

  • duplicateTitle is a pluralized message that references {n}, but it is formatted without providing n, which can cause runtime formatting errors / fallback output.
      open={open}
      primaryLabel={t(translations.duplicateConfirm)}
      title={t(translations.duplicateTitle)}
    >

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +27 to +42
const confirm = async (): Promise<void> => {
setSubmitting(true);
await duplicateListings(
listings.map((l) => l.id),
destinationTabId,
() => {
toast.success(t(translations.duplicateStarted, { n: listings.length }));
setSubmitting(false);
onClose();
},
() => {
toast.error(t(translations.duplicateFailed));
setSubmitting(false);
},
);
};

@LWS49 LWS49 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIxed: confirm now wraps the request in try/catch and releases submitting on rejection. With no job topoll, nothing else could have re-enabled the prompt.

Added a test asserting the confirm button comes back enabled after a 500.

Comment on lines +92 to +96
duplicateBody: {
id: 'course.marketplace.duplicateBody',
defaultMessage:
'{n, plural, one {This assessment will be copied to your course.} other {These assessments will be copied to your course.}}',
},

@LWS49 LWS49 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The inconsistency slipped past me. All pluralised messages (duplicateTitle, duplicateFailed) now receive n.

Comment on lines +1 to +26
import CourseAPI from 'api/course';
import pollJob from 'lib/helpers/jobHelpers';

import { MarketplaceListing } from './types';

export const fetchListings = async (): Promise<MarketplaceListing[]> => {
const response = await CourseAPI.marketplace.index();
return response.data.listings as MarketplaceListing[];
};

export const duplicateListings = async (
listingIds: number[],
destinationTabId: number | null,
onSuccess: (redirectUrl?: string) => void,
onFailure: () => void,
): Promise<void> => {
const response = await CourseAPI.marketplace.duplicate(
listingIds,
destinationTabId,
);
pollJob(
response.data.jobUrl,
(data) => onSuccess(data.redirectUrl),
onFailure,
2000,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not use pollJob, tie the polling directly to the active component

#7426

@LWS49 LWS49 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented. Let me know if there are any discrepancies to flag.

Followed what's documented in #7426: the operation no longer schedules anything, it just POSTs and returns the jobUrl. DuplicateConfirmation holds that URL in state and owns the timer in a useEffect, calling pollJobRequest(jobUrl) directly; the cleanup clears the interval, so unmounting or navigating away tears the poller down. pollJob is gone.

Comment on lines +22 to +29
def duplicate
listings = authorized_listings
job = Course::Assessment::Marketplace::DuplicationJob.perform_later(
listings.map(&:id), current_course, duplicate_params[:destination_tab_id].to_i,
current_user: current_user
).job
render partial: 'jobs/submitted', locals: { job: job }
end

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Fixed according to suggestion to use .presence&.to_i so an omitted tab stays nil and the job lets the duplication fall back to the destination course's first tab.

Comment on lines +10 to +26
def perform_tracked(listing_ids, destination_course, destination_tab_id, options = {})
current_user = options[:current_user]
ActsAsTenant.without_tenant do
listings = Course::Assessment::Marketplace::Listing.published.where(id: listing_ids)
listings.each do |listing|
# The adoption row is written by the duplication service itself, which tracks every copy of a
# listed assessment regardless of the path that produced it. See
# `Course::Duplication::BaseService#record_marketplace_adoptions`.
copy = duplicate_listing(listing, destination_course, current_user)
reparent_into_tab(copy, destination_course, destination_tab_id)
end
redirect_to course_assessments_url(destination_course,
category: destination_course.assessment_categories.first.id,
tab: destination_tab_id,
host: destination_course.instance.host)
end
end

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The job now resolves the requested tab up front, and the redirect points at the tab the copies actually landed in, falling back to the copy's own tab when no tab was requested or the requested one belongs to another course.

Important to ensure error handling behaves correctly for absent and foreign-tab cases, so two specs are added to cover this.

const mock = createMockAdapter(CourseAPI.marketplace.client);
beforeEach(() => mock.reset());

const listings = [{ id: 1, title: 'Recursion Drills' }] as never;

@LWS49 LWS49 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

after_save_success = save_success && after_save(objects, duplicated)
raise ActiveRecord::Rollback unless after_save_success

record_marketplace_adoptions

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this hook should be considered part of the process of duplicating the individual assessments with marketplace listings (or duplicating assessments in general at least)

At minimum, I would expect it to trigger before after_save(...) to prevent unexpected states where all the duplicates save, the after_save hook runs, but this triggers a rollback wiping out the duplicated data.

@LWS49 LWS49 Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Moved the call ahead of after_save, so a failure rolls the transaction back before the models' post-duplication callbacks have done their work rather than undoing it afterwards.

Polling:
- drop `pollJob` (marked DO NOT USE -- it orphans pollers on navigation) in
  favour of a `pollJobRequest` interval owned by DuplicateConfirmation and torn
  down by its effect cleanup
- handle a rejected duplicate request, which previously left the prompt
  disabled with no job to re-enable it

Destination tab:
- keep an omitted `destination_tab_id` nil instead of coercing it to 0, so the
  job falls back to the destination course's default tab
- redirect to the tab the copies actually landed in, and to that tab's own
  category, rather than to `tab=0` or a tab from another course

Adoptions:
- record them before the duplicates' `after_duplicate_save` callbacks run, so a
  failure rolls the transaction back before that work rather than undoing it
- move the per-copy rule onto `Course::Assessment#record_marketplace_adoption`,
  next to the association and `initialize_duplicate`; the service keeps only the
  sweep, which the `after_duplicate_save` hook cannot replace (it never runs
  during course duplication)

Messages:
- pass `n` to the pluralised `duplicateTitle` and `duplicateFailed`, which were
  formatted without it
- give `duplicateBody`'s `other` branch the count its locale strings already had
- drop an `as never` cast that disabled type-checking on the test's props
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants