-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[6.1] Remove ExecRuleHelper #36439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 6.1-dev
Are you sure you want to change the base?
[6.1] Remove ExecRuleHelper #36439
Conversation
- Task::computeNextExecution() replaces ExecRuleHelper::nextExec(). - Replaces ExecRuleHelper() usages.
ExecRuleHelper is made redundant with the addition of Task::computeNextExecution(). Given all its usages have been replaced, this commit safely removes the class.
|
The removed helper PHP will have to be added to the list of files to be deleted on update in script.php. I will do that when this PR has been merged. |
|
@ditsuke I think @chmst indeed found an issue with your changed code, possibly here: https://github.com/joomla/joomla-cms/pull/36439/files#diff-b9f75ae33fad36d7a8968d12a2b78ec972c5e6d2c6fd7ed0024d764191ec48ecR572 . Possibly a type cast to object is not sufficient. I don't have looked up now what the constructor expects. |
Fixes issue identified with creating/saving tasks. Since Task objects are also now created by TaskModel::save(), the parameter resolution in the constructor must accommodate for records from both the database and form submissions (both cast to object). This commit adds the necessary check, so we can deal with both possible types.
|
@chmst @richard67 thanks for identifying the issue and the source! The latest commit should solve it. Please test. |
|
You fixed the code error, but I don't understand why the Task Params are twice. First in tab "New Task" and in tab "Advanced"? |
@chmst I addressed that in #36515 (needs test). We need the routine parameter fieldset to have a |
|
Could we please get a couple tests on this 😃 |
…lper Conflicts: administrator/components/com_scheduler/src/Task/Task.php
|
This pull request has automatically rebased to 4.2-dev. |
|
@ditsuke this PR is still open. could you please resolve the conflicts so that we can re-test? |
|
This pull request has been automatically rebased to 5.0-dev. |
|
This pull request has been automatically rebased to 5.1-dev. |
|
This pull request has been automatically rebased to 5.2-dev. |
|
I have tested this item 🔴 unsuccessfully on 4f91c6a An error has occurred. Function Location1 () JROOT/administrator/components/com_scheduler/src/Model/TaskModel.php:571 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36439. |
|
@cybersalt same as this comment |
|
This pull request has been automatically rebased to 5.3-dev. |
|
This pull request has been automatically rebased to 6.0-dev. |
# Conflicts: # administrator/components/com_scheduler/src/Helper/ExecRuleHelper.php # administrator/components/com_scheduler/src/Model/TaskModel.php # administrator/components/com_scheduler/src/Task/Task.php
| * | ||
| * @return ?Date|string Next due execution. | ||
| * | ||
| * @since 4.1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @since 4.1.0 | |
| * @since __DEPLOY_VERSION__ |
|
This pull request has been automatically rebased to 6.1-dev. |


Summary of Changes
Replace the ExecRuleHelper class with Task::computeNextExecution().
Testing Instructions
Actual result BEFORE applying this Pull Request
ExecRuleHelperexists, for little reason.Expected result AFTER applying this Pull Request
ExecRuleHelperis safely replaced byTask::computeNextExecution()Documentation Changes Required
-