From 2f52b104ebad82e4f83d66c39c8bb5e6b4beda85 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?C=C3=A9cile=20Madjar?= <cecile.madjar@gmail.com>
Date: Thu, 11 May 2023 10:19:06 -0400
Subject: [PATCH] [MRI violations] Fix multiple rows for file protocol
 violations not resolvable (#8662)

This fixes a bug where the violation could not be resolved when there is more than one Protocol Violation returned for a file/timerun combination.
Only show one row per pname/timeRun/mincFile/scanType in the main menu filter table when there are multiple Protocol Violation for a given file (the list of violations will be displayed when clicking on the Protocol Violation link.
When selecting a resolution, loop through all entries in mri_violations_log matching the timeRun/mincFile and update the resolution status for all matching entries in the violations_resolved table.

Resolves  #8626
---
 .../mri_violations/php/provisioner.class.inc  |  5 ++-
 modules/mri_violations/php/resolve.class.inc  | 33 ++++++++++---------
 .../test/mri_violationsTest.php               |  8 ++---
 3 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/modules/mri_violations/php/provisioner.class.inc b/modules/mri_violations/php/provisioner.class.inc
index 9b68588691b..f3150bd856f 100644
--- a/modules/mri_violations/php/provisioner.class.inc
+++ b/modules/mri_violations/php/provisioner.class.inc
@@ -93,7 +93,7 @@ class Provisioner extends \LORIS\Data\Provisioners\DBRowProvisioner
                            PhaseEncodingDirection,EchoTime,EchoNumber
                          )
                        ) AS hash,
-                       mrl.LogID AS join_id,
+                       MIN(mrl.LogID) AS join_id,
                        p.CenterID AS Site,
                        COALESCE(
                          violations_resolved.Resolved, 'unresolved'
@@ -112,6 +112,9 @@ class Provisioner extends \LORIS\Data\Provisioners\DBRowProvisioner
                   mrl.Visit_label = s.Visit_label AND mrl.CandID=s.CandID
                 )
                 LEFT JOIN psc p ON (p.CenterID = s.CenterID)
+                GROUP BY PatientName, TimeRun, Project, Cohort, MincFile,
+                  Scan_type, SeriesUID, Site, hash, Resolved, TarchiveID,
+                  CandID, PSCID
               UNION
                   SELECT PatientName,
                          TimeRun,
diff --git a/modules/mri_violations/php/resolve.class.inc b/modules/mri_violations/php/resolve.class.inc
index c92b8479d7c..e4f73f9cd6a 100644
--- a/modules/mri_violations/php/resolve.class.inc
+++ b/modules/mri_violations/php/resolve.class.inc
@@ -107,12 +107,13 @@ class Resolve extends \NDB_Page
             if (!empty($ID_mri_protocol_violated_scans)) {
                 $newlyResolved['TypeTable'] = 'mri_protocol_violated_scans';
                 $newlyResolved['ExtID']     = $ID_mri_protocol_violated_scans;
+                $DB->insert('violations_resolved', $newlyResolved);
             }
 
-            // Table 2: mri_violations_log
-            $ID_mri_violations_log = $DB->pselectOne(
-                "SELECT LogID
-                FROM mri_violations_log
+            // Table 2: MRICandidateErrors
+            $ID_MRICandidateErrors = $DB->pselectOne(
+                "SELECT ID
+                FROM MRICandidateErrors
                 WHERE (:hash = md5(concat_WS(
                                 ':',MincFile,PatientName,SeriesUID,TimeRun,
                                 PhaseEncodingDirection,EchoTime,EchoNumber)
@@ -121,15 +122,16 @@ class Resolve extends \NDB_Page
                 ['hash' => $hash]
             );
 
-            if (!empty($ID_mri_violations_log)) {
-                $newlyResolved['TypeTable'] = 'mri_violations_log';
-                $newlyResolved['ExtID']     = $ID_mri_violations_log;
+            if (!empty($ID_MRICandidateErrors)) {
+                $newlyResolved['TypeTable'] = 'MRICandidateErrors';
+                $newlyResolved['ExtID']     = $ID_MRICandidateErrors;
+                $DB->insert('violations_resolved', $newlyResolved);
             }
 
-            // Table 3: MRICandidateErrors
-            $ID_MRICandidateErrors = $DB->pselectOne(
-                "SELECT ID
-                FROM MRICandidateErrors
+            // Table 3: mri_violations_log
+            $mri_violations_log_ID_list = $DB->pselect(
+                "SELECT LogID
+                FROM mri_violations_log
                 WHERE (:hash = md5(concat_WS(
                                 ':',MincFile,PatientName,SeriesUID,TimeRun,
                                 PhaseEncodingDirection,EchoTime,EchoNumber)
@@ -137,12 +139,13 @@ class Resolve extends \NDB_Page
                 )",
                 ['hash' => $hash]
             );
+            error_log(print_r($mri_violations_log_ID_list, true));
 
-            if (!empty($ID_MRICandidateErrors)) {
-                $newlyResolved['TypeTable'] = 'MRICandidateErrors';
-                $newlyResolved['ExtID']     = $ID_MRICandidateErrors;
+            foreach ($mri_violations_log_ID_list as $entry) {
+                $newlyResolved['TypeTable'] = 'mri_violations_log';
+                $newlyResolved['ExtID']     = $entry['LogID'];
+                $DB->insert('violations_resolved', $newlyResolved);
             }
-            $DB->insert('violations_resolved', $newlyResolved);
 
             // Created
             $response = $response->withStatus(201);
diff --git a/modules/mri_violations/test/mri_violationsTest.php b/modules/mri_violations/test/mri_violationsTest.php
index 001d1e5e355..c1e89da5832 100644
--- a/modules/mri_violations/test/mri_violationsTest.php
+++ b/modules/mri_violations/test/mri_violationsTest.php
@@ -386,12 +386,12 @@ public function testDashboardWidgetAllSites()
             ]
         );
         $this->safeGet($this->url . '/dashboard/');
-        // Raisin bread has 173 unresolved violated scans. We are adding three
-        // in setup(): one resolved, and two unresolved. The total
-        // number of unresolved violations is thus 175
+        // Raisin bread has 169 unique unresolved violated scans. We are adding
+        // three in setup(): one resolved, and two unresolved. The total
+        // number of unresolved violations is thus 172
         $this->_testMytaskPanelAndLink(
             ".mri_violations",
-            "175",
+            "172",
             "- MRI Violated Scans"
         );
         $this->resetPermissions();