CTP-3137 Add button to remove mapping in dashboard page (#76)

ucl-isd · Nov 19, 2024 · 1e1ee5b · 1e1ee5b
1 parent c91be1b
commit 1e1ee5b
Show file tree

Hide file tree

Showing 8 changed files with 217 additions and 6 deletions.
diff --git a/classes/manager.php b/classes/manager.php
@@ -16,6 +16,7 @@
 
 namespace local_sitsgradepush;
 
+use context_course;
 use core_component;
 use core_course\customfield\course_handler;
 use DirectoryIterator;
@@ -1438,6 +1439,37 @@ public function get_all_summative_grade_items(int $courseid): array {
         return $results;
     }
 
+    /**
+     * Delete assessment mapping.
+     *
+     * @param int $courseid
+     * @param int $mappingid
+     * @return void
+     * @throws \dml_exception
+     * @throws \moodle_exception
+     */
+    public function remove_mapping(int $courseid, int $mappingid): void {
+        global $DB;
+
+        // Check permission. Assume the user who has permission to map assessment is allowed to remove mapping too.
+        if (!has_capability('local/sitsgradepush:mapassessment', context_course::instance($courseid))) {
+            throw new \moodle_exception('error:remove_mapping', 'local_sitsgradepush');
+        }
+
+        // Check the mapping exists.
+        if (!$DB->record_exists(self::TABLE_ASSESSMENT_MAPPING, ['id' => $mappingid])) {
+            throw new \moodle_exception('error:assessmentmapping', 'local_sitsgradepush', '', $mappingid);
+        }
+
+        // Remove mapping is not allowed if grades have been pushed.
+        if ($this->has_grades_pushed($mappingid)) {
+            throw new \moodle_exception('error:mab_has_push_records', 'local_sitsgradepush', '', 'Mapping ID: ' . $mappingid);
+        }
+
+        // Everything is fine, remove the mapping.
+        $DB->delete_records(self::TABLE_ASSESSMENT_MAPPING, ['id' => $mappingid]);
+    }
+
     /**
      * Save transfer log.
      *

diff --git a/classes/output/renderer.php b/classes/output/renderer.php
@@ -214,14 +214,16 @@ public function render_dashboard(array $moduledeliveries, int $courseid, int $re
                     $assessmentmapping->name = $assessmentdata->source->get_assessment_name();
                     $assessmentmapping->url = $assessmentdata->source->get_assessment_url(false);
                     $assessmentmapping->transferhistoryurl = $assessmentdata->source->get_assessment_transfer_history_url(false);
+                    $assessmentmapping->removesourceurl = $this->get_remove_source_url($courseid, $mapping->id)->out(false);
 
                     // Check if there is a task running for the assessment mapping.
                     $taskrunning = taskmanager::get_pending_task_in_queue($mapping->id);
                     $assessmentmapping->taskrunning = !empty($taskrunning);
                     $assessmentmapping->taskprogress = $taskrunning && $taskrunning->progress ? $taskrunning->progress : 0;
 
-                    // Disable the change source button if there is a task running.
-                    $assessmentmapping->disablechangesource =
+                    // Disable the change source button / hide the remove source button
+                    // if grades have been pushed or there is a task running.
+                    $assessmentmapping->disablechangesource = $assessmentmapping->hideremovesourcebutton =
                         !empty($taskrunning) || $this->manager->has_grades_pushed($mapping->id);
 
                     $componentgrade->assessmentmapping = $assessmentmapping;
@@ -445,4 +447,19 @@ private function get_warning_message_for_history_page_table(\stdClass $mapping,
 
         return $warningmessage;
     }
+
+    /**
+     * Get the remove source URL.
+     *
+     * @param int $courseid Course ID
+     * @param int $mapid Assessment mapping ID
+     *
+     * @return \moodle_url
+     */
+    private function get_remove_source_url(int $courseid, int $mapid): \moodle_url {
+        return new \moodle_url(
+            '/local/sitsgradepush/dashboard.php',
+            ['id' => $courseid, 'mapid' => $mapid, 'action' => 'removesource']
+        );
+    }
 }
