Skip to content

[Studio] Fix blank unit selection for the static Quantity Value transformer - #683

Merged
ValeriaMaltseva merged 9 commits into
2026.xfrom
fix/quantity-value-transformer-unit-list-2026.x
Sep 2, 2026
Merged

[Studio] Fix blank unit selection for the static Quantity Value transformer#683
ValeriaMaltseva merged 9 commits into
2026.xfrom
fix/quantity-value-transformer-unit-list-2026.x

Conversation

@ValeriaMaltseva

@ValeriaMaltseva ValeriaMaltseva commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Problem

The unit select of the Quantity Value transformer is always empty when Unit source is set to
Static.

Fixes pimcore/platform-version#148 on 2026.x.

Root cause

UnitDataResponse documented its list under the OpenAPI property name UnitList, while the
serializer emits the actual property name — unitList:

{"unitList":[{"unitId":"kg","abbreviation":"kg"}],"additionalAttributes":[]}

The generated API client therefore typed the response as UnitList, and
quantity-value-transformer-form.tsx read unitData?.UnitList, which was always undefined
empty option list → blank select.

The schema annotation was already corrected to unitList in 307aeb9, but the committed OpenAPI
snapshot and the generated client on 2026.x still carry the old name — so the type checker agrees
with the broken access and never flags it.

Change

Two commits, cherry-picked from #682 so both lines carry byte-identical content:

  1. The fix
    • assets/studio/build/api/docs.jsonopenapi.json — snapshot property UnitListunitList
    • data-importer-api-slice.gen.ts — regenerated via npm run build-api-client
    • quantity-value-transformer-form.tsx — reads unitData?.unitList
    • tests/unit/UnitDataResponseTest.php — regression test: the documented OpenAPI property name
      must match the key the serializer actually emits
  2. Snapshot property metadata refresh — the committed snapshot predated several #[Property]
    annotations, so the client carried no descriptions for currentConfig.label and
    userPermissions.update/delete, and typed dataPreview.data as any.

Only the seven BundleDataImporter* schemas are touched. The snapshot keeps its existing 403 paths,
369 components and info.version: 0.13.20 — verified by structural comparison, changed paths: [].

CalculateTransformationResultTypeParameters::$currentConfig documents dataSourceIndex items as
integer, which contradicts both the frontend (dataSourceIndex?: string[] in types.ts and
throughout the advanced mapping modal) and the bundle's own ColumnHeadersResponse, where id and
dataIndex are string. That one property is deliberately left un-refreshed so dataSourceIndex
stays any until the annotation itself is corrected.

Verification

  • npm run check-types — 7 errors on this branch, the same 7 as on origin/2026.x
    (colorFillAdditional / colorFillActive missing on FullToken, in *.styles.tsx files this PR
    does not touch). No new errors.
  • npm run lint — the same 7 pre-existing no-unsafe-argument errors in those same style files.
    Nothing in the four changed files.
  • Negative test: with the snapshot and client fixed but quantity-value-transformer-form.tsx left
    reading UnitList, check-types fails exactly at the bug site —
    Property 'UnitList' does not exist on type 'BundleDataImporterUnitDataResponse'. Did you mean 'unitList'?
    So the type checker now guards this.
  • The Codeception suite was not run locally (vendor/ is not installed in this workspace; it
    needs the Docker test env + product-registration credentials) — CI validates it.
  • No frontend build artifact is committed; the "Studio Frontend Build" workflow regenerates it.

Manual test

Data Importer config → mapping to a Quantity Value field → add the Quantity Value transformer →
set Unit source to Static → the Unit select lists all quantity-value units and is searchable.

Note for consumers

Anyone who worked around this by renaming unitList back to UnitList in the response (see the
issue comment) must drop that workaround.

Additional schema fixes forward-merged from #682

