[Studio] Fix blank unit selection for the static Quantity Value transformer - #683
Open
ValeriaMaltseva wants to merge 3 commits into
Open
[Studio] Fix blank unit selection for the static Quantity Value transformer#683ValeriaMaltseva wants to merge 3 commits into
ValeriaMaltseva wants to merge 3 commits into
Conversation
…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>
|
Contributor
There was a problem hiding this comment.
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.
| * property name has to be the one the serializer actually emits. When the two drifted apart, | ||
| * the quantity value transformer's unit select silently rendered empty. | ||
| */ | ||
| public function testDocumentedPropertyNameMatchesTheSerializedKey(): void |
| label?: string; | ||
| /** Cell data value */ | ||
| data?: any; | ||
| data?: string; |
| @@ -0,0 +1,57 @@ | |||
| <?php declare(strict_types=1); | |||
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
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.



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
UnitDataResponsedocumented its list under the OpenAPI property nameUnitList, while theserializer emits the actual property name —
unitList:The generated API client therefore typed the response as
UnitList, andquantity-value-transformer-form.tsxreadunitData?.UnitList, which was alwaysundefined→empty option list → blank select.
The schema annotation was already corrected to
unitListin 307aeb9, but the committed OpenAPIsnapshot and the generated client on
2026.xstill carry the old name — so the type checker agreeswith the broken access and never flags it.
Change
Two commits, cherry-picked from #682 so both lines carry byte-identical content:
assets/studio/build/api/docs.jsonopenapi.json— snapshot propertyUnitList→unitListdata-importer-api-slice.gen.ts— regenerated vianpm run build-api-clientquantity-value-transformer-form.tsx— readsunitData?.unitListtests/unit/UnitDataResponseTest.php— regression test: the documented OpenAPI property namemust match the key the serializer actually emits
#[Property]annotations, so the client carried no descriptions for
currentConfig.labelanduserPermissions.update/delete, and typeddataPreview.dataasany.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::$currentConfigdocumentsdataSourceIndexitems asinteger, which contradicts both the frontend (dataSourceIndex?: string[]intypes.tsandthroughout the advanced mapping modal) and the bundle's own
ColumnHeadersResponse, whereidanddataIndexarestring. That one property is deliberately left un-refreshed sodataSourceIndexstays
anyuntil the annotation itself is corrected.Verification
npm run check-types— 7 errors on this branch, the same 7 as onorigin/2026.x(
colorFillAdditional/colorFillActivemissing onFullToken, in*.styles.tsxfiles this PRdoes not touch). No new errors.
npm run lint— the same 7 pre-existingno-unsafe-argumenterrors in those same style files.Nothing in the four changed files.
quantity-value-transformer-form.tsxleftreading
UnitList,check-typesfails exactly at the bug site —Property 'UnitList' does not exist on type 'BundleDataImporterUnitDataResponse'. Did you mean 'unitList'?So the type checker now guards this.
vendor/is not installed in this workspace; itneeds the Docker test env + product-registration credentials) — CI validates 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
unitListback toUnitListin the response (see theissue comment) must drop that workaround.
Relationship to #682
#682 carries the identical change against
2026.2and is still open. This PR lands it directly on2026.xas requested. Because the two branches were byte-identical in all affected paths and this isa clean cherry-pick, the later
2026.2 → 2026.xforward merge resolves to the same content — if itconflicts at all, either side is correct.