diff --git a/dashboard.php b/dashboard.php
@@ -26,6 +26,7 @@
 namespace local_sitsgradepush;
 
 use context_course;
+use core\output\notification;
 use moodle_exception;
 use moodle_url;
 
@@ -51,6 +52,27 @@
 // Check user's capability.
 require_capability('local/sitsgradepush:mapassessment', $context);
 
+// Process remove source action.
+if ($action = optional_param('action', '', PARAM_ALPHA)) {
+    if ($action === 'removesource') {
+        $mapid = required_param('mapid', PARAM_INT);
+        try {
+            manager::get_manager()->remove_mapping($courseid, $mapid);
+            // Reload the page.
+            redirect(new moodle_url('/local/sitsgradepush/dashboard.php', ['id' => $courseid]));
+        } catch (moodle_exception $e) {
+            // Add notification.
+            redirect(new moodle_url(
+                '/local/sitsgradepush/dashboard.php',
+                ['id' => $courseid]),
+                $e->getMessage(),
+                null,
+                notification::NOTIFY_ERROR
+            );
+        }
+    }
+}
+
 $header = get_string('dashboard:header', 'local_sitsgradepush');
 $param = ['id' => $courseid];
 if ($reassess == 1) {

diff --git a/lang/en/local_sitsgradepush.php b/lang/en/local_sitsgradepush.php
@@ -56,6 +56,8 @@
 $string['dashboard:moduledelivery'] = 'MODULE DELIVERY';
 $string['dashboard:moodle_activity'] = 'Moodle activity';
 $string['dashboard:reassessment_page'] = 'Re-assessment';
+$string['dashboard:remove_btn_content'] = 'Are you sure you want to remove source <strong>{$a->sourcename}</strong> for SITS assessment <strong>({$a->mabseq}) {$a->mabname}</strong>?';
+$string['dashboard:remove_btn_title'] = 'Remove source';
 $string['dashboard:seq'] = 'SEQ';
 $string['dashboard:sits_assessment'] = 'SITS assessment';
 $string['dashboard:source'] = 'Source';
@@ -101,7 +103,7 @@
 $string['error:invalid_source_type'] = 'Invalid source type. {$a}';
 $string['error:lesson_practice'] = 'Practice lessons have no grades';
 $string['error:lti_no_grades'] = 'LTI activity is set to not send grades to gradebook';
-$string['error:mab_has_push_records'] = 'Assessment component mapping cannot be updated as marks have been transfered for {$a}';
+$string['error:mab_has_push_records'] = 'Assessment component mapping cannot be updated as marks have been transferred for {$a}';
 $string['error:mab_invalid_for_mapping'] = 'This assessment component is not valid for mapping due to the following reasons: {$a}.';
 $string['error:mab_not_found'] = 'Assessment component not found. ID: {$a}';
 $string['error:mapassessment'] = 'You do not have permission to map assessment.';
@@ -119,6 +121,7 @@
 $string['error:pastcourse'] = 'It looks like this course is from a previous academic year, marks transfer is not allowed.';
 $string['error:pushgradespermission'] = 'You do not have permission to transfer marks.';
 $string['error:reassessmentdisabled'] = 'Re-assessment marks transfer is disabled.';
+$string['error:remove_mapping'] = 'You do not have permission to remove mapping.';
 $string['error:resit_number_zero_for_reassessment'] = 'Student resit number should not be zero for a reassessment.';
 $string['error:same_map_code_for_same_activity'] = 'An activity cannot be mapped to more than one assessment component with same map code';
 $string['error:studentnotfound'] = 'Student with idnumber {$a->idnumber} not found for component grade {$a->componentgrade}';

diff --git a/templates/dashboard.mustache b/templates/dashboard.mustache
@@ -98,7 +98,8 @@
                             "transferhistoryurl": "http://test.m4.local:4001/local/sitsgradepush/index.php?id=22",
                             "taskrunning": false,
                             "taskprogress": "0",
-                            "disablechangesource": false
+                            "disablechangesource": false,
+                            "hideremovesourcebutton": false
                         }
                     }
                 },
