Skip to content

Eop card browser/refactor mentor followups - #3331

Draft
ayman-art wants to merge 4 commits into
masterfrom
eop-card-browser/refactor-mentor-followups
Draft

Eop card browser/refactor mentor followups#3331
ayman-art wants to merge 4 commits into
masterfrom
eop-card-browser/refactor-mentor-followups

Conversation

@ayman-art

@ayman-art ayman-art commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Description

Draft. Applying mentor follow-ups across Eop Card browser project PRs
Done:

  • extracted duplicate getEditions() method
  • Used deckservice.getCards() instead of manually listing editions

Current task:

  • card/order.ts should be generated by a script.

Resolved or fixed issue: related to #1322

AI Tool Disclosure

  • My contribution does not include any AI-generated content
  • My contribution includes AI-generated content, as disclosed below:
    • AI Tools: [e.g. GitHub CoPilot, ChatGPT, JetBrains Junie etc.]
    • LLMs and versions: [e.g. GPT-4.1, Claude Haiku 4.5, Gemini 2.5 Pro etc.]
    • Prompts: [Summarize the key prompts or instructions given to the AI tools]

Affirmation

@ayman-art

Copy link
Copy Markdown
Collaborator Author

I want to note that for refactoring:

decks: decks.set(lang, new Map([...mobileCards, ...webappCards, ...companionCards, ...eopCards]))

I used getCards() existing in deckService, which loops over all editions. So it also adds dbd, which wasn't existing in the old map.

@ayman-art

Copy link
Copy Markdown
Collaborator Author

@rewtd, @sydseter Question regarding cards/order.ts.. Before writing a script for this, do we really need controlling cards order per suit? If not, I can see the order is the same 2,3,4,5,6,7,8,9,X,J,Q,K,A for all suits. so we can use one shared ranking.

@sydseter

sydseter commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Yes, sure

@ayman-art

Copy link
Copy Markdown
Collaborator Author

Yes, sure

just confirming, you're good with one shared rank map, right?

/**
* Full edition name, used in card detail page titles and social meta tags in <svelte:head>.
*/
export const EDITION_FULL_NAMES: Record<string, string> = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This can be moved to yaml

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

This PR refactors how edition display names are produced for card-detail pages and simplifies card loading for the /cards index and /cards/[card] routes by relying on DeckService.getCards().

Changes:

  • Replaces repeated getEdition() logic in multiple Svelte pages with a shared EDITION_FULL_NAMES map.
  • Updates card page <svelte:head> title/social tags to use the shared full edition name.
  • Refactors /cards and /cards/[card] server loads to use deckService.getCards(lang) instead of manually merging per-edition maps.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cornucopia.owasp.org/src/routes/edition/[edition]/[card]/+page.svelte Uses shared EDITION_FULL_NAMES for head title/meta instead of a local helper.
cornucopia.owasp.org/src/routes/edition/[edition]/[card]/[version]/+page.svelte Same shared edition-name refactor for version-scoped card route.
cornucopia.owasp.org/src/routes/edition/[edition]/[card]/[version]/[lang]/+page.svelte Same shared edition-name refactor for lang-scoped card route.
cornucopia.owasp.org/src/routes/cards/+page.server.ts Simplifies deck assembly by using DeckService.getCards(lang).
cornucopia.owasp.org/src/routes/cards/[card]/+page.svelte Uses shared EDITION_FULL_NAMES for head title/meta instead of a local helper.
cornucopia.owasp.org/src/routes/cards/[card]/+page.server.ts Loads cards via DeckService.getCards(lang) and builds decks from that result.
cornucopia.owasp.org/src/lib/services/deckServiceConsts.ts Introduces EDITION_FULL_NAMES constants for consistent full edition display names.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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