fix: page templates date field required#992
Conversation
📝 WalkthroughWalkthroughThe PR threads an ChangesGlobal page template deadline flow
Sequence Diagram(s)sequenceDiagram
participant PageTemplateListPage
participant PageTemplatePopup
participant useFormik
participant mediaModuleSchema
participant PageModules
participant MediaRequestModule
PageTemplateListPage->>PageTemplatePopup: isGlobal
PageTemplatePopup->>useFormik: validationContext { isGlobal }
useFormik->>mediaModuleSchema: validate upload_deadline
mediaModuleSchema-->>PageTemplatePopup: upload_deadline required or nullable
PageTemplatePopup->>PageModules: isGlobal
PageModules->>MediaRequestModule: showUploadDeadline={!isGlobal}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
| <MediaRequestModule | ||
| baseName={name} | ||
| index={index} | ||
| isGlobal={isGlobal} |
There was a problem hiding this comment.
at this point isGlobal means nothing, can we change it to showUploadDeadline={!isGlobal} instead ? and add a default to isGlobal
| name: yup.string().required(T.translate("validation.required")), | ||
| type: yup.string().required(T.translate("validation.required")), | ||
| upload_deadline: yup.date().required(T.translate("validation.required")), | ||
| upload_deadline: yup.date().nullable(), |
There was a problem hiding this comment.
@priscila-moneo this change makes upload_deadline optional for all consumers of the component, not just the global context.
The ticket required:
- Global template -> hide the field + make it optional (correct)
- Summit/sponsor template -> keep the field visible and still required (broken by this change)
The other two consumers of PageTemplatePopup - show-pages-list-page/index.js and sponsor-pages-tab/index.js - do not pass isGlobal, still render the date picker, but now allow the form to submit without a value because the validation is .nullable() instead of .required().
Suggested fix: make the schema conditional using Formik's validationContext:
// In useFormik:
validationContext: { isGlobal }
// In mediaModuleSchema:
upload_deadline: yup.date().when('$isGlobal', {
is: true,
then: (s) => s.nullable(),
otherwise: (s) => s.required(T.translate("validation.required"))
})
There was a problem hiding this comment.
Pull request overview
This PR addresses cases where upload_deadline was being treated as a required field for page template MEDIA modules (notably for global templates), by omitting it from the payload when unset and updating the UI/validation to allow it to be absent.
Changes:
- Update module normalization to remove
upload_deadlinefrom the outgoing payload when it is null/undefined. - Relax form validation for
upload_deadlineand hide the deadline field for global templates in the MEDIA module UI. - Add/adjust constants and unit tests to cover the new normalization behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils/page-template.js | Omits upload_deadline from normalized MEDIA modules when unset. |
| src/utils/constants.js | Adds grid column constants used by the updated MEDIA module layout. |
| src/utils/tests/page-template.test.js | Adds coverage ensuring upload_deadline is omitted when null/undefined. |
| src/pages/sponsors-global/page-templates/page-template-popup/page-template-modules-form.js | Threads isGlobal into MEDIA module rendering. |
| src/pages/sponsors-global/page-templates/page-template-popup/modules/page-template-media-request-module.js | Hides the deadline field for global templates and adjusts layout sizing. |
| src/pages/sponsors-global/page-templates/page-template-popup/index.js | Makes upload_deadline nullable in schema and passes isGlobal down. |
| src/pages/sponsors-global/page-templates/page-template-list-page.js | Marks the popup usage as isGlobal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| PageModules.propTypes = { | ||
| name: PropTypes.string | ||
| name: PropTypes.string, | ||
| isGlobal: PropTypes.bool | ||
| }; |
Signed-off-by: Priscila Moneo <priscila_moneo@hotmail.com.ar>
Signed-off-by: Priscila Moneo <priscila_moneo@hotmail.com.ar>
priscila-moneo
force-pushed
the
fix/page-templates-date-field-required
branch
from
3d3d139 to
26b5622
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/pages/sponsors-global/page-templates/page-template-popup/index.js (1)
125-127: 🎯 Functional Correctness | 🔴 Critical
validationContextis not a Formik option; global template submission is blocked.
useFormik(v2.4.6) ignoresvalidationContext, so the Yup schema resolves$isGlobaltoundefined. This forcesupload_deadlineto remain.required()even when the field is hidden for global templates, preventing form submission.Replace
validationSchemawith a customvalidatefunction to inject the context:🛠️ Fix: Inject context via custom validate
const formik = useFormik({ initialValues: pageTemplate, - validationContext: { isGlobal }, - validationSchema: yup.object().shape({ - code: yup.string().required(T.translate("validation.required")), - name: yup.string().required(T.translate("validation.required")), - ...(showSponsorships && { - sponsorship_types: yup - .array() - .min(1, T.translate("validation.required")) - }), - modules: yup.array().of(moduleSchema) - }), + validate: (values) => + validationSchema + .validate(values, { abortEarly: false, context: { isGlobal } }) + .then(() => ({})) + .catch((err) => yupToFormErrors(err)), });Define
validationSchemacontaining themoduleSchemalogic before this hook.Ensure
yupToFormErrorsis imported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js` around lines 125 - 127, useFormik is currently passing validationContext, which Formik ignores, so the Yup schema in this popup cannot receive isGlobal and global template submission stays blocked. Update the page-template-popup form hook to replace validationSchema usage with a custom validate function that runs the existing moduleSchema validation with context, and convert Yup errors with yupToFormErrors so hidden global-only fields like upload_deadline do not remain required. Keep the fix localized to the useFormik setup in the page template popup component and preserve the current moduleSchema logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js`:
- Around line 125-127: useFormik is currently passing validationContext, which
Formik ignores, so the Yup schema in this popup cannot receive isGlobal and
global template submission stays blocked. Update the page-template-popup form
hook to replace validationSchema usage with a custom validate function that runs
the existing moduleSchema validation with context, and convert Yup errors with
yupToFormErrors so hidden global-only fields like upload_deadline do not remain
required. Keep the fix localized to the useFormik setup in the page template
popup component and preserve the current moduleSchema logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32cffde2-ea83-41d3-9605-c42134967c97
📒 Files selected for processing (7)
src/pages/sponsors-global/page-templates/page-template-list-page.jssrc/pages/sponsors-global/page-templates/page-template-popup/index.jssrc/pages/sponsors-global/page-templates/page-template-popup/modules/page-template-media-request-module.jssrc/pages/sponsors-global/page-templates/page-template-popup/page-template-modules-form.jssrc/utils/__tests__/page-template.test.jssrc/utils/constants.jssrc/utils/page-template.js
ref: https://app.clickup.com/t/9014802374/86b9c2nd4
Summary by CodeRabbit
New Features
Bug Fixes
Tests