-
Notifications
You must be signed in to change notification settings - Fork 2
Listings: supportingCards install with examples but stay out of the detail view (CS-12116) #664
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
Changes from all commits
605bf14
3e65048
bea1e78
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,9 +98,14 @@ export default class ListingInstallCommand extends Command< | |
| // this is intentionally to type because base command cannot interpret Listing type from catalog | ||
| const listing = listingInput as Listing; | ||
|
|
||
| let examplesToInstall = listing.examples; | ||
| if (listing.examples?.length) { | ||
| examplesToInstall = await this.expandInstances(listing.examples); | ||
| // seed examples first so the primary example stays the first planned instance | ||
| let hasPrimaryExamples = (listing.examples?.length ?? 0) > 0; | ||
| let examplesToInstall = [ | ||
| ...(listing.examples ?? []), | ||
| ...(listing.supportingCards ?? []), | ||
| ]; | ||
|
Comment on lines
+102
to
+106
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Claude Code 🤖] Agreed on coverage, but the listing install/create/use acceptance modules are currently module.skip pending the skipped-catalog-tests re-enablement effort, so a new test here wouldn't execute. Supporting-card scenarios (copied on install, supporting-only listing leaves exampleCardId/selectedCodeRef unset) are on the list for when those modules are unskipped. |
||
| if (examplesToInstall.length) { | ||
| examplesToInstall = await this.expandInstances(examplesToInstall); | ||
| } | ||
|
|
||
| // side-effects | ||
|
|
@@ -125,9 +130,11 @@ export default class ListingInstallCommand extends Command< | |
| resolver, | ||
| virtualNetwork, | ||
| ); | ||
| let firstInstance = r.instancesCopy[0]; | ||
| exampleCardId = join(realmUrl, firstInstance.lid); | ||
| selectedCodeRef = firstInstance.targetCodeRef; | ||
| if (hasPrimaryExamples) { | ||
| let firstInstance = r.instancesCopy[0]; | ||
| exampleCardId = join(realmUrl, firstInstance.lid); | ||
| selectedCodeRef = firstInstance.targetCodeRef; | ||
| } | ||
| return r; | ||
| }) | ||
| .addIf(listing.skills?.length > 0, (resolver: ListingPathResolver) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,17 +78,20 @@ export default class ListingUseCommand extends Command< | |
| }); | ||
| } | ||
|
|
||
| if (listing.examples) { | ||
| const sourceCards = (listing.examples as CardAPI.CardDef[]).map( | ||
| (example) => example, | ||
| ); | ||
| for (const card of sourceCards) { | ||
| await new CopyCardToRealmCommand(this.commandContext).execute({ | ||
| sourceCard: card, | ||
| targetRealm: realmUrl, | ||
| localDir, | ||
| }); | ||
| } | ||
| const sourceCards = [ | ||
| ...new Map( | ||
| [ | ||
| ...((listing.examples ?? []) as CardAPI.CardDef[]), | ||
| ...((listing.supportingCards ?? []) as CardAPI.CardDef[]), | ||
| ].map((card) => [card.id ?? card, card]), | ||
| ).values(), | ||
| ]; | ||
| for (const card of sourceCards) { | ||
| await new CopyCardToRealmCommand(this.commandContext).execute({ | ||
| sourceCard: card, | ||
| targetRealm: realmUrl, | ||
| localDir, | ||
| }); | ||
| } | ||
|
Comment on lines
+81
to
95
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Claude Code 🤖] Fixed in bea1e78 — the copy loop now dedupes by card id across the two fields. Install and submission were already safe because expandInstances tracks visited ids. |
||
|
|
||
| if ('skills' in listing && Array.isArray(listing.skills)) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Claude Code 🤖] Same constraint as the install-test note: the listing-create acceptance module is currently module.skip, so classification coverage (non-instance open card lands in supportingCards, load-failure falls back to examples) will be added when that suite is re-enabled. The host-side modal behavior is covered by tests in cardstack/boxel#5536.