From 5245da000952684478d5f846253fcf1bd51c2324 Mon Sep 17 00:00:00 2001 From: Andreas Rosenthal Date: Mon, 27 Jul 2026 14:40:06 +0200 Subject: [PATCH 01/10] fix: allow Support to edit support persons and internal notes Assigned Support on site can update Support persons (same gate as internal notes); other internal fields stay read-only with server-side restore. Updates EN/DE notice and PHPUnit expectations for Spec 091. Co-authored-by: Cursor --- classes/form/edit_event_form.php | 9 ++++++--- classes/local/manager/event_access_manager.php | 5 ++++- lang/de/bookit.php | 2 +- lang/en/bookit.php | 2 +- ...t_event_form_support_internal_fields_test.php | 16 ++++++++++------ 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/classes/form/edit_event_form.php b/classes/form/edit_event_form.php index ac5c7915..ca332f7d 100644 --- a/classes/form/edit_event_form.php +++ b/classes/form/edit_event_form.php @@ -478,7 +478,8 @@ public function definition(): void { $supportpersons[$id] = fullname($user); } $supportpersonselementname = 'supportpersons'; - if (!$caneditinternal && $existingevent) { + // Assigned Support may edit support persons (same gate as internal notes); others stay RO. + if (!$caneditinternal && !$caneditinternalnotes && $existingevent) { $supportpersonselementname = 'supportpersons_readonly'; $mform->addElement( 'static', @@ -503,7 +504,6 @@ public function definition(): void { $userselectoroptions ); $mform->setType('supportpersons', PARAM_TEXT); - $mform->disabledIf('supportpersons', 'editinternal', 'neq', 1); } $mform->addHelpButton($supportpersonselementname, 'event_supportperson', 'mod_bookit'); } else { @@ -1125,7 +1125,10 @@ public function process_dynamic_submission(): array { } if ($currentevent && !$caneditinternal) { - $formdata->supportpersons = $currentevent->supportpersons; + // Support with notes-gate may update supportpersons; other internal fields stay locked. + if (!$caneditinternalnotes) { + $formdata->supportpersons = $currentevent->supportpersons; + } $formdata->extratimebefore = $currentevent->extratimebefore; $formdata->extratimeafter = $currentevent->extratimeafter; $formdata->refcourseid = $currentevent->refcourseid; diff --git a/classes/local/manager/event_access_manager.php b/classes/local/manager/event_access_manager.php index 0bb6ca88..130385f9 100644 --- a/classes/local/manager/event_access_manager.php +++ b/classes/local/manager/event_access_manager.php @@ -524,7 +524,10 @@ public static function can_supportperson_view_internal_fields( } /** - * Check whether the user may edit only the internal notes field. + * Check whether assigned Support may edit Internal notes (and Support persons on the form). + * + * The same gate authorises Support-persons edits in edit_event_form; other internal fields + * remain read-only for Support-only users. * * @param stdClass $event * @param context_module $context diff --git a/lang/de/bookit.php b/lang/de/bookit.php index e11391b4..03969a13 100644 --- a/lang/de/bookit.php +++ b/lang/de/bookit.php @@ -262,7 +262,7 @@ $string['event_students_help'] = 'Tragen Sie die erwartete Anzahl der Teilnehmenden ein.'; $string['event_supportperson'] = 'Support-Personen'; $string['event_supportperson_help'] = 'Diesem Event zugewiesene Support-Personen.'; -$string['event_supportperson_internalnotes_notice'] = 'Support vor Ort darf nur das Feld "Interne Notizen" bearbeiten.'; +$string['event_supportperson_internalnotes_notice'] = 'Support vor Ort darf „Support-Personen“ und „Interne Notizen“ bearbeiten; alle übrigen internen Felder sind schreibgeschützt.'; $string['event_timecompensation'] = 'Zeitnachteilsausgleich'; $string['event_timecompensation_help'] = 'Aktivieren, wenn Teilnehmende einen Zeitnachteilsausgleich haben.'; $string['event_usercreated'] = 'Angelegt von'; diff --git a/lang/en/bookit.php b/lang/en/bookit.php index 6c7776af..29fd0a1c 100644 --- a/lang/en/bookit.php +++ b/lang/en/bookit.php @@ -262,7 +262,7 @@ $string['event_students_help'] = 'Enter amount of participants as a number.'; $string['event_supportperson'] = 'Support persons'; $string['event_supportperson_help'] = 'Support persons assigned to this event.'; -$string['event_supportperson_internalnotes_notice'] = 'Support on site may edit only the internal notes field.'; +$string['event_supportperson_internalnotes_notice'] = 'Support on site may edit Support persons and Internal notes; other internal fields are read-only.'; $string['event_timecompensation'] = 'Time compensation'; $string['event_timecompensation_help'] = 'Check if you have participants entitled to time compensation.'; $string['event_usercreated'] = 'Created by user'; diff --git a/tests/local/form/edit_event_form_support_internal_fields_test.php b/tests/local/form/edit_event_form_support_internal_fields_test.php index 30c1450c..413bd845 100644 --- a/tests/local/form/edit_event_form_support_internal_fields_test.php +++ b/tests/local/form/edit_event_form_support_internal_fields_test.php @@ -419,11 +419,11 @@ public function test_support_sees_refcourseid_readonly_when_enabled(): void { } /** - * Support-only users see internal fields read-only except internal notes. + * Support-only users may edit Support persons and Internal notes; other internal fields stay RO. * * @return void */ - public function test_support_internal_fields_readonly_except_notes(): void { + public function test_support_internal_fields_persons_and_notes_editable(): void { $bookingperson = $this->getDataGenerator()->create_user(); $supportuser = $this->getDataGenerator()->create_user(); $context = $this->create_support_context(); @@ -442,20 +442,24 @@ public function test_support_internal_fields_readonly_except_notes(): void { $form = $this->create_rendered_form($context, (int)$event->id, (int)$supportuser->id); $mform = $this->get_mform($form); - $this->assertTrue($mform->elementExists('supportpersons_readonly')); + $this->assertTrue($mform->elementExists('header_internal')); + $this->assertTrue($mform->elementExists('supportpersons')); + $this->assertFalse($mform->elementExists('supportpersons_readonly')); + $this->assertSame('autocomplete', $mform->getElementType('supportpersons')); $this->assertTrue($mform->elementExists('extratimebefore_readonly')); $this->assertTrue($mform->elementExists('extratimeafter_readonly')); $this->assertTrue($mform->elementExists('bookingstatusreadonly')); $this->assertTrue($mform->elementExists('internalnotes')); + $this->assertTrue($mform->elementExists('supportpersoninternalnotesnotice')); $this->assertNotSame('select', $mform->getElementType('bookingstatus') ?? ''); } /** - * Tampered internal field submissions must not change persisted values. + * Support may change Support persons and notes; tampered other internal fields are ignored. * * @return void */ - public function test_support_tamper_internal_fields_ignored_on_save(): void { + public function test_support_save_persons_and_notes_other_internal_ignored(): void { global $DB; $bookingperson = $this->getDataGenerator()->create_user(); @@ -493,7 +497,7 @@ public function test_support_tamper_internal_fields_ignored_on_save(): void { $persisted = $DB->get_record('bookit_event', ['id' => $event->id], '*', MUST_EXIST); $this->assertSame((int)$refcourse->id, (int)$persisted->refcourseid); - $this->assertSame((string)$supportuser->id, $persisted->supportpersons); + $this->assertSame((string)$otheruser->id, $persisted->supportpersons); $this->assertSame(15, (int)$persisted->extratimebefore); $this->assertSame(30, (int)$persisted->extratimeafter); $this->assertSame(event_access_manager::BOOKINGSTATUS_IN_PROGRESS, (int)$persisted->bookingstatus); From 0b58c01a47fa22986c87a81b5c8171ad06b7c971 Mon Sep 17 00:00:00 2001 From: Andreas Rosenthal Date: Mon, 27 Jul 2026 14:41:55 +0200 Subject: [PATCH 02/10] test: align Behat Support scenario with persons+notes edit matrix Rename Spec-065 wording and assert supportpersons is enabled for assigned Support on the event form (Spec 091). Co-authored-by: Cursor --- tests/behat/role_based_access.feature | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/behat/role_based_access.feature b/tests/behat/role_based_access.feature index cd5510a9..b74966a7 100644 --- a/tests/behat/role_based_access.feature +++ b/tests/behat/role_based_access.feature @@ -41,7 +41,7 @@ Feature: Enforce role-based visibility and editing for booking requests | name | shortname | seats | | Default room | DEF | 0 | - Scenario: Support person sees assigned new and confirmed bookings and may edit only internal notes + Scenario: Support person sees assigned bookings and may edit support persons and internal notes Given the following "mod_bookit > events" exist: | name | username | supportperson_usernames | startdate | enddate | bookingstatus | institution | | Accepted support exam | bookinguser | supportuser | ##tomorrow noon##%Y-%m-%dT%H:%M:%S## | ##tomorrow 14:00##%Y-%m-%dT%H:%M:%S## | 2 | 1 | @@ -55,6 +55,7 @@ Feature: Enforce role-based visibility and editing for booking requests And the Bookit event details control "institutionid" should be disabled And the Bookit event details control "starttime" should be disabled And the Bookit event details control "bookingstatus" should not be visible + And the Bookit event details control "supportpersons" should be enabled And the Bookit event details control "internalnotes" should be enabled And I should see "Internal fields" From 409d3cb721e4fa55e15f2ab2fea1bd4c9b6ab482 Mon Sep 17 00:00:00 2001 From: Andreas Rosenthal Date: Mon, 27 Jul 2026 14:47:22 +0200 Subject: [PATCH 03/10] test: teach Behat modal control asserts about autocomplete widgets Moodle autocomplete hides the original select; treat the visible autocomplete UI as enabled/disabled so Support persons checks work. Co-authored-by: Cursor --- tests/behat/behat_mod_bookit.php | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/behat/behat_mod_bookit.php b/tests/behat/behat_mod_bookit.php index a72335ba..bfb6f0c9 100644 --- a/tests/behat/behat_mod_bookit.php +++ b/tests/behat/behat_mod_bookit.php @@ -2087,7 +2087,43 @@ private function assert_modal_control_state(string $controlname, string $expecte if (!root) { return 'modal-not-found'; } + + // Moodle may append a random suffix to element ids (e.g. id_supportpersons_abc123). + var fitems = Array.from(root.querySelectorAll('[id^="fitem_id_"]')).filter(function(el) { + return el.id.indexOf('fitem_id_' + controlName) === 0; + }); + var readonlyFitem = fitems.find(function(el) { + return el.id.indexOf('fitem_id_' + controlName + '_readonly') === 0 + || el.id.indexOf('_readonly') !== -1; + }); + var editFitem = fitems.find(function(el) { + return el.id.indexOf('readonly') === -1; + }); + + // Static readonly display for this field. + if (readonlyFitem && !editFitem) { + return 'disabled'; + } + + // Autocomplete: the original