@@ -169,6 +170,17 @@
                             {{#str}} dashboard:changesource, local_sitsgradepush {{/str}}
                             <span class="sr-only"> for {{mabseq}} {{mabname}}</span>
                         </a>
+                        <button type="button"
+                                class="btn btn-danger btn-sm {{#hideremovesourcebutton}}d-none{{/hideremovesourcebutton}}"
+                                data-confirmation="modal"
+                                data-confirmation-title-str='["dashboard:remove_btn_title", "local_sitsgradepush"]'
+                                data-confirmation-content-str='["dashboard:remove_btn_content", "local_sitsgradepush", {"sourcename": "{{name}}", "mabseq": "{{mabseq}}", "mabname": "{{mabname}}"}]'
+                                data-confirmation-yes-button-str='["confirm"]'
+                                data-confirmation-destination="{{removesourceurl}}"
+                        >
+                            {{#str}} dashboard:remove_btn_title, local_sitsgradepush {{/str}}
+                            <span class="sr-only"> for {{mabseq}} {{mabname}}</span>
+                        </button>
                     {{/currentacademicyear}}
                     {{/assessmentmapping}}
                     {{#currentacademicyear}}

diff --git a/tests/behat/behat_sitsgradepush.php b/tests/behat/behat_sitsgradepush.php
@@ -120,10 +120,10 @@ public function i_click_on_the_button_for(string $buttontext, string $rowtext):
         $page = $this->getSession()->getPage();
 
         // Buttons which are button type, i.e. not links.
-        $buttons = ['Select', 'Transfer marks'];
+        $buttons = ['Select', 'Transfer marks', 'Remove source'];
 
         // Determine the XPath based on button type.
-        if (in_array($buttontext, ['Select source', 'Change source', 'Transfer marks'])) {
+        if (in_array($buttontext, ['Select source', 'Change source', 'Transfer marks', 'Remove source'])) {
             $xpath = "//tr[th[contains(text(), '$rowtext')]]";
         } else if ($buttontext == 'Select') {
             $xpath = "//tr[td[contains(text(), '$rowtext')]]";
@@ -179,6 +179,22 @@ public function i_should_see_activity_is_mapped_for(string $linktext, string $si
         }
     }
 
+    /**
+     * Check if a SITS assessment is not mapped.
+     *
+     * @param  string $sitsassessment
+     * @throws \Exception
+     *
+     * @Then I should see :sitsassessment is not mapped
+     */
+    public function i_should_see_sits_assessment_is_not_mapped(string $sitsassessment): void {
+        global $DB;
+        $mab = $DB->get_record(manager::TABLE_COMPONENT_GRADE, ['mabname' => $sitsassessment]);
+        if (manager::get_manager()->is_component_grade_mapped($mab->id, 0)) {
+            throw new Exception("SITS assessment '$sitsassessment' is mapped.");
+        }
+    }
+
     /**
      * Set up the course for marks transfer.
      *

diff --git a/tests/behat/remove_source.feature b/tests/behat/remove_source.feature
@@ -0,0 +1,41 @@
+@local @local_sitsgradepush
+
+Feature: Remove Moodle source for SITS assessment component
+  As a teacher with the appropriate permissions
+  I can remove a Moodle source for a SITS assessment component
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | idnumber | email                |
+      | teacher1 | Teacher1  | Test     | tea1     | teacher1@example.com |
+    And the following custom field exists for grade push:
+      | category  | CLC |
+      | shortname | course_year |
+      | name      | Course Year |
+      | type      | text        |
+    And the following "courses" exist:
+      | fullname | shortname | format | customfield_course_year |
+      | Course 1 | C1        | topics | ##now##%Y##             |
+    And the following "course enrolments" exist:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+    And the following "permission overrides" exist:
+      | capability                        | permission | role           | contextlevel | reference |
+      | local/sitsgradepush:mapassessment | Allow      | editingteacher | Course       | C1        |
+    And the course "C1" is set up for marks transfer
+    And the following "activities" exist:
+      | activity        | name       | intro   | course | idnumber    | section |
+      | assign          | Assign 1   | A1 desc | C1     | assign1     | 1       |
+      | quiz            | Quiz 1     | Q1 desc | C1     | quiz1       | 1       |
+    And the course "C1" is regraded
+    And the "mod" "assign1" is mapped to "72hr take-home examination (3000 words)"
+
+  @javascript
+  Scenario: Remove source for a SITS assessment component
+    Given I am on the "Course 1" course page logged in as teacher1
+    And I click on "More" "link" in the ".secondary-navigation" "css_element"
+    And I select "SITS Marks Transfer" from secondary navigation
+    And I should see "Assign 1" is mapped to "72hr take-home examination (3000 words)"
+    And I click on the "Remove source" button for "72hr take-home examination (3000 words)"
+    And I click on "Confirm" "button" in the "Remove source" "dialogue"
+    Then I should see "72hr take-home examination (3000 words)" is not mapped
diff --git a/tests/manager_test.php b/tests/manager_test.php
@@ -26,6 +26,7 @@
 use local_sitsgradepush\api\irequest;
 use local_sitsgradepush\assessment\assessment;
 use local_sitsgradepush\assessment\assessmentfactory;
+use moodle_exception;
 
 defined('MOODLE_INTERNAL') || die();
 global $CFG;
@@ -1286,6 +1287,73 @@ public function test_get_mab_by_mapping_id(): void {
         $this->assertEquals($this->mab1->id, $mab->id);
     }
 
+    /**
+     * Test the remove mapping method.
+     *
+     * @covers \local_sitsgradepush\manager::remove_mapping
+     * @return void
+     * @throws \ReflectionException
+     * @throws \coding_exception
+     * @throws \dml_exception
+     * @throws \moodle_exception
+     */
+    public function test_remove_mapping(): void {
+        global $DB;
+        // Set up the test environment.
+        $this->setup_testing_environment(assessmentfactory::get_assessment('mod', $this->assign1->cmid));
+        $this->setUser($this->teacher1);
+
+        try {
+            // Test removing mapping without capability.
+            $this->manager->remove_mapping($this->course1->id, $this->mappingid1);
+        } catch (\moodle_exception $e) {
+            $this->assertStringContainsString(get_string('error:remove_mapping', 'local_sitsgradepush'), $e->getMessage());
+        }
+
+        // Test mapping not exists.
+        try {
+            // Create role.
+            $roleid = $this->dg->create_role(['shortname' => 'canmapassessment']);
+            assign_capability(
+                'local/sitsgradepush:mapassessment',
+                CAP_ALLOW,
+                $roleid,
+                context_course::instance($this->course1->id)->id
+            );
+            $this->dg->enrol_user($this->teacher1->id, $this->course1->id, 'canmapassessment');
+            $this->manager->remove_mapping($this->course1->id, 0);
+        } catch (\moodle_exception $e) {
+            $this->assertStringContainsString(get_string('error:assessmentmapping', 'local_sitsgradepush', 0), $e->getMessage());
+        }
+
+        // Test push records exist.
+        try {
+            // Add some push logs.
+            $transferlog = new \stdClass();
+            $transferlog->type = 'pushgrade';
+            $transferlog->userid = $this->student1->id;
+            $transferlog->assessmentmappingid = $this->mappingid1;
+            $transferlog->usermodified = $this->teacher1->id;
+            $transferlog->timecreated = time();
+            $transferlogid = $DB->insert_record('local_sitsgradepush_tfr_log', $transferlog);
+
+            // Test push logs exist.
+            $this->manager->remove_mapping($this->course1->id, $this->mappingid1);
+        } catch (\moodle_exception $e) {
+            $this->assertStringContainsString(
+                get_string('error:mab_has_push_records', 'local_sitsgradepush', 'Mapping ID: ' . $this->mappingid1),
+                $e->getMessage()
+            );
+        }
+
+        // Remove the push logs to allow mapping removal.
+        $DB->delete_records('local_sitsgradepush_tfr_log', ['id' => $transferlogid]);
+
+        // Test successful removal of the mapping.
+        $this->manager->remove_mapping($this->course1->id, $this->mappingid1);
+        $this->assertFalse($DB->record_exists('local_sitsgradepush_mapping', ['id' => $this->mappingid1]));
+    }
+
     /**
      * Set default configurations for the tests.
      *