[#37536] - [5.2] Task Scheduler require custom cron execution rules to be filled out
- Pending
- Medium
- Build: 4.2-dev
- # 37536
- Diff
- MagnusSinger:i37313
User tests: Successful: Unsuccessful:
Pull Request for Issue #37313
Summary of Changes
When you create a new scheduled task and select custom execution rules, and you don't specify a execution time, you can't save it now
Testing Instructions
Create Demo Task - Sleep
Select Cron Expression Advanced as Execution Rule
Do not select all values in area Cron match
Try to save, Joomla will show you that you still have empty fields
Actual result BEFORE applying this Pull Request
You were able to click save, but then ran into an error
Expected result AFTER applying this Pull Request
You see that there are required fields left and you cannot save
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration |
| Labels |
Added:
?
|
||
| Title |
|
||||||
@richard67 thanks for the hint, I'll have a look at it
See: #37537 (comment)
| Category | Administration | ⇒ | Administration Repository NPM Change JavaScript |
@bembelimen @richard67 I tried to control it now via JS, please let me know if this is okay
| Labels |
Added:
NPM Resource Changed
|
||
@dgrammatiko Could you do a quick review on the JS in this PR? To me it looks good, but I am not really a JS expert. Thanks in advance.
@MagnusSinger Could you extend your testing instructions by a test if other execution rules than advanced cron expressions don't require the cron parameters (i.e. mainly a test for your last change)? Thanks in advance. In general it's important that testing instructions don't cover only the test for the fixed issue but also the test that other behaviour of the changed code is not broken.
We need for sure also a server side validation for this (e.g. in the models validate method)
This pull requests has been automatically converted to the PSR-12 coding standard.
This pull request has been automatically rebased to 4.3-dev.
| Labels |
Added:
?
bug
|
||
| Labels |
Added:
PR-4.3-dev
|
||
This pull request has been automatically rebased to 4.4-dev.
| Title |
|
||||||
This issue still persists even in version 5.2. The "Cron Expression (Advanced)" option remains virtually unusable that way.
| Title |
|
||||||
@magnussinger Can you please fix the conflicts?
@MagnusSinger My comment from another, closed PR for the same issue also applies here, see #37350 (comment) .
Also the testing instructions should include a test that nothingness broken for other kinds of execution rules that the advanced cron expression .