feat(erli): category/parameter browsing from Allegro's catalog without a required Allegro connection#1407
Merged
Conversation
…ient (#1388) * feat(erli): add Erli-owned Allegro client_credentials category-catalog client Adds AllegroCategoryCatalogClient, a self-contained HTTP client that lets an Erli connection browse Allegro's public /sale/categories and /sale/categories/{id}/parameters catalog via an Allegro app's grant_type=client_credentials token, with no dependency on @openlinker/integrations-allegro (per ADR-030) and no requirement for the operator to own a real Allegro seller connection. Extends ErliConnectionConfig with an optional allegroEnvironment field and ErliCredentials with optional allegroClientId/allegroClientSecret fields to carry the Allegro app credentials alongside Erli's own apiKey. Part of #1381, sub-task 1 of 3 (see #1383, #1384). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RpsVFTpKZ1HoowEUKzbnWu Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(erli): address PR #1388 review findings on AllegroCategoryCatalogClient - Extract inline wire-shape types to allegro-category-catalog-client.types.ts, matching the erli-http-client.ts / erli-http-client.types.ts split. - Add a single-flight guard around token acquisition so two concurrent fetchCategories/fetchCategoryParameters calls on a cold cache issue one grant_type=client_credentials request, not two. - Treat a token response missing expires_in as already-expired instead of caching it indefinitely. - Add tests for the 401/403 branch on a data call (categories/parameters), distinct from the token-endpoint rejection branch already covered. - Add a cross-plugin mapper parity test suite that runs Allegro's real sandbox category-parameters fixture through this client's copy of toNeutralCategoryParameter, mirroring assertions from allegro-category-parameter.mapper.spec.ts, with cross-referencing comments in both spec files so a future mapper change prompts updating both. - Replace the inline 'sandbox' | 'production' union with an as const + runtime-array AllegroCatalogEnvironment type in erli-connection.types.ts, matching AllegroConnectionConfig's own convention. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> --------- Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… credential/config validation (#1399) * feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation (#1383) ErliOfferManagerAdapter now wires fetchCategories/fetchCategoryParameters as optional instance properties (never a static implements clause) so isCategoryBrowser/isCategoryParametersReader reflect whether a specific Erli connection has configured a valid Allegro app credential pair, per ADR-031. ErliAdapterFactory constructs the shared AllegroCategoryCatalogClient only when both allegroClientId and allegroClientSecret resolve to non-empty strings, resolving config.allegroEnvironment (default 'production'). The credentials shape validator now enforces "both or neither" for the Allegro credential pair, and the config shape validator restricts allegroEnvironment to 'sandbox' | 'production'. Verified CategoriesCacheService and ListingsController need no changes: both already resolve capabilities generically via getCapabilityAdapter + is* type guards, so a misconfigured Erli connection behaves like today's "adapter doesn't implement this capability" case. Added an integration test exercising the real ErliAdapterFactory + ErliOfferManagerAdapter + AllegroCategoryCatalogClient (fetch stubbed) through the production adapter-resolution seam and the real HTTP endpoint, covering configured / unconfigured / partially-configured connections. Confirmed the existing #1367 bulk-wizard capability-gate test passes unmodified. Part of #1381, sub-task 2 of 4 (depends on #1382, merged; see also #1384, #1387). Signed-off-by: Norbert Kulus <42zeroo@gmail.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(erli): add allegroCategoryAccessEnabled config flag as the FE-visible signal connection.supportedCapabilities turned out to be a static, per-adapterKey manifest value rather than computed per-connection-instance, so it can't distinguish an Erli connection with configured Allegro app credentials from one without. Add a non-secret ErliConnectionConfig.allegroCategoryAccessEnabled boolean for the frontend to read instead (see ADR-031 "Correction"); the write path that sets allegroClientId/allegroClientSecret must set/clear this flag atomically (tracked in #1384's scope). Also fixes stale ADR-030 references (renumbered to ADR-031 to avoid a collision with #1307) in allegro-category-catalog-client.ts and erli-connection.types.ts. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(erli): hoist Allegro category-token cache to host.cache + dedupe credential resolve (#1399 review) AllegroCategoryCatalogClient built a fresh in-memory token cache per adapter instance, but ErliAdapterFactory builds a fresh instance per getCapabilityAdapter call — so fetchCategoryParameters paid a full client_credentials OAuth round-trip on every request. The client now persists the acquired token in the optional CachePort (host.cache), keyed by clientId, with a TTL matching the token's remaining lifetime, so it survives across per-request instances and processes. Also collapses ErliAdapterFactory.createAdapters's two credentialsResolver.get calls (one for apiKey, one for the Allegro pair) into one shared resolve. Addresses both IMPORTANT findings from the PR #1399 review round (Piotr's token-cache note and the self-review's double-resolve finding). Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> --------- Signed-off-by: Norbert Kulus <42zeroo@gmail.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
…parameters steps (#1401) * feat(erli,web): checkbox-reveal Allegro credentials panel + offer wizard category/parameters steps Adds the operator-facing half of the Erli-owned Allegro category-catalog feature (#1384, part of epic #1381, depends on #1382/#1383): - ErliCredentialsPanel gains a "Browse Allegro categories when creating Erli offers" checkbox that reveals Client ID / Client Secret fields (masked, show/hide toggle). Saving sequences two existing mutations from one click: useUpdateConnectionCredentialsMutation (credentials pair, merged with apiKey) then useUpdateConnectionMutation (config patch for allegroCategoryAccessEnabled) — credentials first so a failure never leaves the flag advertising access the backend can't serve. - ErliCreateOfferWizard renders the reused Allegro CategoryPicker + CategoryParametersStep (fed by the existing useCategoryParametersQuery) as dedicated Category / Category-parameters steps when connection.config.allegroCategoryAccessEnabled is true — per ADR-031's correction, this per-connection-instance config flag is the FE gating signal, not the static, per-adapterKey connection.supportedCapabilities. Falls back to today's plain-text field + a link to the connection's edit page otherwise. Stepper labels and needsProductParameters become capability-conditional; erliCreateOfferSchema gains a parameters slice serialized into overrides.parameters on submit (no BE change needed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RpsVFTpKZ1HoowEUKzbnWu Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(erli,web): address PR #1401 review findings - Gate `canSubmit` in ErliCredentialsPanel on `allegroEnabled` so unchecking the Allegro-access box after typing (but not saving) Client ID/Secret disables Save instead of showing a false-positive "Credentials saved" toast while discarding the typed secret. - Drop the checkbox's invalid inline `accentColor: var(--accent)` (token doesn't exist) — `index.css` already applies `accent-color: var(--accent-primary)` globally. - Restore category-parameter values on Erli offer retry by reusing the Allegro retry mapper's `readParameters` heuristic (exported) instead of always prefilling an empty `parameters` object — both platforms persist the same neutral `overrides.parameters` shape. Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * fix(erli,connections): merge credential rotation + enforce category pick (#1401 second review) - ConnectionService.updateCredentials now merges the submitted payload onto the existing stored credentialsJson instead of replacing it wholesale, so rotating the plain Erli apiKey no longer silently wipes a previously configured allegroClientId/allegroClientSecret pair (and enabling Allegro access without touching apiKey no longer fails shape validation). - ErliCreateOfferWizard now blocks Next on the Category step until a category is actually selected, mirroring AllegroCreateOfferWizard's required categoryId field, instead of silently falling through to Category-parameters/Review with an empty category. - Carried over the approved mockup's helper copy under the Allegro Client ID / Client Secret fields and restored the checkbox's original hint text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> * test(erli,docs): cover mutation-sequencing failure paths + close ADR-031 documentation gaps (#1401 third review) - erli-credentials-panel.test.tsx: add regression tests asserting the config patch never fires when the credentials write rejects, and that a config-patch rejection after a successful credentials write leaves the flag/fields intact with a retryable inline error - the two claims the PR description makes about "the trickiest part of this issue" were previously unverified by the suite. - ADR-031: add a second Correction noting the sequencing is two FE-orchestrated mutations, not a single backend write, since no endpoint accepts both the credential pair and the config flag together; also record the wizard's 3-step-vs-5-step topology and the deferred credential-verification indicator as explicit, confirmed scope cuts rather than implicit deviations from the approved mockup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> --------- Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…tegory access (#1387) (#1405) Extends the #1384 Erli credentials panel with a reuse-vs-manual choice: when the operator already has an Allegro connection, its clientId/clientSecret can be copied server-side into the Erli connection instead of registering a second Allegro app. The raw clientSecret never round-trips through the browser. The credential-copy logic is dispatched through a new, platform-neutral ConnectionCredentialsRewriterPort + registry (mirroring the existing ConnectionConfigShapeValidatorPort/ConnectionCredentialsShapeValidatorPort pair) so the generic ConnectionService.updateCredentials stays unaware that Allegro or Erli exist. The Erli-specific resolution lives entirely in ErliAllegroCredentialsRewriterAdapter, registered from a companion ErliCredentialsRewriterModule (mirrors ErliWebhookProvisioningModule) since it needs ConnectionPort, which is intentionally outside the plugin-neutral HostServices bag. Closes #1387 Signed-off-by: norbert-kulus-blockydevs <norbert.kulus@blockydevs.com>
norbert-kulus-blockydevs
force-pushed
the
epic/1381-erli-allegro-category-catalog
branch
from
1b3f45c to
fbe98f0
Compare
piotrswierzy
approved these changes
Jul 8, 2026
piotrswierzy
left a comment
piotrswierzy
left a comment
Collaborator
There was a problem hiding this comment.
/pr-review — feat(erli): Erli category/parameter browsing from Allegro's catalog (#1381 epic integration)
Summary
This is the epic integration PR to main for #1381, rolling up the four sub-tasks I reviewed and approved individually. Provenance confirms it's exactly the sum of those slices with no surprise additions, and — importantly — it demonstrably resolves the FE-gating concern I carried through the entire epic. Approve.
Provenance — clean sum of the reviewed chain ✅
The four commits map 1:1 to the approved sub-PRs:
bf14c289— #1388 Erli-owned Allegroclient_credentialscatalog clientf39a857b— #1399 per-connection capability wiring + credential/config validators8ed9165c— #1401 FE checkbox + offer-wizard category/parameters steps (the #1384 FE work)fbe98f0f— #1405 credential reuse viaConnectionCredentialsRewriterPort
The 42-file surface is entirely erli / integrations / the FE plugin, plus a lint-script tweak (check-plugin-guide-quotes.mjs) and a one-line offer-form helper — no unexpected cross-context files. Each slice already passed lint/type-check/unit individually.
The FE-gate concern (raised on #1380 and #1399) is resolved correctly ✅
Through the whole epic I flagged one forward risk: the Erli FE must not gate the category UI on the static, per-adapterKey connection.supportedCapabilities (which can't reflect a per-connection config), the way the Allegro bulk wizard does. This PR does the right thing:
- The Erli panel gates on the per-connection
connection.config.allegroCategoryAccessEnabledflag (erli-credentials-panel.tsx:65), a genuine per-connection signal — exactly the "surface a per-connection effective-capability" option I recommended. - A grep for
supportedCapabilities.*CategoryBrowser/includes('CategoryBrowser')across the Erli FE returns nothing — they did not fall into the static-manifest trap. - The write is fail-safe ordered (credentials first, then patch
allegroCategoryAccessEnabled), and the tests cover enable/disable directions, the reuse path (asserting the raw secret never leaves the browser), and the config-patch-rejects-after-credentials-succeed error case.
That's the last open thread from the epic, closed cleanly.
Already-verified (per-slice, still holding)
- ADR-031 per-instance capability model (methods as optional instance properties, structural
is*guards) — #1399. - Server-side-only secret handling with a platform-neutral rewriter registry, admin-gated
204credentials endpoint — #1405. - Mapper-drift parity guard, single-flight + expiry-aware token cache — #1388.
🟢 Notes
- CI-green is the gate. The "CI green on this PR" box is unticked — since this merges the whole chain to
main, confirm the fullpnpm lint(incl.check:invariants— cross-context imports, migration timestamps) +testpass on the merged whole before merging. No schema/migration lands in this epic (the capability is a config-JSON flag, not a column), so there's no timestamp-ordering risk. - The forward note from #1399 stands as a future item, not a blocker: the app-token cache is per-adapter-instance; if category-parameter browsing volume grows, hoist it to
host.cache. mergeable_state: blockedis branch protection (approval + checks), not content.
Verdict
✅ Approve — a faithful integration of a fully-reviewed chain that also lands the correct resolution to the epic's one open design question (per-connection FE gating). Merge once CI is green on the combined branch. This closes #1381.
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
2 participants
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.
Summary
client_credentialscategory-catalog HTTP client ([TASK] Integration — Erli-owned Allegro client_credentials catalog client #1382/feat(erli): Erli-owned Allegro client_credentials category-catalog client #1388)allegroCategoryAccessEnabledconfig flag ([TASK] Integration — Erli adapter capability wiring + credentials/config validators for Allegro category access #1383/feat(erli): wire per-connection Allegro category-catalog capability + credential/config validation #1399)ConnectionCredentialsRewriterPort([TASK] Integration+Frontend — Erli: reuse an existing Allegro connection's credentials for category access #1387/feat(erli): reuse an existing Allegro connection's credentials for category access (#1387) #1405)Closes #1381
Closes #1382
Closes #1383
Closes #1384
Closes #1387
Test plan