Skip to content

fix: stop querying attribute fields removed in 3.24 - #6871

Merged
lkostrowski merged 9 commits into
mainfrom
lkostrowski/staging-schema-attribute-fields
Aug 27, 2026
Merged

fix: stop querying attribute fields removed in 3.24#6871
lkostrowski merged 9 commits into
mainfrom
lkostrowski/staging-schema-attribute-fields

Conversation

@lkostrowski

Copy link
Copy Markdown
Member

The shared attribute fragments still selected availableInGrid,
filterableInStorefront and storefrontSearchPosition. 3.24 removes them from
Attribute, so attribute list, details and update all failed validation against
the staging schema.

The previous deprecation gated every reader behind isMainSchema() but left the
selections in, on the assumption the fields would stay @deprecated on staging
rather than disappear. They disappeared.

Shared fragments are now schema-agnostic — nothing outside src/attributes/
ever read those three fields, which keeps products, product types, model types
and customers out of the split. The fields moved to a main-only
AttributeFacetedNavigation fragment spread by the five documents that render
them, each with a 3.24 twin that omits it:

  • queries.staging.ts: AttributeDetails, AttributeList,
    PageTypeAssignedAttributesForList, ProductTypeAssignedAttributesForList
  • mutations.staging.ts: AttributeUpdate

schemaAwareOperations.ts picks the document per build flag. Results stay typed
after the main variant, a superset, which is safe because the readers are
already gated. AttributeCreate needs no twin: it only selects id.

Under 3.23 the faceted navigation settings, the "Use in faceted search" column
and its sorting all behave exactly as before.

Two latent crashes on 3.24 are fixed on the way: attributePageForm called
.toString() on the now-absent storefrontSearchPosition, and a bookmarked
"Use in faceted search" sort URL sent an enum value 3.24 no longer defines.

