CTP-5980: Add Grading Audit report page.

[cwarwicker]

No description provided.

[Copilot]

Pull request overview

Adds a new “Grading Audit” report page to the mod_coursework activity module, intended for moderators to review grading distributions and moderation/plagiarism-related counts, with supporting UI, language strings, and automated tests.

Changes:

• Introduces a new audit report implementation (classes/audit.php) and a Mustache template to render summary/stats tables.
• Adds a navigation entry (for users with mod/coursework:moderate) and an action endpoint to display the report.
• Adds PHPUnit coverage for the core statistics helpers and a Behat feature covering visibility/access to the report.

Reviewed changes

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

Show a summary per file

File	Description
tests/phpunit/audit_test.php	Adds unit tests for audit statistics and boundary counting.
tests/behat/grading_audit.feature	Adds Behat scenarios validating moderator-only access/visibility for the audit page.
templates/audit/report.mustache	Introduces the report UI layout for summary, moderation, overall stats, and per-marker breakdown.
lib.php	Adds a settings navigation link to the Grading Audit page for moderators.
lang/en/coursework.php	Adds new UI strings for the audit report headings/columns.
classes/models/coursework.php	Adds a helper to fetch distinct assessors with feedback for an activity.
classes/audit.php	Implements data gathering and statistics calculations for the audit report.
actions/audit.php	Adds a page controller to render the audit report for a given cmid.

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

actions/audit.php:38

• This page script doesn't set $PAGE title/heading or add a navbar item (other scripts in actions/ do), and it also contains a typo in the comment plus a leftover commented-out line. Setting page metadata improves consistency and avoids relying on defaults.

// Must have the moderatore capability.
$context = \core\context\module::instance($cmid);
require_capability('mod/coursework:moderate', $context);

// Get the coursework instance.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

actions/audit.php:47

• String component usage is inconsistent with other new usage in this PR (e.g., lib.php:949 and mustache templates use "mod_coursework"). Standardising on the same component avoids subtle string lookup issues and keeps the codebase consistent.

echo $OUTPUT->heading(get_string('gradingaudit', 'coursework'));

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

classes/audit.php:206

• retrieve_submissions_by_coursework() loads all submission records just to count them. For large activities this adds avoidable DB + memory overhead; use a count_records query instead. (This also clarifies the assessor comment: get_all_assessors() returns users who have given feedback, not necessarily allocated markers.)

            'assessors' => implode(', ', $assessors),
            'moderators' => implode(', ', $moderators),
            'submissions' => count($this->coursework->retrieve_submissions_by_coursework()),
        ];
    }

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

classes/audit.php:205

• Counting submissions by loading all submission records (retrieve_submissions_by_coursework()) is unnecessarily expensive for large activities. You can count directly in the DB instead.

            'submissions' => count($this->coursework->retrieve_submissions_by_coursework()),
        ];

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

actions/audit.php:44

• The page heading uses a different string component than the navigation link (lib.php uses 'mod_coursework'). Keeping this consistent avoids accidental [[missingstring]] output if one component isn't mapped/loaded in a given context.

echo $OUTPUT->heading(get_string('gradingaudit', 'coursework'));

classes/audit.php:205

• get_assessor_data() calls get_grades() (and therefore get_feedback()/DB query) once per assessor, which can become an N+1 query pattern for large activities. Consider fetching all relevant feedback grades in one query and grouping in PHP by assessorid to keep the report generation bounded.

        // Now let's calculate everything per marker.
        $data['assessors_data'] = [];
        $assessors = $this->coursework->get_all_assessors();
        foreach ($assessors as $id => $assessor) {
            // Get the grade data for just this assessor.
            $grades = $this->get_grades($assessor->id);

            $assdata = [];
            $assdata['name'] = fullname($assessor);

            // Boundary statistics.
            $assdata['boundaries'] = [];
            foreach ($boundaries as $boundary) {
                $assdata['boundaries'][] = $this->count_grades_within_boundary($grades, $boundary);
            }

            // Calculate their overall statistics.
            $stats = $this->calculate_statistics($grades);
            $assdata['mean'] = $stats['stats_mean'];
            $assdata['median'] = $stats['stats_median'];
            $assdata['sd'] = $stats['stats_sd'];
            $data['assessors_data'][] = $assdata;
        }

