Fixes the issue where localeString custom fields do not appear in the Admin Dashboard despite being correctly defined in the configuration. #4081
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe change refactors how customFields and translations are injected into GraphQL fragment selection sets, shifting from direct mutation to explicit index-based detection and replacement, with improved handling of nested localized field structures. Changes
Possibly related issues
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts (1)
243-263: Potential duplicatecustomFieldsin translations selection set.The code unconditionally pushes a
customFieldsfield intotranslations.selectionSet.selections(line 244) without checking if one already exists. If the original fragment already containstranslations { customFields { ... } }, this will result in duplicatecustomFieldsselections in the GraphQL query, which could cause unexpected behavior or errors.Consider checking for an existing
customFieldsbefore pushing, similar to how it's done for the top-levelcustomFieldsfield.Proposed fix: Check for existing customFields in translations
// Always include locale custom fields inside translations + const translationsSelections = selectionSetForTranslations.selections as SelectionNode[]; + const existingTranslationsCustomFieldsIndex = translationsSelections.findIndex( + selection => isFieldNode(selection) && selection.name.value === 'customFields', + ); + + const translationsCustomFieldsNode: FieldNode = { + name: { + kind: Kind.NAME, + value: 'customFields', + }, + kind: Kind.FIELD, + selectionSet: { + kind: Kind.SELECTION_SET, + selections: localizedFields.map( + customField => + ({ + kind: Kind.FIELD, + name: { + kind: Kind.NAME, + value: customField.name, + }, + }) as FieldNode, + ), + }, + }; + + if (existingTranslationsCustomFieldsIndex === -1) { + translationsSelections.push(translationsCustomFieldsNode); + } else { + translationsSelections[existingTranslationsCustomFieldsIndex] = translationsCustomFieldsNode; + } - (selectionSetForTranslations.selections as SelectionNode[]).push({ - name: { - kind: Kind.NAME, - value: 'customFields', - }, - kind: Kind.FIELD, - selectionSet: { - kind: Kind.SELECTION_SET, - selections: localizedFields.map( - customField => - ({ - kind: Kind.FIELD, - name: { - kind: Kind.NAME, - value: customField.name, - }, - }) as FieldNode, - ), - }, - });
🧹 Nitpick comments (3)
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts (3)
184-193: Redundant condition check.The condition
existingCustomFieldsFieldIndex !== -1on line 184 is redundant. IfexistingCustomFieldsFieldis truthy (which it must be to reach theelsebranch), thenexistingCustomFieldsFieldIndexis already guaranteed to be!== -1.Proposed simplification
- } else if (existingCustomFieldsFieldIndex !== -1) { + } else { // If a customFields field already exists, replace it with an updated node const updatedField: FieldNode = {
200-202: Unused variable:translationsField.The
translationsFieldvariable is computed but never used. The code at lines 206-208 re-finds the translations field usingfindIndexfor index-based replacement. This appears to be dead code from the previous implementation.Proposed fix: Remove unused variable
- const translationsField = selectionSet.selections - .filter(isFieldNode) - .find(field => field.name.value === 'translations'); - if (localizedFields.length) {
233-240: Consider clarifying the object reference pattern.When
existingField.selectionSetexists,ensureTranslationSelectionSetreturns the same object reference. The spread at line 237 and reassignment at line 238 become effectively a no-op for theselectionSetproperty. The subsequent mutation at line 244 works because it modifies the shared reference.This is correct but subtle. A brief comment clarifying this intentional reference sharing would improve maintainability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}: Use React as the UI framework for components in the dashboard package
Use TanStack Query (useQuery or useMutation) for data fetching, not Apollo Client
Import api from '@/graphql/api.js' and graphql from '@/graphql/graphql.js' for GraphQL queries and mutations
Use the api.query() pattern with graphql document when creating useQuery calls, including queryKey and staleTime configuration
Use the api.mutate() pattern with graphql document when creating useMutation calls, with onSuccess and onError handlers
Use FormFieldWrapper component for form controls instead of raw Shadcn react-hook-form components
Use @lingui/react/macro for localization with Trans component for markup and useLingui hook for programmatic strings
Set React component props objects to Readonly<> type for type safety
Files:
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts
packages/dashboard/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
Use Shadcn UI & Tailwind CSS for styling and UI components in the dashboard
Files:
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts
🧬 Code graph analysis (1)
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts (1)
packages/admin-ui/src/lib/core/src/common/utilities/selection-manager.ts (1)
selection(19-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: e2e tests (22.x, postgres)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: e2e tests (24.x, sqljs)
- GitHub Check: e2e tests (24.x, mariadb)
- GitHub Check: e2e tests (24.x, postgres)
- GitHub Check: build (24.x)
- GitHub Check: build (20.x)
- GitHub Check: codegen / codegen
- GitHub Check: unit tests (20.x)
- GitHub Check: unit tests (24.x)
- GitHub Check: build (22.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 20.x)
🔇 Additional comments (1)
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts (1)
121-128: LGTM!The index-based detection of the existing
customFieldsfield is clean and correctly handles both the presence and absence cases. The cast to mutable array is safe here since the document was already deep-cloned viaJSON.parse(JSON.stringify(...)).
starry-osean
changed the title
|
|
Hi，@BibiSebi ！Thank you for taking the time to push forward the resolution of this issue. The current issue is a test timeout: e2e tests are timing out in the MySQL environment, possibly due to:
|
|
Hello @starry-osean thank you for taking your time to submit a PR on this. 🙏 Could you add some screen-recording of a before and after behavior? I believe the issue #4005 is just that the Thank you 🙏 |
|
Just to give you an overview of what I tried: I extended the config with the following values. That rendered an input field and it was possible for me to add translatable content (see recording). Screen.Recording.2026-01-23.at.12.42.46.mov |
|
Hi，@BibiSebi ！This picture is the repaired version. |
|
Thank you for the quick response @starry-osean, in that case I would close this PR and proceed by merging mine. I will keep an eye out on the issue and if something similar/connected re-appears I will revisit your solution. |
2 participants
Summary
When localeString/localeText custom fields exist but a query doesn’t explicitly select translations, the dashboard failed to request translations.customFields, so localized custom fields wouldn’t render.
In add-custom-fields.ts, I now a translations selection set (with languageCode) and inject translations.customFields. whenever localized custom fields are present. This guarantees the data is fetched and the fields render.
Changes
packages/dashboard/src/lib/framework/document-introspection/add-custom-fields.ts
If localized fields are detected, ensure a translations selection set exists and append customFields selections for all localized custom fields.
Validation
Install deps (ensure @vendure/core is available), then: npx vite build --emptyOutDir.
In the admin UI, create/edit an entity with a localeString custom field (e.g., Product.shortName), switch languages, and confirm the field appears and is editable.
CI Status Note
The CI test failure appears to be an environment issue (port conflicts: EADDRINUSE: address already in use :::3000 and Vite port mismatch). The dashboard tests timeout because:
Server port 3000 is already in use
Vite runs on port 5175 instead of the expected 5173. This is unrelated to the code changes. Local validation confirms the fix works correctly. The build and type checks pass locally, and the functionality has been manually verified.
Related Issue: Fixes the issue where localeString custom fields do not appear in the Admin Dashboard despite being correctly defined in the configuration.