aminghadersohi opened a new pull request, #43307:
URL: https://github.com/apache/superset/pull/43307

   ### SUMMARY
   
   The **Schedule** column on Alerts & Reports shows a human-readable rendering 
of the report's crontab, produced by `ReportSchedule.crontab_humanized` → 
`cron_descriptor.get_description()`. When a schedule restricts *both* the 
day-of-month and the day-of-week fields, that description is wrong.
   
   `0 9 7-11,19-23 * 2` renders as:
   
   > At 09:00 AM, on day 7 through 11 and 19 through 23 of the month, only on 
Tuesday
   
   The comma plus "only on" reads as an intersection — "the 7th–11th and 
19th–23rd, but only when that day is a Tuesday". POSIX cron does the opposite: 
when both fields are restricted they are **OR**'d. `croniter`, which selects 
the actual fire times in `superset/tasks/cron_util.py`, implements that 
correctly, so the report also fires on every Tuesday outside those two ranges. 
Verified against `croniter`: for the expression above the first fire time after 
2026-09-01 is 2026-09-01 09:00 — a Tuesday that is in neither day-of-month 
range.
   
   So the picker is right and the description is wrong. The frontend cron 
picker (`react-js-cron`, via `CronPicker`) already renders this correctly: its 
locale sets `prefixWeekDaysForMonthAndYearPeriod: 'or'`, so the builder shows 
"… on day 7-11,19-23 **or** Tuesday". Only the backend-generated text 
disagreed, which is exactly the mismatch users hit — they configure a schedule 
in the picker and then read a contradictory description in the list.
   
   Rather than swapping out `cron_descriptor` (which is otherwise fine and 
handles `L`, `W`, `#n`, i18n, etc.), this adds a small `get_cron_description()` 
wrapper in `superset/tasks/cron_util.py` — the module that already owns cron 
interpretation via `croniter`. It only changes the output for the ambiguous 
case: when neither the day-of-month nor the day-of-week field is `*`/`?`, the 
day-of-week clause is rejoined with "or". Every other expression is passed 
through `cron_descriptor` untouched.
   
   | crontab | before | after |
   |---|---|---|
   | `0 9 7-11,19-23 * 2` | …of the month, only on Tuesday | …of the month, 
**or on** Tuesday |
   | `0 9 1,15 * 1-5` | …of the month, Monday through Friday | …of the month, 
**or** Monday through Friday |
   | `0 9 L * 5` | on the last day of the month, only on Friday | on the last 
day of the month, **or on** Friday |
   | `0 9 * * 2` | only on Tuesday | *(unchanged)* |
   | `0 9 7-11 * *` | on day 7 through 11 of the month | *(unchanged)* |
   
   Known limitations: the rewrite is applied to standard 5-field expressions 
only (what the picker and the API validator produce); 6-field expressions fall 
through to the unmodified `cron_descriptor` output because the field offsets 
are ambiguous. The "only " prefix strip is English-specific — `cron_descriptor` 
is invoked with its default `en_US` locale here, and if the prefix isn't 
present the clause is emitted verbatim, so a different locale degrades to ", or 
<clause>" rather than breaking.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   No UI code changed — this is the text returned by the `crontab_humanized` 
field of `GET /api/v1/report/`, rendered verbatim in the Schedule column. The 
before/after strings are in the table above.
   
   ### TESTING INSTRUCTIONS
   
   Regression tests in `tests/unit_tests/tasks/test_cron_util.py`:
   
   - `test_get_cron_description` — parametrized over the ambiguous and 
unambiguous cases; the ambiguous rows fail on `master` (`assert '…of the month, 
only on Tuesday' == '…of the month, or on Tuesday'`).
   - `test_get_cron_description_matches_fire_times` — pins the description 
against what `croniter` actually schedules, so the two can't drift apart again.
   
   ```
   pytest tests/unit_tests/tasks/test_cron_util.py tests/unit_tests/reports/ -q
   281 passed
   ```
   
   Manually: create a report with schedule type "CRON Schedule" and crontab `0 
9 7-11,19-23 * 2`, then look at the Schedule column on the Alerts & Reports 
list. It should read "At 09:00 AM, on day 7 through 11 and 19 through 23 of the 
month, or on Tuesday", matching what the recurring picker shows for the same 
expression.
   
   ### ADDITIONAL INFORMATION
   
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to