Update for Modx3 #38

biz87 · 2025-12-01T10:53:42Z

No description provided.

add icon for menu

Remove run

add file browser for task's type as file

1.5.0 version

Mark-H · 2025-12-01T17:46:36Z

Thanks for the pull request! Some cool new features in there, too.

Looks good from a code review, only spotted very minor things I would want to change. I'll try to find the time to do some proper testing with this on both 2.x and 3.x in the next few weeks.

Copilot

Pull request overview

This PR updates Scheduler to better support MODX 3 while introducing reliability and performance improvements.

Add MODX 3 compatibility (service container bootstrap, namespaced processors, dedicated MODX 3 build script).
Introduce task retry logic (max_retries/retry_delay), improved failure notifications (templateable emails), and fix a run-processor race condition with atomic status updates.
Add composite index (status, timing) for faster task selection and several UI updates (history grid retry count, icons, and settings).

Reviewed changes

Copilot reviewed 28 out of 30 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
core/components/scheduler/processors/mgr/runs/remove.class.php	Minor formatting; no functional changes
core/components/scheduler/processors/mgr/runs/multiple.class.php	Minor formatting; no functional changes
core/components/scheduler/processors/mgr/runs/history.class.php	Coding style cleanup; update action icon
core/components/scheduler/model/schema/scheduler.mysql.schema.xml	Add retry fields, composite index for sTaskRun
core/components/scheduler/model/scheduler/staskrun.class.php	Safer/clearer timing rounding and parsing
core/components/scheduler/model/scheduler/stask.class.php	Retry handling, Throwable catching, templateable failure emails
core/components/scheduler/model/scheduler/sprocessortask.class.php	MODX 3 namespaced processors support and path resolution
core/components/scheduler/model/scheduler/mysql/staskrun.map.inc.php	Map retry_count and status_timing index
core/components/scheduler/model/scheduler/mysql/stask.map.inc.php	Map max_retries and retry_delay
core/components/scheduler/lexicon/ru/default.inc.php	New settings/labels for retries and email templating (RU)
core/components/scheduler/lexicon/en/default.inc.php	New settings/labels (EN); minor text edit
core/components/scheduler/index.class.php	MODX 2/3 initialization; JS includes formatting
core/components/scheduler/docs/changelog.txt	New 1.7.0 section and reworked chronology/content
core/components/scheduler/controllers/index.class.php	Minor formatting
core/components/scheduler/bootstrap.php	MODX 3 service bootstrap (registers scheduler service)
assets/components/scheduler/run.php	Atomic status update to prevent race condition; timing/cleanup
assets/components/scheduler/js/mgr/widgets/windows.tasks.js	UI updates incl. retry fields; minor cleanups
assets/components/scheduler/js/mgr/widgets/grid.history.js	History grid columns (retry_count), actions, and refactors
assets/components/scheduler/js/mgr/misc/scheduler.utils.js	Minor whitespace cleanup
assets/components/scheduler/js/mgr/combos.js	Style cleanup; no functional changes
assets/components/scheduler/css/bootstrap.buttons.css	Formatting
_build/validators/requirements.script.php	Version checks updated; MODX 2/3 compatibility
_build/resolvers/resolve.tables.php	Table create/remove with MODX 2/3 compatibility
_build/resolvers/resolve.dbchanges.php	Upgrades include retry fields and composite index
_build/data/transport.menu.php	Menu icon updated (font icon)
_build/data/settings.php	New settings: email template/subject, tasks_per_run
_build/build.transport.php	Bump version to 1.7.0
_build/build.transport.modx3.php	New MODX 3 build script
_build/build.schema.modx3.php	New MODX 3 schema build helper
_bootstrap/index.php	Path handling cleanup

Comments suppressed due to low confidence (1)

assets/components/scheduler/js/mgr/widgets/grid.history.js:302

This property is overwritten by another property in the same object literal.

    , removeRun: function (btn, e) {
        const ids = this._getSelectedIds();
        if (!ids.length) {
            return false;
        }
        Ext.MessageBox.confirm(
            _('ms2_menu_remove_title'),
            ids.length > 1
                ? _('scheduler.run_multiple_remove_confirm')
                : _('scheduler.run_remove_confirm'),
            function (val) {
                if (val === 'yes') {
                    this.runAction('remove');
                }
            }, this
        );
    },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-12-01T17:52:53Z

assets/components/scheduler/js/mgr/widgets/grid.history.js