codegen-staging and graphql.config now also load ./src/fragments/*.ts so the
twins resolve shared fragments, which doubles as a drift guard: every shared
fragment is validated against 3.24.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com## Scope of the change

  • I confirm I added ripples for changes (see src/ripples) or my feature doesn't contain any user-facing changes
  • I used analytics "trackEvent" for important events

lkostrowski and others added 2 commits August 26, 2026 12:18
The shared attribute fragments still selected `availableInGrid`,
`filterableInStorefront` and `storefrontSearchPosition`. 3.24 removes them from
`Attribute`, so attribute list, details and update all failed validation against
the staging schema.

The previous deprecation gated every reader behind `isMainSchema()` but left the
selections in, on the assumption the fields would stay `@deprecated` on staging
rather than disappear. They disappeared.

Shared fragments are now schema-agnostic — nothing outside `src/attributes/`
ever read those three fields, which keeps products, product types, model types
and customers out of the split. The fields moved to a main-only
`AttributeFacetedNavigation` fragment spread by the five documents that render
them, each with a 3.24 twin that omits it:

- `queries.staging.ts`: AttributeDetails, AttributeList,
  PageTypeAssignedAttributesForList, ProductTypeAssignedAttributesForList
- `mutations.staging.ts`: AttributeUpdate

`schemaAwareOperations.ts` picks the document per build flag. Results stay typed
after the main variant, a superset, which is safe because the readers are
already gated. AttributeCreate needs no twin: it only selects `id`.

Under 3.23 the faceted navigation settings, the "Use in faceted search" column
and its sorting all behave exactly as before.

Two latent crashes on 3.24 are fixed on the way: `attributePageForm` called
`.toString()` on the now-absent `storefrontSearchPosition`, and a bookmarked
"Use in faceted search" sort URL sent an enum value 3.24 no longer defines.

codegen-staging and graphql.config now also load `./src/fragments/*.ts` so the
twins resolve shared fragments, which doubles as a drift guard: every shared
fragment is validated against 3.24.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the hand-maintained 3.24 twins of the attribute documents with a
directive the client resolves at build time. Fields that exist in only one
schema version are marked in place:

    availableInGrid @lockSchema(schema: "main")

and are stripped from the document when the build targets the other version.
One source document, no twin to drift.

Stripping happens on the cache, not in a link. Links run after the cache, so a
link-level transform leaves InMemoryCache normalising against fields the API was
never asked for: every write logs "Missing field ...", every cache-first read
misses, and the query refetches on every mount. `transformDocument` feeds both
the cache and the link, so the two stay in sync. Apollo Client 3.8 has a
first-class `documentTransform` for this; the subclass goes away when we get
there.

Two passes over the document, because `removeDirectivesFromDocument` derives
`shouldRemoveField` from `directives.some(d => d.remove)` — a document-wide flag
rather than a per-config one. A single call carrying both configs deletes every
`@lockSchema` field, not just the locked-out ones.

`AttributeFacetedNavigation` keeps an unconditional `id`. A fragment whose every
field is stripped prints as an empty selection set, which `graphql.validate`
accepts and the API rejects as a syntax error; `lockSchema.test.ts` re-parses
after printing so that can't slip through again. The test resolves the five real
generated documents for both versions and validates them against schema-main and
schema-staging loaded without the directive declaration, so a surviving
`@lockSchema` fails as an unknown directive exactly as the API would.

Behaviour is unchanged on both schemas: readers of the three fields stay gated
behind `isMainSchema()` and the types remain a superset.

Deleted: queries.staging.ts, mutations.staging.ts, schemaAwareOperations.ts and
its test. Call sites are back on the generated hooks.

Not covered: `cache.readQuery`/`writeQuery` reach `read`/`write` directly and
bypass `transformDocument`. No caller does that with a `@lockSchema` document
today, and the gap is marked where the override would go.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a37b122

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
saleor-dashboard Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.78%. Comparing base (ce6966f) to head (a37b122).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/attributes/utils/applyAttributeListFilters.ts 0.00% 1 Missing ⚠️
src/attributes/utils/filterAssignedAttributes.ts 50.00% 1 Missing ⚠️
src/attributes/views/AttributeList/sort.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #6871     +/-   ##
=========================================
  Coverage   56.78%   56.78%             
=========================================
  Files        3542     3542             
  Lines       73484    73488      +4     
  Branches    18646    19030    +384     
=========================================
+ Hits        41725    41728      +3     
+ Misses      31077    29813   -1264     
- Partials      682     1947   +1265     
Flag Coverage Δ
storybook 49.51% <75.00%> (+<0.01%) ⬆️
units 51.98% <50.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lkostrowski and others added 2 commits August 26, 2026 13:45
Folds `AttributeFacetedNavigation` back into the shared fragments. The separate
fragment existed so the 3.24 twin documents could omit one spread; with the
twins gone and `@lockSchema` resolving per build, it only bought indirection.

The three fields return to exactly where they sat before any of this:
`filterableInStorefront` on `Attribute`, `availableInGrid` and
`storefrontSearchPosition` on `AttributeUpdateResult` and `AttributeDetails`.
That placement matches the readers — the list datagrid reads
`filterableInStorefront` off a row, `attributePageForm` reads all three off the
details query — and is narrower than putting all three on `Attribute`, which
this first did and which eight fixture type errors caught.

Gone with it: `src/attributes/types.ts`, whose `Partial<>` wrappers only existed
because the twins disagreed about which fields a row carried, and the `id`
placeholder that kept the fragment from collapsing into an empty selection set.

Resolution is now a plain `graphql` visitor. Apollo's
`removeDirectivesFromDocument` throws on the multi-fragment document files
codegen hands it, and its document-wide `remove` flag meant it could not keep
and drop `@lockSchema` fields in one pass.

The staging codegen no longer loads `./src/fragments/*.ts`. `documentTransforms`
do run ahead of validation, but the import-types preset never forwards them, so
hooksStaging validated the unresolved documents and failed. lockSchema.test.ts
guards this instead, and guards it better: it sweeps all 445 generated
documents, resolves `@lockSchema` for both versions, re-parses and validates
against the real schemas loaded without the directive declaration. That covers
the whole app rather than the attribute module, and would have caught the
original 3.24 breakage.

That sweep found one unrelated pre-existing break: ExportGiftCards still selects
`exportGiftCards` and `ExportGiftCardsInput`, both removed in 3.24. Listed in
KNOWN_324_GAPS so it is visible rather than silent; not fixed here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lkostrowski lkostrowski added the test deployment Deploy Pull Request to *.saleor.rocks environment label Aug 26, 2026
lkostrowski and others added 5 commits August 26, 2026 17:18
`exportGiftCards` is deprecated in 3.23 and removed from `Mutation` in 3.24, but
nothing gated the UI. On a 3.24 build the menu item rendered, the dialog opened,
and the failure only landed when the user pressed Export — on a request the API
rejects as invalid.

Gated at all three entry points behind `isMainSchema()`, matching how the
attribute module handles its removed fields:

- the "Export codes" item in the gift card list menu
- the "Export to email" button in the bulk-create success dialog
- the dialog render in GiftCardListDialogsProvider

The last one is not redundant. The dialog renders off `params.action === EXPORT`,
so a bookmarked ?action=EXPORT URL would still reach it with the menu item gone —
the same hole this branch already closed for the attribute sort field. Gating sits
at the render sites rather than inside GiftCardExportDialogContent, which calls
`useExportGiftCardsMutation` and `useGiftCardTotalCountQuery` up front and cannot
return early without breaking hook order.

ExportGiftCards stays in KNOWN_324_GAPS: this removes the document's
reachability, not the document, so codegen still emits it and lockSchema.test.ts
still sees it. The comment there now records why `@lockSchema` cannot cover this
case at all — the mutation is the operation's only root field, so stripping it
leaves an empty selection set, and `$input: ExportGiftCardsInput!` is a variable
definition the directive cannot reach.

playwright/tests/giftCards.spec.ts clicks `exportCodesMenuItem` in two tests. On
3.23 they are unaffected; on a 3.24 run they now fail at the click rather than at
the mutation. The Playwright suite is not schema-aware, so it is left as is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The field form cannot express a mutation that disappears entirely: deleting
the only root field leaves an empty selection set, and a variable definition
naming a removed input type is out of the directive's reach. So `ExportGiftCards`
sat in a hardcoded `KNOWN_324_GAPS` list in the sweep test instead.

An operation-level lock is a build-time marker — nothing is stripped at runtime,
entry-point gating still does that — but it lets the gap be declared next to the
mutation, and the sweep read it instead of a name list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`exportGiftCards` is gone from the 3.24 API and deprecated on 3.23, where it
stays reachable directly through the API. Rather than keep the UI alive behind
a schema check, the export dialog and its entry points are removed.

The bulk-issue success dialog goes with them — offering to email the codes was
its only action, so the success notification is now the only confirmation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lkostrowski lkostrowski removed the test deployment Deploy Pull Request to *.saleor.rocks environment label Aug 27, 2026
@lkostrowski
lkostrowski marked this pull request as ready for review August 27, 2026 06:25
@lkostrowski
lkostrowski requested review from a team and NyanKiyoshi August 27, 2026 06:25
@lkostrowski
lkostrowski enabled auto-merge (squash) August 27, 2026 06:25
@lkostrowski
lkostrowski requested review from mirekm and removed request for NyanKiyoshi August 27, 2026 06:29
@lkostrowski
lkostrowski merged commit bc08ccf into main Aug 27, 2026
16 of 18 checks passed
@lkostrowski
lkostrowski deleted the lkostrowski/staging-schema-attribute-fields branch August 27, 2026 06:31
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.

2 participants