MDL Shield

Activity dates

tool_activitydates

Print Report
Plugin Information

An admin tool (tool_activitydates) that bulk-schedules a course's activity open/close dates (and, where the module table supports it, due dates) and gradebook lock dates across a series of timed sessions. It writes each activity's own native dates and grade-item lock dates, refreshes the related calendar events, and can optionally show students a note about when their grades lock, both on the activity page and the course page.

Version:2026092904
Release:2.1.0
Reviewed for:5.2
Privacy API
Unit Tests
Behat Tests
Reviewed:2026-09-30
57 files·10,725 lines
Grade Justification

This is an exemplary, security-conscious plugin. Every entry point enforces the correct controls: view.php validates courseid with PARAM_INT, calls require_login($course), resolves the course context, and gates access with has_any_capability(['tool/activitydates:manage', 'tool/activitydates:managelocks']). Both capabilities are declared at CONTEXT_COURSE with RISK_DATALOSS and default to editing teachers and managers. All state changes flow through a moodleform, whose get_data() enforces sesskey (verified in core's formslib.php), and the extra table inputs read via optional_param_array() are read only after get_data() succeeds, so CSRF is covered.

The write paths are tightly scoped and were traced end to end: dates are written only with :manage, grade locks only with :managelocks, and save() re-checks both flags rather than trusting the submission. Every course-module id is resolved from get_fast_modinfo($courseid), intersected with the valid set, and the lock path re-validates each cmid with get_coursemodule_from_id('', $cmid, $courseid, ...). Forged, unselected, cross-course, and out-of-window ids are ignored — behavior explicitly covered by the plugin's own tests. The activity-type name that becomes a DB table name is validated by the form's validation() against the allowed type list before any query. Output is escaped throughout (Mustache auto-escaping, format_string() on the course name, strip_tags() on the intro); the JS parses cmids to integers and moves DOM nodes rather than injecting HTML. The Privacy API null_provider is correct — the schema contains no personal-data columns — and course/module deletion observers clean up the plugin's rows.

No security vulnerabilities were found. The only findings are low-severity code-quality observations: the plugin writes activity dates directly to each module's instance table (a recognized but largely unavoidable pattern here, and well mitigated with event refresh, the course_module_updated event, and a cache rebuild), and the table builder issues per-row database queries. Both are teacher-scoped and carry no security impact. Test coverage (PHPUnit and Behat) is unusually thorough, including negative security scenarios.

AI Summary

tool_activitydates is a course-level admin tool that bulk-schedules activities' native open/close/due dates and gradebook lock dates over timed sessions, with optional student-facing grade-lock notes.

The review read every PHP, JavaScript, and Mustache file (production and tests) in full and verified the relevant behaviors against Moodle 5.2 core (/moodle/public).

Security posture: strong. Key controls are all present and correctly implemented:

  • Access control — view.php enforces require_login($course) plus has_any_capability(['tool/activitydates:manage', 'tool/activitydates:managelocks']) at course context; both capabilities are RISK_DATALOSS, CONTEXT_COURSE, editing-teacher/manager.
  • CSRF — all writes go through a moodleform; the extra table inputs are read only after get_data() (which enforces sesskey, confirmed in core formslib.php).
  • Per-capability separation — dates require :manage, locks require :managelocks, and save() re-checks both; verified by the plugin's tests.
  • Input trust boundaries — cmids are resolved from get_fast_modinfo($courseid) and re-validated; the module-type value that becomes a table name is validated against the allowed list before any query; all SQL is parameterized.
  • Output — Mustache escaping, format_string() on the course name (with an explicit XSS test), integer-parsed cmids in JS.
  • Privacy — null_provider, matching a schema that stores only course-level configuration; deletion observers clean up.

Findings: two low-severity, non-security items only:

  1. Direct writes to core module instance tables (quiz, choice, …) bypassing each module's update_instance()/update_moduleinfo() pipeline — well mitigated and largely unavoidable.
  2. Per-row database queries in the table builder (an N+1 pattern) on a teacher-facing page.

Overall this is a model Moodle plugin: clean, well-documented, and backed by comprehensive PHPUnit and Behat tests that include negative security cases (forged cmids, capability separation, stale-table server enforcement, stealth/restricted-activity note leakage).

Findings

code qualityLow
Activity dates are written directly to module instance tables, bypassing the module update API

When applying a schedule, the plugin updates each activity's open/close (and due) dates by writing straight to the module's own instance table (e.g. quiz, choice) with $DB->update_record($modtype, ...), and clears them the same way when resetting unselected activities.

This writes to tables owned by other components (mod_quiz, mod_choice, etc.) rather than going through that module's xxx_update_instance() callback or core's update_moduleinfo() in course/modlib.php. Moodle's plugin guidelines discourage writing to another component's tables directly, because the module's own update routine may perform date-derived side effects that a bare column write skips.

The plugin mitigates this carefully: after each write it recreates the module's open/close calendar events via component_callback('mod_' . $modtype, 'refresh_events', ...), triggers \core\event\course_module_updated, sets timemodified, adjusts visibility, and calls rebuild_course_cache(). The dates themselves are validated (close after open; due after open and no later than close) before writing.

The residual gap is any module-specific logic in xxx_update_instance() beyond standard event refresh. This is essentially unavoidable given the plugin's purpose (setting native module dates in bulk, for which core offers no lightweight generic API), so it is reported as a low code-quality observation, not a defect requiring a redesign.

Risk Assessment

Low risk. This is a code-quality/architecture observation, not a security issue. Reaching the write requires the tool/activitydates:manage capability (editing teacher or manager) in the course, and such a user can already edit these same dates one activity at a time through each module's own settings form. There is no privilege escalation and no cross-user or cross-course impact: the blast radius is the teacher's own course. The plugin refreshes calendar events, fires course_module_updated, and rebuilds the course cache, so the most visible downstream effects of a date change are handled. The unmitigated part is limited to any module-specific date-handling logic inside a module's update_instance() that goes beyond standard event refresh, which for date-only changes is minimal.

Context

apply_dates() iterates the previewed table rows and, for each selected and scheduled row with a validated value, writes the new dates to the module instance. process_unselected() handles rows the teacher left unticked, optionally hiding them and/or clearing their dates. Both are reached only from activitydates::save(), which runs the dates branch only when the caller passed $canmanage === true; view.php sets that flag from has_capability('tool/activitydates:manage', $context) on the course. All course modules are drawn from get_fast_modinfo($courseid), so writes are confined to the current course's activities of the validated type.

Identified Code
            $DB->update_record($modtype, $instance);
            set_coursemodule_visible($cm->id, true, true);
            // Recreate the open/close calendar events. Pass the instance ID (not an
            // object) so the callback re-reads the freshly updated record.
            component_callback(
                'mod_' . $modtype,
                'refresh_events',
                [$settings->courseid, $cm->instance, $cm]
            );
            \core\event\course_module_updated::create_from_cm($cm)->trigger();
Suggested Fix

No change is required for correctness or security. The current approach is capability-gated, validated, and refreshes events and the course cache.

If stricter alignment with core is desired, consider routing date changes through update_moduleinfo($cm, $moduleinfo, $course) (course/modlib.php), which runs each module's full xxx_update_instance() pipeline. Note this is significantly heavier (it expects a full moduleinfo object shaped like the module edit form) and may not be worth it for a bulk date-only tool. At minimum, keep the current event refresh and cache rebuild as they are.

Identified Code
            $reset = (object) ['id' => $cm->instance, 'timeopen' => 0, 'timeclose' => 0];
            if (self::has_duedate($settings->modtype)) {
                $reset->duedate = 0;
            }
            $DB->update_record($settings->modtype, $reset);
Suggested Fix

Same as above — this is the reset-unselected counterpart of the direct write and carries the same trade-off. No change needed for security; consider the module-API route only if stricter core alignment is wanted.

best practiceLow
Per-row database queries when building the schedule table (N+1 pattern)

get_table_data() builds the preview/edit table by querying the database once per course module rather than in a batch:

  • one $DB->get_record($settings->modtype, ...) per module to read its current dates, and
  • locks\manager::current_locktime() per module, which calls \grade_item::fetch_all().

Separately, while building each data row, locks\manager::has_grade_item() is called per module, which again calls \grade_item::fetch_all() for the same course module — so each gradable row triggers two effectively identical grade-item lookups.

For a course with N activities of the chosen type this is on the order of 3N queries. By contrast, locknote::course_page_notes() already demonstrates the efficient approach, fetching all of a course's mod grade items in a single \grade_item::fetch_all(['courseid' => ..., 'itemtype' => 'mod']) call and grouping them in PHP.

Risk Assessment

Low risk. This is a performance/best-practice note with no security impact. The page is teacher/manager-facing (not a student hot path), and the number of activities of one type in one course is normally small (dozens at most), so the practical cost is modest. It is worth tidying because the fix is straightforward and the plugin already uses the batched pattern elsewhere, but it does not affect correctness.

Context

get_table_data() is invoked on every load, preview, and save of the Activity dates page, which is reachable only by users with tool/activitydates:manage or tool/activitydates:managelocks in the course. The query count scales with the number of activities of the selected type present in a single course.

Identified Code
        foreach ($modules as $cmid => $cm) {
            $instance = $DB->get_record($settings->modtype, ['id' => $cm->instance], $fields);
            $instances[$cmid] = $instance;
            $current[$cmid] = [
                'timeopen' => (int) ($instance->timeopen ?? 0),
                'duedate' => $hasdue ? (int) ($instance->duedate ?? 0) : null,
                'timeclose' => (int) ($instance->timeclose ?? 0),
                'timelock' => $lockmanager->current_locktime($courseid, $cm),
            ];
        }
Suggested Fix

Read all instances of the type in one query and index by instance id, and fetch the course's mod grade items once (as course_page_notes() does), then look each cm up in PHP:

$instances = $DB->get_records_list($settings->modtype, 'id', array_map(fn($cm) => $cm->instance, $modules), '', 'id, ...');
// Fetch grade items once for the whole course and group by module+instance,
// then reuse for both current_locktime() and has_grade_item().
Identified Code
                    'hasgradeitem' => $lockmanager->has_grade_item($courseid, $cm),
Suggested Fix

Reuse a single per-course grade-item lookup (computed once, grouped by cm) for both the lock time and the hasgradeitem flag, instead of calling \grade_item::fetch_all() again here. This removes the duplicate grade-item query per gradable row.

Additional AI Notes

Access control and CSRF were verified against core, not assumed. moodleform::_process_submission() in /moodle/public/lib/formslib.php throws invalidsesskey whenever the form marker is present with a bad sesskey, and get_data() returns data only for a submitted, validated form. In view.php every extra table input (*_rows, fix_*, shownote_cmids, tablefingerprint) is read only inside the get_data() branch (or the is_submitted() re-render branch, which also runs after the constructor's sesskey check), so all state changes are sesskey-protected.

The module-type value that becomes a table name is safely constrained. The display type comes from optional_param('modtype', '', PARAM_ALPHANUMEXT) gated by array_key_exists($requested, $types), and the submitted type is rejected by the form's validation() unless it is a key of the allowed module list before any get_columns()/get_record()/update_record() call. The allowed list is built from installed course modules, so the string is always a real module table name — no arbitrary-table access or SQL injection is possible here.

Privacy implementation is correct. The null_provider claim was checked against db/install.xml: all five tables store only course-level configuration (course id, module type, schedule settings, selected cmids, note flags, held-date flags) with no user-identifying columns. Course and course-module deletion observers (classes/observer.php) remove the plugin's rows so nothing is orphaned, and the historic cleanup::orphans() step is confined to the upgrade path.

Student-facing lock notes are correctly scoped. locknote::for_cm() and locknote::course_page_notes() gate on $cm->uservisible, is_visible_on_course_page(), deletion-in-progress, and a course-scoped join to the plugin's own lock configuration, so no note leaks for a restricted, hidden, stealth, or other-course activity — behaviors covered directly by tests/locks/local/locknote_test.php.

No third-party code is bundled. The amd/build/*.min.js files are the compiled output of the plugin's own amd/src/*.js (standard Moodle Grunt output), so the absence of thirdpartylibs.xml is correct and no bundled-library version conflicts apply.

Test coverage is a notable strength. The PHPUnit suite and Behat features exercise the security-relevant paths explicitly, including rejection of forged/foreign/unselected cmids, strict per-capability separation (:manage-only vs :managelocks-only), server-side enforcement of the settings fingerprint even when the client Save button is re-enabled, format_string() escaping of a malicious course shortname, and note suppression for stealth/restricted activities.

This review was generated by an AI system and may contain inaccuracies. Findings should be verified by a human reviewer before acting on them.