tests/phpunit/audit/audit_testable.php:27

• This helper test class is missing the standard MOODLE_INTERNAL guard used throughout tests/phpunit (e.g. tests/phpunit/models/coursework_test.php:27).

namespace mod_coursework;

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

classes/models/coursework.php:1262

• get_all_assessors() currently returns users from any feedback record on the activity, including moderation/final-stage feedback. That can cause non-marker users to appear in the audit “Markers” list and per-marker table. Other parts of the codebase treat marker feedback as ismoderation = 0, isfinalgrade = 0, and stageidentifier LIKE 'assessor%' (e.g. lib.php:1091-1094). Filter this query accordingly.

              FROM {coursework_feedbacks} f
              JOIN {coursework_submissions} s ON s.id = f.submissionid
              JOIN {user} u ON u.id = f.assessorid
             WHERE s.courseworkid = :courseworkid
          ORDER BY u.lastname, u.firstname", [

classes/audit.php:188

• get_assessor_data() calls get_grades() inside a loop over assessors, and get_grades() performs a DB query via get_feedback(). This creates an N+1 query pattern (one query per assessor) which may be slow on modules with many markers/feedback records. Consider fetching all relevant feedback/grades in a single query and grouping in PHP (or using an aggregated SQL query grouped by assessor) to keep the report page performant.

        // Now let's calculate everything per marker.
        $data['assessors_data'] = [];
        $assessors = $this->coursework->get_all_assessors();
        foreach ($assessors as $id => $assessor) {
            // Get the grade data for just this assessor.
            $grades = $this->get_grades($assessor->id);

[DavidUCL]

A few things to be checked.

Looks like the automated checks haven't run

[Copilot]

Pull request overview

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

Suppressed comments (7)

tests/phpunit/audit_test.php:50

• @covers points to audit::calculate_statistics, but the implementation under test is audit::calculate_grade_statistics(). As written, coverage mapping won’t match the real method.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit_test.php:65

• @covers points to audit::calculate_statistics, but the implementation under test is audit::calculate_grade_statistics(). As written, coverage mapping won’t match the real method.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit_test.php:82

• @covers points to audit::calculate_statistics, but the implementation under test is audit::calculate_grade_statistics(). As written, coverage mapping won’t match the real method.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit/audit_testable.php:38

• The docblock says this exposes calculate_statistics, but the underlying method is calculate_grade_statistics(). Updating the comment avoids confusion when navigating the tests.

     * Expose calculate_statistics for testing.

classes/audit.php:366

• count_marked_submissions() currently treats any non-final-agreed feedback as “marked” via submission::get_assessor_feedbacks(), which includes all feedback records (draft/unfinalised, moderation, etc.). This can over-count “marked” submissions and make the summary inconsistent with get_feedback()/get_grades(), which explicitly filters to finalised assessor_* feedback.

    protected function count_marked_submissions(array $submissions, int $assessorid = null): int {
        $marked = 0;
        foreach ($submissions as $submission) {
            // Get all assessor stage feedbacks for this submission.
            $feedback = $submission->get_assessor_feedbacks();

            // If we specify an assessor, filter by just theirs.
            if (!is_null($assessorid)) {
                $feedback = array_filter($feedback, function ($f) use ($assessorid) {
                    return (int)$f->assessorid === $assessorid;
                });
            }

            // If any exist, yes this was marked.
            if ($feedback) {
                $marked++;
            }
        }
        return $marked;
    }

classes/audit.php:392

• adjust_grades_to_percentages() rounds converted grades to whole numbers, which can shift grades across class boundaries when boundaries include decimals (e.g. 69.50–69.99) and also reduces precision for mean/median/sd calculations.

        return array_map(fn($grade) => round(($grade / $maxgrade) * 100), $grades);

actions/audit.php:44

• This page doesn’t set $PAGE title/heading before output. Other actions scripts set these so the browser title, navbar, and accessibility landmarks are correct; currently only an H1 is printed.

$PAGE->set_context($context);
$PAGE->set_url(new moodle_url('/mod/coursework/actions/audit.php', ['cmid' => $cmid]));

echo $OUTPUT->header();
echo $OUTPUT->heading(get_string('gradingaudit', 'coursework'));

[Copilot]

Pull request overview

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

Suppressed comments (13)

actions/audit.php:44

• get_string() uses the wrong component ('coursework'), so the heading will likely render as a missing-string placeholder. The string is defined under the plugin component (mod_coursework).

echo $OUTPUT->heading(get_string('gradingaudit', 'coursework'));

tests/phpunit/audit_test.php:50

• The @covers annotation references audit::calculate_statistics, but the implementation under test is audit::calculate_grade_statistics(). This makes coverage reporting misleading.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit_test.php:65

• The @covers annotation references audit::calculate_statistics, but the implementation under test is audit::calculate_grade_statistics(). This makes coverage reporting misleading.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit_test.php:82

• The @covers annotation references audit::calculate_statistics, but the implementation under test is audit::calculate_grade_statistics(). This makes coverage reporting misleading.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit/audit_testable.php:38

• Docblock mentions calculate_statistics, but this helper actually exposes calculate_grade_statistics(). Keeping the naming consistent avoids confusion when maintaining tests.

     * Expose calculate_statistics for testing.

classes/audit.php:187

• summary_stats is used as a Mustache section ({{#summary_stats}}) and should be an object context. Using a PHP array here risks Mustache iterating over the values, which would leave max/min/flagged unresolved in the template.

            'summary_stats' => $this->calculate_grade_statistics($this->get_grades()),

classes/audit.php:323

• Use the new $includeflagged switch so per-assessor stats don’t trigger a DB query for flagged submissions.

            $stats = $this->calculate_grade_statistics($grades);

classes/audit.php:143

• moderation_sample (and nested marked/moderated blocks) are used as Mustache sections. Converting these contexts to objects before returning avoids section-iteration issues in Mustache rendering.

        return $data;

classes/audit.php:168

• moderation_stats is used as a Mustache section ({{#moderation_stats}}). Casting it to an object before returning avoids Mustache treating it as an iterable list of scalar values.

        return $data;

classes/audit.php:432

• calculate_grade_statistics() always queries flagged submissions. This method is called per assessor in get_marker_statistics(), but that table doesn’t use flagged counts. Add a switch to skip the DB query when not needed.

    protected function calculate_grade_statistics(array $grades): array {

classes/audit.php:441

• When $includeflagged is false, avoid querying flagged submissions in the empty-grades case.

            $data['flagged'] = $this->count_flagged_submissions();

classes/audit.php:464

• When $includeflagged is false, avoid querying flagged submissions in the non-empty-grades case.

        $data['flagged'] = $this->count_flagged_submissions();

classes/audit.php:392

• Rounding converted percentages to 0 decimals can reduce accuracy (and can push grades across boundaries). Using 2 decimals keeps stats and boundary comparisons consistent with typical boundary formats (e.g. 70.00–100.00).

        return array_map(fn($grade) => round(($grade / $maxgrade) * 100), $grades);

[DavidUCL]

A couple things. My dev environment is a little dirty - can you confirm that a grade type of None on a coursework (no submissions required I think) breaks your grading audit page?

[cwarwicker]

A couple things. My dev environment is a little dirty - can you confirm that a grade type of None on a coursework (no submissions required I think) breaks your grading audit page?

I've hidden the link to the page now if it's not numeric grading.

[Copilot]

Pull request overview

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

Suppressed comments (8)

tests/phpunit/audit_test.php:66

• @covers refers to audit::calculate_statistics, but the method under test is audit::calculate_grade_statistics (exposed via calculate_statistics_public()). This breaks coverage mapping.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */
    public function test_calculate_statistics_for_odd_grade_count(): void {

tests/phpunit/audit_test.php:83

• @covers refers to audit::calculate_statistics, but the method under test is audit::calculate_grade_statistics (exposed via calculate_statistics_public()). This breaks coverage mapping.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */
    public function test_calculate_statistics_for_even_grade_count(): void {

classes/audit.php:168

• moderation_stats is an associative array, but the template uses it as a Mustache section ({{#moderation_stats}} ... {{/moderation_stats}}). Arrays are treated as iterables, so this will not provide a total/total_agreed/total_disagreed context unless moderation_stats is an object (stdClass).

        $data['moderation_stats'] = [
            'total' => 0,
            'total_agreed' => 0,
            'total_disagreed' => 0,
        ];

classes/audit.php:187

• summary_stats is returned as an associative array, but templates/audit/report.mustache uses it as a Mustache section ({{#summary_stats}}). To ensure the section renders once with mean/median/max/min/sd/flagged available, pass summary_stats as an object (stdClass).

            'submissions' => $submissions,
            'total_submissions' => count($submissions),
            'marked' => $marked,
            'unmarked' => count($submissions) - $marked,
            'summary_stats' => $this->calculate_grade_statistics($this->get_grades()),
        ];

classes/audit.php:486

• array_column($feedback, 'submissionid') will not work reliably here because $feedback is an array of stdClass records from $DB->get_records_sql(). This can result in an empty $ids list (or warnings) and an incorrect plagiarism flag count.

        $feedback = $this->get_feedback();
        $ids = array_unique(array_column($feedback, 'submissionid'));
        if ($ids) {

tests/phpunit/audit/audit_testable.php:42

• Docblock refers to calculate_statistics, but this helper actually exposes calculate_grade_statistics(). This mismatch makes the test helper harder to understand/maintain.

    /**
     * Expose calculate_statistics for testing.
     *
     * @param array $grades
     * @return array
     */
    public function calculate_statistics_public(array $grades): array {
        return $this->calculate_grade_statistics($grades);

tests/phpunit/audit_test.php:50

• @covers refers to audit::calculate_statistics, but the method under test is audit::calculate_grade_statistics (exposed via calculate_statistics_public()). This breaks coverage mapping.

This issue also appears in the following locations of the same file:

• line 63
• line 80

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */
    public function test_calculate_statistics_returns_zeroes_for_empty_grades(): void {

classes/audit.php:143

• moderation_sample (and its marked/moderated children) are associative arrays, but the template uses them via Mustache sections ({{#moderation_sample}}, then {{#marked}} / {{#moderated}}). Arrays are treated as iterables, so the nested keys won't be available as section context unless these are objects (stdClass).

        return $data;

[Copilot]

Pull request overview

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

Suppressed comments (9)

tests/phpunit/audit_test.php:49

• The @covers annotation refers to a non-existent method calculate_statistics. The implementation under test is calculate_grade_statistics, so this annotation is currently inaccurate for coverage reporting.

     * @covers \mod_coursework\audit::calculate_statistics

tests/phpunit/audit_test.php:64

• The @covers annotation refers to a non-existent method calculate_statistics. The implementation under test is calculate_grade_statistics, so this annotation is currently inaccurate for coverage reporting.

     * @covers \mod_coursework\audit::calculate_statistics

tests/phpunit/audit_test.php:81

• The @covers annotation refers to a non-existent method calculate_statistics. The implementation under test is calculate_grade_statistics, so this annotation is currently inaccurate for coverage reporting.

     * @covers \mod_coursework\audit::calculate_statistics

tests/phpunit/audit/audit_testable.php:28

• Most PHP files in this plugin (including other PHPUnit test files) include a defined('MOODLE_INTERNAL') || die(); guard. This helper file can be requested directly, so it should include the same guard for consistency and safety.

namespace mod_coursework;

/**

tests/phpunit/audit/audit_testable.php:44

• The docblock says this exposes calculate_statistics, but the method actually proxies calculate_grade_statistics, which can confuse future maintenance of the tests.

    /**
     * Expose calculate_statistics for testing.
     *
     * @param array $grades
     * @return array
     */
    public function calculate_statistics_public(array $grades): array {
        return $this->calculate_grade_statistics($grades);

lib.php:923

• The navigation link is currently restricted to single-marker courseworks, but the added behat scenario for this feature uses a 2-marker activity (Coursework-A). With get_max_markers() === 1 the "Grading Audit" link will not appear, so the scenario will fail and the report becomes inaccessible for multi-marker activities.

    // Link to statistics page.
    // Currently this is restricted to activities using numeric grading, and only 1 marker (with moderation).
    if (
        has_capability('mod/coursework:moderate', $context)
        && $coursework->uses_numeric_grade()
        && $coursework->get_max_markers() === 1
    ) {

actions/audit.php:40

• This page uses the 'coursework' string component for the title, but the surrounding codebase (and navigation link) uses 'mod_coursework'. Additionally, direct access to this URL for non-numeric grading will later throw when the report calls get_max_grade(). Validate numeric grading here and use the consistent string component.

$audit = new \mod_coursework\audit($cmid);
$title = get_string('gradingaudit', 'coursework');

templates/audit/report.mustache:175

• The first column in the "Moderator statistics" table is labelled as "Marker" (assessor), but the values rendered are moderator names. This is user-facing and misleading.

            <th>{{#str}} assessor, mod_coursework {{/str}}</th>

classes/audit.php:35

• Other classes in this plugin include a defined('MOODLE_INTERNAL') || die(); guard. Adding it here improves consistency and prevents direct access to the class file outside Moodle bootstrap.

use stdClass;

/**
 * Audit class.
 */

[Copilot]

Pull request overview

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

Suppressed comments (7)

classes/audit.php:92

• Mustache sections like {{#summary_stats}}, {{#moderation_stats}}, and {{#moderation_sample}} will treat PHP arrays as iterables and iterate their values. Because summary_stats, moderation_stats, and moderation_sample are currently nested associative arrays, the report will render multiple/incorrect rows and inner keys like {{max}}/{{total}} may not resolve as intended. Cast these section contexts to stdClass (and do the same for moderation_sample.marked / .moderated) before rendering.

        // Merge all the data into one object for the template.
        $data = (object)array_merge(
            $assessmentinfo,
            $markingsummary,
            $markerstats,

tests/phpunit/audit_test.php:65

• The @covers annotation references audit::calculate_statistics, but the implementation method is calculate_grade_statistics(). This breaks/invalidates PHPUnit coverage mapping.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit_test.php:82

• The @covers annotation references audit::calculate_statistics, but the implementation method is calculate_grade_statistics(). This breaks/invalidates PHPUnit coverage mapping.

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

templates/audit/report.mustache:176

• The first column header in the "Moderator statistics" table uses the assessor string, but the rows are moderators. This makes the UI label incorrect/misleading.

            <th>{{#str}} assessor, mod_coursework {{/str}}</th>
            {{#marker_stats_headers}}

lib.php:923

• The comment says the Grading Audit link is restricted to single-marker activities "(with moderation)", but the condition does not currently check whether moderation agreement is enabled. This can expose a report link for activities where the moderation sections will be irrelevant/empty.

    // Link to statistics page.
    // Currently this is restricted to activities using numeric grading, and only 1 marker (with moderation).
    if (
        has_capability('mod/coursework:moderate', $context)
        && $coursework->uses_numeric_grade()
        && $coursework->get_max_markers() === 1
    ) {

tests/phpunit/audit_test.php:50

• The @covers annotation references audit::calculate_statistics, but the implementation method is calculate_grade_statistics(). This breaks/invalidates PHPUnit coverage mapping.

This issue also appears in the following locations of the same file:

• line 63
• line 80

    /**
     * @covers \mod_coursework\audit::calculate_statistics
     */

tests/phpunit/audit/audit_testable.php:45

• calculate_statistics_public() is currently a wrapper around calculate_grade_statistics(), so the method name and docblock are misleading. Adding a correctly named wrapper (and keeping the existing name as a backwards-compatible alias) improves clarity and keeps call sites working.

    /**
     * Expose calculate_statistics for testing.
     *
     * @param array $grades
     * @return array
     */
    public function calculate_statistics_public(array $grades): array {
        return $this->calculate_grade_statistics($grades);
    }