Skip to content

Conversation

@KBorm
Copy link
Contributor

@KBorm KBorm commented Feb 9, 2026

Note: Please fill out all relevant sections and remove irrelevant ones.

🔀 Purpose of this PR:

  • Fixes a bug
  • Updates for a new Moodle version
  • [] Adds a new feature of functionality
  • Improves or enhances existing features
  • Refactoring: restructures code for better performance or maintainability
  • Testing: add missing or improve existing tests
  • Miscellaneous: code cleaning (without functional changes), documentation, configuration, ...

📝 Description:

Please describe the purpose of this PR in a few sentences.

  • What feature or bug does it address?

This PR is a supplement to "Customise buttons in adminapprove step #266".
#266 allows to customise the adminapprove buttons in order to clarify what will happen in case of proceed or rollback.

Unfortunately the final code adds six configuraion values instead of two, and I was not happy with this solution ;-)

The reason for this PR is that the decision table contains buttons for changing "all" records, "selected" records, and "individual" records. So there are several ways to do the same thing.
If you take a look at Moodle, for example, in the admin user overview or the participant overview in courses, you will see that there is a different approach.
Therefore, I have now adjusted the decision table so that a configuration of the original two values is sufficient.

  • Why is this change or addition necessary?
  • clearer user interface on step configuration.
  • clearer user interface on approve step with less buttons
  • UI on approval step is more consistent with other moodle table handling
  • less code because of using more core code
  • a racing problem when handling ‘all’ entries has been avoided. Previously, it could happen that the user saw X entries in the table, but Y entries were processed because a new SQL query was executed during processing.
  • What is the expected behavior after the change?

admin approve works properly with only two button labels configured


📋 Checklist

Please confirm the following (check all that apply):

  • I have phpunit and/or behat tests that cover my changes or additions.
  • Code passes the code checker without errors and warnings.
  • Code passes the moodle-ci/cd pipeline on all supported Moodle versions or the ones the plugin supports.
  • Code does not have var_dump() or var_export or any other debugging statements (or commented out code) that
    should not appear on the productive branch.
  • Code only uses language strings instead of hard-coded strings.
  • If there are changes in the database: I updated/created the necessary upgrade steps in db/upgrade.php and
    updated the version.php.
  • If there are changes in javascript: I build new .min files with the grunt amd command.
  • If it is a Moodle update PR: I read the release notes, updated the version.php and the CHANGES.md.
    I ran all tests thoroughly checking for errors. I checked if bootstrap had any changes/deprecations that require
    changes in the plugins UI.

🔍 Related Issues


🧾📸🌐 Additional Information (like screenshots, documentation, links, etc.)

Any other relevant information.

  • Creating backups (Create backup step) with behat is tricky because of a static counter
    that is incremented with each backup but only reset in a cron run.
    This counter affects all behat test cases within a testsuite.
    Backups are skipped if 10 backups are already created and a test case will fail
    without warning if only scheduled task "tool_lifecycle\task\lifecycle_task" is used. (trigger cron is required.)
    This counter should be reset with every single test setup (without doing it in the test case itself).
    This is way I switched to adhoc create backup step in the adminapprove step tests.

  • I added a sort condition to the decision table because the postgresql database
    returns the records in a (for me) unpredictable order.

  • Decision table: One could consider whether to remove the buttons inside of the decision table,
    because the corresponding actions can also be performed by selecting and triggering the action.
    These actions are still duplicated in the UI.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant