[8416] Add Expired Date to the Content Type Editor Add Field's System types#4811
[8416] Add Expired Date to the Content Type Editor Add Field's System types#4811jvega190 wants to merge 3 commits intocraftercms:developfrom
Conversation
WalkthroughThese changes introduce a new "expired-date" control to the Content Type Management system by registering it across descriptors, control maps, value retrievers and serializers, while updating field initialization logic and read-only status handling in the UI layer. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
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/ContentTypeManagement/components/EditTypeView.tsx (1)
1249-1260:⚠️ Potential issue | 🟠 MajorAdd
'expired-date': 'expired_dt'tosystemFieldsIdsMapinutils.ts.The
expired-datefield type is missing fromsystemFieldsIdsMap. Since theexpiredDatedescriptor marks the Variable Name field as readonly withdefaultValue: 'expired_dt', new expired-date fields must initialize with a proper ID set via the map. Without this entry,systemFieldsIdsMap['expired-date']returnsundefined, and the?? nullfallback causes newly added fields to be saved withid: null, producing a broken content type definition. Add the mapping:'expired-date': 'expired_dt'(or extract the appropriate variable name from the descriptor's metadata/defaultValue).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsx` around lines 1249 - 1260, Add the missing mapping for the expired-date system field in the systemFieldsIdsMap so new fields get the correct ID; update systemFieldsIdsMap to include the entry 'expired-date': 'expired_dt' (or use the variable name used as defaultValue in the expiredDate descriptor) so getNewFieldFromDescriptor will set id from systemFieldsIdsMap['expired-date'] instead of falling back to null.
🧹 Nitpick comments (1)
ui/app/src/components/ContentTypeManagement/descriptors/controls/expiredDate.ts (1)
89-111: Consider a future-orientedpopulateDateExpdefault for the content expiration fieldWith
populate: trueandpopulateDateExp: 'now', the expiration date auto-populates to the end of the current minute (sinceallowPastDate: falseconstrains the'now'expression). This means content will expire almost immediately by default unless the content type editor changes the expression.For a system field specifically designed to track content expiration, a more future-biased default expression (e.g.,
now+1year) would provide a safer starting point and reduce the risk of content type editors accidentally shipping with misconfigured expiration dates.♻️ Suggested defaults
Option 1: Disable auto-population
populate: { id: 'populate', type: 'boolean', name: defineMessage({ defaultMessage: 'Populated' }), - defaultValue: true, + defaultValue: false, validations: immutableEmptyObject },Option 2: Use a future-oriented expression
populateDateExp: { id: 'populateDateExp', type: 'date-time-expression-input', name: defineMessage({ defaultMessage: 'Populate Expression' }), - defaultValue: 'now', + defaultValue: 'now+1year', validations: { type: createValidation('type', 'dateTime') } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/src/components/ContentTypeManagement/descriptors/controls/expiredDate.ts` around lines 89 - 111, The populateDateExp default currently set to 'now' causes immediate expirations when populate is true and allowPastDate is false; change the default in the control definition (populateDateExp) to a future-oriented expression or disable auto-population: either set populateDateExp to a future expression like 'now+1year' (or another agreed offset) or set populate: false by default so the field is not auto-filled; update the descriptor for populateDateExp and/or the populate property in expiredDate control to reflect the chosen safer default (ensure validations using createValidation('type','dateTime') remain intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ui/app/src/components/ContentTypeManagement/components/EditTypeView.tsx`:
- Around line 1249-1260: Add the missing mapping for the expired-date system
field in the systemFieldsIdsMap so new fields get the correct ID; update
systemFieldsIdsMap to include the entry 'expired-date': 'expired_dt' (or use the
variable name used as defaultValue in the expiredDate descriptor) so
getNewFieldFromDescriptor will set id from systemFieldsIdsMap['expired-date']
instead of falling back to null.
---
Nitpick comments:
In
`@ui/app/src/components/ContentTypeManagement/descriptors/controls/expiredDate.ts`:
- Around line 89-111: The populateDateExp default currently set to 'now' causes
immediate expirations when populate is true and allowPastDate is false; change
the default in the control definition (populateDateExp) to a future-oriented
expression or disable auto-population: either set populateDateExp to a future
expression like 'now+1year' (or another agreed offset) or set populate: false by
default so the field is not auto-filled; update the descriptor for
populateDateExp and/or the populate property in expiredDate control to reflect
the chosen safer default (ensure validations using
createValidation('type','dateTime') remain intact).
craftercms/craftercms#8416
Summary by CodeRabbit
New Features
Improvements