Ceciles review comments

Author: driusan

modules/mri_violations/jsx/protocolModal.js

@@ -53,7 +53,9 @@ function ProtocolViolationModal(props) {
 
     let violations = [];
 
+    let title = 'Violations for SeriesUID ' + props.SeriesUID;
     for (const violation of data) {
+      title = 'Violations for ' + violation[4];
       violations.push(
         <div key={violation[4]}>
           <dl style={{display: 'flex', width: '100%',
@@ -76,13 +78,21 @@ function ProtocolViolationModal(props) {
               <dd>{violation[3]}</dd>
             </div>
             <div>
-              <dt>Minc Location</dt>
+              <dt>Image Location</dt>
               <dd>{violation[4]}</dd>
             </div>
             <div>
               <dt>Patient Name</dt>
               <dd>{violation[5]}</dd>
             </div>
+            <div>
+              <dt>Series UID</dt>
+              <dd>{props.SeriesUID}</dd>
+            </div>
+            <div>
+              <dt>Echo Time</dt>
+              <dd>{violation[8]}</dd>
+            </div>
           </dl>
 
           <table className="table table-hover table-primary table-bordered">
@@ -214,15 +224,16 @@ function ProtocolViolationModal(props) {
       }
     }
     pushgroup(curgroupname, curgroup);
+    console.log(props);
 
     return <Modal onClose={props.onClose}
              show={true}
              width="90%"
-             title={'Violations for SeriesUID ' + props.SeriesUID}>
+             title={title}>
+              <h2>Image Protocol</h2>
+              {violations}
               <h2>Study Protocols</h2>
               {protocolgroups}
-              <h2>Violations</h2>
-              {violations}
            </Modal>;
 }
 

modules/mri_violations/jsx/resolvedViolations.js

@@ -54,7 +54,7 @@ export function resolvedFilters(fieldoptions) {
       },
     },
     {
-      label: 'Minc File', show: true, filter: {
+      label: 'Image File', show: true, filter: {
         name: 'mincFile',
         type: 'text',
       },

modules/mri_violations/jsx/unresolvedViolations.js

@@ -46,7 +46,7 @@ export function unresolvedFilters(fieldoptions) {
       },
     },
     {
-      label: 'Minc File', show: true, filter: {
+      label: 'Image File', show: true, filter: {
         name: 'mincFile',
         type: 'text',
       },

modules/mri_violations/php/mri_violations.class.inc

@@ -135,13 +135,26 @@ class Mri_Violations extends \DataFrameworkMenu
             return (new ProtocolViolationProvisioner($this->seriesUID));
         case 'protocolcheck':
             return (new ProtocolCheckViolationProvisioner($this->seriesUID))
-              ->filter(new UserCenterMatchOrNull);
+              ->filter(new UserCenterMatchOrNullOrAnyPermission(
+                      $this->allSitePermissionNames(),
+              ));
         default:
             return (new Provisioner())
               ->filter(new UserProjectMatchOrNull);
         }
     }
 
+    /**
+     * Overload filters to allow posssibility of a null CenterID. Filters are
+     * added in getBaseDataProvisioner depending on the type
+     *
+     * @return \LORIS\Data\Provisioner
+     */
+    public function getDataProvisionerWithFilters() : \LORIS\Data\Provisioner
+    {
+        return $this->getBaseDataProvisioner();
+    }
+
     /**
      * Include additional JS files
      *

modules/mri_violations/php/resolved_violations.class.inc

@@ -84,5 +84,21 @@ class Resolved_Violations extends \DataFrameworkMenu
         return (new ResolvedViolationsProvisioner())
             ->filter(new UserProjectMatchOrNull);
     }
+
+    /**
+     * Overload filters to allow posssibility of a null CenterID
+     *
+     * @return \LORIS\Data\Provisioner
+     */
+    public function getDataProvisionerWithFilters() : \LORIS\Data\Provisioner
+    {
+        $provisioner = $this->getBaseDataProvisioner()->filter(
+            new UserCenterMatchOrNullOrAnyPermission(
+                $this->allSitePermissionNames()
+            )
+        );
+
+        return $provisioner;
+    }
 }
 

modules/mri_violations/php/resolvedviolation.class.inc

@@ -7,8 +7,7 @@ namespace LORIS\mri_violations;
  *
  * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
  */
-class ResolvedViolation implements \LORIS\Data\DataInstance,
-    \LORIS\StudyEntities\SiteHaver
+class ResolvedViolation implements \LORIS\Data\DataInstance
 {
     protected $DBRow;
     protected $CenterID;
@@ -37,11 +36,14 @@ class ResolvedViolation implements \LORIS\Data\DataInstance,
      * Returns the CenterID for this instance, for filters such as
      * \LORIS\Data\Filters\UserSiteMatch to match against.
      *
-     * @return \CenterID
+     * @return ?\CenterID
      */
-    public function getCenterID(): \CenterID
+    public function getCenterID(): ?\CenterID
     {
-        return new \CenterID($this->DBRow['CenterId']);
+        if ($this->DBRow['Site'] === null) {
+            return null;
+        }
+        return new \CenterID($this->DBRow['Site']);
     }
 
     /**

modules/mri_violations/php/usercentermatchornulloranypermission.class.inc

@@ -0,0 +1,52 @@
+<?php
+namespace LORIS\mri_violations;
+
+/**
+ * UserCenterMatchOrNull filters out data for any resource which is not
+ * part of one of the user's centers. If the resource has no CenterID
+ * defined, it is not excluded from the results.
+ *
+ * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
+ */
+class UserCenterMatchOrNullOrAnyPermission implements \LORIS\Data\Filter
+{
+    public function __construct(protected array $permissions)
+    {
+    }
+
+    /**
+     * Implements the \LORIS\Data\Filter interface
+     *
+     * @param \User                    $user     The user that the data is being
+     *                                           filtered for.
+     * @param \LORIS\Data\DataInstance $resource The data being filtered.
+     *
+     * @return bool
+     */
+    public function filter(\User $user, \Loris\Data\DataInstance $resource) : bool
+    {
+        // phan only understands method_exists on simple variables.
+        // Assigning to a variable is the a workaround
+        // for false positive 'getProjectID doesn't exist errors suggested
+        // in https://github.com/phan/phan/issues/2628
+        $res = $resource;
+        '@phan-var object $res';
+
+        if ($user->hasAnyPermission($this->permissions)) {
+            return true;
+        }
+
+        if (method_exists($res, 'getCenterID')) {
+            $resourceCenter = $res->getCenterID();
+            if (!is_null($resourceCenter)) {
+                return $user->hasCenter(
+                    new \CenterID(strval($resourceCenter))
+                );
+            }
+            return true;
+        }
+        throw new \LorisException(
+            "Can not implement filter on a resource type that has no centers."
+        );
+    }
+}