feat(admin): bulk import products from the PriceCharting collection - #299
Merged
Conversation
Adds Inventory > "Add from PC Collection", the bulk sibling of the existing single-item Add-from-PriceCharting page. The operator scans their own PriceCharting collection, filters it, and onboards many holdings in one pass through the SAME creation path — same per-grade price lookup, same single-item endpoint, same batch runner — so an imported product is indistinguishable from a hand-searched one. Shape follows what the live API actually returns (measured against the real collection, 9,000+ offers): - The collection is five figures and still growing, so the route serves ONE upstream page per request and hands back the cursor; the admin page scans with live counters, a Stop/Continue control, and a 300-row render cap. A server-side walk would have been a multi-minute request that reports no progress and dies on any timeout. - `seller` is NOT required: the API token identifies the account (a wrong seller 400s upstream with "user not found"). PRICECHARTING_SELLER_ID stays supported for the case where the token's account is not the owner. - Grade tags upstream are free text — "Ungraded", "PSA 10", "Graded 9", "BGS 10 Black", "ACE 10", plus the video-game condition family. gradeForIncludeString maps them onto PRICE_FIELDS labels and returns null for anything PriceCharting has no field for, so the operator picks the tier rather than the import guessing on a money field. Money and inventory correctness: - market_value is ALWAYS the live per-grade FMV re-read through /admin/pricecharting/product, never the offer's own valuation. The nightly sync skips moves beyond MAX_SYNC_DELTA_RATIO, so an offer-valued import would not be corrected later — it would stick. - Holdings that would mint the same product (same PriceCharting product at the same grade tag) are merged into one draft with units summed. Importing them separately collided on the generated handle and undercounted stock. - The offer's own grade tag wins over the price tier when recording grader/grade: a collection offer IS the slab, unlike a search hit. Half grades stay blank rather than minting an off-scale grade. Also: offer images are accepted only from PriceCharting's own bucket (the create step stores non-PC URLs verbatim instead of ingesting them), and CardPokemonFields takes an idPrefix so N pickers on one screen stop sharing three hardcoded DOM ids. Tests: unit coverage for the tag mapping and offer normalization from real upstream rows, a stubbed-fetch spec pinning the outgoing request (status, cursor pass-through, optional seller), and an http spec for mounting + auth + the missing-token setup message. The happy path was verified once against the live API through the booted route (cursor pages disjoint; a "Graded 9" holding priced at its Grade 9 tier) — that spec is not committed, as CI has no token. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Reviewer's GuideImplements a new admin flow to bulk-import products from a seller’s PriceCharting collection, including backend collection proxy/normalization and a frontend inventory page that reuses the existing single-item creation path while handling pagination, filtering, per-offer grade mapping, and draft batching. Sequence diagram for bulk import from PriceCharting collectionsequenceDiagram
actor AdminUser
participant AddFromPcCollectionPage
participant AdminRest as admin-rest
participant PcCollectionRoute as pc_collection_route_GET
participant PriceChartingAPI as pricecharting_api
participant ProductsBatch as useCreateProductsFromPriceChartingBatch
AdminUser->>AddFromPcCollectionPage: click_scan
AddFromPcCollectionPage->>AdminRest: getPriceChartingCollection(cursor)
AdminRest->>PcCollectionRoute: GET /admin/pricecharting/collection
PcCollectionRoute->>PriceChartingAPI: pcFetch /api/offers status=collection
PriceChartingAPI-->>PcCollectionRoute: offers_page + cursor
PcCollectionRoute->>PcCollectionRoute: normalizeOffer(raw)
PcCollectionRoute-->>AdminRest: {offers, cursor}
AdminRest-->>AddFromPcCollectionPage: PcOffer[]
AddFromPcCollectionPage->>AddFromPcCollectionPage: filter_offers(cardsOnly, gradedOnly, text)
AdminUser->>AddFromPcCollectionPage: select_offers
AdminUser->>AddFromPcCollectionPage: click_import_selected
loop per_group(product_id, include)
AddFromPcCollectionPage->>AdminRest: getPriceChartingProduct(product_id)
AdminRest-->>AddFromPcCollectionPage: product_with_prices
AddFromPcCollectionPage->>AddFromPcCollectionPage: graderFromInclude(include)
AddFromPcCollectionPage->>AddFromPcCollectionPage: gradeToGrader(pcGrade)
AddFromPcCollectionPage->>AddFromPcCollectionPage: build Draft
end
AdminUser->>AddFromPcCollectionPage: complete_drafts(pixel_pokemon_id, stock,...)
AddFromPcCollectionPage->>AddFromPcCollectionPage: toItem(Draft) -> PcQueueItem
AdminUser->>AddFromPcCollectionPage: click_save_drafts
AddFromPcCollectionPage->>ProductsBatch: mutateAsync(PcQueueItem[])
ProductsBatch-->>AddFromPcCollectionPage: {created, skipped}
AddFromPcCollectionPage-->>AdminUser: toasts + updated_inventory
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds PriceCharting collection retrieval and admin import support with offer normalization, seller filtering, grading helpers, cursor validation, draft grading, product handle generation, localized copy, and namespaced Pokémon picker identifiers. ChangesPriceCharting collection import
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The new AddFromPcCollectionPage component is very large and mixes scanning, filtering, importing, and draft editing logic in one file; consider extracting some of this into reusable hooks or smaller subcomponents to simplify maintenance and future changes.
- The cursor length guard in the collection route currently just drops overlong values; you may want to add some lightweight logging/metrics around that branch so it’s easier to detect and debug upstream or client-side issues that produce invalid cursors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new AddFromPcCollectionPage component is very large and mixes scanning, filtering, importing, and draft editing logic in one file; consider extracting some of this into reusable hooks or smaller subcomponents to simplify maintenance and future changes.
- The cursor length guard in the collection route currently just drops overlong values; you may want to add some lightweight logging/metrics around that branch so it’s easier to detect and debug upstream or client-side issues that produce invalid cursors.
## Individual Comments
### Comment 1
<location path="backend/apps/admin/src/routes/products/from-pc-collection/page.tsx" line_range="80-91" />
<code_context>
+ * the created handle is name-grader-grade-pc_product_id, so importing them
+ * separately would collide (the second create fails) and the units held would
+ * be undercounted. Grouped on import, with stock summed instead. */
+const productKey = (o: PcOffer): string => `${o.product_id}|${o.include}`;
+
+/** Graded in PriceCharting's sense: anything it did not tag "Ungraded". The
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Normalize the include string when building productKey so semantically identical tags dedupe correctly.
Because `productKey` uses the raw `include` value, minor formatting differences (e.g. casing, trailing spaces, or extra spaces) will cause logically identical offers to be grouped separately, which can undercount units by preventing drafts from merging. Please normalize `include` (trim, lowercase, and consider collapsing internal whitespace) before building `productKey` so equivalent tags consistently dedupe.
```suggestion
const offerKey = (o: PcOffer): string =>
o.offer_id ?? `${o.product_id}|${o.include}`;
/** Normalize the include/grade tag so semantically identical tags dedupe:
* - trim leading/trailing whitespace
* - lowercase
* - collapse internal whitespace runs to a single space */
const normalizedInclude = (include: string): string =>
include.trim().toLowerCase().replace(/\s+/g, " ");
/** Two holdings of the SAME product at the SAME grade tier become ONE product:
* the created handle is name-grader-grade-pc_product_id, so importing them
* separately would collide (the second create fails) and the units held would
* be undercounted. Grouped on import, with stock summed instead. */
const productKey = (o: PcOffer): string =>
`${o.product_id}|${normalizedInclude(o.include)}`;
/** Graded in PriceCharting's sense: anything it did not tag "Ungraded". The
* slab tier is what a gacha storefront onboards, so the table defaults to it. */
const isGraded = (o: PcOffer): boolean => !/^\s*ungraded\s*$/i.test(o.include);
```
</issue_to_address>
### Comment 2
<location path="backend/apps/admin/src/routes/products/from-pc-collection/page.tsx" line_range="244-254" />
<code_context>
+ }
+ };
+
+ const matches = useMemo(() => {
+ const q = filter.trim().toLowerCase();
+ return (offers ?? []).filter((o) => {
+ if (cardsOnly && !o.is_card) return false;
+ if (gradedOnly && !isGraded(o)) return false;
+ if (q === '') return true;
+ return (
+ o.name.toLowerCase().includes(q) || o.set.toLowerCase().includes(q)
+ );
+ });
+ }, [offers, filter, cardsOnly, gradedOnly]);
+
+ const visible = matches.slice(0, MAX_VISIBLE_ROWS);
+
+ // Picks survive a filter change: selecting a row and then narrowing the
</code_context>
<issue_to_address>
**suggestion:** Filtering logic ignores the include/condition fields, which might limit findability of certain offers.
Currently the filter only matches on `name` and `set`. Since operators may search by grade or condition (e.g. "PSA 10", "Ungraded"), consider also matching against `include` and/or `condition` so users can more easily target specific tiers within large collections.
```suggestion
const matches = useMemo(() => {
const q = filter.trim().toLowerCase();
return (offers ?? []).filter((o) => {
if (cardsOnly && !o.is_card) return false;
if (gradedOnly && !isGraded(o)) return false;
if (q === '') return true;
const { name, set, include, condition } = o;
return (
name.toLowerCase().includes(q) ||
set.toLowerCase().includes(q) ||
(include && include.toLowerCase().includes(q)) ||
(condition && condition.toLowerCase().includes(q))
);
});
}, [offers, filter, cardsOnly, gradedOnly]);
```
</issue_to_address>
### Comment 3
<location path="backend/packages/api/src/api/admin/pricecharting/__tests__/collection-offers.unit.spec.ts" line_range="88-101" />
<code_context>
+ });
+});
+
+describe('penniesToUsd', () => {
+ it('converts integer pennies from a number or a string', () => {
+ expect(penniesToUsd(1999)).toBe(19.99);
+ expect(penniesToUsd('1999')).toBe(19.99);
+ expect(penniesToUsd('$1,999')).toBe(19.99);
+ });
+
+ it('treats absent / zero / junk as no price', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Extend `penniesToUsd` tests to cover negative and non-integer values.
Current tests only exercise standard integer inputs plus zero/null/undefined/junk strings. Since `penniesToUsd` also handles negatives and non-finite values, please add assertions for cases like `-100`, `1.5`, `Number.POSITIVE_INFINITY`, and a whitespace-padded string (e.g. `' 1999 '`) to capture the expected behavior and protect against regressions if the parsing logic changes.
```suggestion
describe('penniesToUsd', () => {
it('converts integer pennies from a number or a string', () => {
expect(penniesToUsd(1999)).toBe(19.99);
expect(penniesToUsd('1999')).toBe(19.99);
expect(penniesToUsd('$1,999')).toBe(19.99);
expect(penniesToUsd(' 1999 ')).toBe(19.99);
});
it('handles negative and non-integer values', () => {
expect(penniesToUsd(-100)).toBe(-1);
expect(penniesToUsd(1.5)).toBe(0.02);
expect(penniesToUsd(Number.POSITIVE_INFINITY)).toBeNull();
});
it('treats absent / zero / junk as no price', () => {
expect(penniesToUsd(0)).toBeNull();
expect(penniesToUsd(null)).toBeNull();
expect(penniesToUsd(undefined)).toBeNull();
expect(penniesToUsd('abc')).toBeNull();
});
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/admin/src/routes/products/from-pc-collection/page.tsx`:
- Around line 614-621: Add an accessible aria-label to the row Checkbox in the
product table, deriving the label from the current offer associated with key.
Keep the existing checked state and setSelected behavior unchanged, and ensure
each row’s label identifies its offer clearly.
- Around line 80-91: The API normalization boundary in normalizeOffer must
reject rows whose offer-id is null, empty, or whitespace-only before they reach
the admin offers list; update the check there and ensure collection/route.ts
excludes those normalization results. Tighten PcOffer.offer_id from nullable to
string, while preserving offerKey and productKey behavior for valid offers.
- Around line 403-407: Update saveDrafts to wrap batchCreate.mutateAsync in a
try/catch, showing err.message through the existing error-toast mechanism when
the batch request fails, then clear drafts as required. Preserve the current
created/skipped handling for successful requests.
- Around line 316-363: Update the draft creation flow around rows.push and
getTcgCardMeta so each newly created draft is appended to drafts state
immediately before starting the metadata lookup, allowing the continuation to
match its key. Remove the trailing bulk append of rows after the loop, while
preserving the existing metadata fill-only behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d49c4a85-77af-454e-9409-b580669c1c09
📒 Files selected for processing (14)
backend/apps/admin/src/components/GraderGradeSelect.tsxbackend/apps/admin/src/i18n/en.jsonbackend/apps/admin/src/lib/admin-rest.tsbackend/apps/admin/src/lib/format.test.tsbackend/apps/admin/src/lib/format.tsbackend/apps/admin/src/routes/cards/CardPokemonFields.tsxbackend/apps/admin/src/routes/products/from-pc-collection/page.tsxbackend/packages/api/.env.templatebackend/packages/api/integration-tests/http/pc-collection.spec.tsbackend/packages/api/src/api/admin/pricecharting/__tests__/collection-offers.unit.spec.tsbackend/packages/api/src/api/admin/pricecharting/__tests__/collection-route.unit.spec.tsbackend/packages/api/src/api/admin/pricecharting/collection-offers.tsbackend/packages/api/src/api/admin/pricecharting/collection/route.tsbackend/packages/api/src/modules/packs/pricecharting-grades.ts
Label prefill was a real bug: drafts were held in a local array and committed to state only after the whole import loop finished, while the fire-and-forget getTcgCardMeta continuation patched state by key. Any response arriving while later products were still being priced found no such key and no-oped, so year/rarity were dropped for every draft but (maybe) the last. Each draft is now committed as it is created. Also from review: - Require a non-empty per-row offer id at the API normalization boundary and tighten PcOffer.offer_id to string. The admin keyed rows by offer_id with a fallback to product+tag — which is exactly productKey, so two distinct holdings of the same card at the same grade shared a key, one was deduped away during the scan, and the units held were undercounted. - Normalize the grade tag (case + whitespace) when grouping holdings, so semantically identical tags still merge into one product. - Search the grade tag and condition too: inside a five-figure collection "psa 10" is as natural a query as a card name. - Catch a rejected batch save. Per-item failures already fold into `skipped`, but a rejection of the mutation itself was an unhandled promise from an onClick — nothing happened and the drafts stayed on screen unexplained. - Give each row checkbox an accessible name (card + grade tag). - penniesToUsd: cover padded strings, sub-cent rounding, and the no-price cases. Note the review's suggested `-100 -> -1` is wrong: a negative valuation is not a price, and the guard returns null — asserted as such. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The collection page was importing STRANGERS' cards. PriceCharting's /api/offers takes `status` as its only required parameter and the API token authenticates but does NOT scope the query to the token's account, so a call without `seller` returns every user's offers with that status. Verified live: a seller-less call came back with thousands of other people's cards (baseball, Magic, video games), every row carrying `user-viewing-own-offers: false`, while the operator's own PriceCharting collection page showed 0 items. An earlier commit removed the seller requirement on the strength of two bad inferences: that a 200 with data meant "our account", and that `seller=me` 400ing meant the parameter was unnecessary. It 400s because the parameter takes a 26-character user id, not a username — as does the account's own display name. The route now refuses to run without PRICECHARTING_SELLER_ID, with a message naming where to find the id (the URL of pricecharting.com/selling-available). Refusing is the only safe default here: the seller-less response is not a degraded result, it is confidently wrong data that would onboard other people's cards as our stock at our prices. Specs updated to match: the unit spec now asserts the seller is sent and that a missing one short-circuits before any fetch; the http spec covers both configuration guards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/packages/api/.env.template`:
- Around line 73-78: Add the missing PRICECHARTING_SELLER_ID= declaration
directly beneath the existing PriceCharting user ID documentation in the
environment template, preserving the comments and leaving the value empty for
users to configure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9855a737-4edb-4fbc-99fe-e41653f20d36
📒 Files selected for processing (9)
backend/apps/admin/src/i18n/en.jsonbackend/apps/admin/src/lib/admin-rest.tsbackend/apps/admin/src/routes/products/from-pc-collection/page.tsxbackend/packages/api/.env.templatebackend/packages/api/integration-tests/http/pc-collection.spec.tsbackend/packages/api/src/api/admin/pricecharting/__tests__/collection-offers.unit.spec.tsbackend/packages/api/src/api/admin/pricecharting/__tests__/collection-route.unit.spec.tsbackend/packages/api/src/api/admin/pricecharting/collection-offers.tsbackend/packages/api/src/api/admin/pricecharting/collection/route.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- backend/apps/admin/src/lib/admin-rest.ts
- backend/apps/admin/src/i18n/en.json
- backend/packages/api/src/api/admin/pricecharting/tests/collection-offers.unit.spec.ts
- backend/packages/api/src/api/admin/pricecharting/collection-offers.ts
- backend/apps/admin/src/routes/products/from-pc-collection/page.tsx
… checks Second review round. The headline fix is a collision the FIRST round's fix did not actually cover. Product handle collided across generic tiers. The handle is `name-grader-grade-pc_product_id`, and PriceCharting drops the grading company below its own top-tier price fields — so a "Graded 9" holding imports with grader='' AND grade='' (a grade is unrepresentable without a grader, §3a). "Ungraded" and "Graded 9" of the same card therefore slugged identically (charizard-v-79-836945 for both) and the second create died on the unique handle/sku index. Grouping picks by (product_id, tag) did not help: that key is finer than the handle key, and it does nothing at all across separate batches, which is exactly how this page is meant to be used. Fixed at the one place every caller routes through — buildPcProductHandle falls back to the PriceCharting tier when neither grader nor grade is asserted. Handles that carry a grader are byte-identical to before, so nothing in the catalog moves; only previously-impossible handles change shape. Re-importing the same holding at the same tier still collides, which is what keeps the unique index working as the idempotency guard. Ownership is now verified per row rather than assumed from configuration. PRICECHARTING_SELLER_ID being SET only proves it is set; a stale or typo'd 26-char id returns another account's holdings that import exactly like ours. Upstream marks every row with `user-viewing-own-offers` — the field that exposed the original site-wide-feed bug — so the route drops rows explicitly marked as not ours and reports the count, which the page surfaces as a warning naming the likely cause. An absent flag is tolerated: a correctly-scoped response with rows in it has not been observable while the collection is empty. Grader/grade is now one operator-controlled value. It was derived by two different rules — display used the offer tag alone, submit used the tag with a tier fallback — so "BGS 10 Black" showed "no grader" while submitting BGS 10, and "Graded 9" (the most common graded tag upstream) submitted as raw: no isGraded, no slab bake, a real PSA 9 landing in the catalog as ungraded with Grade-9 FMV and no way to correct it on the page. Each draft now mounts GraderGradeSelect, seeded from the tag or the tier, and that single value is both shown and submitted. Also: a failed scan no longer renders as "your collection is empty" (the 503 message is shown in place, and the partial badge is suppressed); numeric offer-ids are accepted rather than silently emptying the page; clear-selection for picks hidden behind a filter; label fields capped at the server's 64 chars; filter placeholder now mentions the grade tag it already searched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… the scan Review feedback: the guard on the upstream cursor silently dropped an overlong value and answered with page 1. That restart is invisible to the scan loop — its dedupe discards every row it already has — so the loop spins to the page cap looking like a hang. A cursor that long is not a cursor, so the route 400s with a message telling the operator to rescan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Addressing the two high-level review points (no inline thread for these): Cursor guard — done, and stronger than logging ( Component size — declining, with reasoning. Also for the record: CodeRabbit hit its review rate limit ("next review available in 26 minutes") before it could review 🤖 Addressed by Claude Code |
|
Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/packages/api/src/api/admin/pricecharting/collection-offers.ts`:
- Around line 80-83: Update the offerId normalization in the collection-offers
parsing flow to accept only string values or finite numeric values from
raw['offer-id']; trim strings and stringify valid numbers, while converting
objects, booleans, nullish values, and non-finite numbers to the existing
empty-value path so they cannot be deduplicated as "[object Object]".
In `@backend/packages/api/src/api/admin/pricecharting/collection/route.ts`:
- Around line 66-71: Update the cursor validation in the collection route so a
supplied req.query.cursor is rejected with HTTP 400 when it is not a string,
rather than coerced to an empty cursor; continue trimming and enforcing
MAX_CURSOR_LENGTH for valid strings. Add a route test covering a non-string
cursor input and verify PriceCharting is not called.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 17e1d656-6265-4cfd-9856-33f040f9916d
📒 Files selected for processing (9)
backend/apps/admin/src/i18n/en.jsonbackend/apps/admin/src/lib/admin-rest.tsbackend/apps/admin/src/routes/products/from-pc-collection/page.tsxbackend/packages/api/src/api/admin/pricecharting/__tests__/collection-offers.unit.spec.tsbackend/packages/api/src/api/admin/pricecharting/__tests__/collection-route.unit.spec.tsbackend/packages/api/src/api/admin/pricecharting/collection-offers.tsbackend/packages/api/src/api/admin/pricecharting/collection/route.tsbackend/packages/api/src/workflows/steps/__tests__/pc-product-handle.unit.spec.tsbackend/packages/api/src/workflows/steps/create-product-from-pricecharting.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/apps/admin/src/i18n/en.json
Two review findings, both the same shape: a value coerced instead of refused, producing a wrong result that looks like a normal one. - offer-id: String() on an object yields "[object Object]" — a key EVERY such row would share. The scan keys rows by offer_id, so two distinct holdings would dedupe into one and undercount the units held. Now only a string or a finite number is accepted; anything else takes the existing drop path. - cursor: a repeated ?cursor=a&cursor=b arrives as an ARRAY, which was coerced to '' and answered with page 1. That restart is invisible to the scan loop (its dedupe discards every row it already holds) and spins to the page cap looking like a hang — exactly what the length guard already rejects, so a non-string now 400s the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Inventory › "Add from PC Collection" — the bulk sibling of the existing single-item Add from PriceCharting page. Scan your own PriceCharting collection, filter it, onboard many holdings in one pass through the same creation path (same per-grade price lookup, same single-item endpoint, same batch runner), so an imported product is indistinguishable from a hand-searched one.
Shape follows the live API, not assumptions
Measured against the real collection (9,000+ offers, 8,122 cards):
GET /admin/pricecharting/collectionserves one upstream page per request plus the cursor. The admin page scans with live counters, Stop/Continue, and a 300-row render cap. A server-side walk would be a multi-minute request that reports no progress and dies on any timeout.selleris REQUIRED/api/offerstakesstatusas its only mandatory parameter, so a seller-less call returns every user's offers — verified live: thousands of strangers' cards, alluser-viewing-own-offers: false, while the operator's own collection was empty. The route refuses to run withoutPRICECHARTING_SELLER_ID(a 26-char user id from the URL ofpricecharting.com/selling-available).Ungraded,PSA 10,Graded 9,Graded 9.5,BGS 10 Black,CGC 10,SGC 10,ACE 10, plus the video-game condition family (Item, Box, and Manual).gradeForIncludeStringmaps them ontoPRICE_FIELDSlabels and returnsnullwhen PriceCharting has no field for one — the operator picks the tier rather than the import guessing on a money field.Money + inventory correctness
market_valueis always the live per-grade FMV, re-read through/admin/pricecharting/product— never the offer's own valuation. The nightly sync skips moves beyondMAX_SYNC_DELTA_RATIO, so an offer-valued import wouldn't be corrected later, it would stick.name-grader-grade-pc_product_id); importing them separately collided and undercounted stock. They now collapse into one draft with units summed.BGS 9.5) stay blank rather than minting an off-scale grade.Also
CardPokemonFieldstakes an optionalidPrefix(defaults tocard, existing call sites unchanged) so N pickers on one screen stop sharing three hardcoded DOM ids.PSA_GRADESmoved tolib/format.tsbesidegradeToGrader.Verification
status=collection, cursor pass-through, seller omitted when unset, junk-row filtering, 502/503 paths)tscclean (api + admin), eslint cleanGraded 9mapped to tierGrade 9priced at $10.50 with an image. That spec is not committed — CI has no token.Reviewed via
/superpowers:requesting-code-review(zero Critical); all 5 Important findings fixed in this branch.Follow-up (not in this PR)
Mark already-imported offers in the table — needs a lookup of existing
metadata.pc_product_id. Would save the operator scanning 9,000 rows for what they already onboarded.🤖 Generated with Claude Code
Summary by Sourcery
Add an admin bulk-import flow that scans a seller’s live PriceCharting collection, normalizes offers server-side, and turns selected holdings into product drafts that share the existing single-item PriceCharting creation pipeline.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
foreign_dropped).Bug Fixes
offer-id.cursorvalidation and filter behavior to exclude foreign offers.Documentation / Chores