Merge latest LORIS Core 25.0-release branch (#37)

* [Issue tracker] - fix can't Delete attachment (aces#8337) Allow users to delete their own attachment. If a user does not have Issue Tracker: Close/Edit/Re-assign/Comment on All Issues, they cannot currently delete their own attachment. Resolves aces#8006 * [configuration] Fix error messages & saving with null Alias (aces#8349) This prevents the user from saving a Project configuration with a null Alias in a new project as well as existing projects. * [webpack] remove util shortcut as it is unusedand causes conflicts (aces#8634) The util shortcut seems to be completely unused in the code, the only place using the content of that directory references the whole path of the directory Resolves aces#8577 * [Tools] double_escape_report tool breaking on non-string values (aces#8484) This tool tries to run pregmatch on every value pulled from the instrument Data. there is a possibility that values coming from json_decode() are decoded into floats or arrays if that is how they have been saved. if it's the case the script should just skip them and not fail * [issue tracker] Populate reporter dropdown (aces#8469) In the issue tracker module, the reporter dropdown is populated with the assignee users instead of the reporter users. This fills the reporter dropdown with the right reporter values. Fixes aces#8429 * [examiner] Column for 'Instrument' in certification menu appears narrow (aces#8453) This is changing the CSS class used in the examiner page. The former class had a small percentage of the width causing the first field become narrow, by changing it to another class, the problem is solved. Fixes aces#8026 * [tools] support JSON instruments in fix_candidate_age.php (aces#8286) Fixes aces#8090 * [bvl_feedback] Fix Permissions for Feedback Summary & Thread List (aces#7826) Currently, only users with the access_all_profiles permission can see Feedback Threads and Open Thread Summary at the profile level. Even if a user adds a feedback entry, they won't be able to see their own feedback thread unless they have the access_all_profiles permission. This change allows users to see threads and the summary that exists for candidates that they have access to (i.e., if a user is affiliated with MTL, they can now see the feedback threads & summary of MTL candidates). Fixes aces#7190 * [new_profile/sex] Remove strtolower() and force uppercase (aces#8633) The keys in the new_profile module were lower case which forces the Sex library class to use a strtolower() function to validate the value and then submits the lowercase value in the SQL insert statement where SQL implicitly converts it to uppercase. This workflow is very risky as different versions of SQL or different databases may not recognise the lowercase and uppercase as the same word and treat it as a truncation. This is also simply bad practice and unnecessary here. * [NDB_BVL_Feedback] Fixing 500 error on instrument list page (aces#8694) A recent change (aces#7826) to the NDB_BVL_Feedback class causes the following 500 Error to appear when the instrument_list page is loaded. This is because references are made to Timepoint::singleton instead of TimePoint::singleton. Fix casing. * [Login] Case insensitive comparison of Authentication header The HTTP spec says the header name is case-insensitive. Some clients send it as "authentication" (lowercase). This makes our check for the header case-insensitive by lower-casing the headers before doing the comparison. * Fix PHPCS (aces#8719) * Fix nonaggregated column error in mri_violation provisioner (aces#8716) Fixes aces#8705, Fixes aces#8697 * [EEG Uploader] Handle Checksum value (aces#8729) Include checksum in value saved to database. * [instruments] Fix properly disabling/enabling 'Delete instrument data' button display (aces#8686) Currently, if the 'InstrumentResetting' config is set to 'No', the 'Delete instrument data' button is displayed and the user is able to clear the instrument. This fixes that by letting the if statement check a true boolean instead of string "true" by calling 'settingEnabled'. The PR also modifies the testing plan so this is tested in the future. * [electrophysiology_uploader] Added 'Help' content for the elctrophysiology_uploader module (aces#8728) Added 'Help' content for electrophysiology_uploader module. Fixes aces#8597 * [module_manager] Visibility of 'My Preferences' in menu reflects state (aces#8726) The 'My Preferences' menu item is no longer visible if the my_preferences module is not Active. Fixes aces#8695 * [Media] Fix Input callback not set (aces#8720) Add a onUserInput prop to the "Update File" ButtonElement Fixes aces#8700 * [battery_manager] React warning messages in console (aces#8724) A few number values were being passed as Strings in the props to Form elements. Change them to be numbers. Fixes aces#8703 * [user_accounts] Update TestPlan wording (aces#8718) This changes the wording slightly in the TestPlan of user_accounts to indicate that an email is only sent to a new user if the "Send email to user" button is selected. Fixes aces#6971 * 24.1 to 25.0 release patch (aces#8715) Combine files in the New_patches into one release file. * [document_repository] Include steps to test for multiple files (aces#8758) Updated some steps to include the newly added support for uploading multiple files at once. For testing assignment aces#8519. * [document_repository] Update help content (aces#8761) Added reference to new feature to support uploading multiple files at once. * [user_accounts] Update TestPlan.md (aces#8754) The pwned password example was not complex enough to pass the complexity check and trigger the pwned check, so change the example to another one which is more secure but also in a pwned database. * [acknowledgements] Update test plan (aces#8763) Correct step about clearing filters wording and add step to check the citation policy works. Fixes aces#8736 * [instrument & instrument_manager] test plans update (aces#8765) Transfer the permission test step from instruments module to instrument_manager module, since it is now managed there instead of in the config.xml * [dashboard] Load project description from ajax and run through DOMPurify (aces#8762) This makes sure the Project Description on the dashboard runs through DOMPurify. In order to do that, it was also necessary to move the description from being loaded in a smarty template to being loaded from an AJAX call (so that we can import the DOMPurify module.) Fixes aces#8750 * [configuration] Use unsafeInsert/unsafeUpdate for saving values (aces#8759) The values get double-escaped when modified now if they contain HTML. Use unsafe variants of database calls so that the values to not get modified when re-saved. Fixes aces#8748 * [new_profile] Fix date requirements and formats with EDC (aces#8767) Fixed NewProfileIndex to use the correct variable containing the EDC date To create a candidate, the date of birth for a candidate is now required only when useEDC = no in Candidate.class.inc. If useEDC = yes, DoB is null unless it's specified. Added the validation of the EDC date format If the Ym format is selected in the configuration, added a '-15' to the end of the EDC date to be compatible with the SQL type date Resolves aces#8742 * [Media] Fix warning invalid prop options (aces#8722) Update the prop option hiddenOptions for the fileVisibility SelectElement to be an object instead of an array. Resolves aces#8696 * [Survey] Fix loading of survey instruments (aces#8647) The module search path for survey instruments was incorrect. Set to the same search path as in tools/generic_includes.php Resolves aces#8546 * [new_profile] Fix Date Picker (aces#8775) Removed the legacy JQuery month picker, without it the issue is gone. Resolves aces#8753 * [api] Test multiple versions (aces#8778) This attempts to add tests for multiple versions of the API. The tests from v0.0.4-dev are copied to v0.0.3 classes and the version in the new classes changed to v0.0.3. * [survey_accounts] Fix help text (aces#8789) Replace `<br>` tag with ` \n` as per markdown spec in order to fix rendering of help text. * [Timepoint List] Hide visits that are from user unaffiliated projects (aces#8723) Adjusted the Timepoint_List module to also filter out the visits of projects that are not affiliated to the current user when they have permission to all sites. Resolves aces#8710 * [Candidate] Fix wrong comments to avoid confusion in future (aces#8791) The middle day of the month is used when date format is Y-M, not the first. --------- Co-authored-by: Shen <kongtiaowangshen@gmail.com> Co-authored-by: CamilleBeau <51176779+CamilleBeau@users.noreply.github.com> Co-authored-by: Rida Abou-Haidar <ridz1208@users.noreply.github.com> Co-authored-by: regis <regisoc@users.noreply.github.com> Co-authored-by: miladheshmati <51356424+miladheshmati@users.noreply.github.com> Co-authored-by: Suzanne Lee <suzannebhlee@gmail.com> Co-authored-by: Alexandra Livadas <alexandra.livadas@mail.mcgill.ca> Co-authored-by: Dave MacFarlane <driusan@mcin.ca> Co-authored-by: Laetitia Fesselier <laetitia.fesselier@mail.mcgill.ca> Co-authored-by: jeffersoncasimir <15801528+jeffersoncasimir@users.noreply.github.com> Co-authored-by: charlottesce <75381352+charlottesce@users.noreply.github.com> Co-authored-by: Saagar Arya <51128536+skarya22@users.noreply.github.com>
zaliqarosli · Jun 15, 2023 · d087c1d · d087c1d
1 parent 4aafee2
commit d087c1d
Show file tree

Hide file tree

Showing 63 changed files with 4,657 additions and 205 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -148,6 +148,7 @@ the `Data` key instead of `$InstrumentName` (PR #7857)
 - Add psr/log to composer (PR #8109)
 - Fixed broken DB calls in `assign_missing_instruments` and `instruments` (PR #8162)
 - Add support for PHP 8.1 (PR #7989)
+- Fix Project tab of Configuration module to give correct errors, and prevent saving without Alias (PR #8349)
 ### Modules
 #### API
 - Ability to use PSCID instead of the CandID in the candidates API (PR #8138)

diff --git a/Makefile b/Makefile
@@ -64,3 +64,5 @@ login:
 mri_violations:
 	target=mri_violations npm run compile
 
+dashboard:
+	target=dashboard npm run compile
diff --git a/htdocs/survey.php b/htdocs/survey.php
@@ -86,7 +86,10 @@ function initialize()
         $this->loris     = new \LORIS\LorisInstance(
             $DB,
             $config,
-            []
+            [
+                __DIR__ . "/../project/modules",
+                __DIR__ . "/../modules/",
+            ]
         );
         $this->TestName  = $DB->pselectOne(
             "SELECT Test_name FROM participant_accounts

diff --git a/jsx/Form.js b/jsx/Form.js
@@ -1568,25 +1568,6 @@ class DateElement extends Component {
     if (this.props.maxYear === '' || this.props.maxYear === null) {
       maxYear = '9999';
     }
-    let monthInputs = $('input[type=month][name=' + this.props.name+']');
-    monthInputs.datepicker({
-      dateFormat: 'yy-mm',
-      changeMonth: true,
-      changeYear: true,
-      yearRange: minYear + ':' + maxYear,
-      constrainInput: true,
-      onChangeMonthYear: (y, m, d) => {
-        // Update date in the input field
-        $(this).datepicker('setDate', new Date(y, m - 1, d.selectedDay));
-      },
-      onSelect: (dateText, picker) => {
-        this.props.onUserInput(this.props.name, dateText);
-      },
-    });
-    monthInputs.attr('placeholder', 'yyyy-mm');
-    monthInputs.on('keydown paste', (e) => {
-      e.preventDefault();
-    });
   }
 
   // //////// START OF COPN OVERRIDE //////////
@@ -1738,8 +1719,8 @@ DateElement.propTypes = {
   label: PropTypes.string,
   value: PropTypes.string,
   id: PropTypes.string,
-  maxYear: PropTypes.string,
-  minYear: PropTypes.string,
+  maxYear: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
+  minYear: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
   dateFormat: PropTypes.string,
   disabled: PropTypes.bool,
   required: PropTypes.bool,

diff --git a/modules/acknowledgements/test/TestPlan.md b/modules/acknowledgements/test/TestPlan.md
@@ -5,10 +5,11 @@
 2. Access the acknowledgements module page, ensure that it renders.[Automation Testing]
 3. Verify that a new record can not be added Without acknowledgements_edit permission [Manual Testing]
 4. Check that each dropdown has the correct options.[Manual Testing]
-5. Click "Clear Form" and ensure filters are reset to same state.[Automation Testing]
+5. Click "Clear Filter" and ensure filters are reset the default state.[Automation Testing]
 6. Verify that the "Save" button on the modal window form inserts a new record in the 
    data table and that the data table is refreshed automatically.[Automation Testing]
 7. Start entering information in the modal window form and close the modal window 
    without saving. A message asking 'Are You Sure?' should appear to confirm that the
    form should indeed be closed without saving the information previously entered [Manual Testing]
-
+8. Modify the "citation_policy" setting in the LORIS configuration module and ensure that the citation policy
+   at the top of the page is modified.
diff --git a/modules/api/php/endpoint.class.inc b/modules/api/php/endpoint.class.inc
@@ -59,7 +59,9 @@ abstract class Endpoint extends LORISEndpoint implements RequestHandlerInterface
         $versions = $this->supportedVersions() ?? [];
         $version  = $request->getAttribute("LORIS-API-Version") ?? "unknown";
         if (!in_array($version, $versions)) {
-            return new \LORIS\Http\Response\JSON\BadRequest('Unsupported version');
+            // If it's not supported by a version of the API, that means the
+            // endpoint for that version should be not found
+            return new \LORIS\Http\Response\JSON\NotFound('Unsupported version');
         }
 
         return parent::process($request, $handler);

diff --git a/modules/battery_manager/jsx/batteryManagerForm.js b/modules/battery_manager/jsx/batteryManagerForm.js
@@ -67,8 +67,8 @@ class BatteryManagerForm extends Component {
           name="ageMinDays"
           label="Minimum age (days)"
           onUserInput={setTest}
-          min="0"
-          max="99999"
+          min={0}
+          max={99999}
           required={true}
           value={test.ageMinDays}
           errorMessage={errors.ageMinDays}
@@ -78,8 +78,8 @@ class BatteryManagerForm extends Component {
           name="ageMaxDays"
           label="Maximum age (days)"
           onUserInput={setTest}
-          min="0"
-          max="99999"
+          min={0}
+          max={99999}
           required={true}
           value={test.ageMaxDays}
           errorMessage={errors.ageMaxDays}
@@ -134,8 +134,8 @@ class BatteryManagerForm extends Component {
           label="Instrument Order"
           onUserInput={setTest}
           required={false}
-          min="0"
-          max="127" // max value allowed by default column type of instr_order
+          min={0}
+          max={127} // max value allowed by default column type of instr_order
           value={test.instrumentOrder}
         />
          <ButtonElement

diff --git a/modules/candidate_profile/test/TestPlan.md b/modules/candidate_profile/test/TestPlan.md
@@ -50,6 +50,7 @@ that widget (ie. the media module for CandID 587630 (DCC090) or CandID 300001 (M
 4. Ensure that, when the module which added the extra `CandidateInfo` terms
    is disabled, the terms from that module no longer show up in the
    `Candidate Info` card.
+5. Ensure that you can always only see visits from projects that you are affiliated with.
 
 All other widgets are part of other modules, and should be tested as
 part of that module's testing.
diff --git a/modules/configuration/ajax/process.php b/modules/configuration/ajax/process.php
@@ -60,7 +60,7 @@
                 }
             }
             // Update the config setting to the new value.
-            $DB->update(
+            $DB->unsafeUpdate(
                 'Config',
                 ['Value' => $value],
                 ['ID' => $key]
@@ -108,7 +108,7 @@
                 }
             }
             // Add the new setting
-            $DB->insert(
+            $DB->unsafeInsert(
                 'Config',
                 [
                     'ConfigID' => $ConfigSettingsID, // FK to ConfigSettings.

diff --git a/modules/configuration/ajax/updateProject.php b/modules/configuration/ajax/updateProject.php
@@ -44,18 +44,35 @@
         exit;
     }
 
+    if (empty($_POST['Name'])
+        || empty($_POST['Alias'])
+        || strlen($_POST['Alias']) > 4
+    ) {
+        http_response_code(400);
+        echo json_encode(['error' => 'Bad Request']);
+        exit;
+    }
+
     $project = \Project::createNew($projectName, $projectAlias, $recTarget);
     $db->insert(
         'user_project_rel',
         ["UserID"=>$user->getId(),"ProjectID"=>$project->getId()]
     );
 } else {
+    if (empty($_POST['Name'])
+        || empty($_POST['Alias'])
+        || strlen($_POST['Alias']) > 4
+    ) {
+        http_response_code(400);
+        echo json_encode(['error' => 'Bad Request']);
+        exit;
+    }
+
     // Update Project fields
     $project = \Project::getProjectFromID(new \ProjectID($projectID));
     $project->updateName($projectName);
     $project->updateAlias($projectAlias);
     $project->updateRecruitmentTarget($recTarget);
-
 }
 
 // Cohort information isn't mandatory. If the array is empty, give an

diff --git a/modules/configuration/js/project.js b/modules/configuration/js/project.js
@@ -31,15 +31,23 @@ $(document).ready(function() {
         }
 
         var errorClosure = function(i, form) {
-          if (isNaN(recruitmentTarget)) {
+          if (!Name) {
+            return function () {
+              $(form.find(".saveStatus")).text("Failed to save, must enter a Project Name!").css({'color': 'red'}).fadeIn(500).delay(1000).fadeOut(500);
+            }
+          } else if (!Alias) {
+            return function () {
+              $(form.find(".saveStatus")).text("Failed to save, must enter an Alias!").css({'color': 'red'}).fadeIn(500).delay(1000).fadeOut(500);
+            }
+          } else if (isNaN(recruitmentTarget)) {
             return function () {
               $(form.find(".saveStatus")).text("Failed to save, recruitment target must be an integer!").css({'color': 'red'}).fadeIn(500).delay(1000).fadeOut(500);
             }
           } else if (Alias.length > 4){
             return function () {
               $(form.find(".saveStatus")).text("Failed to save, Alias should be at most 4 characters long!").css({'color': 'red'}).fadeIn(500).delay(1000).fadeOut(500);
             }
-          }else {
+          } else {
             return function () {
               $(form.find(".saveStatus")).text("Failed to save, same name already exist!").css({'color': 'red'}).fadeIn(500).delay(1000).fadeOut(500);
             }

diff --git a/modules/dashboard/.gitignore b/modules/dashboard/.gitignore
@@ -0,0 +1 @@
+js/welcome.js
diff --git a/modules/dashboard/jsx/welcome.js b/modules/dashboard/jsx/welcome.js
@@ -0,0 +1,13 @@
+import DOMPurify from 'dompurify';
+
+window.addEventListener('load', () => {
+    fetch(loris.BaseURL + '/dashboard/projectdescription').then( (resp) => {
+        if (!resp.ok) {
+            throw new Error('Could not get project description');
+        }
+        return resp.json();
+    }).then( (json) => {
+      const el = document.getElementById('project-description');
+      el.innerHTML = DOMPurify.sanitize(json.Description);
+    }).catch( (e) => console.error(e));
+});
diff --git a/modules/dashboard/php/projectdescription.class.inc b/modules/dashboard/php/projectdescription.class.inc
@@ -0,0 +1,38 @@
+<?php
+namespace LORIS\dashboard;
+use \Psr\Http\Message\ServerRequestInterface;
+use \Psr\Http\Message\ResponseInterface;
+
+/**
+ * Get the project description to display in the welcome widget.
+ *
+ * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
+ */
+
+class ProjectDescription extends \LORIS\Http\Endpoint
+{
+    /**
+     * {@inheritDoc}
+     *
+     * @param ServerRequestInterface $request The incoming request
+     *
+     * @return ResponseInterface
+     */
+    public function handle(ServerRequestInterface $request) : ResponseInterface
+    {
+        $desc = $this->loris->getConfiguration()->getSetting('projectDescription');
+        return new \LORIS\Http\Response\JSON\OK(['Description' => $desc]);
+    }
+
+    /**
+     * The hasAccess call called by the Module class before loading the page.
+     *
+     * @param \User $user The user accessing the page
+     *
+     * @return bool
+     */
+    function _hasAccess(\User $user)
+    {
+        return true;
+    }
+}
diff --git a/modules/dashboard/templates/welcomebody.tpl b/modules/dashboard/templates/welcomebody.tpl
@@ -1,5 +1,4 @@
 <h3 class="welcome">Welcome, {$username}.</h3>
 <p class="pull-right small login-time">Last login: {$last_login}</p>
-{if !is_null($project_description)}
-<p class="project-description">{$project_description}</p>
-{/if}
+<p id="project-description" class="project-description"></p>
+<script src="/dashboard/js/welcome.js"></script>
diff --git a/modules/document_repository/help/document_repository.md b/modules/document_repository/help/document_repository.md
@@ -5,6 +5,6 @@ This module provides studies with a centralized location for important documents
 In the **Browse** tab, you can search for specific files using the **Selection Filter** section. Click on any file name in the data table to download. Click on any of the folders to reveal subfolders and files organized within. You can click on **Edit** or **Delete** in any cell, in those respective columns, to modify a file. Click on certain column headers to sort the list of documents.
 The **Filter globally** only works in the current directory (root folder only contains directories).
 The **Filter globally** will filter the file across the directories.
-Navigate to the **Upload** tab to upload a file. Populate the fields and upload your file. Your newly uploaded file should now appear in the list within the Browse tab.
+Navigate to the **Upload** tab to upload one or more files. Populate the fields and upload your files. Your newly uploaded files should now appear in the list within the Browse tab.
 
 If you need to add a new category or subcategory, you can do so in the **Category** tab. Populate the fields and click **Add Category**.
diff --git a/modules/document_repository/test/TestPlan.md b/modules/document_repository/test/TestPlan.md
@@ -6,7 +6,7 @@
    [Manual Testing]
 3. Check that the comments for a category are displayed properly as a tooltip.
    [Manual Testing]
-4. Upload a file.
+4. Upload one file. Upload multiple files at once.
    [Manual Testing]
 5. User is able to delete a file if they have the "Delete files in Document Repository" permission or is the super user.
    [Manual Testing]
@@ -29,13 +29,13 @@
    [Manual Testing]
 14. Edit a file in the repository: check that “Date Uploaded” date is updated.
     [Manual Testing]
-15. Visit the My Preferences module and enable notifications for the document_repository.
+15. Visit the "My Preferences" module and enable notifications for the document_repository.
     Make sure that you are notified for the follow changes:
        - Addition, deletion or modification of a file (by a user other than yourself)
        - Addition of a category (by a user other than yourself)
     [Manual Testing]
 16. Try uploading a file that exceeds the max upload limit. Ensure that an error message occurs
-    when the server has detected that the file is too large.
+    when the server has detected that the file is too large. Do this again, with multiple files at once, to ensure proper individual error reporting for all files.
     [Manual Testing]
 
 ### Widget registration on the dashboard page

diff --git a/modules/electrophysiology_uploader/help/electrophysiology_uploader.md b/modules/electrophysiology_uploader/help/electrophysiology_uploader.md
@@ -0,0 +1,13 @@
+# Electrophysiology Uploader
+
+This module allows you to upload EEG recordings and browse past uploads.
+
+In the **Browse** tab, you can use the *Selection Filter* section to search for existing uploads.
+
+In the data table, you can click any link in the *Upload Location* column to download the uploaded files.
+
+To upload recordings, click on the **Upload** tab. Select your file for upload and enter the required information about the recording.
+
+Note: The candidate must already be registered in LORIS and the visit label must be known for the study.
+
+Pay attention to the *Notes* for upload rules. Your dataset should be [BIDS-compliant](https://bids-specification.readthedocs.io).
diff --git a/modules/electrophysiology_uploader/php/upload.class.inc b/modules/electrophysiology_uploader/php/upload.class.inc
@@ -46,7 +46,7 @@ class Upload extends \NDB_Page
         // Ensure GET or POST request.
         switch ($request->getMethod()) {
         case 'GET':
-            return $this->_handleGet($request);
+            return $this->_handleGET($request);
         case 'POST':
             return $this->_handlePOST($request);
         default:
@@ -63,7 +63,7 @@ class Upload extends \NDB_Page
      *
      * @return ResponseInterface
      */
-    private function _handleGet(ServerRequestInterface $request) : ResponseInterface
+    private function _handleGET(ServerRequestInterface $request) : ResponseInterface
     {
         $qParams = $request->getQueryParams();
         if (!isset($qParams['upload_id'])) {
@@ -244,6 +244,7 @@ class Upload extends \NDB_Page
             'UploadDate'     => date('Y-m-d H:i:s'),
             'UploadLocation' => "$filename",
             'SessionID'      => $session['ID'],
+            'Checksum'       => $values['checksum'] ?? null,
         ];
 
         $db->insert('electrophysiology_uploader', $saveValues);

diff --git a/modules/examiner/templates/form_editExaminer.tpl b/modules/examiner/templates/form_editExaminer.tpl
@@ -12,7 +12,7 @@
                     </div>
                     {/foreach}
                     <div class="row hidden-xs hidden-sm">
-                        <div class="col-md-1">
+                        <div class="col-md-3">
                             <label>Instrument</label>
                         </div>
                         <div class="col-md-2">
@@ -21,19 +21,20 @@
                         <div class="col-md-2 col-md-offset-1">
                             <label>Certification Date</label>
                         </div>
-                        <div class="col-md-4 col-md-offset-1">
+                        <div class="col-md-2 col-md-offset-1">
                             <label>Comments</label>
                         </div>
                     </div>
                     <hr class="row hidden-xs hidden-sm">
                     {foreach from=$instruments key=ID item=name}
                     <div class="row">
-                        <div class="col-md-1">
+                        <div class="col-md-3">
                             <label>{$form.$name.label}</label>
                         </div>
                         {$form.$name.html}
                     </div>
                     {/foreach}
+                    <hr class="row hidden-xs hidden-sm">
                     <div class="row">
                         {if not $success|default}
                         <div class="col-xs-12">

diff --git a/modules/instrument_manager/test/TestPlan.md b/modules/instrument_manager/test/TestPlan.md
@@ -21,4 +21,5 @@ Check that function CheckTable performs the appropriate checks (catches these di
 eg "Table Valid" column is valid when instrument fields match up and "Pages Valid" is valid when subpages are valid.
    [Manual Testing]
 9. With a user who has the `instrument_manager_write` permission, click the button in the `Permission Required` column. A modal should popup where you can edit the permissions.
-Check that modifying the permissions saves the new permissions.
+   - Check that modifying the permissions saves the new permissions.
+   - Check that a user can access that instruments if and only if they has that permission.