+            return false;
+        }
+        Ext.MessageBox.confirm(
+            _('ms2_menu_remove_title'),


The confirmation title key references a MiniShop2 lexicon ('ms2_menu_remove_title'). Use a Scheduler lexicon key instead (e.g., _('scheduler.run_remove') or add a dedicated key like _('scheduler.menu_remove_title')).

Suggested change

_('ms2_menu_remove_title'),

_('scheduler.run_remove'),

Copilot · 2025-12-01T17:52:53Z

assets/components/scheduler/js/mgr/widgets/grid.history.js

+    , removeRun: function (btn, e) {
+        const ids = this._getSelectedIds();
+        if (!ids.length) {
+            return false;
+        }
+        Ext.MessageBox.confirm(
+            _('ms2_menu_remove_title'),
+            ids.length > 1
+                ? _('scheduler.run_multiple_remove_confirm')
+                : _('scheduler.run_remove_confirm'),
+            function (val) {
+                if (val === 'yes') {
+                    this.runAction('remove');
+                }
+            }, this
+        );
+    },
+    runAction(method) {
+        const ids = this._getSelectedIds();
+        if (!ids.length) {
+            return false;
+        }
+        MODx.Ajax.request({
+            url: this.config.url,
+            params: {
+                action: 'mgr/runs/multiple',
+                method: method,
+                ids: Ext.util.JSON.encode(ids),
+            },
+            listeners: {
+                success: {
+                    fn: function () {
+                        //noinspection JSUnresolvedFunction
+                        this.refresh();
+                    }, scope: this
+                },
+                failure: {
+                    fn: function (response) {
+                        MODx.msg.alert(_('error'), response.message);
+                    }, scope: this
+                },
+            }
+        })
    }


This block duplicates removeRun/runAction which are already defined earlier in the same object. Having duplicate method names in the object literal is confusing and the latter silently overrides the former. Please remove one set and keep a single implementation.

Copilot · 2025-12-01T17:52:54Z

core/components/scheduler/index.class.php

+        $this->addJavascript($this->scheduler->config['jsUrl'] . 'mgr/scheduler.class.js');
+        $this->addJavascript($this->scheduler->config['jsUrl'] . 'mgr/combos.js');
        $this->addHtml('<script type="text/javascript">
        Ext.onReady(function() {
-            Scheduler.config = '.$this->modx->toJSON($this->scheduler->config).';
+            Scheduler.config = ' . $this->modx->toJSON($this->scheduler->config) . ';
        });


Potential null dereference: if the MODX 3 service lookup fails (e.g., service not registered), $this->scheduler is null and accessing ->config will fatal. Guard against a missing service and either fall back to getService() or return an error/notice before using $this->scheduler.

Copilot · 2025-12-01T17:52:54Z

core/components/scheduler/model/scheduler/stask.class.php

+            $newRun->setTiming('+' . $retryDelay . ' seconds');
+
+            if ($newRun->save()) {
+                $this->xpdo->log(\xPDO::LOG_LEVEL_INFO,
+                    "[Scheduler] Retry {$newRun->get('retry_count')}/{$maxRetries} scheduled for task {$this->get('namespace')}::{$this->get('reference')}"
+                );
+            }
+        }


Check the return value of setTiming(). If it returns false (e.g., invalid strtotime input), saving will schedule a run with an invalid or zero timing. Suggest: if (!$newRun->setTiming('+' . $retryDelay . ' seconds')) { log and return; } before save.

Copilot · 2025-12-01T17:52:54Z

core/components/scheduler/lexicon/en/default.inc.php

 $_lang['scheduler.run_update'] = "Update scheduled run";
 $_lang['scheduler.run_remove'] = "Remove scheduled run";
-$_lang['scheduler.runs_remove'] = "Remove scheduled runs";
+$_lang['scheduler.runs_remove'] = "Remove  scheduled runs";


Correct double space in the string.

Suggested change

$_lang['scheduler.runs_remove'] = "Remove scheduled runs";

$_lang['scheduler.runs_remove'] = "Remove scheduled runs";

core/components/scheduler/docs/changelog.txt

Copilot · 2025-12-01T17:52:55Z

core/components/scheduler/index.class.php

+                        $this->modx->log(1, $e->getMessage());
+                    } catch (\Psr\Container\ContainerExceptionInterface $e) {
+                        $this->modx->log(1, $e->getMessage());


Use the appropriate log level constants (e.g., modX::LOG_LEVEL_ERROR) instead of magic numbers to improve readability and maintain consistency.

Suggested change

$this->modx->log(1, $e->getMessage());

} catch (\Psr\Container\ContainerExceptionInterface $e) {

$this->modx->log(1, $e->getMessage());

$this->modx->log(modX::LOG_LEVEL_ERROR, $e->getMessage());

} catch (\Psr\Container\ContainerExceptionInterface $e) {

$this->modx->log(modX::LOG_LEVEL_ERROR, $e->getMessage());

Copilot · 2025-12-01T17:52:55Z

assets/components/scheduler/js/mgr/widgets/grid.history.js

+    , removeRun: function (btn, e) {
+        const ids = this._getSelectedIds();
+        if (!ids.length) {
+            return false;
+        }
+        Ext.MessageBox.confirm(
+            _('ms2_menu_remove_title'),
+            ids.length > 1
+                ? _('scheduler.run_multiple_remove_confirm')
+                : _('scheduler.run_remove_confirm'),
+            function (val) {
+                if (val === 'yes') {
+                    this.runAction('remove');
+                }
+            }, this
+        );
+    },
+    runAction(method) {
+        const ids = this._getSelectedIds();
+        if (!ids.length) {
+            return false;
+        }
+        MODx.Ajax.request({
+            url: this.config.url,
+            params: {
+                action: 'mgr/runs/multiple',
+                method: method,
+                ids: Ext.util.JSON.encode(ids),
+            },
+            listeners: {
+                success: {
+                    fn: function () {
+                        //noinspection JSUnresolvedFunction
+                        this.refresh();
+                    }, scope: this
+                },
+                failure: {
+                    fn: function (response) {
+                        MODx.msg.alert(_('error'), response.message);
+                    }, scope: this
+                },
+            }
+        })
    }


This property is overwritten by another property in the same object literal.

Suggested change

, removeRun: function (btn, e) {

const ids = this._getSelectedIds();

if (!ids.length) {

return false;

}

Ext.MessageBox.confirm(

_('ms2_menu_remove_title'),

ids.length > 1

? _('scheduler.run_multiple_remove_confirm')

: _('scheduler.run_remove_confirm'),

function (val) {

if (val === 'yes') {

this.runAction('remove');

}

}, this

);

},

runAction(method) {

const ids = this._getSelectedIds();

if (!ids.length) {

return false;

}

MODx.Ajax.request({

url: this.config.url,

params: {

action: 'mgr/runs/multiple',

method: method,

ids: Ext.util.JSON.encode(ids),

},

listeners: {

success: {

fn: function () {

//noinspection JSUnresolvedFunction

this.refresh();

}, scope: this

},

failure: {

fn: function (response) {

MODx.msg.alert(_('error'), response.message);

}, scope: this

},

}

})

}

Copilot · 2025-12-01T17:52:55Z

assets/components/scheduler/js/mgr/combos.js

+        if (!/^\//.test(config.openTo)) {
+            config.openTo = '/' + config.openTo
+        }
+        if (!/$\//.test(config.openTo)) {


This assertion can never match.

Suggested change

if (!/$\//.test(config.openTo)) {

if (!/\/$/.test(config.openTo)) {

Mark-H · 2025-12-01T18:20:55Z

(You don't have to do anything with the copilot review, I'll check it out.)

biz87 and others added 16 commits December 3, 2023 14:34

remove history run

25d312c

miltiple remove runs

5ec5b42

english lexicon

c951518

add icon for menu

7fa0828

add file browser for task's type as file

7c66645

Merge pull request #1 from biz87/icon

dab97f5

add icon for menu

Merge pull request #2 from biz87/remove-run

ba54c3d

Remove run

Merge pull request #3 from biz87/file-browser

95c9d3e

add file browser for task's type as file

1.5.0 version

39024df

Merge pull request #4 from biz87/1-5-0

d5687da

1.5.0 version

1.6.0 - support MODX 3

9dfcd91

fix some errors for modx3 init

f6d3e08

Добавил несколько недостающих классов Модели

f23be0b

refactoring support MODX

91cbc5c

version 1.7.0

a5ee5ea

Merge branch 'master' into modx-3-rebase

707d3cb

biz87 changed the title ~~Подогнал под MODX3 + MiniShop3~~ Update for Modx3 Dec 1, 2025

Mark-H requested a review from Copilot December 1, 2025 17:46

Copilot started reviewing on behalf of Mark-H December 1, 2025 17:47 View session

Copilot finished reviewing on behalf of Mark-H December 1, 2025 17:50

Copilot AI reviewed Dec 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update for Modx3 #38

Update for Modx3 #38

Uh oh!

biz87 commented Dec 1, 2025

Uh oh!

Mark-H commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Mark-H commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	$_lang['scheduler.runs_remove'] = "Remove scheduled runs";
	$_lang['scheduler.runs_remove'] = "Remove scheduled runs";

	if (!/$\//.test(config.openTo)) {
	if (!/\/$/.test(config.openTo)) {

Update for Modx3 #38

Are you sure you want to change the base?

Update for Modx3 #38

Uh oh!

Conversation

biz87 commented Dec 1, 2025

Uh oh!

Mark-H commented Dec 1, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Mark-H commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants