[5761] Enhance macro support for populating dates and times in the date time picker#4864
[5761] Enhance macro support for populating dates and times in the date time picker#4864jvega190 wants to merge 7 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 4 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR expands date-populate expression support in the forms engine by enhancing validation to recognize day-of-week macros, extended now-expressions with multiple time units, and {macro} patterns with optional static times, then refactoring the computation logic to resolve these new formats using helper functions for day-of-week calculations and time application. The help text in DateTimeExpressionInput is updated to document these new formats separately. ChangesDate populate-expression macro expansion
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@ui/app/src/components/FormsEngine/lib/controlHelpers.tsx`:
- Around line 486-487: Validation accepts offset-only macros like "+1d" or
"-2weeks" but the processing/parsing code only handles forms starting with
"now"; update the parsing logic that consumes the variable macro (the same
variable named macro in controlHelpers.tsx) to normalize offset-only inputs by
prepending "now" before further parsing (i.e., if macro matches
/^([+-]\d+(d|days|w|weeks|y|years|h|hours|m|minutes))$/i then set macro = "now"
+ macro). Apply this normalization in the parsing code paths referenced around
the existing /^now.../ checks so the validation and computation logic (the macro
parsing block near the later processing code) treat "+/-N(unit)" and
"now+/-N(unit)" equivalently.
- Around line 567-569: The code currently bails out when expression doesn't
match the {macro} regex (macroMatch) and later only validates the macro part (so
malformed full expressions with a bad time suffix slip through); update the
parsing routine in controlHelpers.tsx to first attempt to parse both forms: if
expression matches {macro} with an optional time suffix, validate the time
suffix (HH:MM[:SS]) and the macro token before computing the date, and if
expression does not contain braces treat it as a plain expression and validate
it with the same validator used for macros instead of returning immediately;
ensure the branches referenced by macroMatch and the later validation blocks
(the code around the existing checks at the spots corresponding to lines 595-596
and 628-633) consistently validate the full expression (macro + optional time)
and only compute a Date when both macro and any time suffix are valid, otherwise
return new Date() or a clear error path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15d2f5ad-83e3-4230-8d4e-860e7bbb08be
📒 Files selected for processing (1)
ui/app/src/components/FormsEngine/lib/controlHelpers.tsx
|
@coderabbitai please enumerate the features and routes that QA needs to test based on these changes. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 26040
🗺️ Routes / Entry Points to TestAll tests are exercised through the FormsEngine date-time picker. QA needs a content type with a Date/Time field whose populate expression (default value macro) is set to each expression below.
✅ Feature Areas & Test Cases1. Regression – Existing
|
| Expression | Expected result |
|---|---|
now |
Current date/time; with allowPastDate=false seconds are set to :59 |
now+5days |
Today + 5 days |
now-3weeks |
Today − 3 weeks |
now+1years |
Today + 1 year |
now-4hours |
Now − 4 hours |
now+30minutes |
Now + 30 minutes |
2. 🆕 Short unit abbreviations (d, w, y, h, m)
These are new in this PR and were previously rejected by the validator.
| Expression | Expected result |
|---|---|
now+2d |
Today + 2 days |
now-3w |
Today − 3 weeks |
now+1y |
Today + 1 year |
now-4h |
Now − 4 hours |
now+30m |
Now + 30 minutes |
+5d (no now prefix) |
Today + 5 days |
-2w (no now prefix) |
Today − 2 weeks |
3. 🆕 Day-of-week macros (plain text)
The picker should populate the next occurrence of the given weekday (never the current day, even if today matches).
| Expression | Expected result |
|---|---|
monday / mon |
Next Monday |
tuesday / tue |
Next Tuesday |
wednesday / wed |
Next Wednesday |
thursday / thu |
Next Thursday |
friday / fri |
Next Friday |
saturday / sat |
Next Saturday |
sunday / sun |
Next Sunday |
Edge case: run test on the exact day (e.g., monday on a Monday) |
Should still return next Monday (+7 days), not today |
4. 🆕 {macro} wrapper format
Same semantics as plain macros but wrapped in curly braces.
| Expression | Expected result |
|---|---|
{now} |
Same as plain now |
{now+5d} |
Today + 5 days |
{monday} |
Next Monday |
{fri} |
Next Friday |
{NOW} / {MONDAY} |
Case-insensitive; same results as lowercase |
{ now } (spaces inside braces) |
Should be trimmed and accepted |
5. 🆕 {macro} with static time suffix
The date is computed from the macro, then the time component is overwritten with the static value.
| Expression | Expected result |
|---|---|
{now} 09:00 |
Today at 09:00:00 |
{now} 14:30:45 |
Today at 14:30:45 |
{monday} 08:00 |
Next Monday at 08:00:00 |
{now+5days} 23:59 |
Today + 5 days at 23:59:00 |
{fri} 00:00:00 |
Next Friday at midnight |
Edge: midnight {now} 00:00 |
Today at 00:00 (not rejected as "past") |
Edge: single-digit hour {now} 9:05 |
Should be accepted ([01]?\d) |
6. 🆕 Validation – expressions that must be rejected
Verify the date-time picker shows a validation error or falls back to now for these.
| Invalid expression | Why it should fail |
|---|---|
{foobar} |
Unrecognised macro |
{now} 25:00 |
Invalid hour (> 23) |
{now} 9:70 |
Invalid minutes (> 59) |
{now} 9:30:70 |
Invalid seconds (> 59) |
{now} 9 |
Missing minutes |
now+ |
Incomplete relative expression |
nowdays |
Missing +/- and numeric value |
| `` (empty string) | Falsy guard at top of validator |
null / undefined |
Falsy guard at top of validator |
7. allowPastDate flag behaviour
| Scenario | allowPastDate |
Expected |
|---|---|---|
| Invalid expression fallback | false |
Returns current time with seconds set to :59 |
| Invalid expression fallback | true |
Returns current time with actual seconds |
now expression |
false |
Seconds set to :59 |
now expression |
true |
Seconds unchanged |
| Day-of-week macro | either | Seconds not forced to :59 (only now path applies this) |
8. Case-insensitivity (cross-cutting)
Test each category above with UPPER, lower, and Mixed case to confirm the .toLowerCase() normalisation works end-to-end (e.g., MONDAY, NOW+5D, {FRI} 14:00).
|
Expressions in Type Builder control tooltip need to be updated |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx`:
- Around line 31-33: The three constant strings DATE_TIME_FORMAT_OFFSET,
DATE_TIME_FORMAT_DAY, and DATE_TIME_EXAMPLE in the DateTimeExpressionInput
component are incomplete and missing documentation for newly supported
expression variants. Update DATE_TIME_FORMAT_OFFSET to include short unit
variants (d, w, y, h, m) in addition to the long forms. Update
DATE_TIME_FORMAT_DAY to document that plain day names without braces are also
accepted. Update DATE_TIME_EXAMPLE to include concrete examples showing short
unit usage (like now+5d, now-2w), offset-only forms (like +5d, -2w), and plain
day names (like monday, friday) to give users complete information about all
supported formats.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9411534b-528f-4458-adfc-cc404da264ff
📒 Files selected for processing (1)
ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
…rity and consistency
|
@coderabbitai, how does this work look to be merged? Any issues you can point out? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== CHANGED FILES =="
git diff --name-only origin/develop...HEAD
echo
echo "== OUTLINE: ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
ast-grep outline ui/app/src/components/FormsEngine/lib/controlHelpers.tsx --view expanded || true
echo
echo "== DIFF: ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
git diff --unified=80 origin/develop...HEAD -- ui/app/src/components/FormsEngine/lib/controlHelpers.tsx
echo
echo "== OUTLINE: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
ast-grep outline ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx --view expanded || true
echo
echo "== DIFF: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
git diff --unified=80 origin/develop...HEAD -- ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
echo
echo "== SEARCH FOR RELATED TESTS =="
rg -n --hidden -S "validateDatePopulateExpression|processPopulateExpression|DateTimeExpressionInput|populate expression|allowPastDate" ui/app -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || true
echo
echo "== FIND CANDIDATE TEST FILES =="
fd -i 'controlHelpers' ui/app || true
fd -i 'DateTimeExpressionInput' ui/app || true
fd -i 'test' ui/app/src/components/FormsEngine || trueLength of output: 23138
Two issues stand out:
My read on overall quality:
Suggested minimum before merge:
So: probably safe after one more pass, but I wouldn’t merge as-is without those fixes. |
rart
left a comment
rart
left a comment
There was a problem hiding this comment.
See rabbit. Make sure rabbit clears your last commit (always).
…er clarity and consistency; improve date validation logic in processPopulateExpression to prevent past date selection.
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== PR HEAD =="
git rev-parse --short HEAD
echo
echo "== CHANGED FILES =="
git diff --name-only origin/develop...HEAD
echo
echo "== OUTLINE: ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
ast-grep outline ui/app/src/components/FormsEngine/lib/controlHelpers.tsx --view expanded || true
echo
echo "== DIFF: ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
git diff --unified=40 origin/develop...HEAD -- ui/app/src/components/FormsEngine/lib/controlHelpers.tsx
echo
echo "== OUTLINE: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
ast-grep outline ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx --view expanded || true
echo
echo "== DIFF: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
git diff --unified=40 origin/develop...HEAD -- ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
echo
echo "== SEARCH FOR RELATED TESTS / USAGES =="
rg -n -C2 --hidden -S "validateDatePopulateExpression|processPopulateExpression|DateTimeExpressionInput|allowPastDate|getNextDayOfWeek|setTimeOnDate" ui/app \
-g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || true
echo
echo "== CANDIDATE TEST FILES =="
fd -i 'controlHelpers' ui/app || true
fd -i 'DateTimeExpressionInput' ui/app || true
fd -i 'test' ui/app/src/components/FormsEngine || true
fd -i 'spec' ui/app/src/components/FormsEngine || true
echo
echo "== READ RELEVANT TOOLTIP SECTION =="
sed -n '1,220p' ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
echo
echo "== READ RELEVANT HELPER SECTION =="
sed -n '250,470p' ui/app/src/components/FormsEngine/lib/controlHelpers.tsxLength of output: 44606
Bottom line: this looks close, but I wouldn’t hand it off as fully ready yet. The core parser changes look reasonable, but I still see a few important gaps:
So my final recommendation is:
If those are addressed, I’d be much more comfortable calling it ready. |
…larity; enhance validation documentation in controlHelpers to specify valid expression formats and behavior regarding past dates.
|
@coderabbitai can you check if everything looks good? Please give it a final review before I hand over for human review. Tests won't be added yet, and the point number 2 of the review is intended |
|
Tip For best results, initiate chat on the files or code changes. #!/bin/bash
set -euo pipefail
echo "== PR HEAD =="
git rev-parse --short HEAD
echo
echo "== CHANGED FILES =="
git diff --name-only origin/develop...HEAD
echo
echo "== DIFF: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
git diff --unified=60 origin/develop...HEAD -- ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
echo
echo "== READ: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
sed -n '1,220p' ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
echo
echo "== DIFF: ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
git diff --unified=80 origin/develop...HEAD -- ui/app/src/components/FormsEngine/lib/controlHelpers.tsx
echo
echo "== READ: validator/processor section in ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
sed -n '520,760p' ui/app/src/components/FormsEngine/lib/controlHelpers.tsx#!/bin/bash
set -euo pipefail
echo "== PR HEAD =="
git rev-parse --short HEAD
echo
echo "== CHANGED FILES =="
git diff --name-only origin/develop...HEAD
echo
echo "== DIFF: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
git diff --unified=60 origin/develop...HEAD -- ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
echo
echo "== READ: ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx =="
sed -n '1,220p' ui/app/src/components/ContentTypeManagement/controls/DateTimeExpressionInput.tsx
echo
echo "== DIFF: ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
git diff --unified=80 origin/develop...HEAD -- ui/app/src/components/FormsEngine/lib/controlHelpers.tsx
echo
echo "== READ: validator/processor section in ui/app/src/components/FormsEngine/lib/controlHelpers.tsx =="
sed -n '520,760p' ui/app/src/components/FormsEngine/lib/controlHelpers.tsx |
|
With that context, I’d say this is good to hand over for human review. My final take:
So my recommendation is:
I wouldn’t block this handoff on anything else from my side. ✏️ Learnings added
✅ Action performedReview finished.
|
2 participants
craftercms/craftercms#5761
Summary by CodeRabbit
nowwith ± offsets (days/weeks/years/hours/minutes), and optional static time (HH:mm or HH:mm:ss).