CTP-6576: Enable auto calculation of grades when using advanced grading method and simple final grading method.

[cwarwicker]

This simply removes the restrictions which were in place to stop auto calculation when using advanced grading methods, and adds an additional check to allow it if the final stage method is different (in the average_grade class - the others had no restriction anyway, but were restricted by the mod_form not letting you select them).

[Copilot]

Pull request overview

This PR aims to enable automatic grade calculation (“automatic agreement”) when initial marking uses an advanced grading method (e.g., rubric) but the final agreed stage uses simple direct grading, and adds Behat coverage for these scenarios.

Changes:

• Adds a new Behat feature covering automatic agreement behavior with rubric-based initial stages and simple final-stage grading.
• Updates the module form to stop hiding automatic agreement settings when advanced grading is configured.
• Adjusts the average_grade auto-grader to allow auto-calculation when advanced grading is used initially but not for the final stage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
tests/behat/automatic_agreement_advanced_grading.feature	Adds Behat scenarios for advanced-initial + simple-final automatic agreement, plus a “no auto agree” case when advanced grading is used throughout.
mod_form.php	Removes UI hiding rules that previously prevented configuring automatic agreement alongside advanced grading.
classes/auto_grader/average_grade.php	Permits auto-agreed grade calculation when final-stage grading is simple, while still skipping when advanced grading is used throughout.

---

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

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

mod_form.php:1362

• By removing the advanced-grading hideIf() rules here, the form now allows selecting percentage_distance while advanced grading is active and finalstagegrading remains “same for all stages” (0). The percentage_distance auto-grader currently has no equivalent guard (unlike average_grade), so it can create a final-agreed feedback record with only a simple numeric grade even though the final agreed stage is still configured to use advanced grading. That leaves no rubric/guide instance/filling data for the agreed feedback and can lead to broken/blank advanced-grading details (and potentially errors) when viewing the agreed stage.

Fix should be to block auto-agreement when advanced grading is used for the final stage (e.g., add the same is_using_advanced_grading() && finalstagegrading == 0 early-return in classes/auto_grader/percentage_distance.php, and ideally cover it with a PHPUnit/Behat check).

        $this->form()->hideif('automaticagreementstrategy', 'numberofmarkers', 'eq', 1);
        $this->form()->hideif('automaticagreementrange', 'automaticagreementstrategy', 'eq', 'average_grade');
        $this->form()->hideif('automaticagreementrange', 'automaticagreementstrategy', 'eq', 'none');

[Copilot]

Pull request overview

Copilot reviewed 5 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

amd/src/mod_form.js:24

• The header block looks copy/pasted from a rubric module: the description and @module name don’t match this file/module (mod_coursework/mod_form). This makes it harder to trace AMD modules and can confuse automated docs/search.

/**
 * Rubric labels and make radio buttons work.
 *
 * @module    mod_coursework/rubric
 * @author    Conn Warwicker <conn.warwicker@catalyst-eu.net>
 * @copyright 2026 onwards Catalyst IT EU {@link https://catalyst-eu.net}
 * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
 */

amd/src/mod_form.js:44

• This toggle directly sets .fitem display to '' when shouldHide is false, which can override Moodle hideIf() rules for automaticagreementrange/roundingrule (e.g. strategy none should keep them hidden). Also closest('.fitem') can return null and throw. Consider only undoing visibility changes that this script applied, and re-trigger core hideIf when un-hiding.

    const shouldHide = gradingMethod.value !== '' && finalStageGrading.value === '0';

    [strategyEl, rangeEl, roundEl].forEach(el => {
        el.closest('.fitem').style.display = shouldHide ? 'none' : '';
    });

lib.php:552

• With the previous finalstagegrading == 1 override removed, there’s currently no server-side guard preventing unsupported automatic-agreement strategies being saved when advanced grading is configured for all stages (finalstagegrading == 0). Relying on the new JS-only hiding is easy to bypass (no-JS, crafted POST), and percentage_distance/average_grade_no_straddle will still auto-create numeric agreed grades without an advanced-grading breakdown. Consider enforcing a safe default here during update.

    $coursework->timemodified = time();
    $coursework->id = $coursework->instance;

[Copilot]

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

amd/src/mod_form.js:24

• The file header comment appears to be copied from amd/src/rubric.js (it says “Rubric labels…”, and @module mod_coursework/rubric). This module is mod_coursework/mod_form and is specifically about toggling automatic-agreement fields, so the header is misleading and makes it harder to search/trace modules.

/**
 * Rubric labels and make radio buttons work.
 *
 * @module    mod_coursework/rubric
 * @author    Conn Warwicker <conn.warwicker@catalyst-eu.net>
 * @copyright 2026 onwards Catalyst IT EU {@link https://catalyst-eu.net}
 * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
 */

amd/src/mod_form.js:44

• toggleAutomaticAgreementFields() sets el.closest('.fitem').style.display = ... and forces display = '' when shouldHide is false. This can override Moodle’s built-in hideIf() behaviour for these same elements (e.g. automaticagreementstrategy hidden when numberofmarkers == 1, and automaticagreementrange/roundingrule hidden based on strategy), making fields reappear when they should stay hidden.

A safer approach is to apply/remove a dedicated CSS class (e.g. mod-coursework-autoagreement-hidden) that sets display:none !important (you can inject a <style> tag once in init()), and only toggle that class; this avoids clobbering other hide/show logic. Also guard against closest('.fitem') returning null.

    const shouldHide = gradingMethod.value !== '' && finalStageGrading.value === '0';

    [strategyEl, rangeEl, roundEl].forEach(el => {
        el.closest('.fitem').style.display = shouldHide ? 'none' : '';
    });

[DavidUCL]

Setting display directly here later gets overridden by moodle core.
Either toggle a class instead (e.g. 'd-none') or rework hideIf (hideIf might be a lost cause)

1. Set number of markers to 2
2. Use rubric as the grading method.
3. Final stage grading to "Same for all stages"
4. Fields hide correctly
5. Change Number of markers from 2 to 1
6. Change it back to 2

Do you see the strategy dropdown?

[DavidUCL]

Can we put a $PAGE->pagetype check here like there is on line 52?
I haven't looked into it - but it references MDL-78528.

If not required, that's fine

[DavidUCL]

needs a version bump. That's pretty much it.