#682 (2026.2 line) had two more schema-type bugs surface after its own snapshot refresh, both from
the same 307aeb9 PR that this description already calls out for dataSourceIndex:

  1. dataSourceIndex typed as integer — this PR's own snapshot refresh left it deliberately
    un-refreshed (see above), so it never broke here, but the underlying PHP annotation in
    CalculateTransformationResultTypeParameters.php was still wrong. Corrected to string,
    matching ColumnHeadersResponse and every frontend caller.
  2. dataPreview[].data typed as stringPreviewData::getDataPreview() forwards interpreter
    values unchanged, and JsonFileInterpreter reads straight from json_decode, so a cell can be a
    string, number, boolean, array, object, or null. Three frontend call sites already worked around
    the incorrect narrowing (normalize-data-row.ts, preview-import-step.tsx,
    use-preview-record-query.ts). Reverted to unconstrained (no type key = arbitrary JSON value),
    matching the schema's original pre-307aeb9 shape.

Both PHP annotations were corrected, and docs.jsonopenapi.json was refreshed fresh from
https://feature.nightly.pimcore.app/pimcore-studio/api/docs/json?locale=en (2026.x's own nightly
instance — already reflects the unitList casing fix, but not these two, since neither PHP fix has
been merged/deployed there yet). Same caveat as #682: a plain re-fetch from that URL before this
merges will silently reintroduce both bugs; re-apply the corrections if that happens.

Relationship to #682

#682 carries the identical change against 2026.2 and is still open. This PR lands it directly on
2026.x as requested. Because the two branches were byte-identical in all affected paths and this is
a clean cherry-pick, the later 2026.2 → 2026.x forward merge resolves to the same content — if it
conflicts at all, either side is correct.

ValeriaMaltseva and others added 2 commits August 25, 2026 11:55
…former

The unit select of the Quantity Value transformer was always empty when the
unit source was set to "Static".

`UnitDataResponse` documented its list under the OpenAPI property name
`UnitList`, while the serializer emits the actual property name `unitList`.
The generated API client therefore typed the response as `UnitList` and the
transformer form read `unitData?.UnitList`, which was always undefined.

The schema annotation was already corrected to `unitList`, but the committed
OpenAPI snapshot and the generated client still carried the old name, so the
type checker agreed with the broken access. Refresh the snapshot property,
regenerate the client and read `unitList` in the transformer form.

Consumers that worked around this by renaming `unitList` back to `UnitList`
in the response must drop that workaround.

Fixes pimcore/platform-version#148

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The committed OpenAPI snapshot predated several `#[Property]` annotations in
the bundle's schema classes. The generated API client therefore carried no
descriptions for `currentConfig.label` and `userPermissions.update`/`delete`,
and typed `dataPreview.data` as `any`.

Refresh only the `BundleDataImporter*` schemas in the snapshot — 21 additions,
all property descriptions and examples plus the `dataPreview.data` string type
— and regenerate the client. Nothing outside the data-importer schemas is
touched, so the snapshot keeps its existing 403 paths and 369 components.

`CalculateTransformationResultTypeParameters::$currentConfig` documents
`dataSourceIndex` items as `integer`, which contradicts both the frontend
(string column identifiers throughout) and the bundle's own
`ColumnHeadersResponse`, where `id` and `dataIndex` are `string`. That one
property is deliberately left un-refreshed so `dataSourceIndex` stays `any`
until the annotation itself is corrected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 25, 2026 09:58

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

Verdict: Needs changes. The unitList fix addresses the root cause at the schema, generated-client, and UI boundaries, and the only frontend caller is updated.

Changes:

  • Aligns the OpenAPI snapshot and generated client with serialized unitList.
  • Updates the Quantity Value transformer to consume unitList.
  • Adds regression tests and refreshes schema metadata.

The upgrade guide already documents the rename. However, the regression test does not validate the stale generated artifacts, and dataPreview.data is incorrectly narrowed to string.

Reviewed changes

Copilot reviewed 11 out of 32 changed files in this pull request and generated 3 comments.

