From 5165a39a25f9137f949dd110bab99630a7b53931 Mon Sep 17 00:00:00 2001 From: Victoria Foing Date: Thu, 19 Jul 2018 18:21:46 -0400 Subject: [PATCH 01/14] Create module Battery Manager made sure everything is functional minor format change fixed error in code reverted Modal form to original state fixed user perm rel file --- SQL/0000-00-02-Modules.sql | 1 + ...2018-07-23-battery_manager_permissions.sql | 27 + jsx/FilterableDataTable.js | 1 + jsx/Form.js | 2 +- modules/battery_manager/.gitignore | 1 + modules/battery_manager/README.md | 27 + .../battery_manager/help/battery_manager.md | 116 ++++ .../battery_manager/jsx/batteryManagerForm.js | 143 +++++ .../jsx/batteryManagerIndex.js | 496 ++++++++++++++++++ .../php/battery_manager.class.inc | 91 ++++ modules/battery_manager/php/module.class.inc | 66 +++ modules/battery_manager/php/test.class.inc | 84 +++ .../php/testendpoint.class.inc | 262 +++++++++ .../php/testoptionsendpoint.class.inc | 102 ++++ .../php/testprovisioner.class.inc | 70 +++ .../test/BatteryManagerTest.php | 64 +++ modules/battery_manager/test/TestPlan.md | 104 ++++ raisinbread/RB_files/RB_modules.sql | 1 + raisinbread/RB_files/RB_permissions.sql | 2 + raisinbread/RB_files/RB_user_perm_rel.sql | 2 + webpack.config.js | 1 + 21 files changed, 1662 insertions(+), 1 deletion(-) create mode 100644 SQL/New_patches/2018-07-23-battery_manager_permissions.sql create mode 100644 modules/battery_manager/.gitignore create mode 100644 modules/battery_manager/README.md create mode 100644 modules/battery_manager/help/battery_manager.md create mode 100644 modules/battery_manager/jsx/batteryManagerForm.js create mode 100644 modules/battery_manager/jsx/batteryManagerIndex.js create mode 100644 modules/battery_manager/php/battery_manager.class.inc create mode 100644 modules/battery_manager/php/module.class.inc create mode 100644 modules/battery_manager/php/test.class.inc create mode 100644 modules/battery_manager/php/testendpoint.class.inc create mode 100644 modules/battery_manager/php/testoptionsendpoint.class.inc create mode 100644 modules/battery_manager/php/testprovisioner.class.inc create mode 100644 modules/battery_manager/test/BatteryManagerTest.php create mode 100644 modules/battery_manager/test/TestPlan.md diff --git a/SQL/0000-00-02-Modules.sql b/SQL/0000-00-02-Modules.sql index ebe02de10a7..3cdf4ba0883 100644 --- a/SQL/0000-00-02-Modules.sql +++ b/SQL/0000-00-02-Modules.sql @@ -8,6 +8,7 @@ CREATE TABLE `modules` ( INSERT INTO modules (Name, Active) VALUES ('acknowledgements', 'Y'); INSERT INTO modules (Name, Active) VALUES ('api', 'Y'); +INSERT INTO modules (Name, Active) VALUES ('battery_manager', 'Y'); INSERT INTO modules (Name, Active) VALUES ('behavioural_qc', 'Y'); INSERT INTO modules (Name, Active) VALUES ('brainbrowser', 'Y'); INSERT INTO modules (Name, Active) VALUES ('bvl_feedback', 'Y'); diff --git a/SQL/New_patches/2018-07-23-battery_manager_permissions.sql b/SQL/New_patches/2018-07-23-battery_manager_permissions.sql new file mode 100644 index 00000000000..25cbfdb1869 --- /dev/null +++ b/SQL/New_patches/2018-07-23-battery_manager_permissions.sql @@ -0,0 +1,27 @@ +-- Add view permission for battery manager +INSERT INTO permissions (code, description, categoryID) + VALUES ( + 'battery_manager_view', + 'View Battery Manager', + (SELECT ID FROM permissions_category WHERE Description = 'Permission') + ); + +-- Add edit permission for battery manager +INSERT INTO permissions (code, description, categoryID) + VALUES ( + 'battery_manager_edit', + 'Add, activate, and deactivate entries in Test Battery', + (SELECT ID FROM permissions_category WHERE Description = 'Permission') + ); + +-- Give view permission to admin +INSERT INTO user_perm_rel (userID, permID) +SELECT ID, permID FROM users u JOIN permissions p +WHERE UserID='admin' AND code = 'battery_manager_view'; + +-- Give edit permission to admin +INSERT INTO user_perm_rel (userID, permID) +SELECT ID, permID FROM users u JOIN permissions p +WHERE UserID='admin' AND code = 'battery_manager_edit'; + +INSERT INTO modules (Name, Active) VALUES ('battery_manager', 'Y'); diff --git a/jsx/FilterableDataTable.js b/jsx/FilterableDataTable.js index 7804244b33c..e5484cf0329 100644 --- a/jsx/FilterableDataTable.js +++ b/jsx/FilterableDataTable.js @@ -111,6 +111,7 @@ FilterableDataTable.propTypes = { name: PropTypes.string.isRequired, title: PropTypes.string, data: PropTypes.object.isRequired, + filterPresets: PropTypes.object, fields: PropTypes.object.isRequired, columns: PropTypes.number, getFormattedCell: PropTypes.func, diff --git a/jsx/Form.js b/jsx/Form.js index 441b25fd43f..d67d464bc7f 100644 --- a/jsx/Form.js +++ b/jsx/Form.js @@ -1225,7 +1225,7 @@ class NumericElement extends Component { id={this.props.id} min={this.props.min} max={this.props.max} - value={this.props.value} + value={this.props.value || ''} disabled={disabled} required={required} onChange={this.handleChange} diff --git a/modules/battery_manager/.gitignore b/modules/battery_manager/.gitignore new file mode 100644 index 00000000000..235c1debb3b --- /dev/null +++ b/modules/battery_manager/.gitignore @@ -0,0 +1 @@ +js/batteryManagerIndex.js \ No newline at end of file diff --git a/modules/battery_manager/README.md b/modules/battery_manager/README.md new file mode 100644 index 00000000000..87c7e8c49b7 --- /dev/null +++ b/modules/battery_manager/README.md @@ -0,0 +1,27 @@ +# Battery Manager + +## Purpose +The Battery Manager module allows users to **browse**, **add**, +**edit**, **activate** and **deactivate** entries in the Test Battery. The +Test Battery is used to determine which Instruments are administered at +different timepoints. + +## Intended Users +The Battery Manager module is used by study +administrators. + +## Scope +The Battery Manager module provides a tool for browsing, adding, +editing, activating, and deactivating entries in the the Test Battery. + +#### Interactions with LORIS +Changes, additions and deletion of data in this module affects the test +battery assigned to a candidate at each timepoint + +## Permissions +In order to use the Battery Manager module the user needs +one or both of the following permissions: +- `battery_manager_view`: gives user read-only access to Battery Manager +module (browsing the Test Battery). +- `battery_manager_edit`: gives user edit access to Battery +Manager module (add/edit/activate/deactivate entries in Test Battery). diff --git a/modules/battery_manager/help/battery_manager.md b/modules/battery_manager/help/battery_manager.md new file mode 100644 index 00000000000..c549ed19d17 --- /dev/null +++ b/modules/battery_manager/help/battery_manager.md @@ -0,0 +1,116 @@ +# Battery Manager +The Battery Manager module serves as a front-end for manipulating the Test Battery. +This includes browsing, adding, editing, activating, and deactivating entries. + +## Searching for an entry in the Test Battery +Under the `Browse` tab, use the `Selection Filters` to search for entries by fields such as: +`Instrument`, +`Minimum age (days)`, +`Maximum age (days)`, +`Stage`, +`Subproject`, +`Visit Label`, +`Site`, +`First Visit`, +`Instrument Order`, +`Active`. +As filters are selected, the data table below will dynamically update with relevant results. +Click the **Clear Filters** button to reset all filters. + +Within the data table, results can be sorted in ascending or descending order by +clicking on any column header. + +## Adding an entry to the Test Battery +Under the `Add` tab, you can add a new entry to the Test Battery. +You can specify information about the entry by using the searchable dropdowns, dropdown menus, and numeric text fields. +You will have to fill out the required fields `Instrument`, `Minimum age (days)`, `Maximum age (days)`, and `Stage`. +Finally, press the **Add entry** button to add the entry to the Test Battery. +You cannot add an entry if it has a duplicate entry in the Test Battery. + +*For more information on the behaviour of each parameter refer to the [Behaviour of Parameters](#behaviour-of-parameters) section of this document* + +## Editing an entry to the Test Battery +Under the `Browse` tab, you can edit an entry by clicking on the `Edit` link in the `Edit Metadata` column of the Menu Table. +The link will display a form that is populated with the values of the entry. +You can update information in the form by selecting from the dropdown menus and filling in the numeric text fields. +You will have to fill out the required fields `Instrument`, `Minimum age (days)`, `Maximum age (days)`, `Stage`, and `Active`. +Finally, press the **Edit entry** button to edit the entry in the Test Battery. +You cannot edit an entry if you make no changes in the form. +You cannot edit an entry if it becomes the same as another active entry in the Test Battery. + +*For more information on the behaviour of each parameter refer to the [Behaviour of Parameters](#behaviour-of-parameters) section of this document* + +## Activating/Deactivating an entry in the Test Battery + +### Browse tab (activate/deactivate) +In the `Change Status` column of the Menu Table, you press the **Activate** or +**Deactivate** button to directly change the status of an entry. + +### Add tab (activate) +Under the `Add` tab, you can add an entry that already exists in the Test +Battery but has been deactivated. A pop up will appear that will give you +the option to activate the existing entry. + +### Edit window (activate/deactivate) +Select an entry in the Menu table and click on `Edit`. +In the `Edit` window, edit an entry and make sure the new entry has no duplicate in the Test Battery. +This will add the new entry to the table and deactivate the original one. +Alternatively, edit an entry so that it becomes the same as another deactivated entry in the Test Battery. +A pop up will appear that will give you the option to activate the other entry and deactivate the original one. + +## Behaviour of Parameters + +### Subproject: + - If the test battery entry does NOT have a `subprojectID` + (`subprojectID=NULL`), the instrument gets administered to ALL subprojects. + - If the test battery entry has a `subprojectID` set and the `subprojectID` + matches the one of the timepoint, the instrument is administered only to + that subproject. + +### Stage: + - If the test battery entry has a `stage` set and the `stage` matches the + one of the timepoint, the instrument is administered at that stage. + +### Center: + - If the test battery entry does NOT have a `CenterID` (`CenterID=NULL`), + the instrument gets administered at ALL centers. + - If the test battery entry has a `CenterID` set and the `CenterID` matches + the one of the timepoint, the instrument is administered at that CenterID. + +### AgeMinDays/AgeMaxDays: + - If the test battery entry has `AgeMinDays` and `AgeMaxDays` set and they + are both set to `0`, the instrument gets administered at ALL ages; + - If the test battery entry has `AgeMinDays` and `AgeMaxDays` set and they + are set to any value other than `0`, the instrument gets administered IF AND + ONLY IF the age of the candidate at the timepoint is between `AgeMinDays` + and `AgeMaxDays`; + +### VisitLabel: + - If the test battery entry does NOT have a `Visit_label` + (`Visit_label=NULL`), the instrument gets administered IF AND ONLY IF no + other test battery entries matches the timepoint's visit label and + subproject. + - If the test battery entry has a `Visit_label` set and the `Visit_label` + matches the one of the timepoint, the instrument is administered at that + Visit. + - **NOTE:** *In order to administer an instrument at all visits without + defining each visit individually in the battery, the test battery table + should NOT contain any entries for the subproject/visit_label combination + of the timepoint. If the timepoint's subproject/visit_label combination + has a specified set of instruments defined in the test_battery, all entries + of the battery with no visit labels (`Visit_label=NULL`) will be ignored.* + + ### FirstVisit: + - If `firstVisit` is set to `Y`, the test battery instance is applied + only if it is the first visit. + - If `firstVisit` is to `N`, the test battery instance is applied only if it + *not* the first visit. + - If `firstVisit` is set to null, the test battery instance is applied to any + visit. + - **NOTE:** *The `firstVisit` flag can allow to bypass all other rules in + some instances. In these instances, the `stage`, `CenterID`, `Visit_label`, + `AgeMinDays` and `AgeMaxDays` will not affect in any way the administration + of the instrument. Only the `subprojectID` value will impact if the + instrument gets administered or not; the `subprojectID` value must match + the timepoint's in order for the instrument to be administered in these + instances.* diff --git a/modules/battery_manager/jsx/batteryManagerForm.js b/modules/battery_manager/jsx/batteryManagerForm.js new file mode 100644 index 00000000000..444821d5f38 --- /dev/null +++ b/modules/battery_manager/jsx/batteryManagerForm.js @@ -0,0 +1,143 @@ +import React, {Component} from 'react'; +import PropTypes from 'prop-types'; + +/** + * Battery Manager Form + * + * Module component rendering Add tab + * + * @author Victoria Foing + * + */ +class BatteryManagerForm extends Component { + /** + * Render function + * + * @return {*} + */ + render() { + const {test, options, setTest, add} = this.props; + + // Inform users about duplicate entries + const renderHelpText = () => { + if (add) { + return ( + + You cannot add an entry if it has a duplicate entry in the test + battery.
+ If the duplicate entry is inactive, you will be given the option + to activate it. + + ); + } else { + return ( + + Editing an entry will deactivate the current entry and create a + new entry.
+ You cannot edit an entry to have the same values as another active + entry.
+ If the duplicate entry is inactive, you will be given the option + to active it. +
+
+ + ); + } + }; + + return ( + + + + + + + + + + + + + ); + } +} + +BatteryManagerForm.propTypes = { + test: PropTypes.object.isRequired, + setTest: PropTypes.func.isRequired, + options: PropTypes.object.isRequired, +}; + +export default BatteryManagerForm; diff --git a/modules/battery_manager/jsx/batteryManagerIndex.js b/modules/battery_manager/jsx/batteryManagerIndex.js new file mode 100644 index 00000000000..90b137aac49 --- /dev/null +++ b/modules/battery_manager/jsx/batteryManagerIndex.js @@ -0,0 +1,496 @@ +import React, {Component} from 'react'; +import PropTypes from 'prop-types'; + +import Loader from 'Loader'; +import FilterableDataTable from 'FilterableDataTable'; +import Modal from 'Modal'; +import swal from 'sweetalert2'; + +import BatteryManagerForm from './batteryManagerForm'; + +/** + * Battery Manager + * + * Main module component rendering tab pane with Browse and Add tabs + * + * @author Victoria Foing + * @author Henri Rabalais + */ +class BatteryManagerIndex extends Component { + /** + * Constructor + * + * @param {object} props + */ + constructor(props) { + super(props); + + this.state = { + tests: {}, + test: {}, + show: {testForm: false, editForm: false}, + error: false, + isLoaded: false, + }; + + this.fetchData = this.fetchData.bind(this); + this.postData = this.postData.bind(this); + this.formatColumn = this.formatColumn.bind(this); + this.addTest = this.addTest.bind(this); + this.setTest = this.setTest.bind(this); + this.clearTest = this.clearTest.bind(this); + this.closeForm = this.closeForm.bind(this); + this.activateTest = this.activateTest.bind(this); + this.deactivateTest = this.deactivateTest.bind(this); + this.updateTest = this.updateTest.bind(this); + } + + /** + * Component did mount lifecycle method. + */ + componentDidMount() { + this.fetchData(this.props.testEndpoint, 'GET', 'tests') + .then(() => this.fetchData(this.props.optionEndpoint, 'GET', 'options')) + .then(() => this.setState({isLoaded: true})); + } + + /** + * Retrieve data from the provided URL and store it in state + * + * @param {string} url + * @param {string} method + * @param {string} state + * + * @return {object} promise + * + * XXX: This should eventually be moved to a library function + */ + fetchData(url, method, state) { + return new Promise((resolve, reject) => { + return fetch(url, {credentials: 'same-origin', method: method}) + .then((resp) => resp.json()) + .then((data) => this.setState({[state]: data}, resolve())) + .catch((error) => { + this.setState({error: true}, reject()); + console.error(error); + }); + }); + } + + /** + * Posts data from the provided URL to the server. + * + * @param {string} url + * @param {object} data + * @param {string} method + * + * @return {object} promise + * + * XXX: This should eventually be moved to a library function + */ + postData(url, data, method) { + return new Promise((resolve, reject) => { + const dataClone = JSON.parse(JSON.stringify(data)); + return fetch(url, { + credentials: 'same-origin', + method: method, + body: JSON.stringify(dataClone), + }) + .then((response) => response.text() + .then((body) => { + body = JSON.parse(body); + const status = response.ok ? 'success' : 'error'; + const message = response.ok ? body.message : body.error; + swal.fire(message, '', status); + response.ok ? resolve() : reject(); + }) + .catch(() => reject())); + }); + } + + /** + * Modify behaviour of specified column cells in the Data Table component + * + * @param {string} column - column name + * @param {string} cell - cell content + * @param {object} row - row content indexed by column + * + * @return {*} a formated table cell for a given column + */ + formatColumn(column, cell, row) { + let result = {cell}; + const testId = row['ID']; + switch (column) { + case 'Instrument': + result = {this.state.options.instruments[cell]}; + break; + case 'Subproject': + result = {this.state.options.subprojects[cell]}; + break; + case 'Site': + result = {this.state.options.sites[cell]}; + break; + case 'Change Status': + if (row.Active === 'Y') { + // Pass ID of row to deactivate function + result = { + this.deactivateTest(testId); + }}/>; + } else if (row.Active === 'N') { + // Pass ID of row to activate function + result = { + this.activateTest(testId); + }}/>; + } + break; + case 'Edit Metadata': + const editButton = { + this.loadTest(testId); + this.show('editForm'); + }}/>; + result = {editButton}; + break; + } + + return result; + } + + /** + * Show a given form based on the passed 'state' + * + * @param {string} item - the item to be shown + */ + show(item) { + let show = this.state.show; + show[item] = true; + this.setState({show}); + } + + /** + * Hige a given form based on the passed 'state' + * + * @param {string} item - the item to be hidden + */ + hide(item) { + let show = this.state.show; + show[item] = false; + this.setState({show}); + } + + /** + * Set the form data based on state values of child elements/components + * + * @param {string} name - name of the selected element + * @param {string} value - selected value for corresponding form element + */ + setTest(name, value) { + const test = this.state.test; + test[name] = value; + this.setState({test}); + } + + /** + * Loads a test into the current state based on the testId + * + * @param {string} testId + */ + loadTest(testId) { + const test = JSON.parse(JSON.stringify(this.state.tests + .find((test) => test.id === testId))); + this.setState({test}); + } + + /** + * Clear the state of the current test + */ + clearTest() { + const test = {}; + this.setState({test}); + } + + /** + * Close the Form + */ + closeForm() { + this.hide('testForm'); + this.hide('editForm'); + this.clearTest(); + } + + /** + * Display popup so user can confirm activation of row + * Refresh page if entry in Test Battery is successfully activated + * + * @param {int} testId + * + * @return {object} + */ + activateTest(testId) { + return new Promise((resolve, reject) => { + const test = this.state.tests.find((test) => test.id === testId); + test.active = 'Y'; + this.updateTest(test) + .then(() => resolve()) + .then(() => reject()); + }); + } + + /** + * Display popup so user can confirm deactivation of row + * Refresh page if entry in Test Battery is successfully deactivated + * + * @param {int} testId + * + * @return {object} + */ + deactivateTest(testId) { + return new Promise((resolve, reject) => { + const test = this.state.tests.find((test) => test.id === testId); + test.active = 'N'; + this.updateTest(test) + .then(() => resolve()) + .catch(() => reject()); + }); + } + + /** + * Updates a previously existing Test with an updated Test. + * + * @param {object} test + * + * @return {object} promise + */ + updateTest(test) { + return new Promise((resolve, reject) => { + this.postData(this.props.testEndpoint+test.id, test, 'PATCH') + .then(() => { + const index = this.state.tests + .findIndex((element) => element.id === test.id); + const tests = this.state.tests; + tests[index] = test; + this.setState({tests}, resolve()); + }) + .catch(() => reject()); + }); + } + + /** + * save test to database + * + * @return {object} promise + */ + addTest() { + return new Promise((resolve, reject) => { + const test = this.state.test; + this.checkDuplicate(test) + .then(() => this.postData(this.props.testEndpoint, test, 'POST')) + .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) + .then(() => test.id && this.deactivateTest(test.id)) + .then(() => this.closeForm()) + .then(() => resolve()) + .catch(() => reject()); + }); + } + + /** + * Checks whether the Test is a duplicate of an existing Test. + * + * @param {object} test + * + * @return {object} promise + */ + checkDuplicate(test) { + return new Promise((resolve, reject) => { + let duplicate; + this.state.tests.forEach((testCheck) => { + if ( + test.testName === testCheck.testName && + test.ageMinDays === testCheck.ageMinDays && + test.ageMaxDays === testCheck.ageMaxDays && + test.stage === testCheck.stage && + test.subproject === testCheck.subproject && + test.visitLabel === testCheck.visitLabel && + test.centerId === testCheck.centerId && + test.firstVisit === testCheck.firstVisit && + test.instrumentOrder === testCheck.instrumentOrder + ) { + duplicate = testCheck; + } + }); + + if (duplicate) { + if (duplicate.active === 'N') { + swal.fire({ + title: 'Test Duplicate', + text: 'Would you to like activate this test?', + type: 'warning', + confirmButtonText: 'Activate', + showCancelButton: true, + }).then((result) => { + if (result.value) { + this.activateTest(duplicate.id); + if (test.id && (test.id !== duplicate.id)) { + this.deactivateTest(test.id); + } + this.closeForm(); + } + }); + } else if (duplicate.active === 'Y') { + swal.fire( + 'Test Duplicate', 'You cannot duplicate an active test', 'error' + ); + } + reject(); + } else { + resolve(); + } + }); + } + + /** + * Render Method + * + * @return {*} + */ + render() { + // If error occurs, return a message. + // XXX: Replace this with a UI component for 500 errors. + if (this.state.error) { + return

An error occured while loading the page.

; + } + + // Waiting for async data to load + if (!this.state.isLoaded) { + return ; + } + + /** + * XXX: Currently, the order of these fields MUST match the order of the + * queried columns in _setupVariables() in batter_manager.class.inc + */ + const {options, test, tests, show} = this.state; + const {hasPermission} = this.props; + const fields = [ + {label: 'ID', show: false}, + {label: 'Instrument', show: true, filter: { + name: 'testName', + type: 'select', + options: options.instruments, + }}, + {label: 'Minimum Age', show: true, filter: { + name: 'minimumAge', + type: 'text', + }}, + {label: 'Maximum Age', show: true, filter: { + name: 'maximumAge', + type: 'text', + }}, + {label: 'Stage', show: true, filter: { + name: 'stage', + type: 'select', + options: options.stages, + }}, + {label: 'Subproject', show: true, filter: { + name: 'subproject', + type: 'select', + options: options.subprojects, + }}, + {label: 'Visit Label', show: true, filter: { + name: 'visitLabel', + type: 'select', + options: options.visits, + }}, + {label: 'Site', show: true, filter: { + name: 'site', + type: 'select', + options: options.sites, + }}, + {label: 'First Visit', show: true, filter: { + name: 'firstVisit', + type: 'select', + options: options.firstVisit, + }}, + {label: 'Instrument Order', show: true, filter: { + name: 'instrumentOrder', + type: 'text', + }}, + {label: 'Active', show: true, filter: { + name: 'active', + type: 'select', + options: options.active, + }}, + {label: 'Change Status', show: hasPermission('batter_manager_edit')}, + {label: 'Edit Metadata', show: hasPermission('batter_manager_edit')}, + ]; + + const testForm = ( + + + + ); + + const actions = [ + { + label: 'New Test', + action: () => this.show('testForm'), + show: hasPermission('battery_manager_edit'), + }, + ]; + + const testsArray = tests.map((test) => { + return [ + test.id, + test.testName, + test.ageMinDays, + test.ageMaxDays, + test.stage, + test.subproject, + test.visitLabel, + test.centerId, + test.firstVisit, + test.instrumentOrder, + test.active, + ]; + }); + + return ( +
+ {testForm} + +
+ ); + } +} + +BatteryManagerIndex.propTypes = { + dataURL: PropTypes.string.isRequired, + hasPermission: PropTypes.func.isRequired, +}; + +window.addEventListener('load', () => { + ReactDOM.render( + , + document.getElementById('lorisworkspace') + ); +}); diff --git a/modules/battery_manager/php/battery_manager.class.inc b/modules/battery_manager/php/battery_manager.class.inc new file mode 100644 index 00000000000..093c228aca2 --- /dev/null +++ b/modules/battery_manager/php/battery_manager.class.inc @@ -0,0 +1,91 @@ + + * @author Henri Rabalais + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +namespace LORIS\battery_manager; +use \Psr\Http\Message\ServerRequestInterface; +use \Psr\Http\Message\ResponseInterface; + +/** + * Main class for battery manager module corresponding to /battery_manager/ URL + * Admin section of the LorisMenu. + * + * Displays a list of records in the test battery and control panel to search them + * Allows user to add, activate, and deactivate entries in the test battery + * + * PHP Version 7 + * + * @category Module + * @package Battery_Manager + * @author Victoria Foing + * @author Henri Rabalais + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +class Battery_Manager extends \NDB_Page +{ + public $skipTemplate = true; + + /** + * Returns true if user has access to this page. + * + * @param \User $user The user whose access is being checked + * + * @return bool + */ + function _hasAccess(\User $user) : bool + { + return $user->hasPermission('battery_manager_view') || + $user->hasPermission('battery_manager_edit'); + } + + /** + * This acts as an Ajax enpoint that handles all action requests from the + * Battery Manager Module. + * + * @param ServerRequestInterface $request The incoming PSR7 request + * + * @return ResponseInterface The outgoing PSR7 response + */ + public function handle(ServerRequestInterface $request) : ResponseInterface + { + $resp = parent::handle($request); + switch ($resp->getStatusCode()) { + case 200: + break; + default: + return $resp; + } + $this->setup(); + return (new \LORIS\Http\Response()) + ->withBody(new \LORIS\Http\StringStream($this->display())); + } + + /** + * Include additional JS files. + * + * @return array of javascript to be inserted + */ + public function getJSDependencies() : array + { + $factory = \NDB_Factory::singleton(); + $baseURL = $factory->settings()->getBaseURL(); + $deps = parent::getJSDependencies(); + return array_merge( + $deps, + array( + $baseURL . "/battery_manager/js/batteryManagerIndex.js", + ) + ); + } +} + diff --git a/modules/battery_manager/php/module.class.inc b/modules/battery_manager/php/module.class.inc new file mode 100644 index 00000000000..ba6ae6a8f72 --- /dev/null +++ b/modules/battery_manager/php/module.class.inc @@ -0,0 +1,66 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris-Trunk/ + */ +namespace LORIS\battery_manager; + +/** + * Class module implements the basic LORIS module functionality + * + * @category Behavioural + * @package Main + * @subpackage Battery_Manager + * @author Victoria Foing + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris-Trunk/ + */ +class Module extends \Module +{ + /** + * {@inheritDoc} + * + * @param \User $user The user whose access is being checked. + * + * @return bool whether access is granted + */ + public function hasAccess(\User $user) : bool + { + return parent::hasAccess($user) && + $user->hasAnyPermission( + [ + 'battery_manager_view', + 'battery_manager_edit' + ] + ); + } + + /** + * {@inheritDoc} + * + * @return string The menu category for this module + */ + public function getMenuCategory() : string + { + return "Tools"; + } + + /** + * {@inheritDoc} + * + * @return string The human readable name for this module + */ + public function getLongName() : string + { + return "Battery Manager"; + } +} diff --git a/modules/battery_manager/php/test.class.inc b/modules/battery_manager/php/test.class.inc new file mode 100644 index 00000000000..245eb483794 --- /dev/null +++ b/modules/battery_manager/php/test.class.inc @@ -0,0 +1,84 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ + +namespace LORIS\battery_manager; + +/** + * A Test represents a row in the Battery Manager menu table. + * + * @category Behavioural + * @package Main + * @subpackage Imaging + * @author Henri Rabalais + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +class Test implements \LORIS\Data\DataInstance +{ + public $row; + + /** + * Create a new Test Instance. + * + * @param array $row The row (in the same format as \Database::pselectRow returns + * returns + */ + public function __construct(array $row) + { + $this->row = $row; + } + + /** + * Implements \LORIS\Data\DataInstance interface for this row. + * + * @return string the row data. + */ + public function toJSON() : string + { + return json_encode($this->row); + } + + /** + * Returns the CenterID for this row, for filters such as + * \LORIS\Data\Filters\UserSiteMatch to match again. + * + * @return integer The CenterID + */ + public function getCenterID() : int + { + return $this->row['centerId']; + } + + /** + * Convert data from Instance to a format suitable for SQL. + * + * @return array + */ + public function toSQL() : array + { + return array( + 'Test_name' => $this->row['testName'] ?? null, + 'AgeMinDays' => $this->row['ageMinDays'] ?? null, + 'AgeMaxDays' => $this->row['ageMaxDays'] ?? null, + 'Stage' => $this->row['stage'] ?? null, + 'SubprojectID' => $this->row['subproject'] ?? null, + 'Visit_label' => $this->row['visitLabel'] ?? null, + 'CenterID' => $this->row['centerId'] ?? null, + 'firstVisit' => $this->row['firstVisit'] ?? null, + 'instr_order' => $this->row['instrumentOrder'] ?? null, + 'Active' => $this->row['active'] ?? null, + ); + } +} diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc new file mode 100644 index 00000000000..5fd5e5635d6 --- /dev/null +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -0,0 +1,262 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +namespace LORIS\battery_manager; +use \Psr\Http\Message\ServerRequestInterface; +use \Psr\Http\Server\RequestHandlerInterface; +use \Psr\Http\Message\ResponseInterface; + +/** + * Main class for managing Test Instances + * + * Handles requests for retrieving and saving Test Instances. + * Allows users to add, activate, and deactivate entries in the test battery. + * + * PHP Version 7 + * + * @category Module + * @package Battery_Manager + * @author Henri Rabalais + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +class TestEndpoint implements RequestHandlerInterface +{ + + /** + * Returns true if user has access to this endpoint. + * + * @param \User $user The user whose access is being checked + * + * @return bool + */ + function _hasAccess(\User $user) : bool + { + return true; + } + + /** + * This function passes the request to the handler. This is necessary since + * the Endpoint bypass the Module class. + * + * XXX: This function should be extracted to a parent class. + * + * @param ServerRequestInterface $request The PSR7 request. + * @param RequestHandlerInterface $handler The request handler. + * + * @return ResponseInterface The outgoing PSR7 response. + */ + public function process( + ServerRequestInterface $request, + RequestHandlerInterface $handler + ) : ResponseInterface { + return $handler->handle($request); + } + + /** + * This acts as an Ajax enpoint that handles all action requests from the + * Battery Manager Module. + * + * @param ServerRequestInterface $request The incoming PSR7 request + * + * @return ResponseInterface The outgoing PSR7 response + */ + public function handle(ServerRequestInterface $request) : ResponseInterface + { + $this->db = \Database::singleton(); + $this->user = $request->getAttribute('user'); + $method = $request->getMethod(); + + switch($method) { + case 'GET': + return $this->_getInstances(); + case 'POST': + return $this->_postInstance($request); + case 'PATCH': + return $this->_patchInstance($request); + } + } + + /** + * Gets the data source for battery manager tests. + * + * @return \LORIS\Data\Provisioner + */ + private function _getDataProvisioner() : \LORIS\Data\Provisioner + { + $provisioner = new TestProvisioner(); + + if ($this->user->hasPermission('access_all_profiles') == false) { + $provisioner = $provisioner->filter( + new \LORIS\Data\Filters\UserSiteMatch() + ); + } + return $provisioner; + } + + /** + * Updates the status of the given test from Active to Deactive or + * viceversa. + * + * @param ServerRequestInterface $request Test to be saved + * + * @return ResponseInterface response + */ + private function _patchInstance(ServerRequestInterface $request) + { + $testArray = json_decode($request->getBody()->getContents(), true); + $test = new Test($testArray); + + // validate instance; + $errors = $this->_validateInstance($test); + if (!empty($errors)) { + return new \LORIS\Http\Response\JSON\BadRequest( + implode(' ', $errors) + ); + } + + $testArray = $test->toSQL(); + try { + $this->db->update( + 'test_battery', + $testArray, + array('ID' => $testArray['ID']) + ); + return new \LORIS\Http\Response\JSON\OK( + ['message'=>'Availability Updated Successfully'] + ); + } catch (\DatabaseException $e) { + return new \LORIS\Http\Response\JSON\InternalServerError( + 'Could not update entry in the Test Battery' + ); + } + } + + /** + * Adds a new test to the test_battery in the database. + * + * @param ServerRequestInterface $request Test to be posted + * + * @return ResponseInterface response + */ + private function _postInstance(ServerRequestInterface $request) + { + $testArray = json_decode($request->getBody()->getContents(), true); + $test = new Test($testArray); + + if (!$this->user->hasPermission('battery_manager_edit')) { + return new \LORIS\Http\Response\JSON\Forbidden('Edit Permission Denied'); + } + + // validate instance + $errors = $this->_validateInstance($test); + if (!empty($errors)) { + return new \LORIS\Http\Response\JSON\BadRequest( + implode(' ', $errors) + ); + } + + // check if instance is duplicate + if ($this->_isDuplicate($test)) { + return new \LORIS\Http\Response\JSON\Conflict( + 'This Test already exists in the database' + ); + } + + $test->row['active'] = 'Y'; + $testArray = $test->toSQL(); + + try { + $this->db->insert('test_battery', $testArray); + return new \LORIS\Http\Response\JSON\OK( + ['message'=>'Test Submission Successful'] + ); + } catch (\DatabaseException $e) { + return new \LORIS\Http\Response\JSON\InternalServerError( + 'Could not add entry to the Test Battery' + ); + } + } + + /** + * Checks if the entry is an exact duplicate of a previous entry. + * + * @param Test $test Test to be checked. + * + * @return bool + */ + private function _isDuplicate(Test $test) : bool + { + // Build SQL query based on values entered by user + $query = "SELECT Test_name as testName, + AgeMinDays as ageMinDays, + AgeMaxDays as ageMaxDays, + Stage as stage, + SubprojectID as subproject, + Visit_label as visitLabel, + CenterID as centerId, + firstVisit, + instr_order as instrumentOrder + FROM test_battery"; + // Select duplicate entry from Test Battery + $entries = $this->db->pselect($query, array()); + + foreach ($entries as $entry) { + if ($test->row === $entry) { + return true; + } + } + + return false; + } + + /** + * Converts the results of this menu filter to a JSON format. + * + * @return ResponseInterface The outgoing PSR7 with a string of json + * encoded tests as the body. + */ + private function _getInstances() : ResponseInterface + { + $instances = (new \LORIS\Data\Table()) + ->withDataFrom($this->_getDataProvisioner($this->user)) + ->toArray($this->user); + return new \LORIS\Http\Response\JSON\OK($instances); + } + + /** + * Validates the Test Instance and collects in errors in an array. + * + * @param Test $test The Test instance to be validated + * + * @return array $errors An array string errors. + */ + private function _validateInstance(Test $test) : array + { + $errors = []; + if (!isset($test->row['testName'])) { + $errors[] = 'Test Name is a required field.'; + } + if (!isset($test->row['ageMinDays'])) { + $errors[] = 'Minimum age is a required field.'; + } + if (!isset($test->row['ageMaxDays'])) { + $errors[] = 'Maximum age is a required field.'; + } + if (!isset($test->row['stage'])) { + $errors[] = 'Stage is a required field.'; + } + + return $errors; + } +} + diff --git a/modules/battery_manager/php/testoptionsendpoint.class.inc b/modules/battery_manager/php/testoptionsendpoint.class.inc new file mode 100644 index 00000000000..aa14c41f76e --- /dev/null +++ b/modules/battery_manager/php/testoptionsendpoint.class.inc @@ -0,0 +1,102 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +namespace LORIS\battery_manager; +use \Psr\Http\Message\ServerRequestInterface; +use \Psr\Http\Message\ResponseInterface; + +/** + * Main class for battery manager module corresponding to /battery_manager/ URL + * Admin section of the LorisMenu. + * + * Displays a list of records in the test battery and control panel to search them + * Allows user to add, activate, and deactivate entries in the test battery + * + * PHP Version 7 + * + * @category Module + * @package Battery_Manager + * @author Henri Rabalais + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +class TestOptionsEndpoint extends \NDB_Page +{ + /** + * This acts as an Ajax enpoint that handles all action requests from the + * Battery Manager Module. + * + * @param ServerRequestInterface $request The incoming PSR7 request + * + * @return ResponseInterface The outgoing PSR7 response + */ + public function handle(ServerRequestInterface $request) : ResponseInterface + { + $method = $request->getMethod(); + + switch($method) { + case 'GET': + return new \LORIS\Http\Response\JSON\OK($this->_getOptions()); + } + + return new \LORIS\Http\Response\JSON\BadRequest('Unspecified Request'); + } + + /** + * Gets the field options for this module. + * + * @return array + */ + private function _getOptions() : array + { + return array( + 'instruments' => \Utility::getAllInstruments(), + 'stages' => $this->_getStageList(), + 'subprojects' => \Utility::getSubprojectList(null), + 'visits' => \Utility::getVisitList(), + 'sites' => \Utility::getSiteList(false), + 'firstVisits' => $this->_getYesNoList(), + 'active' => $this->_getYesNoList(), + ); + } + + /** + * Return associative array of stages. + * + * @return array + */ + private function _getStageList() : array + { + return array( + "Not Started" => 'Not Started', + "Screening" => 'Screening', + "Visit" => 'Visit', + "Approval" => "Approval", + "Subject" => "Subject", + "Recycling Bin" => "Recycling Bin", + ); + } + + /** + * Return associative array of yes and no values. + * + * @return array + */ + private function _getYesNoList() : array + { + return array( + 'Y' => 'Yes', + 'N' => 'No', + ); + } +} + diff --git a/modules/battery_manager/php/testprovisioner.class.inc b/modules/battery_manager/php/testprovisioner.class.inc new file mode 100644 index 00000000000..3dd6ad2ce2e --- /dev/null +++ b/modules/battery_manager/php/testprovisioner.class.inc @@ -0,0 +1,70 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ + +namespace LORIS\battery_manager; + +/** + * This class implements a data provisioner to get all possible tests + * for the battery_manager menu page. + * + * PHP Version 7 + * + * @category Behavioural + * @package Main + * @subpackage Imaging + * @author Henri Rabalais + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +class TestProvisioner extends \LORIS\Data\Provisioners\DBRowProvisioner +{ + /** + * Create a Test Instance, which get rows fro mthe test_battery table. + */ + public function __construct() + { + parent::__construct( + "SELECT b.id, + t.Test_name as testName, + b.AgeMinDays as ageMinDays, + b.AgeMaxDays as ageMaxDays, + b.Stage as stage, + s.SubprojectID as subproject, + b.Visit_label as visitLabel, + p.CenterID as centerId, + b.firstVisit, + b.instr_order as instrumentOrder, + b.Active as active + FROM test_battery b + JOIN test_names t USING (Test_name) + LEFT JOIN subproject s USING (SubprojectID) + LEFT JOIN psc p USING (CenterID) + ORDER BY t.Test_name", + array() + ); + } + + /** + * Returns an instance of a Test object for a given table row. + * + * @param array $row The database row from the LORIS Database class. + * + * @return \LORIS\Data\DataInstance An instance representing a test. + */ + public function getInstance($row) : \LORIS\Data\DataInstance + { + return new Test($row); + } +} diff --git a/modules/battery_manager/test/BatteryManagerTest.php b/modules/battery_manager/test/BatteryManagerTest.php new file mode 100644 index 00000000000..3b8210f64f7 --- /dev/null +++ b/modules/battery_manager/test/BatteryManagerTest.php @@ -0,0 +1,64 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://github.com/aces/Loris + */ + +require_once __DIR__ . + "/../../../test/integrationtests/LorisIntegrationTest.class.inc"; + +/** + * Battery Manager module automated integration tests + * + * PHP Version 5 + * + * @category Test + * @package Loris + * @author Wang Shen + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://github.com/aces/Loris + */ +class BatteryManagerTest extends LorisIntegrationTest +{ + + /** + * Tests that the page does not load if the user does not have correct + * permissions + * + * @return void + */ + function testLoadsWithPermissionRead() + { + $this->setupPermissions(array("battery_manager_view")); + $this->safeGet($this->url . "/battery_manager/"); + $bodyText = $this->webDriver->findElement( + WebDriverBy::cssSelector("body") + )->getText(); + $this->assertNotContains("You do not have access to this page.", $bodyText); + $this->resetPermissions(); + } + /** + * Tests that the page does not load if the user does not have correct + * permissions + * + * @return void + */ + function testDoesNotLoadWithoutPermission() + { + $this->setupPermissions(array()); + $this->safeGet($this->url . "/battery_manager/"); + $bodyText = $this->webDriver->findElement( + WebDriverBy::cssSelector("body") + )->getText(); + $this->assertContains("You do not have access to this page.", $bodyText); + $this->resetPermissions(); + } + +} diff --git a/modules/battery_manager/test/TestPlan.md b/modules/battery_manager/test/TestPlan.md new file mode 100644 index 00000000000..aa694fa997d --- /dev/null +++ b/modules/battery_manager/test/TestPlan.md @@ -0,0 +1,104 @@ +# Battery Manager Module Test Plan + +## Overview + +Battery Manager module allows users to search for, add, edit, activate, and deactivate entries in the +Test Battery. + +## Features + +1. **Browse** entries in the Test Battery. +2. **Add** new entry to the `test_battery` table. +3. **Edit** entry in the `test_battery` table. +4. **Activate** entry in the `test_battery` table. +5. **Deactivate** entry in the `test_battery` table. + +--- + +## Testing Procedure + +### Permissions + +**Testing with no permissions** [Automation Testing] + 1. Access the module with a regular user (without superuser permissions). + 2. By default, the access to module should be denied. + +**Testing with view permission** [Automation Testing] + 1. Add view permission to the aforementioned user. + 2. Battery Manager module should be accessible and only present with **one** tab (Browse). + 3. The **Change Status** column should not be in the data table. + 4. The **Edit Metadata** column should not be in the data table. + +**Testing with edit permission** [Automation Testing] + 1. Add edit permission. + 2. Battery Manager module should now have **two** tabs (Browse) and (Add). + 3. The **Change Status** column should be in the data table. + 4. The **Edit Metadata** column should be in the data table. + 5. Clicking on Add tab should hide the data table and display a form with the following fields: + `Instrument`, `Minimum age (days)`, `Maximum age(days)`, `Stage`, `Subproject`, `Visit Label`, `Site`, `First Visit`, + and `Instrument Order`. + 6. Clicking on Edit in **Edit Metadata** takes you to a new page with a form with the following fields: + `Instrument`, `Minimum age (days)`, `Maximum age(days)`, `Stage`, `Subproject`, `Visit Label`, `Site`, `First Visit`, + and `Instrument Order`. + +### Add tab + +**Testing add functionality** + 1. Check that you cannot add an entry without filling out the required fields: `Instrument`, `Minimum age (days)`, `Maximum age (days)`, `Stage`. + 2. Check that you can only enter a site that exists. + 3. Check that you can only enter numbers between 0 and 99999 in Minumum age (days) and Maximum age (days). + 4. Check that you can only enter numbers between 0 and 127 in Instrument order. + 5. Check that when you try to add an entry that has an active duplicate in the table (Active = 'Y'), you receive an error message. + 6. Try to add an entry that does not have a duplicate. + - Ensure that a success message appears and the page goes back to the Browse tab. + - Ensure the entry you just added is shown in data table. + +**Testing activate functionality** + 1. Try to add an entry that has an inactive duplicate in the table (Active = 'N'). + - Ensure that you receive a warning message that allows you to activate the duplicate. + - Ensure that when you press "Yes", a success message appears and the page goes back to the Browse tab. + - Ensure the entry you just activated is activated in the data table. + +### Browse tab + +**Testing data table** + 1. After a couple of entries are added, ensure they are properly displayed in the data table. + 2. Ensure that information in the data table corresponds to the information in the `test_battery` table. + 3. Click on **column headers** to ensure sorting functionality is working as expected (Ascending/Descending). + +**Testing Change Status column** + 1. Press the `Deactivate` button in the `Change Status` column on an entry in the data table. + - Ensure that a warning message appears that asks you to confirm the action. + - Ensure that when you press "Yes", a success message appears and the page refreshes. + - Ensure the entry has the new Active status in the data table. + 2. Repeat step 1 using the `Activate` button. + +**Testing Edit Metadata column** + 1. Press the `Edit` link in the `Edit Metadata` column on an entry in the data table to edit. + - Ensure that you are taken to an Edit page with a form that is populated with the entry's values. + +**Test filters** + 1. Under **Browse** tab, a selection filter should be present on top of the page containing the following fields: + - Minimum age, Maximum age, and Instrument Order (as text fields). + - Instrument, Stage, Subproject, Visit Label, Site, First Visit, Instrument Order, and Active (as dropdown fields with blank default option). + 2. Type text in the Minimum age and verify that the table gets filtered as you type. + 3. Type text in the Maximum age and verify that the table gets filtered as you type. + 4. Select values from the dropdown filters (independently and combined) to filter table further. + - The table should update and display filtered records accordingly. + +### Edit window + +**Testing edit (activate/deactivate/add) functionality** + 1. Check that you cannot edit an entry without filling out the required fields: `Instrument`, `Minimum age (days)`, `Maximum age (days)`, `Stage`. + 2. Check that you can only enter a site that exists. + 3. Check that you can only enter numbers between 0 and 99999 in Minumum age (days) and Maximum age (days). + 4. Check that you can only enter numbers between 0 and 127 in Instrument order. + 5. Check that when you try to edit an entry without making changes to the form, you receive an error message. + 6. Check that when the edited entry has the same values as another active entry in the Test Battery, you receive an error message. + 7. Try to edit an entry so that it has the same values as another deactivated entry in the Test Battery. + - Ensure that a warning message appears giving the option to activate the other entry and deactivate the original entry. + - Ensure that when you press "Yes", a success message appears and the page goes back to the Browse tab. + - Ensure the original entry was deactivated and the other duplicate entry has been activated in the data table. + 8. Try to edit an entry so that it does not have a duplicate (i.e. itself or another entry). + - Ensure that a success message appears and the page goes back to the `Browse` tab. + - Ensure the original entry was deactivated and the new entry has been added to the data table. diff --git a/raisinbread/RB_files/RB_modules.sql b/raisinbread/RB_files/RB_modules.sql index aa529c88b62..a803b6326d3 100644 --- a/raisinbread/RB_files/RB_modules.sql +++ b/raisinbread/RB_files/RB_modules.sql @@ -39,5 +39,6 @@ INSERT INTO `modules` (`ID`, `Name`, `Active`) VALUES (35,'statistics','Y'); INSERT INTO `modules` (`ID`, `Name`, `Active`) VALUES (36,'survey_accounts','Y'); INSERT INTO `modules` (`ID`, `Name`, `Active`) VALUES (37,'timepoint_list','Y'); INSERT INTO `modules` (`ID`, `Name`, `Active`) VALUES (38,'user_accounts','Y'); +INSERT INTO `modules` (`ID`, `Name`, `Active`) VALUES (39,'battery_manager','Y'); UNLOCK TABLES; SET FOREIGN_KEY_CHECKS=1; diff --git a/raisinbread/RB_files/RB_permissions.sql b/raisinbread/RB_files/RB_permissions.sql index a182842e420..6dbc2bb6aba 100644 --- a/raisinbread/RB_files/RB_permissions.sql +++ b/raisinbread/RB_files/RB_permissions.sql @@ -57,5 +57,7 @@ INSERT INTO `permissions` (`permID`, `code`, `description`, `categoryID`) VALUES INSERT INTO `permissions` (`permID`, `code`, `description`, `categoryID`) VALUES (55,'publication_approve','Publication - Approve or reject proposed publication projects',2); INSERT INTO `permissions` (`permID`, `code`, `description`, `categoryID`) VALUES (56,'data_release_view','Data Release: View releases',2); INSERT INTO `permissions` (`permID`, `code`, `description`, `categoryID`) VALUES (57,'candidate_dob_edit','Edit dates of birth',2); +INSERT INTO `permissions` (`permID`, `code`, `description`, `categoryID`) VALUES (58,'battery_manager_view','View Battery Manager',2); +INSERT INTO `permissions` (`permID`, `code`, `description`, `categoryID`) VALUES (59,'battery_manager_edit','Add, activate, and deactivate entries in Test Battery',2); UNLOCK TABLES; SET FOREIGN_KEY_CHECKS=1; diff --git a/raisinbread/RB_files/RB_user_perm_rel.sql b/raisinbread/RB_files/RB_user_perm_rel.sql index 616c972b333..c2e3c0b8155 100644 --- a/raisinbread/RB_files/RB_user_perm_rel.sql +++ b/raisinbread/RB_files/RB_user_perm_rel.sql @@ -57,5 +57,7 @@ INSERT INTO `user_perm_rel` (`userID`, `permID`) VALUES (1,53); INSERT INTO `user_perm_rel` (`userID`, `permID`) VALUES (1,54); INSERT INTO `user_perm_rel` (`userID`, `permID`) VALUES (1,55); INSERT INTO `user_perm_rel` (`userID`, `permID`) VALUES (1,56); +INSERT INTO `user_perm_rel` (`userID`, `permID`) VALUES (1,58); +INSERT INTO `user_perm_rel` (`userID`, `permID`) VALUES (1,59); UNLOCK TABLES; SET FOREIGN_KEY_CHECKS=1; diff --git a/webpack.config.js b/webpack.config.js index fa00c58b532..e621887b538 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -18,6 +18,7 @@ const config = [{ './modules/configuration/js/SubprojectRelations.js': './modules/configuration/jsx/SubprojectRelations.js', './modules/conflict_resolver/js/conflictResolverIndex.js': './modules/conflict_resolver/jsx/conflictResolverIndex.js', './modules/conflict_resolver/js/resolvedConflictsIndex.js': './modules/conflict_resolver/jsx/resolvedConflictsIndex.js', + './modules/battery_manager/js/batteryManagerIndex.js': './modules/battery_manager/jsx/batteryManagerIndex.js', './modules/bvl_feedback/js/react.behavioural_feedback_panel.js': './modules/bvl_feedback/jsx/react.behavioural_feedback_panel.js', './modules/behavioural_qc/js/behavioural_qc_module.js': './modules/behavioural_qc/jsx/behavioural_qc_module.js', './modules/candidate_list/js/openProfileForm.js': './modules/candidate_list/jsx/openProfileForm.js', From 0a1f6963b8854de267adc98cd0d4f8baafe3e899 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Thu, 6 Feb 2020 17:29:59 -0500 Subject: [PATCH 02/14] addressed Rida's Review --- jsx/Form.js | 16 ++++-- .../battery_manager/jsx/batteryManagerForm.js | 10 +++- .../jsx/batteryManagerIndex.js | 57 ++++++++++++++++++- modules/battery_manager/php/test.class.inc | 1 + .../php/testendpoint.class.inc | 5 +- 5 files changed, 78 insertions(+), 11 deletions(-) diff --git a/jsx/Form.js b/jsx/Form.js index d67d464bc7f..88668d7fef6 100644 --- a/jsx/Form.js +++ b/jsx/Form.js @@ -1207,12 +1207,19 @@ class NumericElement extends Component { } render() { - let disabled = this.props.disabled ? 'disabled' : null; - let required = this.props.required ? 'required' : null; - let requiredHTML = null; + const {disabled, required} = this.props; + let requiredHTML = required ? * : null; + let errorMessage = null; + let elementClass = 'row form-group'; + + // Add error message + if (this.props.errorMessage) { + errorMessage = {this.props.errorMessage}; + elementClass = 'row form-group has-error'; + } return ( -
+
); diff --git a/modules/battery_manager/jsx/batteryManagerForm.js b/modules/battery_manager/jsx/batteryManagerForm.js index 444821d5f38..4521170cc8f 100644 --- a/modules/battery_manager/jsx/batteryManagerForm.js +++ b/modules/battery_manager/jsx/batteryManagerForm.js @@ -16,7 +16,7 @@ class BatteryManagerForm extends Component { * @return {*} */ render() { - const {test, options, setTest, add} = this.props; + const {test, options, setTest, add, errors} = this.props; // Inform users about duplicate entries const renderHelpText = () => { @@ -59,6 +59,8 @@ class BatteryManagerForm extends Component { onUserInput={setTest} required={true} value={test.testName} + errorMessage={errors.testName} + hasError={errors.testName} /> { const test = this.state.test; this.checkDuplicate(test) + .then(() => this.validateTest(test)) .then(() => this.postData(this.props.testEndpoint, test, 'POST')) .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) .then(() => test.id && this.deactivateTest(test.id)) @@ -311,8 +333,7 @@ class BatteryManagerIndex extends Component { test.subproject === testCheck.subproject && test.visitLabel === testCheck.visitLabel && test.centerId === testCheck.centerId && - test.firstVisit === testCheck.firstVisit && - test.instrumentOrder === testCheck.instrumentOrder + test.firstVisit === testCheck.firstVisit ) { duplicate = testCheck; } @@ -347,6 +368,34 @@ class BatteryManagerIndex extends Component { }); } + validateTest(test) { + return new Promise((resolve, reject) => { + console.log('validate'); + const errors = {}; + if (test.testName == null) { + errors.testName = 'This field is required'; + } + if (test.ageMinDays == null) { + errors.ageMinDays = 'This field is required'; + } + if (test.ageMaxDays == null) { + errors.ageMaxDays = 'This field is required'; + } + if (test.stage == null) { + errors.stage = 'This field is required'; + } + + console.log(errors); + if (Object.entries(errors).length === 0) { + console.log('resolve'); + resolve(); + } else { + console.log('reject'); + this.setState({errors}, reject()); + } + }); + } + /** * Render Method * @@ -368,7 +417,7 @@ class BatteryManagerIndex extends Component { * XXX: Currently, the order of these fields MUST match the order of the * queried columns in _setupVariables() in batter_manager.class.inc */ - const {options, test, tests, show} = this.state; + const {options, test, tests, show, errors} = this.state; const {hasPermission} = this.props; const fields = [ {label: 'ID', show: false}, @@ -436,6 +485,7 @@ class BatteryManagerIndex extends Component { setTest={this.setTest} options={options} add={show.testForm} + errors={errors} /> ); @@ -473,6 +523,7 @@ class BatteryManagerIndex extends Component { fields={fields} actions={actions} getFormattedCell={this.formatColumn} + getMappedCell={this.mapColumn} /> ); diff --git a/modules/battery_manager/php/test.class.inc b/modules/battery_manager/php/test.class.inc index 245eb483794..189153a37b2 100644 --- a/modules/battery_manager/php/test.class.inc +++ b/modules/battery_manager/php/test.class.inc @@ -69,6 +69,7 @@ class Test implements \LORIS\Data\DataInstance public function toSQL() : array { return array( + 'ID' => $this->row['id'] ?? null, 'Test_name' => $this->row['testName'] ?? null, 'AgeMinDays' => $this->row['ageMinDays'] ?? null, 'AgeMaxDays' => $this->row['ageMaxDays'] ?? null, diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc index 5fd5e5635d6..bfd9c49e851 100644 --- a/modules/battery_manager/php/testendpoint.class.inc +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -132,7 +132,7 @@ class TestEndpoint implements RequestHandlerInterface array('ID' => $testArray['ID']) ); return new \LORIS\Http\Response\JSON\OK( - ['message'=>'Availability Updated Successfully'] + ['message'=>'Status Updated Successfully'] ); } catch (\DatabaseException $e) { return new \LORIS\Http\Response\JSON\InternalServerError( @@ -204,8 +204,7 @@ class TestEndpoint implements RequestHandlerInterface SubprojectID as subproject, Visit_label as visitLabel, CenterID as centerId, - firstVisit, - instr_order as instrumentOrder + firstVisit FROM test_battery"; // Select duplicate entry from Test Battery $entries = $this->db->pselect($query, array()); From ee5f391161ecea1d3d13f83e49e63320e47da648 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Thu, 6 Feb 2020 18:48:21 -0500 Subject: [PATCH 03/14] made more adjustments to address review --- jsx/Filter.js | 3 + .../jsx/batteryManagerIndex.js | 69 +++++++++++-------- .../php/testendpoint.class.inc | 26 ++++--- 3 files changed, 60 insertions(+), 38 deletions(-) diff --git a/jsx/Filter.js b/jsx/Filter.js index d995eac34fc..26f779fa38f 100644 --- a/jsx/Filter.js +++ b/jsx/Filter.js @@ -71,6 +71,9 @@ class Filter extends Component { case 'multiselect': element = ; break; + case 'numeric': + element = ; + break; case 'date': element = ; break; diff --git a/modules/battery_manager/jsx/batteryManagerIndex.js b/modules/battery_manager/jsx/batteryManagerIndex.js index 40cbab51596..2bb6f861361 100644 --- a/modules/battery_manager/jsx/batteryManagerIndex.js +++ b/modules/battery_manager/jsx/batteryManagerIndex.js @@ -101,10 +101,12 @@ class BatteryManagerIndex extends Component { .then((response) => response.text() .then((body) => { body = JSON.parse(body); - const status = response.ok ? 'success' : 'error'; - const message = response.ok ? body.message : body.error; - swal.fire(message, '', status); - response.ok ? resolve() : reject(); + if (response.ok) { + resolve(body.message); + } else { + swal.fire(body.error, '', 'error'); + reject(body.error); + } }) .catch(() => reject())); }); @@ -155,12 +157,14 @@ class BatteryManagerIndex extends Component { if (row.Active === 'Y') { // Pass ID of row to deactivate function result = { - this.deactivateTest(testId); + this.deactivateTest(testId) + .then((message) => swal.fire(message, '', 'success')); }}/>; } else if (row.Active === 'N') { // Pass ID of row to activate function result = { - this.activateTest(testId); + this.activateTest(testId) + .then((message) => swal.fire(message, '', 'success')); }}/>; } break; @@ -251,7 +255,7 @@ class BatteryManagerIndex extends Component { const test = this.state.tests.find((test) => test.id === testId); test.active = 'Y'; this.updateTest(test) - .then(() => resolve()) + .then((message) => resolve(message)) .then(() => reject()); }); } @@ -269,7 +273,7 @@ class BatteryManagerIndex extends Component { const test = this.state.tests.find((test) => test.id === testId); test.active = 'N'; this.updateTest(test) - .then(() => resolve()) + .then((message) => resolve(message)) .catch(() => reject()); }); } @@ -284,12 +288,12 @@ class BatteryManagerIndex extends Component { updateTest(test) { return new Promise((resolve, reject) => { this.postData(this.props.testEndpoint+test.id, test, 'PATCH') - .then(() => { + .then((message) => { const index = this.state.tests .findIndex((element) => element.id === test.id); const tests = this.state.tests; tests[index] = test; - this.setState({tests}, resolve()); + this.setState({tests}, resolve(message)); }) .catch(() => reject()); }); @@ -304,8 +308,9 @@ class BatteryManagerIndex extends Component { return new Promise((resolve, reject) => { const test = this.state.test; this.checkDuplicate(test) - .then(() => this.validateTest(test)) - .then(() => this.postData(this.props.testEndpoint, test, 'POST')) + .then((test) => this.validateTest(test)) + .then((test) => this.postData(this.props.testEndpoint, test, 'POST')) + .then((message) => swal.fire(message, '', 'success')) .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) .then(() => test.id && this.deactivateTest(test.id)) .then(() => this.closeForm()) @@ -324,16 +329,22 @@ class BatteryManagerIndex extends Component { checkDuplicate(test) { return new Promise((resolve, reject) => { let duplicate; + console.log(test); + Object.keys(test).forEach((key) => { + if (test[key] == '') { + test[key] = null; + } + }); this.state.tests.forEach((testCheck) => { if ( - test.testName === testCheck.testName && - test.ageMinDays === testCheck.ageMinDays && - test.ageMaxDays === testCheck.ageMaxDays && - test.stage === testCheck.stage && - test.subproject === testCheck.subproject && - test.visitLabel === testCheck.visitLabel && - test.centerId === testCheck.centerId && - test.firstVisit === testCheck.firstVisit + test.testName == testCheck.testName && + test.ageMinDays == testCheck.ageMinDays && + test.ageMaxDays == testCheck.ageMaxDays && + test.stage == testCheck.stage && + test.subproject == testCheck.subproject && + test.visitLabel == testCheck.visitLabel && + test.centerId == testCheck.centerId && + test.firstVisit == testCheck.firstVisit ) { duplicate = testCheck; } @@ -349,9 +360,11 @@ class BatteryManagerIndex extends Component { showCancelButton: true, }).then((result) => { if (result.value) { - this.activateTest(duplicate.id); + this.activateTest(duplicate.id) + .then((message) => swal.fire(message, '', 'success')); if (test.id && (test.id !== duplicate.id)) { - this.deactivateTest(test.id); + this.deactivateTest(test.id) + .then((message) => swal.fire(message, '', 'success')); } this.closeForm(); } @@ -363,14 +376,13 @@ class BatteryManagerIndex extends Component { } reject(); } else { - resolve(); + resolve(test); } }); } validateTest(test) { return new Promise((resolve, reject) => { - console.log('validate'); const errors = {}; if (test.testName == null) { errors.testName = 'This field is required'; @@ -385,12 +397,9 @@ class BatteryManagerIndex extends Component { errors.stage = 'This field is required'; } - console.log(errors); if (Object.entries(errors).length === 0) { - console.log('resolve'); - resolve(); + this.setState({errors}, resolve(test)); } else { - console.log('reject'); this.setState({errors}, reject()); } }); @@ -428,11 +437,11 @@ class BatteryManagerIndex extends Component { }}, {label: 'Minimum Age', show: true, filter: { name: 'minimumAge', - type: 'text', + type: 'numeric', }}, {label: 'Maximum Age', show: true, filter: { name: 'maximumAge', - type: 'text', + type: 'numeric', }}, {label: 'Stage', show: true, filter: { name: 'stage', diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc index bfd9c49e851..13bd28abde6 100644 --- a/modules/battery_manager/php/testendpoint.class.inc +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -172,6 +172,7 @@ class TestEndpoint implements RequestHandlerInterface ); } + $test->row['id'] = null; $test->row['active'] = 'Y'; $testArray = $test->toSQL(); @@ -197,20 +198,29 @@ class TestEndpoint implements RequestHandlerInterface private function _isDuplicate(Test $test) : bool { // Build SQL query based on values entered by user - $query = "SELECT Test_name as testName, - AgeMinDays as ageMinDays, - AgeMaxDays as ageMaxDays, - Stage as stage, - SubprojectID as subproject, - Visit_label as visitLabel, - CenterID as centerId, + $query = "SELECT Test_name, + AgeMinDays, + AgeMaxDays, + Stage, + SubprojectID, + Visit_label, + CenterID, firstVisit FROM test_battery"; // Select duplicate entry from Test Battery $entries = $this->db->pselect($query, array()); + $testArray = $test->toSQL(); foreach ($entries as $entry) { - if ($test->row === $entry) { + if ($testArray['Test_name'] == $entry['Test_name'] && + $testArray['AgeMinDays'] == $entry['AgeMinDays'] && + $testArray['AgeMaxDays'] == $entry['AgeMaxDays'] && + $testArray['Stage'] == $entry['Stage'] && + $testArray['SubprojectID'] == $entry['SubprojectID'] && + $testArray['Visit_label'] == $entry['Visit_label'] && + $testArray['CenterID'] == $entry['CenterID'] && + $testArray['firstVisit'] == $entry['firstVisit'] + ) { return true; } } From 2a0553edaf0c3aa31a2be35b410ebc814f209673 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Thu, 6 Feb 2020 19:34:58 -0500 Subject: [PATCH 04/14] addressed final parts of review --- modules/battery_manager/help/battery_manager.md | 5 +++-- .../battery_manager/jsx/batteryManagerIndex.js | 17 ++++++++++++++++- .../php/testoptionsendpoint.class.inc | 4 ++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/modules/battery_manager/help/battery_manager.md b/modules/battery_manager/help/battery_manager.md index c549ed19d17..678bb080db3 100644 --- a/modules/battery_manager/help/battery_manager.md +++ b/modules/battery_manager/help/battery_manager.md @@ -29,12 +29,13 @@ You cannot add an entry if it has a duplicate entry in the Test Battery. *For more information on the behaviour of each parameter refer to the [Behaviour of Parameters](#behaviour-of-parameters) section of this document* -## Editing an entry to the Test Battery +## Editing an entry in the Test Battery Under the `Browse` tab, you can edit an entry by clicking on the `Edit` link in the `Edit Metadata` column of the Menu Table. The link will display a form that is populated with the values of the entry. You can update information in the form by selecting from the dropdown menus and filling in the numeric text fields. You will have to fill out the required fields `Instrument`, `Minimum age (days)`, `Maximum age (days)`, `Stage`, and `Active`. -Finally, press the **Edit entry** button to edit the entry in the Test Battery. +Finally, press the **Submit** button to edit the entry in the Test Battery. +This will create a new Test with the information supplied, and deactivate the Test that was edited. You cannot edit an entry if you make no changes in the form. You cannot edit an entry if it becomes the same as another active entry in the Test Battery. diff --git a/modules/battery_manager/jsx/batteryManagerIndex.js b/modules/battery_manager/jsx/batteryManagerIndex.js index 2bb6f861361..b7e572eacea 100644 --- a/modules/battery_manager/jsx/batteryManagerIndex.js +++ b/modules/battery_manager/jsx/batteryManagerIndex.js @@ -122,6 +122,18 @@ class BatteryManagerIndex extends Component { */ mapColumn(column, value) { switch (column) { + case 'First Visit': + if (value == 'Y') { + return 'Yes'; + } else { + return 'No'; + } + case 'Active': + if (value == 'Y') { + return 'Yes'; + } else { + return 'No'; + } case 'Change Status': return ''; case 'Edit Metadata': @@ -141,6 +153,7 @@ class BatteryManagerIndex extends Component { * @return {*} a formated table cell for a given column */ formatColumn(column, cell, row) { + cell = this.mapColumn(column, cell); let result = {cell}; const testId = row['ID']; switch (column) { @@ -354,7 +367,9 @@ class BatteryManagerIndex extends Component { if (duplicate.active === 'N') { swal.fire({ title: 'Test Duplicate', - text: 'Would you to like activate this test?', + text: 'The information provided corresponds with a deactivated '+ + 'test that already exists in the system. Would you to like '+ + 'activate that test?', type: 'warning', confirmButtonText: 'Activate', showCancelButton: true, diff --git a/modules/battery_manager/php/testoptionsendpoint.class.inc b/modules/battery_manager/php/testoptionsendpoint.class.inc index aa14c41f76e..9324c7fc8ba 100644 --- a/modules/battery_manager/php/testoptionsendpoint.class.inc +++ b/modules/battery_manager/php/testoptionsendpoint.class.inc @@ -64,7 +64,7 @@ class TestOptionsEndpoint extends \NDB_Page 'subprojects' => \Utility::getSubprojectList(null), 'visits' => \Utility::getVisitList(), 'sites' => \Utility::getSiteList(false), - 'firstVisits' => $this->_getYesNoList(), + 'firstVisit' => $this->_getYesNoList(), 'active' => $this->_getYesNoList(), ); } @@ -95,7 +95,7 @@ class TestOptionsEndpoint extends \NDB_Page { return array( 'Y' => 'Yes', - 'N' => 'No', + 'N' => 'No', ); } } From 828900545158b947646e18f4dfbc7bd981c763f5 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Mon, 10 Feb 2020 12:25:30 -0500 Subject: [PATCH 05/14] changed edit functionality --- .../battery_manager/jsx/batteryManagerForm.js | 2 +- .../jsx/batteryManagerIndex.js | 55 +++++++++---------- .../php/testendpoint.class.inc | 6 +- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/modules/battery_manager/jsx/batteryManagerForm.js b/modules/battery_manager/jsx/batteryManagerForm.js index 4521170cc8f..ad50dde0ed6 100644 --- a/modules/battery_manager/jsx/batteryManagerForm.js +++ b/modules/battery_manager/jsx/batteryManagerForm.js @@ -123,7 +123,7 @@ class BatteryManagerForm extends Component { { - this.deactivateTest(testId) - .then((message) => swal.fire(message, '', 'success')); + this.deactivateTest(testId); }}/>; } else if (row.Active === 'N') { // Pass ID of row to activate function result = { - this.activateTest(testId) - .then((message) => swal.fire(message, '', 'success')); + this.activateTest(testId); }}/>; } break; @@ -300,14 +298,16 @@ class BatteryManagerIndex extends Component { */ updateTest(test) { return new Promise((resolve, reject) => { - this.postData(this.props.testEndpoint+test.id, test, 'PATCH') - .then((message) => { - const index = this.state.tests - .findIndex((element) => element.id === test.id); - const tests = this.state.tests; - tests[index] = test; - this.setState({tests}, resolve(message)); - }) + Object.keys(test).forEach((key) => { + if (test[key] == '') { + test[key] = null; + } + }); + this.checkDuplicate(test) + .then((test) => this.validateTest(test)) + .then((test) => this.postData(this.props.testEndpoint+test.id, test, 'PUT')) + .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) + .then(() => resolve()) .catch(() => reject()); }); } @@ -315,18 +315,22 @@ class BatteryManagerIndex extends Component { /** * save test to database * + * @param {object} test + * * @return {object} promise */ - addTest() { + addTest(test) { return new Promise((resolve, reject) => { - const test = this.state.test; + Object.keys(test).forEach((key) => { + if (test[key] == '') { + test[key] = null; + } + }); this.checkDuplicate(test) .then((test) => this.validateTest(test)) .then((test) => this.postData(this.props.testEndpoint, test, 'POST')) .then((message) => swal.fire(message, '', 'success')) .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) - .then(() => test.id && this.deactivateTest(test.id)) - .then(() => this.closeForm()) .then(() => resolve()) .catch(() => reject()); }); @@ -342,12 +346,6 @@ class BatteryManagerIndex extends Component { checkDuplicate(test) { return new Promise((resolve, reject) => { let duplicate; - console.log(test); - Object.keys(test).forEach((key) => { - if (test[key] == '') { - test[key] = null; - } - }); this.state.tests.forEach((testCheck) => { if ( test.testName == testCheck.testName && @@ -363,23 +361,22 @@ class BatteryManagerIndex extends Component { } }); - if (duplicate) { + if (duplicate && duplicate.id !== test.id) { if (duplicate.active === 'N') { + const edit = test.id ? 'This will deactivate the current test.' : ''; swal.fire({ title: 'Test Duplicate', text: 'The information provided corresponds with a deactivated '+ 'test that already exists in the system. Would you to like '+ - 'activate that test?', + 'activate that test? '+edit, type: 'warning', confirmButtonText: 'Activate', showCancelButton: true, }).then((result) => { if (result.value) { - this.activateTest(duplicate.id) - .then((message) => swal.fire(message, '', 'success')); + this.activateTest(duplicate.id); if (test.id && (test.id !== duplicate.id)) { - this.deactivateTest(test.id) - .then((message) => swal.fire(message, '', 'success')); + this.deactivateTest(test.id); } this.closeForm(); } @@ -501,7 +498,7 @@ class BatteryManagerIndex extends Component { title="Add New Test" show={show.testForm || show.editForm} onClose={this.closeForm} - onSubmit={this.addTest} + onSubmit={show.testForm ? () => this.addTest(test) : () => this.updateTest(test)} throwWarning={Object.keys(test).length !== 0} > _getInstances(); case 'POST': return $this->_postInstance($request); - case 'PATCH': - return $this->_patchInstance($request); + case 'PUT': + return $this->_putInstance($request); } } @@ -111,7 +111,7 @@ class TestEndpoint implements RequestHandlerInterface * * @return ResponseInterface response */ - private function _patchInstance(ServerRequestInterface $request) + private function _putInstance(ServerRequestInterface $request) { $testArray = json_decode($request->getBody()->getContents(), true); $test = new Test($testArray); From 17c39e8495dd55d823359143206c497cdbf508ae Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Mon, 10 Feb 2020 16:19:39 -0500 Subject: [PATCH 06/14] cleaned up code --- .../jsx/batteryManagerIndex.js | 356 +++++++----------- .../php/testendpoint.class.inc | 1 - 2 files changed, 143 insertions(+), 214 deletions(-) diff --git a/modules/battery_manager/jsx/batteryManagerIndex.js b/modules/battery_manager/jsx/batteryManagerIndex.js index a0238b4a323..1080b031c4b 100644 --- a/modules/battery_manager/jsx/batteryManagerIndex.js +++ b/modules/battery_manager/jsx/batteryManagerIndex.js @@ -28,7 +28,8 @@ class BatteryManagerIndex extends Component { this.state = { tests: {}, test: {}, - show: {testForm: false, editForm: false}, + add: false, + edit: false, error: false, errors: {}, isLoaded: false, @@ -37,13 +38,9 @@ class BatteryManagerIndex extends Component { this.fetchData = this.fetchData.bind(this); this.postData = this.postData.bind(this); this.formatColumn = this.formatColumn.bind(this); - this.addTest = this.addTest.bind(this); + this.saveTest = this.saveTest.bind(this); this.setTest = this.setTest.bind(this); - this.clearTest = this.clearTest.bind(this); this.closeForm = this.closeForm.bind(this); - this.activateTest = this.activateTest.bind(this); - this.deactivateTest = this.deactivateTest.bind(this); - this.updateTest = this.updateTest.bind(this); this.validateTest = this.validateTest.bind(this); } @@ -64,8 +61,6 @@ class BatteryManagerIndex extends Component { * @param {string} state * * @return {object} promise - * - * XXX: This should eventually be moved to a library function */ fetchData(url, method, state) { return new Promise((resolve, reject) => { @@ -87,8 +82,6 @@ class BatteryManagerIndex extends Component { * @param {string} method * * @return {object} promise - * - * XXX: This should eventually be moved to a library function */ postData(url, data, method) { return new Promise((resolve, reject) => { @@ -108,7 +101,7 @@ class BatteryManagerIndex extends Component { reject(body.error); } }) - .catch(() => reject())); + .catch((e) => reject(e))); }); } @@ -123,16 +116,18 @@ class BatteryManagerIndex extends Component { mapColumn(column, value) { switch (column) { case 'First Visit': - if (value == 'Y') { - return 'Yes'; - } else { - return 'No'; + switch (value) { + case 'Y': + return 'Yes'; + case 'N': + return 'No'; } case 'Active': - if (value == 'Y') { - return 'Yes'; - } else { - return 'No'; + switch (value) { + case 'Y': + return 'Yes'; + case 'N': + return 'No'; } case 'Change Status': return ''; @@ -156,6 +151,7 @@ class BatteryManagerIndex extends Component { cell = this.mapColumn(column, cell); let result = {cell}; const testId = row['ID']; + const test = this.state.tests.find((test) => test.id === testId); switch (column) { case 'Instrument': result = {this.state.options.instruments[cell]}; @@ -168,21 +164,19 @@ class BatteryManagerIndex extends Component { break; case 'Change Status': if (row.Active === 'Y') { - // Pass ID of row to deactivate function result = { - this.deactivateTest(testId); + this.deactivateTest(test); }}/>; } else if (row.Active === 'N') { - // Pass ID of row to activate function result = { - this.activateTest(testId); + this.activateTest(test); }}/>; } break; case 'Edit Metadata': const editButton = { this.loadTest(testId); - this.show('editForm'); + this.setState({edit: true}); }}/>; result = {editButton}; break; @@ -191,28 +185,6 @@ class BatteryManagerIndex extends Component { return result; } - /** - * Show a given form based on the passed 'state' - * - * @param {string} item - the item to be shown - */ - show(item) { - let show = this.state.show; - show[item] = true; - this.setState({show}); - } - - /** - * Hige a given form based on the passed 'state' - * - * @param {string} item - the item to be hidden - */ - hide(item) { - let show = this.state.show; - show[item] = false; - this.setState({show}); - } - /** * Set the form data based on state values of child elements/components * @@ -236,67 +208,42 @@ class BatteryManagerIndex extends Component { this.setState({test}); } - /** - * Clear the state of the current test - */ - clearTest() { - const test = {}; - this.setState({test}); - } - /** * Close the Form */ closeForm() { - this.hide('testForm'); - this.hide('editForm'); - this.clearTest(); + this.setState({add: false, edit: false, test: {}}); } /** - * Display popup so user can confirm activation of row - * Refresh page if entry in Test Battery is successfully activated - * - * @param {int} testId + * Activate Test * - * @return {object} + * @param {object} test */ - activateTest(testId) { - return new Promise((resolve, reject) => { - const test = this.state.tests.find((test) => test.id === testId); - test.active = 'Y'; - this.updateTest(test) - .then((message) => resolve(message)) - .then(() => reject()); - }); + activateTest(test) { + test.active = 'Y'; + this.saveTest(test, 'PUT'); } /** - * Display popup so user can confirm deactivation of row - * Refresh page if entry in Test Battery is successfully deactivated - * - * @param {int} testId + * Deactivate Test * - * @return {object} + * @param {int} test */ - deactivateTest(testId) { - return new Promise((resolve, reject) => { - const test = this.state.tests.find((test) => test.id === testId); - test.active = 'N'; - this.updateTest(test) - .then((message) => resolve(message)) - .catch(() => reject()); - }); + deactivateTest(test) { + test.active = 'N'; + this.saveTest(test, 'PUT'); } /** * Updates a previously existing Test with an updated Test. * * @param {object} test + * @param {string} request * * @return {object} promise */ - updateTest(test) { + saveTest(test, request) { return new Promise((resolve, reject) => { Object.keys(test).forEach((key) => { if (test[key] == '') { @@ -305,115 +252,10 @@ class BatteryManagerIndex extends Component { }); this.checkDuplicate(test) .then((test) => this.validateTest(test)) - .then((test) => this.postData(this.props.testEndpoint+test.id, test, 'PUT')) + .then((test) => this.postData(this.props.testEndpoint+test.id, test, request)) .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) .then(() => resolve()) - .catch(() => reject()); - }); - } - - /** - * save test to database - * - * @param {object} test - * - * @return {object} promise - */ - addTest(test) { - return new Promise((resolve, reject) => { - Object.keys(test).forEach((key) => { - if (test[key] == '') { - test[key] = null; - } - }); - this.checkDuplicate(test) - .then((test) => this.validateTest(test)) - .then((test) => this.postData(this.props.testEndpoint, test, 'POST')) - .then((message) => swal.fire(message, '', 'success')) - .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) - .then(() => resolve()) - .catch(() => reject()); - }); - } - - /** - * Checks whether the Test is a duplicate of an existing Test. - * - * @param {object} test - * - * @return {object} promise - */ - checkDuplicate(test) { - return new Promise((resolve, reject) => { - let duplicate; - this.state.tests.forEach((testCheck) => { - if ( - test.testName == testCheck.testName && - test.ageMinDays == testCheck.ageMinDays && - test.ageMaxDays == testCheck.ageMaxDays && - test.stage == testCheck.stage && - test.subproject == testCheck.subproject && - test.visitLabel == testCheck.visitLabel && - test.centerId == testCheck.centerId && - test.firstVisit == testCheck.firstVisit - ) { - duplicate = testCheck; - } - }); - - if (duplicate && duplicate.id !== test.id) { - if (duplicate.active === 'N') { - const edit = test.id ? 'This will deactivate the current test.' : ''; - swal.fire({ - title: 'Test Duplicate', - text: 'The information provided corresponds with a deactivated '+ - 'test that already exists in the system. Would you to like '+ - 'activate that test? '+edit, - type: 'warning', - confirmButtonText: 'Activate', - showCancelButton: true, - }).then((result) => { - if (result.value) { - this.activateTest(duplicate.id); - if (test.id && (test.id !== duplicate.id)) { - this.deactivateTest(test.id); - } - this.closeForm(); - } - }); - } else if (duplicate.active === 'Y') { - swal.fire( - 'Test Duplicate', 'You cannot duplicate an active test', 'error' - ); - } - reject(); - } else { - resolve(test); - } - }); - } - - validateTest(test) { - return new Promise((resolve, reject) => { - const errors = {}; - if (test.testName == null) { - errors.testName = 'This field is required'; - } - if (test.ageMinDays == null) { - errors.ageMinDays = 'This field is required'; - } - if (test.ageMaxDays == null) { - errors.ageMaxDays = 'This field is required'; - } - if (test.stage == null) { - errors.stage = 'This field is required'; - } - - if (Object.entries(errors).length === 0) { - this.setState({errors}, resolve(test)); - } else { - this.setState({errors}, reject()); - } + .catch((e) => reject(e)); }); } @@ -438,7 +280,7 @@ class BatteryManagerIndex extends Component { * XXX: Currently, the order of these fields MUST match the order of the * queried columns in _setupVariables() in batter_manager.class.inc */ - const {options, test, tests, show, errors} = this.state; + const {options, test, tests, errors, add, edit} = this.state; const {hasPermission} = this.props; const fields = [ {label: 'ID', show: false}, @@ -493,28 +335,10 @@ class BatteryManagerIndex extends Component { {label: 'Edit Metadata', show: hasPermission('batter_manager_edit')}, ]; - const testForm = ( - this.addTest(test) : () => this.updateTest(test)} - throwWarning={Object.keys(test).length !== 0} - > - - - ); - const actions = [ { label: 'New Test', - action: () => this.show('testForm'), + action: () => this.setState({add: true}), show: hasPermission('battery_manager_edit'), }, ]; @@ -535,9 +359,12 @@ class BatteryManagerIndex extends Component { ]; }); + const modalTitle = edit ? 'Edit Test' : 'Add New Test'; + const request = edit ? 'PUT' : 'POST'; + const handleSubmit = () => this.saveTest(test, request); + return (
- {testForm} + + +
); } + + /** + * Checks whether the Test is a duplicate of an existing Test. + * + * @param {object} test + * + * @return {object} promise + */ + checkDuplicate(test) { + return new Promise((resolve, reject) => { + let duplicate; + this.state.tests.forEach((testCheck) => { + if ( + test.testName == testCheck.testName && + test.ageMinDays == testCheck.ageMinDays && + test.ageMaxDays == testCheck.ageMaxDays && + test.stage == testCheck.stage && + test.subproject == testCheck.subproject && + test.visitLabel == testCheck.visitLabel && + test.centerId == testCheck.centerId && + test.firstVisit == testCheck.firstVisit + ) { + duplicate = testCheck; + } + }); + + if (duplicate && duplicate.id !== test.id) { + if (duplicate.active === 'N') { + const edit = test.id ? 'This will deactivate the current test.' : ''; + swal.fire({ + title: 'Test Duplicate', + text: 'The information provided corresponds with a deactivated '+ + 'test that already exists in the system. Would you to like '+ + 'activate that test? '+edit, + type: 'warning', + confirmButtonText: 'Activate', + showCancelButton: true, + }).then((result) => { + if (result.value) { + this.activateTest(duplicate); + if (test.id && (test.id !== duplicate.id)) { + this.deactivateTest(test); + } + this.closeForm(); + } + }); + } else if (duplicate.active === 'Y') { + swal.fire( + 'Test Duplicate', 'You cannot duplicate an active test', 'error' + ); + } + reject(); + } else { + resolve(test); + } + }); + } + + + /** + * Checks that test fields are valide + * + * @return {object} promise + */ + validateTest(test) { + return new Promise((resolve, reject) => { + const errors = {}; + if (test.testName == null) { + errors.testName = 'This field is required'; + } + if (test.ageMinDays == null) { + errors.ageMinDays = 'This field is required'; + } + if (test.ageMaxDays == null) { + errors.ageMaxDays = 'This field is required'; + } + if (test.stage == null) { + errors.stage = 'This field is required'; + } + + if (Object.entries(errors).length === 0) { + this.setState({errors}, resolve(test)); + } else { + this.setState({errors}, reject()); + } + }); + } } BatteryManagerIndex.propTypes = { - dataURL: PropTypes.string.isRequired, + testEndpoint: PropTypes.string.isRequired, + optionEndpoint: PropTypes.string.isRequired, hasPermission: PropTypes.func.isRequired, }; diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc index 8d924e74648..ee5e9b2c87a 100644 --- a/modules/battery_manager/php/testendpoint.class.inc +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -172,7 +172,6 @@ class TestEndpoint implements RequestHandlerInterface ); } - $test->row['id'] = null; $test->row['active'] = 'Y'; $testArray = $test->toSQL(); From c2d7533d854a3d491cae1d6e49d6628bd9e28166 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Mon, 10 Feb 2020 16:33:22 -0500 Subject: [PATCH 07/14] further cleaning --- .../php/testendpoint.class.inc | 74 ++++++++----------- 1 file changed, 31 insertions(+), 43 deletions(-) diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc index ee5e9b2c87a..4206f9ef5fe 100644 --- a/modules/battery_manager/php/testendpoint.class.inc +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -104,8 +104,7 @@ class TestEndpoint implements RequestHandlerInterface } /** - * Updates the status of the given test from Active to Deactive or - * viceversa. + * Puts a test to the test_battery in the database. * * @param ServerRequestInterface $request Test to be saved * @@ -113,36 +112,17 @@ class TestEndpoint implements RequestHandlerInterface */ private function _putInstance(ServerRequestInterface $request) { - $testArray = json_decode($request->getBody()->getContents(), true); - $test = new Test($testArray); - - // validate instance; - $errors = $this->_validateInstance($test); - if (!empty($errors)) { - return new \LORIS\Http\Response\JSON\BadRequest( - implode(' ', $errors) - ); + if (!$this->user->hasPermission('battery_manager_edit')) { + return new \LORIS\Http\Response\JSON\Forbidden('Edit Permission Denied'); } - $testArray = $test->toSQL(); - try { - $this->db->update( - 'test_battery', - $testArray, - array('ID' => $testArray['ID']) - ); - return new \LORIS\Http\Response\JSON\OK( - ['message'=>'Status Updated Successfully'] - ); - } catch (\DatabaseException $e) { - return new \LORIS\Http\Response\JSON\InternalServerError( - 'Could not update entry in the Test Battery' - ); - } + $testArray = json_decode($request->getBody()->getContents(), true); + $test = new Test($testArray); + return $this->_saveInstance($test); } /** - * Adds a new test to the test_battery in the database. + * Posts a test to the test_battery in the database. * * @param ServerRequestInterface $request Test to be posted * @@ -150,20 +130,13 @@ class TestEndpoint implements RequestHandlerInterface */ private function _postInstance(ServerRequestInterface $request) { - $testArray = json_decode($request->getBody()->getContents(), true); - $test = new Test($testArray); - if (!$this->user->hasPermission('battery_manager_edit')) { return new \LORIS\Http\Response\JSON\Forbidden('Edit Permission Denied'); } - // validate instance - $errors = $this->_validateInstance($test); - if (!empty($errors)) { - return new \LORIS\Http\Response\JSON\BadRequest( - implode(' ', $errors) - ); - } + $testArray = json_decode($request->getBody()->getContents(), true); + $test = new Test($testArray); + $test->row['active'] = 'Y'; // check if instance is duplicate if ($this->_isDuplicate($test)) { @@ -172,14 +145,29 @@ class TestEndpoint implements RequestHandlerInterface ); } - $test->row['active'] = 'Y'; - $testArray = $test->toSQL(); + return $this->_saveInstance($test); + } - try { - $this->db->insert('test_battery', $testArray); - return new \LORIS\Http\Response\JSON\OK( - ['message'=>'Test Submission Successful'] + /** + * Generic save function for Test Instances. + * + * @param Test $test The Test Instance to be saved. + * + * @return ResponseInterface response + */ + private function _saveInstance(Test $test) + { + // validate instance + $errors = $this->_validateInstance($test); + if (!empty($errors)) { + return new \LORIS\Http\Response\JSON\BadRequest( + implode(' ', $errors) ); + } + + try { + $this->db->insertOnDuplicateUpdate('test_battery', $test->toSQL()); + return new \LORIS\Http\Response\JSON\OK(); } catch (\DatabaseException $e) { return new \LORIS\Http\Response\JSON\InternalServerError( 'Could not add entry to the Test Battery' From 2548e65f5dfe1081f44a59fda4b86ff84436bbe8 Mon Sep 17 00:00:00 2001 From: Rida Date: Tue, 11 Feb 2020 09:39:57 -0500 Subject: [PATCH 08/14] fix travis --- CHANGELOG.md | 6 ++++++ modules/battery_manager/jsx/batteryManagerIndex.js | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3cb14a7d6f..1800113ca0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,4 +16,10 @@ changes in the following format: PR #1234*** - Very old instrument relying on QuickForm may have issues due to code changes (PR #4928) #### Modules + +##### Battery Manager + - New module created to manage the entries in the test_battery table of the database. + This allows projects to modify their instrument battery without requiring backend access. + (https://github.com/aces/Loris/pull/4221) + ##### module1 diff --git a/modules/battery_manager/jsx/batteryManagerIndex.js b/modules/battery_manager/jsx/batteryManagerIndex.js index 1080b031c4b..d338a7db63a 100644 --- a/modules/battery_manager/jsx/batteryManagerIndex.js +++ b/modules/battery_manager/jsx/batteryManagerIndex.js @@ -252,7 +252,11 @@ class BatteryManagerIndex extends Component { }); this.checkDuplicate(test) .then((test) => this.validateTest(test)) - .then((test) => this.postData(this.props.testEndpoint+test.id, test, request)) + .then((test) => this.postData( + this.props.testEndpoint+test.id, + test, + request) + ) .then(() => this.fetchData(this.props.testEndpoint, 'GET', 'tests')) .then(() => resolve()) .catch((e) => reject(e)); @@ -453,6 +457,8 @@ class BatteryManagerIndex extends Component { /** * Checks that test fields are valide * + * @param {object} test + * * @return {object} promise */ validateTest(test) { From 582aadea9d18b8027dcb33e9dac8bfb1c3744af6 Mon Sep 17 00:00:00 2001 From: Rida Date: Tue, 11 Feb 2020 09:52:40 -0500 Subject: [PATCH 09/14] fix 2 --- .../battery_manager/php/testendpoint.class.inc | 18 +++++++++--------- .../php/testoptionsendpoint.class.inc | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc index 4206f9ef5fe..2797b31e6d3 100644 --- a/modules/battery_manager/php/testendpoint.class.inc +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -199,15 +199,15 @@ class TestEndpoint implements RequestHandlerInterface $testArray = $test->toSQL(); foreach ($entries as $entry) { - if ($testArray['Test_name'] == $entry['Test_name'] && - $testArray['AgeMinDays'] == $entry['AgeMinDays'] && - $testArray['AgeMaxDays'] == $entry['AgeMaxDays'] && - $testArray['Stage'] == $entry['Stage'] && - $testArray['SubprojectID'] == $entry['SubprojectID'] && - $testArray['Visit_label'] == $entry['Visit_label'] && - $testArray['CenterID'] == $entry['CenterID'] && - $testArray['firstVisit'] == $entry['firstVisit'] - ) { + if ($testArray['Test_name'] == $entry['Test_name'] + && $testArray['AgeMinDays'] == $entry['AgeMinDays'] + && $testArray['AgeMaxDays'] == $entry['AgeMaxDays'] + && $testArray['Stage'] == $entry['Stage'] + && $testArray['SubprojectID'] == $entry['SubprojectID'] + && $testArray['Visit_label'] == $entry['Visit_label'] + && $testArray['CenterID'] == $entry['CenterID'] + && $testArray['firstVisit'] == $entry['firstVisit'] + ) { return true; } } diff --git a/modules/battery_manager/php/testoptionsendpoint.class.inc b/modules/battery_manager/php/testoptionsendpoint.class.inc index 9324c7fc8ba..fc20c6fde60 100644 --- a/modules/battery_manager/php/testoptionsendpoint.class.inc +++ b/modules/battery_manager/php/testoptionsendpoint.class.inc @@ -95,7 +95,7 @@ class TestOptionsEndpoint extends \NDB_Page { return array( 'Y' => 'Yes', - 'N' => 'No', + 'N' => 'No', ); } } From 0ff818614aa3431b4f772c510847f4a6664a0291 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Tue, 11 Feb 2020 14:36:04 -0500 Subject: [PATCH 10/14] cleaned more code --- modules/battery_manager/php/testendpoint.class.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc index 4206f9ef5fe..c97119f20d5 100644 --- a/modules/battery_manager/php/testendpoint.class.inc +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -100,6 +100,7 @@ class TestEndpoint implements RequestHandlerInterface new \LORIS\Data\Filters\UserSiteMatch() ); } + return $provisioner; } From f099d57b190d31408855cb5a1175e395a626d9bc Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Tue, 11 Feb 2020 14:41:32 -0500 Subject: [PATCH 11/14] fixed help text on form --- modules/battery_manager/jsx/batteryManagerForm.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/battery_manager/jsx/batteryManagerForm.js b/modules/battery_manager/jsx/batteryManagerForm.js index ad50dde0ed6..6efadc822a8 100644 --- a/modules/battery_manager/jsx/batteryManagerForm.js +++ b/modules/battery_manager/jsx/batteryManagerForm.js @@ -32,8 +32,7 @@ class BatteryManagerForm extends Component { } else { return ( - Editing an entry will deactivate the current entry and create a - new entry.
+ Editing an entry will alter the current entry.
You cannot edit an entry to have the same values as another active entry.
If the duplicate entry is inactive, you will be given the option From 71a6d34628b2840278f8542e33bf3405c3dcc9e4 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Tue, 11 Feb 2020 16:11:52 -0500 Subject: [PATCH 12/14] finalized code clean up --- .../jsx/batteryManagerIndex.js | 21 +++++++----- .../php/testendpoint.class.inc | 33 +++++++++---------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/modules/battery_manager/jsx/batteryManagerIndex.js b/modules/battery_manager/jsx/batteryManagerIndex.js index d338a7db63a..22d256a01af 100644 --- a/modules/battery_manager/jsx/batteryManagerIndex.js +++ b/modules/battery_manager/jsx/batteryManagerIndex.js @@ -151,7 +151,6 @@ class BatteryManagerIndex extends Component { cell = this.mapColumn(column, cell); let result = {cell}; const testId = row['ID']; - const test = this.state.tests.find((test) => test.id === testId); switch (column) { case 'Instrument': result = {this.state.options.instruments[cell]}; @@ -165,11 +164,11 @@ class BatteryManagerIndex extends Component { case 'Change Status': if (row.Active === 'Y') { result = { - this.deactivateTest(test); + this.deactivateTest(testId); }}/>; } else if (row.Active === 'N') { result = { - this.activateTest(test); + this.activateTest(testId); }}/>; } break; @@ -218,9 +217,10 @@ class BatteryManagerIndex extends Component { /** * Activate Test * - * @param {object} test + * @param {int} id */ - activateTest(test) { + activateTest(id) { + const test = this.state.tests.find((test) => test.id === id); test.active = 'Y'; this.saveTest(test, 'PUT'); } @@ -228,9 +228,10 @@ class BatteryManagerIndex extends Component { /** * Deactivate Test * - * @param {int} test + * @param {int} id */ - deactivateTest(test) { + deactivateTest(id) { + const test = this.state.tests.find((test) => test.id === id); test.active = 'N'; this.saveTest(test, 'PUT'); } @@ -423,6 +424,7 @@ class BatteryManagerIndex extends Component { if (duplicate && duplicate.id !== test.id) { if (duplicate.active === 'N') { + console.log('Duplicate Not Active'); const edit = test.id ? 'This will deactivate the current test.' : ''; swal.fire({ title: 'Test Duplicate', @@ -434,14 +436,15 @@ class BatteryManagerIndex extends Component { showCancelButton: true, }).then((result) => { if (result.value) { - this.activateTest(duplicate); + this.activateTest(duplicate.id); if (test.id && (test.id !== duplicate.id)) { - this.deactivateTest(test); + this.deactivateTest(test.id); } this.closeForm(); } }); } else if (duplicate.active === 'Y') { + console.log('Duplicate Active'); swal.fire( 'Test Duplicate', 'You cannot duplicate an active test', 'error' ); diff --git a/modules/battery_manager/php/testendpoint.class.inc b/modules/battery_manager/php/testendpoint.class.inc index 3fcd613e9de..9f543264374 100644 --- a/modules/battery_manager/php/testendpoint.class.inc +++ b/modules/battery_manager/php/testendpoint.class.inc @@ -113,10 +113,6 @@ class TestEndpoint implements RequestHandlerInterface */ private function _putInstance(ServerRequestInterface $request) { - if (!$this->user->hasPermission('battery_manager_edit')) { - return new \LORIS\Http\Response\JSON\Forbidden('Edit Permission Denied'); - } - $testArray = json_decode($request->getBody()->getContents(), true); $test = new Test($testArray); return $this->_saveInstance($test); @@ -131,21 +127,9 @@ class TestEndpoint implements RequestHandlerInterface */ private function _postInstance(ServerRequestInterface $request) { - if (!$this->user->hasPermission('battery_manager_edit')) { - return new \LORIS\Http\Response\JSON\Forbidden('Edit Permission Denied'); - } - $testArray = json_decode($request->getBody()->getContents(), true); $test = new Test($testArray); $test->row['active'] = 'Y'; - - // check if instance is duplicate - if ($this->_isDuplicate($test)) { - return new \LORIS\Http\Response\JSON\Conflict( - 'This Test already exists in the database' - ); - } - return $this->_saveInstance($test); } @@ -158,6 +142,10 @@ class TestEndpoint implements RequestHandlerInterface */ private function _saveInstance(Test $test) { + if (!$this->user->hasPermission('battery_manager_edit')) { + return new \LORIS\Http\Response\JSON\Forbidden('Edit Permission Denied'); + } + // validate instance $errors = $this->_validateInstance($test); if (!empty($errors)) { @@ -166,6 +154,13 @@ class TestEndpoint implements RequestHandlerInterface ); } + // check if instance is duplicate + if ($this->_isDuplicate($test)) { + return new \LORIS\Http\Response\JSON\Conflict( + 'This Test already exists in the database' + ); + } + try { $this->db->insertOnDuplicateUpdate('test_battery', $test->toSQL()); return new \LORIS\Http\Response\JSON\OK(); @@ -186,7 +181,8 @@ class TestEndpoint implements RequestHandlerInterface private function _isDuplicate(Test $test) : bool { // Build SQL query based on values entered by user - $query = "SELECT Test_name, + $query = "SELECT ID, + Test_name, AgeMinDays, AgeMaxDays, Stage, @@ -200,7 +196,8 @@ class TestEndpoint implements RequestHandlerInterface $testArray = $test->toSQL(); foreach ($entries as $entry) { - if ($testArray['Test_name'] == $entry['Test_name'] + if ($testArray['ID'] !== $entry['ID'] + && $testArray['Test_name'] == $entry['Test_name'] && $testArray['AgeMinDays'] == $entry['AgeMinDays'] && $testArray['AgeMaxDays'] == $entry['AgeMaxDays'] && $testArray['Stage'] == $entry['Stage'] From 6a062ec56ad996b4eb7d07a33b118202e6db4324 Mon Sep 17 00:00:00 2001 From: Rida Date: Wed, 12 Feb 2020 13:48:58 -0500 Subject: [PATCH 13/14] added permissions --- SQL/0000-00-01-Permission.sql | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/SQL/0000-00-01-Permission.sql b/SQL/0000-00-01-Permission.sql index beaf82f1136..db77c0d9a3f 100644 --- a/SQL/0000-00-01-Permission.sql +++ b/SQL/0000-00-01-Permission.sql @@ -104,7 +104,10 @@ INSERT INTO `permissions` VALUES (56,'publication_approve', 'Publication - Approve or reject proposed publication projects', 2), (57, 'candidate_dob_edit', 'Edit dates of birth', 2), (58,'electrophysiology_browser_view_allsites', 'View all-sites Electrophysiology Browser pages', 2), - (59,'electrophysiology_browser_view_site', 'View own site Electrophysiology Browser pages', 2); + (59,'electrophysiology_browser_view_site', 'View own site Electrophysiology Browser pages', 2), + (60,'battery_manager_view','View Battery Manager',2), + (60,'battery_manager_edit','Add, activate, and deactivate entries in Test Battery',2); + INSERT INTO `user_perm_rel` (userID, permID) From bf4f6ea1b7bba3ce26b4997813552d1fc16a5302 Mon Sep 17 00:00:00 2001 From: Loris Admin Date: Wed, 12 Feb 2020 15:06:53 -0500 Subject: [PATCH 14/14] fixed SQL error --- SQL/0000-00-01-Permission.sql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/SQL/0000-00-01-Permission.sql b/SQL/0000-00-01-Permission.sql index db77c0d9a3f..99654c4835b 100644 --- a/SQL/0000-00-01-Permission.sql +++ b/SQL/0000-00-01-Permission.sql @@ -106,8 +106,7 @@ INSERT INTO `permissions` VALUES (58,'electrophysiology_browser_view_allsites', 'View all-sites Electrophysiology Browser pages', 2), (59,'electrophysiology_browser_view_site', 'View own site Electrophysiology Browser pages', 2), (60,'battery_manager_view','View Battery Manager',2), - (60,'battery_manager_edit','Add, activate, and deactivate entries in Test Battery',2); - + (61,'battery_manager_edit','Add, activate, and deactivate entries in Test Battery',2); INSERT INTO `user_perm_rel` (userID, permID)