[8507] FE2 - change content type functionality #4750
Conversation
WalkthroughIntroduces an optional Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
✏️ Tip: You can disable this entire section by setting 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)
ui/app/src/components/FormsEngine/lib/formUtils.tsx (1)
379-393: Type mismatch:changeTypeIdshould allowundefined.The parameter type is
string, but the caller inFormsEngine.tsx(line 451) passesupdate.changeTypeIdwhich isstring | undefined. This will cause a TypeScript error in strict mode.🔧 Suggested fix
export function fetchUpdateRequirements({ siteId, path, modelId, readonly, contentTypesById, changeTypeId }: { siteId: string; path: string; modelId: string; readonly: boolean; contentTypesById: LookupTable<ContentType>; - changeTypeId: string; + changeTypeId?: string; }): Observable<FormRequirementsResponse> {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ui/app/src/components/FormsEngine/FormsEngine.tsxui/app/src/components/FormsEngine/lib/formUtils.tsxui/app/src/utils/system.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-19T20:34:01.813Z
Learnt from: jvega190
Repo: craftercms/studio-ui PR: 4600
File: ui/app/src/components/PluginManagement/PluginManagement.tsx:361-361
Timestamp: 2025-08-19T20:34:01.813Z
Learning: In the EnhancedDialog migration, only dialog components that have been refactored to use context-based state management should have their onSubmittingAndOrPendingChange prop renamed to updateSubmittingOrHasPendingChanges. Components that haven't been migrated to the new EnhancedDialog context system should continue using the original onSubmittingAndOrPendingChange prop name.
Applied to files:
ui/app/src/components/FormsEngine/FormsEngine.tsxui/app/src/utils/system.ts
📚 Learning: 2025-08-19T18:43:20.285Z
Learnt from: jvega190
Repo: craftercms/studio-ui PR: 4600
File: ui/app/src/components/CreatePreviewTokenDialog/CreatePreviewTokenDialog.tsx:149-151
Timestamp: 2025-08-19T18:43:20.285Z
Learning: The useUpdateRefs hook in ui/app/src/hooks/useUpdateRefs.ts has both a named export and a default export, so both import styles (default import and named import) are valid and will work correctly.
Applied to files:
ui/app/src/components/FormsEngine/FormsEngine.tsx
📚 Learning: 2025-08-19T18:43:20.285Z
Learnt from: jvega190
Repo: craftercms/studio-ui PR: 4600
File: ui/app/src/components/CreatePreviewTokenDialog/CreatePreviewTokenDialog.tsx:149-151
Timestamp: 2025-08-19T18:43:20.285Z
Learning: The useUpdateRefs hook in ui/app/src/hooks/useUpdateRefs.ts has both a named export (export function useUpdateRefs) and a default export (export default useUpdateRefs), making both import styles valid: default import and named import.
Applied to files:
ui/app/src/components/FormsEngine/FormsEngine.tsx
📚 Learning: 2025-12-11T21:18:51.227Z
Learnt from: jvega190
Repo: craftercms/studio-ui PR: 4708
File: ui/app/src/components/PreviewAudiencesPanel/PreviewAudiencesPanel.tsx:72-74
Timestamp: 2025-12-11T21:18:51.227Z
Learning: When reviewing components in ui/app/src/components, verify that props expecting a title (e.g., EmptyState) allow ReactNode or MessageDescriptor (e.g., React elements like FormattedMessage and MessageDescriptor objects). Ensure TS types reflect ReactNode | MessageDescriptor where appropriate, and audit usages to avoid passing incompatible values. Update tests or PropTypes/TypeScript types as needed to maintain correct typing and rendering behavior.
Applied to files:
ui/app/src/components/FormsEngine/FormsEngine.tsxui/app/src/components/FormsEngine/lib/formUtils.tsx
📚 Learning: 2025-08-27T23:08:05.012Z
Learnt from: jvega190
Repo: craftercms/studio-ui PR: 4451
File: ui/app/src/components/ContentTypeManagement/utils.ts:253-264
Timestamp: 2025-08-27T23:08:05.012Z
Learning: In the ContentTypeManagement utils.ts reverseTypeFieldValuesObject function, properties that are not available in the descriptor values should be removed/cleaned up, not preserved. This keeps the field configuration in sync with the current descriptor definitions and prevents accumulation of obsolete properties.
Applied to files:
ui/app/src/components/FormsEngine/lib/formUtils.tsxui/app/src/utils/system.ts
📚 Learning: 2025-12-22T18:26:52.231Z
Learnt from: jvega190
Repo: craftercms/studio-ui PR: 4663
File: ui/app/src/components/FormsEngine/FormsEngine.tsx:147-147
Timestamp: 2025-12-22T18:26:52.231Z
Learning: In useSaveForm.tsx for shared content creation/updates, the path variable is computed (lines 111-114) but needs to be passed to the onSave callback on line 123. For embedded components, path doesn't exist and shouldn't be set. NodeSelector.tsx expects result.path for shared content to use as the item key.
Applied to files:
ui/app/src/utils/system.ts
📚 Learning: 2025-08-19T20:31:06.188Z
Learnt from: jvega190
Repo: craftercms/studio-ui PR: 4600
File: ui/app/src/components/DeletePluginDialog/utils.ts:29-31
Timestamp: 2025-08-19T20:31:06.188Z
Learning: In the CrafterCMS studio-ui codebase, not all usages of `onSubmittingAndOrPendingChange` should be migrated to use the EnhancedDialog context API. Some components are intentionally meant to continue using the legacy prop pattern, particularly parent/management components that pass handlers down to child dialogs.
Applied to files:
ui/app/src/utils/system.ts
🧬 Code graph analysis (2)
ui/app/src/components/FormsEngine/FormsEngine.tsx (1)
ui/app/src/hooks/useMount.ts (1)
useMount(19-22)
ui/app/src/components/FormsEngine/lib/formUtils.tsx (2)
ui/app/src/models/LookupTable.ts (1)
LookupTable(17-17)ui/app/src/models/ContentType.ts (1)
ContentType(166-191)
⏰ 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). (2)
- GitHub Check: Travis CI - Pull Request
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (6)
ui/app/src/components/FormsEngine/FormsEngine.tsx (4)
122-122: LGTM!Import of
useMountis appropriate for the one-time initialization of pending changes state whenchangeTypeIdis present.
153-160: LGTM!The optional
changeTypeIdproperty correctly extendsUpdateModePropswhile maintaining backward compatibility.
445-452: LGTM!The
changeTypeIdparameter is correctly propagated tofetchUpdateRequirements, enabling content type override during the update flow.
620-625: LGTM!The
useMounthook correctly initializeshasPendingChangestotruewhenchangeTypeIdis present, ensuring the user is prompted to save the content type change. The optional chaining (update?.changeTypeId) safely handles create and repeat modes whereupdateis undefined.ui/app/src/components/FormsEngine/lib/formUtils.tsx (1)
423-427: LGTM!The content type resolution correctly uses
changeTypeIdwhen provided, falling back toitem.contentTypeId. The error handling for missing content types is properly maintained.ui/app/src/utils/system.ts (1)
121-131: LGTM! The mapping fromchangeTemplatetochangeTypeIdis correct.Verification confirms
changeTemplateexists onLegacyFormDialogPropsas an optional string property, and it correctly maps tochangeTypeIdonUpdateModePropsfor the update mode of the FormsEngine. Both properties are type-compatible.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Outside diff range comments addressed |
1 participant
craftercms/craftercms#8507
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.