File Description
assets/studio/build/api/docs.jsonopenapi.json Refreshes Data Importer OpenAPI schemas.
assets/studio/js/src/modules/data-importer/data-importer-api-slice.gen.ts Regenerates client types, including unitList.
assets/studio/js/src/modules/data-importer/dynamic-types/transformer/quantity-value/quantity-value-transformer-form.tsx Reads units from unitData.unitList.
tests/unit/UnitDataResponseTest.php Adds DTO serialization/schema checks.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/unit/UnitDataResponseTest.php
Comment thread assets/studio/js/src/modules/data-importer/data-importer-api-slice.gen.ts Outdated
Comment thread tests/unit/UnitDataResponseTest.php
Forward-merges the same two schema fixes made on the 2026.2 line
(#682, commits 59ee843 and 5f59833) onto 2026.x.
Both bugs stem from the same PR (307aeb9, "fix errors form schema
validation") and were still present here since this branch predates
that later work.

1. CalculateTransformationResultTypeParameters::$currentConfig documented
   dataSourceIndex items as integer, contradicting the string column
   identifiers used throughout the frontend (types.ts's MappingConfigItem,
   ColumnHeadersResponse) and both callers of the generated query
   (advanced-mapping-modal.tsx, use-auto-recalculate-type.ts). Corrected
   to string.

2. DataPreviewResponse typed dataPreview[].data as string, but
   PreviewData::getDataPreview() forwards interpreter values unchanged
   and JsonFileInterpreter reads straight from json_decode, so a cell can
   be a string, number, boolean, array, object, or null. Three frontend
   call sites already worked around this (normalize-data-row.ts,
   preview-import-step.tsx, use-preview-record-query.ts). Reverted to
   unconstrained (no `type` key = arbitrary JSON value), matching the
   original pre-307aeb9 definition.

docs.jsonopenapi.json refreshed from
https://feature.nightly.pimcore.app/pimcore-studio/api/docs/json?locale=en
(2026.x's nightly instance), which already reflects the earlier `unitList`
casing fix but not these two — same two properties were patched in the
fetched snapshot before regenerating the client, for the same reason as
on the 2026.2 line: the nightly instance won't carry this PHP fix until
it's merged and redeployed.

check-types is unaffected outside the two intended type changes.

Co-Authored-By: Claude <noreply@anthropic.com>
@ValeriaMaltseva ValeriaMaltseva added this to the 2026.3.0 milestone Sep 2, 2026
ValeriaMaltseva and others added 4 commits September 2, 2026 10:54
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
testDocumentedPropertyNameMatchesTheSerializedKey only compared the DTO's
#[Property] annotation against ad-hoc Serializer output, entirely within
PHP. The annotation had already been unitList before this PR; the actual
regression was drift between it and the committed docs.jsonopenapi.json
snapshot / generated data-importer-api-slice.gen.ts client, which this
test never reads. It would have passed identically before this PR and
cannot catch the same failure again.

Add two tests that read the committed artifacts directly and assert each
DTO-documented property is present in them:
- testCommittedOpenApiSnapshotMatchesTheDocumentedPropertyNames parses
  docs.jsonopenapi.json and checks the BundleDataImporterUnitDataResponse
  schema's property keys.
- testGeneratedClientMatchesTheDocumentedPropertyNames extracts the
  generated TS type block and checks the same, independently of the
  snapshot, in case the client is ever stale or hand-edited without the
  snapshot being touched.

Verified both would have failed on the pre-fix commit (ea10435, where the
snapshot/client still carried UnitList) and pass on the current tree —
checked with a standalone script mirroring the test logic, since this
workspace has no PHP toolchain installed to run Codeception directly.

Co-Authored-By: Claude <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Sep 2, 2026

Copy link
Copy Markdown

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.

🟡 Changes recommended

The OpenAPI snapshot contains unreported version drift, and the narrowed dataSourceIndex contract is not documented for consumers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 13/34 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread assets/studio/build/api/docs.jsonopenapi.json
Comment thread src/Schema/CalculateTransformationResultTypeParameters.php
@ValeriaMaltseva
ValeriaMaltseva merged commit 229b8e1 into 2026.x Sep 2, 2026
1 check passed
@ValeriaMaltseva
ValeriaMaltseva deleted the fix/quantity-value-transformer-unit-list-2026.x branch September 2, 2026 11:09
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DataHub Transformation Pipeline for Static Quantity Value always blank

3 participants