From dde06dd93e2739ee10728edb68e67b32bc4a84cd Mon Sep 17 00:00:00 2001 From: Jefferson Casimir Date: Mon, 28 Oct 2024 16:41:41 -0400 Subject: [PATCH 1/8] Add CSV parser --- php/libraries/CSVParser.class.inc | 139 ++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 php/libraries/CSVParser.class.inc diff --git a/php/libraries/CSVParser.class.inc b/php/libraries/CSVParser.class.inc new file mode 100644 index 00000000000..5ae492b6444 --- /dev/null +++ b/php/libraries/CSVParser.class.inc @@ -0,0 +1,139 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ + +namespace LORIS; + +use SplFileInfo; +use SplFileObject; + +/** + * This class is used as a helper for parsing csv files + * + * PHP Version 8 + * + * @category Main + * @package LORIS + * @author Jefferson Casimir + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +class CSVParser +{ + private SplFileInfo $csvFile; + /** + * Construct parser + * + * @param SplFileInfo $csvFile csv file path + * + * @return void + */ + public function __construct(SplFileInfo $csvFile) + { + if ($csvFile->getExtension() !== 'csv') { + throw new \RuntimeException('Invalid file extension'); + } + $this->csvFile = $csvFile; + } + + /** + * Check that headers match (equivalent arrays) + * + * @param array $headers Headers parsed from csv file + * @param array $expectedHeaders Expected headers (ex: from instrument) + * @param bool $strict If true, $headers cannot be a superset of $expectedHeaders + * + * @return bool Returns true if headers are valid + */ + private function _validateHeaders(array $headers, array $expectedHeaders, bool $strict = true): bool + { + if ($strict && count($headers) !== count($expectedHeaders)) { + return false; + } + + $matchingHeaders = true; + foreach ($expectedHeaders as $expectedHeader) { + if (!in_array($expectedHeader, $headers)) { + $matchingHeaders = false; + break; + } + } + return $matchingHeaders; + } + + /** + * Parses the csv file + * + * @param array $expectedHeaders Expected headers. Leave empty to skip validation + * + * @param string $separator The field delimiter (one character only). + * Defaults as a comma. + * + * @param string $enclosure The field enclosure character (one character only). + * Defaults as a double quotation mark + * + * @param string $escape The escape character (one character only). + * Defaults as a backslash (\) + * + * @param int $flags Bit mask of the flags to set. + * See SplFileObject constants for the available flags. + * Default is: SplFileObject::READ_CSV | + * SplFileObject::DROP_NEW_LINE | + * SplFileObject::SKIP_EMPTY | + * SplFileObject::READ_AHEAD + * + * + * @return array Returns array of header to row value mapping + * + * Example with defaults: + * csv file: A,B,C + * 1,e,3 + * 4,"5",w + * + * output: [ + * [ [A] => 1, [B] => e, [C] => 3 ], + * [ [A] => 4, [B] => 5, [C] => w ] + * ] + * + */ + public function parse( + array $expectedHeaders = [], + string $separator = ',', + string $enclosure = '"', + string $escape = '\\', + int $flags = + SplFileObject::READ_CSV | + SplFileObject::DROP_NEW_LINE | + SplFileObject::SKIP_EMPTY | + SplFileObject::READ_AHEAD, + ): array + { + $fileObject = new SplFileObject($this->csvFile->getFilename()); + $fileObject->setFlags($flags); + $csvHeaders = $fileObject->fgetcsv(); + + // Validate expected headers, if provided + if (count($expectedHeaders) > 0) { + if (!$this->_validateHeaders($csvHeaders, $expectedHeaders)) { + throw new \RuntimeException('Headers do not match expected headers'); + } + } + + // Parse and create header to value mapping + $csv = []; + while ($row = $fileObject->fgetcsv($separator, $enclosure, $escape)) { + $csv[] = array_combine($csvHeaders, $row); + } + return $csv; + } + } From 5213b824dee077670ab7db1a887b280fd2beaca5 Mon Sep 17 00:00:00 2001 From: Jefferson Casimir Date: Mon, 28 Oct 2024 16:41:55 -0400 Subject: [PATCH 2/8] Add Instrument Data parser --- php/libraries/InstrumentDataParser.class.inc | 141 +++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 php/libraries/InstrumentDataParser.class.inc diff --git a/php/libraries/InstrumentDataParser.class.inc b/php/libraries/InstrumentDataParser.class.inc new file mode 100644 index 00000000000..46e8966f6b1 --- /dev/null +++ b/php/libraries/InstrumentDataParser.class.inc @@ -0,0 +1,141 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ + +namespace LORIS; +require_once __DIR__ . "/../../tools/generic_includes.php"; // TODO: Delete line + +include_once 'NDB_Factory.class.inc'; +include_once 'CSVParser.class.inc'; +use SplFileInfo; + +/** + * This class is used as a helper for parsing csv files + * + * PHP Version 8 + * + * @category Main + * @package LORIS + * @author Jefferson Casimir + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris/ + */ +class InstrumentDataParser extends CSVParser +{ + private string $instrumentName; + /** + * Construct parser + * + * @param string $instrumentName Instrument name + * @param SplFileInfo $csvFile csv file path + * + * @return void + */ + public function __construct(string $instrumentName, SplFileInfo $csvFile) + { + parent::__construct($csvFile); + $this->instrumentName = $instrumentName; + } + + /** + * Get columns from DB table + * + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string + * + * @return array Returns columns from DB table + */ + public static function getDBColumns(string $tableName, array $excluded = ['']): array + { + $db = \NDB_Factory::singleton()->database(); + + $query = "SELECT column_name + FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = :table + AND column_name NOT IN (" . + implode(',', array_map( + function ($col) { return "'$col'"; }, + $excluded, + )) . ")"; + $results = $db->pselect($query, [ + 'table' => $tableName, + ]); + + $headers = []; + foreach ($results as $row) { + $headers[] = $row['column_name']; + } + return $headers; + } + + /** + * Get expected columns from `flag` table + * + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string + * + * @return array Returns columns from flag table + */ + public static function getFlagHeaders(array $excluded = ['']): array + { + return InstrumentDataParser::getDBColumns('flag', $excluded); + } + + /** + * Get expected columns from the instrument's table + * + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string + * + * @return array Returns columns from flag table + */ + public static function getInstrumentHeaders(string $instrumentName, array $excluded = ['']): array + { + return InstrumentDataParser::getDBColumns($instrumentName, $excluded); + } + + /** + * Get expected csv columns for an instrument + * + * @return array Returns array of column names as strings + */ + public static function getCSVHeaders(string $instrumentName): array + { + return array_merge( + ['PSCID', 'Visit_label'], + InstrumentDataParser::getFlagHeaders(['ID', 'CommentID', 'SessionID']), + InstrumentDataParser::getInstrumentHeaders($instrumentName, ['CommentID', 'UserID', 'Testdate']), + ); + } + + /** + * Parse the csv file + * + * @return array Returns array of column names as strings + */ + public function parseCSV(): array + { + try + { + $expectedHeaders = $this->getCSVHeaders($this->instrumentName); + return $this->parse($expectedHeaders); + } catch (\Exception $e) { + error_log("An error occurred while parsing {$e->getMessage()}"); + return []; + } + } + } + From 9cf8dfac85d1d73f4d5555e569c927942a053643 Mon Sep 17 00:00:00 2001 From: Jefferson Casimir Date: Mon, 28 Oct 2024 16:44:24 -0400 Subject: [PATCH 3/8] Instrument Data Form: Get header template --- .../jsx/instrumentManagerIndex.js | 2 +- modules/instrument_manager/jsx/uploadForm.js | 127 ++++++++++--- .../php/instrument_manager.class.inc | 169 +++++++++++++++++- 3 files changed, 272 insertions(+), 26 deletions(-) diff --git a/modules/instrument_manager/jsx/instrumentManagerIndex.js b/modules/instrument_manager/jsx/instrumentManagerIndex.js index 0d34901b62c..dadd31410af 100644 --- a/modules/instrument_manager/jsx/instrumentManagerIndex.js +++ b/modules/instrument_manager/jsx/instrumentManagerIndex.js @@ -266,7 +266,7 @@ class InstrumentManagerIndex extends Component { } else if (this.state.data.fieldOptions.writable) { let url = loris.BaseURL.concat('/instrument_manager/'); content = ( - + ); } else { content = ( diff --git a/modules/instrument_manager/jsx/uploadForm.js b/modules/instrument_manager/jsx/uploadForm.js index a0ba79ae257..aad923263c7 100644 --- a/modules/instrument_manager/jsx/uploadForm.js +++ b/modules/instrument_manager/jsx/uploadForm.js @@ -2,6 +2,7 @@ import React, {Component} from 'react'; import PropTypes from 'prop-types'; import swal from 'sweetalert2'; import {FileElement} from 'jsx/Form'; +import {SelectElement} from '../../../jsx/Form'; /** * Instrument Upload Form component @@ -15,12 +16,13 @@ class InstrumentUploadForm extends Component { super(props); this.state = { - data: {}, selectedFile: null, + selectedInstrument: '', }; - this.fileSelected = this.fileSelected.bind(this); - this.upload = this.upload.bind(this); + this.instrumentFileSelected = this.instrumentFileSelected.bind(this); + this.uploadInstrument = this.uploadInstrument.bind(this); + this.getInstrumentOptions = this.getInstrumentOptions.bind(this); } /** @@ -29,7 +31,7 @@ class InstrumentUploadForm extends Component { * @param {string} element - Element name * @param {string} file */ - fileSelected(element, file) { + instrumentFileSelected(element, file) { this.setState({ selectedFile: file, }); @@ -38,7 +40,7 @@ class InstrumentUploadForm extends Component { /** * Upload instrument */ - upload() { + uploadInstrument() { const data = new FormData(); data.append('install_file', this.state.selectedFile); @@ -81,6 +83,17 @@ class InstrumentUploadForm extends Component { }); } + getInstrumentOptions() { + const instruments = {}; + if (this.props.data) { + this.props.data.Data.map((instrument) => { + const instrumentName = instrument[0]; + instruments[instrumentName] = instrumentName; + }); + } + return instruments; + } + /** * Renders the React component. * @@ -88,38 +101,112 @@ class InstrumentUploadForm extends Component { */ render() { const disabled = () => this.state.selectedFile === null; + const instrumentSelected = this.state.selectedInstrument !== ''; return ( -
-
-
-
Upload Instrument
-
-
+ <> +
+
+ + + + +
+ {/*
*/} + {/*
*/} +
+ +
+ this.setState({ + ...this.state, + selectedInstrument: value, + })} + emptyOption={true} + sortByValue={true} + /> +
+
+
+
+ + Download Expected Template +
-
+
-
+ ); } } InstrumentUploadForm.propTypes = { action: PropTypes.string.isRequired, + data: PropTypes.object.isRequired, }; - export default InstrumentUploadForm; + +/** + * Create a componenet to select permissions from a list of available + * permissions. + * + * @param {object} props - react props + * @return {JSX.Element} + */ +function UploadPanel(props) { + const children = React.Children.toArray(props.children); + return
+
+ {props.title} +
+
+
+ {children} +
+
+
; +} + +UploadPanel.propTypes = { + children: PropTypes.array, + title: PropTypes.string, +}; + + diff --git a/modules/instrument_manager/php/instrument_manager.class.inc b/modules/instrument_manager/php/instrument_manager.class.inc index 1f530e8b397..6ca8b2e709e 100644 --- a/modules/instrument_manager/php/instrument_manager.class.inc +++ b/modules/instrument_manager/php/instrument_manager.class.inc @@ -1,6 +1,7 @@ getMethod(); if ($method == 'POST') { return $this->handlePOST($request); + } else if ($method == 'GET') { + return $this->handleGET($request); + } + } + + /** + * Handle a GET requests + * + * @param ServerRequestInterface $request The PSR15 Request being handled + * + * @return ResponseInterface The PSR15 response for the page. + */ + protected function handleGET(ServerRequestInterface $request): ResponseInterface + { + // Ensure the user is allowed to upload. + if (! $request->getAttribute('user')->hasPermission( + 'instrument_manager_read' + ) + ) { + return new \LORIS\Http\Response\JSON\Forbidden(); + } + + $params = $request->getQueryParams(); + if (isset($params['instrument'])) { + $instrument = $params['instrument']; + + if ($this->instrumentExists($instrument)) { + $headers = InstrumentDataParser::getCSVHeaders($instrument); + + $fileName = "${instrument}_headers.csv"; + $fileObject = new \SplFileObject(sys_get_temp_dir() . "/$fileName", 'w'); + $fileObject->fputcsv($headers); + + $downloader = new \LORIS\FilesDownloadHandler( + new \SPLFileInfo(sys_get_temp_dir()) + ); + return $downloader->handle( + $request->withAttribute('filename', $fileName) + ); + } + + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid request' + ); + } return parent::handle($request); } + /** - * Handle a POST request for instrument upload. It moves the uploaded file - * in the project/instruments directory then call the php script from the - * tools directory to generate sql statement required for the installation - * of the instrument. Finaly, it source the sql file to complete the - * installation. + * Handle a POST requests * * @param ServerRequestInterface $request The PSR15 Request being handled * @@ -90,6 +133,33 @@ class Instrument_Manager extends \DataFrameworkMenu ); } + $data = json_decode((string) $request->getBody(), true); + + if ($data['upload_type'] == 'instrument') { + return $this->handleInstrumentUpload($request); + } else if ($data['upload_type'] == 'data') { + return $this->handleDataUpload($request); + } + + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid request' + ); + } + + + /** + * Handle a POST request for uploading a new instrument. It moves the uploaded file + * in the project/instruments directory then call the php script from the + * tools directory to generate sql statement required for the installation + * of the instrument. Finally, it sources the sql file to complete the + * installation. + * + * @param ServerRequestInterface $request The PSR15 Request being handled + * + * @return ResponseInterface The PSR15 response for the page. + */ + protected function handleInstrumentUpload(ServerRequestInterface $request): ResponseInterface + { $uploaded_file = $request->getUploadedFiles()['install_file']; $filename = $uploaded_file->getClientFilename(); $instrument = pathinfo($filename)['filename']; @@ -165,6 +235,95 @@ class Instrument_Manager extends \DataFrameworkMenu } return new \LORIS\Http\Response\JSON\Created(["ok"=>"ok"]); } + + + /** + * Handle a POST request for uploading instrument data for an existing + * instrument. + * + * @param ServerRequestInterface $request The PSR15 Request being handled + * + * @return ResponseInterface The PSR15 response for the page. + */ + protected function handleDataUpload(ServerRequestInterface $request): ResponseInterface + { + $uploaded_file = $request->getUploadedFiles()['install_file']; + $filename = $uploaded_file->getClientFilename(); + $instrument = pathinfo($filename)['filename']; + $targetdir = new \SplFileInfo($this->_path . 'project/instruments/'); + + if ($this->instrumentExists($instrument)) { + // An instrument with this name already exists in the test_names + // table. Abort upload. + return new \LORIS\Http\Response\JSON\Conflict(self::ALREADY_INSTALLED); + } + + /* Although only .linst files should be uploaded to this module, and + * they are just special text files, they are marked as + * application/octet-stream upon upload. + */ + $uploader = (new \LORIS\FilesUploadHandler($targetdir)) + ->withPermittedMIMETypes( + 'application/octet-stream' + ); + + $response = $uploader->handle($request); + + if (!in_array($response->getStatusCode(), [200, 201], true)) { + if ($response->getStatusCode() == 409) { + /* Don't update the file if it already exists on the back-end. + * Instead, inform users that an administrator must install it + * on their behalf. + * This should only happen for users on a system where automatic + * installation is disabled (ie. has no adminUser). + */ + return new \LORIS\Http\Response\JSON\Conflict( + self::FILE_ALREADY_EXISTS + ); + } + return $response; + } + + try { + + $inst = \NDB_BVL_Instrument::factory( + $this->loris, + $instrument, + '', + '', + ); + + $DB = $this->loris->getDatabaseConnection(); + $DB->insert( + "test_names", + [ + 'test_name' => $instrument, + 'Full_name' => $inst->getFullName(), + 'Sub_group' => 1, + ] + ); + file_put_contents( + \Utility::pathJoin( + $targetdir->getPath(), + $targetdir->getFilename(), + $instrument . ".meta" + ), + "jsondata{@}true\n" + ); + + } catch (\NotFound $e) { + return (new \LORIS\Http\Response\JSON\NotFound( + $e->getMessage() + )); + } catch (\Exception $e) { + return new \LORIS\Http\Response\JSON\InternalServerError( + $e->getMessage() + ); + } + return new \LORIS\Http\Response\JSON\Created(["ok"=>"ok"]); + } + + /** * Tells the base class that this page's provisioner can support the * HasAnyPermissionOrUserSiteMatch filter. From b87e1b1bcfb68f398d7d91c1961a67154bc20ba7 Mon Sep 17 00:00:00 2001 From: Jefferson Casimir Date: Fri, 1 Nov 2024 13:24:14 -0400 Subject: [PATCH 4/8] First iteration. Pre-NDB_BVL for all --- .../jsx/instrumentManagerIndex.js | 26 +- modules/instrument_manager/jsx/uploadForm.js | 87 +++- .../php/instrument_manager.class.inc | 121 ++--- php/libraries/CSVParser.class.inc | 56 +-- php/libraries/Database.class.inc | 26 +- php/libraries/InstrumentDataParser.class.inc | 431 ++++++++++++++++-- php/libraries/Utility.class.inc | 5 +- 7 files changed, 565 insertions(+), 187 deletions(-) diff --git a/modules/instrument_manager/jsx/instrumentManagerIndex.js b/modules/instrument_manager/jsx/instrumentManagerIndex.js index dadd31410af..5ba1a7f3e88 100644 --- a/modules/instrument_manager/jsx/instrumentManagerIndex.js +++ b/modules/instrument_manager/jsx/instrumentManagerIndex.js @@ -241,19 +241,19 @@ class InstrumentManagerIndex extends Component { tabs.push({id: 'upload', label: 'Upload'}); } - const feedback = () => { - if (!this.state.data.fieldOptions.caninstall - && this.props.hasPermission('instrument_manager_write')) { - return ( -
- Instrument installation is not possible given the current server - configuration; the LORIS 'adminUser' is not configured properly. - File upload is still possible but instruments will need to be - installed manually -
- ); - } - }; + const feedback = () => {}; + // if (!this.state.data.fieldOptions.caninstall + // && this.props.hasPermission('instrument_manager_write')) { + // return ( + //
+ // Instrument installation is not possible given the current server + // configuration; the LORIS 'adminUser' is not configured properly. + // File upload is still possible but instruments will need to be + // installed manually + //
+ // ); + // } + // }; const uploadTab = () => { let content = null; diff --git a/modules/instrument_manager/jsx/uploadForm.js b/modules/instrument_manager/jsx/uploadForm.js index aad923263c7..8d6fb005722 100644 --- a/modules/instrument_manager/jsx/uploadForm.js +++ b/modules/instrument_manager/jsx/uploadForm.js @@ -16,24 +16,40 @@ class InstrumentUploadForm extends Component { super(props); this.state = { - selectedFile: null, + selectedInstrumentFile: null, + selectedDataFile: null, selectedInstrument: '', }; this.instrumentFileSelected = this.instrumentFileSelected.bind(this); this.uploadInstrument = this.uploadInstrument.bind(this); this.getInstrumentOptions = this.getInstrumentOptions.bind(this); + this.dataFileSelected = this.dataFileSelected.bind(this); + this.uploadInstrumentData = this.uploadInstrumentData.bind(this); + } /** - * Update selectedFile on file selection + * Update selectedInstrumentFile on instrument file selection * * @param {string} element - Element name * @param {string} file */ instrumentFileSelected(element, file) { this.setState({ - selectedFile: file, + selectedInstrumentFile: file, + }); + } + + /** + * Update selectedDataFile on data file selection + * + * @param {string} element - Element name + * @param {string} file + */ + dataFileSelected(element, file) { + this.setState({ + selectedDataFile: file, }); } @@ -42,7 +58,8 @@ class InstrumentUploadForm extends Component { */ uploadInstrument() { const data = new FormData(); - data.append('install_file', this.state.selectedFile); + data.append('upload_type', 'instrument'); + data.append('install_file', this.state.selectedInstrumentFile); fetch(this.props.action, { method: 'POST', @@ -83,6 +100,47 @@ class InstrumentUploadForm extends Component { }); } + + /** + * Upload instrument data + */ + uploadInstrumentData() { + const data = new FormData(); + data.append('upload_type', 'data'); + data.append('instrument', this.state.selectedInstrument); + data.append('data_file', this.state.selectedDataFile); + + fetch(this.props.action, { + method: 'POST', + credentials: 'same-origin', + body: data, + }) + .then((resp) => { + return resp.json(); + }) + .then((data) => { + console.log('data', data); + if (data.success) { + swal.fire({ + title: 'Upload Successful!', + type: 'success', + text: data.message, + }).then(function() { + // window.location.assign(loris.BaseURL + '/instrument_manager/'); + }); + } else { + swal.fire({ + title: 'No data uploaded', + type: 'warn', + text: data.message, + }); + } + }) + .catch((error) => { + this.setState({error: true}); + }); + } + getInstrumentOptions() { const instruments = {}; if (this.props.data) { @@ -94,14 +152,17 @@ class InstrumentUploadForm extends Component { return instruments; } + + /** * Renders the React component. * * @return {JSX} - React markup for the component */ render() { - const disabled = () => this.state.selectedFile === null; + const uploadInstrumentDisabled = () => this.state.selectedInstrumentFile === null; const instrumentSelected = this.state.selectedInstrument !== ''; + const uploadDataDisabled = () => !instrumentSelected || this.state.selectedDataFile === null; return ( <> @@ -114,12 +175,12 @@ class InstrumentUploadForm extends Component { name='install_file' label='Instrument file' onUserInput={this.instrumentFileSelected} - value={this.state.selectedFile} + value={this.state.selectedInstrumentFile} /> @@ -148,16 +209,16 @@ class InstrumentUploadForm extends Component {
diff --git a/modules/instrument_manager/php/instrument_manager.class.inc b/modules/instrument_manager/php/instrument_manager.class.inc index 6ca8b2e709e..c2b1fc8e632 100644 --- a/modules/instrument_manager/php/instrument_manager.class.inc +++ b/modules/instrument_manager/php/instrument_manager.class.inc @@ -4,6 +4,9 @@ namespace LORIS\instrument_manager; use LORIS\InstrumentDataParser; use \Psr\Http\Message\ServerRequestInterface; use \Psr\Http\Message\ResponseInterface; +use SplFileInfo; + +use NDB_BVL_Instrument; /** * Contains functions used to view and manage instruments installed in LORIS. @@ -62,6 +65,8 @@ class Instrument_Manager extends \DataFrameworkMenu } else if ($method == 'GET') { return $this->handleGET($request); } + + return parent::handle($request); } /** @@ -70,6 +75,7 @@ class Instrument_Manager extends \DataFrameworkMenu * @param ServerRequestInterface $request The PSR15 Request being handled * * @return ResponseInterface The PSR15 response for the page. + * @throws \NotFound|\LorisException */ protected function handleGET(ServerRequestInterface $request): ResponseInterface { @@ -86,9 +92,9 @@ class Instrument_Manager extends \DataFrameworkMenu $instrument = $params['instrument']; if ($this->instrumentExists($instrument)) { - $headers = InstrumentDataParser::getCSVHeaders($instrument); + $headers = InstrumentDataParser::getCSVHeaders($this->loris, $instrument); - $fileName = "${instrument}_headers.csv"; + $fileName = "${instrument}_headers.csv"; $fileObject = new \SplFileObject(sys_get_temp_dir() . "/$fileName", 'w'); $fileObject->fputcsv($headers); @@ -133,11 +139,12 @@ class Instrument_Manager extends \DataFrameworkMenu ); } - $data = json_decode((string) $request->getBody(), true); + $request_body = (array) $request->getParsedBody(); + $uploadType = $request_body['upload_type']; - if ($data['upload_type'] == 'instrument') { + if ($uploadType == 'instrument') { return $this->handleInstrumentUpload($request); - } else if ($data['upload_type'] == 'data') { + } else if ($uploadType == 'data') { return $this->handleDataUpload($request); } @@ -247,82 +254,53 @@ class Instrument_Manager extends \DataFrameworkMenu */ protected function handleDataUpload(ServerRequestInterface $request): ResponseInterface { - $uploaded_file = $request->getUploadedFiles()['install_file']; - $filename = $uploaded_file->getClientFilename(); - $instrument = pathinfo($filename)['filename']; - $targetdir = new \SplFileInfo($this->_path . 'project/instruments/'); + $data_file = $request->getUploadedFiles()['data_file']; + $request_body = (array)$request->getParsedBody(); - if ($this->instrumentExists($instrument)) { - // An instrument with this name already exists in the test_names - // table. Abort upload. - return new \LORIS\Http\Response\JSON\Conflict(self::ALREADY_INSTALLED); - } + $fileLocation = sys_get_temp_dir() . '/' . $data_file->getClientFilename(); + $data_file->moveTo($fileLocation); // TODO: Explore alternatives - /* Although only .linst files should be uploaded to this module, and - * they are just special text files, they are marked as - * application/octet-stream upon upload. - */ - $uploader = (new \LORIS\FilesUploadHandler($targetdir)) - ->withPermittedMIMETypes( - 'application/octet-stream' - ); + $userID = $request->getAttribute('user')->getUsername(); - $response = $uploader->handle($request); + if (isset($request_body['instrument'])) { + $instrument = $request_body['instrument']; + if ($this->instrumentExists($instrument)) { - if (!in_array($response->getStatusCode(), [200, 201], true)) { - if ($response->getStatusCode() == 409) { - /* Don't update the file if it already exists on the back-end. - * Instead, inform users that an administrator must install it - * on their behalf. - * This should only happen for users on a system where automatic - * installation is disabled (ie. has no adminUser). - */ - return new \LORIS\Http\Response\JSON\Conflict( - self::FILE_ALREADY_EXISTS + // Parse + $dataParser = new InstrumentDataParser( + $instrument, + new SplFileInfo($fileLocation) ); - } - return $response; - } - - try { - - $inst = \NDB_BVL_Instrument::factory( - $this->loris, - $instrument, - '', - '', - ); - $DB = $this->loris->getDatabaseConnection(); - $DB->insert( - "test_names", - [ - 'test_name' => $instrument, - 'Full_name' => $inst->getFullName(), - 'Sub_group' => 1, - ] - ); - file_put_contents( - \Utility::pathJoin( - $targetdir->getPath(), - $targetdir->getFilename(), - $instrument . ".meta" - ), - "jsondata{@}true\n" - ); + try { + $data = $dataParser->parseCSV($this->loris); + $validData = $dataParser->validateData( + $data, + [ + 'UserID' => $userID, + 'Examiner' => $userID + ] + ); + + $result = $dataParser->insertInstrumentData($this->loris, $validData); + + return new \LORIS\Http\Response\JSON\Created($result); + } catch (\LorisException | \Exception $e) { + return new \LORIS\Http\Response\JSON\OK([ + 'message' => $e->getMessage() + ]); + } + } - } catch (\NotFound $e) { - return (new \LORIS\Http\Response\JSON\NotFound( - $e->getMessage() - )); - } catch (\Exception $e) { - return new \LORIS\Http\Response\JSON\InternalServerError( - $e->getMessage() + return new \LORIS\Http\Response\JSON\BadRequest( + 'Instrument does not exist' ); } - return new \LORIS\Http\Response\JSON\Created(["ok"=>"ok"]); - } + return new \LORIS\Http\Response\JSON\BadRequest( + 'Invalid request' + ); + } /** * Tells the base class that this page's provisioner can support the @@ -381,6 +359,7 @@ class Instrument_Manager extends \DataFrameworkMenu $this->_path = $this->loris->getConfiguration()->getSetting('base'); $instrument_dir = $this->_path . 'project/instruments'; + error_log($instrument_dir . ' writeable? ' . (is_writable($instrument_dir) ? 'Y':'N')); return is_writable($instrument_dir); } diff --git a/php/libraries/CSVParser.class.inc b/php/libraries/CSVParser.class.inc index 5ae492b6444..ac899251e2f 100644 --- a/php/libraries/CSVParser.class.inc +++ b/php/libraries/CSVParser.class.inc @@ -49,13 +49,13 @@ class CSVParser /** * Check that headers match (equivalent arrays) * - * @param array $headers Headers parsed from csv file - * @param array $expectedHeaders Expected headers (ex: from instrument) - * @param bool $strict If true, $headers cannot be a superset of $expectedHeaders + * @param array $headers Headers parsed from csv file + * @param array $expectedHeaders Expected headers (ex: from instrument) + * @param bool $strict If true, $headers cannot be a superset of $expectedHeaders * * @return bool Returns true if headers are valid */ - private function _validateHeaders(array $headers, array $expectedHeaders, bool $strict = true): bool + private function _validateHeaders(array $headers, array $expectedHeaders, bool $strict = false): bool { if ($strict && count($headers) !== count($expectedHeaders)) { return false; @@ -64,6 +64,7 @@ class CSVParser $matchingHeaders = true; foreach ($expectedHeaders as $expectedHeader) { if (!in_array($expectedHeader, $headers)) { + error_log("Header not found: $expectedHeader"); $matchingHeaders = false; break; } @@ -74,24 +75,25 @@ class CSVParser /** * Parses the csv file * - * @param array $expectedHeaders Expected headers. Leave empty to skip validation + * @param array $expectedHeaders Expected headers. Leave empty to skip validation * - * @param string $separator The field delimiter (one character only). - * Defaults as a comma. + * @param string $separator The field delimiter (one character only). + * Defaults as a comma. * - * @param string $enclosure The field enclosure character (one character only). - * Defaults as a double quotation mark + * @param string $enclosure The field enclosure character (one character only). + * Defaults as a double quotation mark * - * @param string $escape The escape character (one character only). - * Defaults as a backslash (\) - * - * @param int $flags Bit mask of the flags to set. - * See SplFileObject constants for the available flags. - * Default is: SplFileObject::READ_CSV | - * SplFileObject::DROP_NEW_LINE | - * SplFileObject::SKIP_EMPTY | - * SplFileObject::READ_AHEAD + * @param string $escape The escape character (one character only). + * Defaults as a backslash (\) * + * @param int $flags Bit mask of the flags to set. + * See SplFileObject constants + * for the available flags. + * Default is: + * SplFileObject::READ_CSV | + * SplFileObject::DROP_NEW_LINE + * | SplFileObject::SKIP_EMPTY | + * SplFileObject::READ_AHEADSplFileObject::READ_AHEAD * * @return array Returns array of header to row value mapping * @@ -104,7 +106,6 @@ class CSVParser * [ [A] => 1, [B] => e, [C] => 3 ], * [ [A] => 4, [B] => 5, [C] => w ] * ] - * */ public function parse( array $expectedHeaders = [], @@ -112,13 +113,12 @@ class CSVParser string $enclosure = '"', string $escape = '\\', int $flags = - SplFileObject::READ_CSV | - SplFileObject::DROP_NEW_LINE | - SplFileObject::SKIP_EMPTY | - SplFileObject::READ_AHEAD, - ): array - { - $fileObject = new SplFileObject($this->csvFile->getFilename()); + SplFileObject::READ_CSV | + SplFileObject::DROP_NEW_LINE | + SplFileObject::SKIP_EMPTY | + SplFileObject::READ_AHEAD, + ): array { + $fileObject = new SplFileObject($this->csvFile->getRealPath()); $fileObject->setFlags($flags); $csvHeaders = $fileObject->fgetcsv(); @@ -129,11 +129,11 @@ class CSVParser } } - // Parse and create header to value mapping + // Parse and create header => value mapping $csv = []; while ($row = $fileObject->fgetcsv($separator, $enclosure, $escape)) { $csv[] = array_combine($csvHeaders, $row); } return $csv; } - } +} diff --git a/php/libraries/Database.class.inc b/php/libraries/Database.class.inc index c348633be37..87d225c925e 100644 --- a/php/libraries/Database.class.inc +++ b/php/libraries/Database.class.inc @@ -771,8 +771,8 @@ class Database implements LoggerAwareInterface if (is_null($uniqueKey) || empty($uniqueKey)) { throw new LorisException( - "The pselectWithIndexKey() function expects the uniqueKey parameter - to not be null or empty. If re-indexing on the primary key is + "The pselectWithIndexKey() function expects the uniqueKey parameter + to not be null or empty. If re-indexing on the primary key is not necessary please use the pselect() function instead." ); } @@ -789,9 +789,9 @@ class Database implements LoggerAwareInterface // Check first that the row contains the primary key supplied if (!array_key_exists($uniqueKey, $row)) { throw new DatabaseException( - "The query supplied to pselectWithIndexKey() does not contain - the unique key to re-index on. Make sure to supply the - appropriate key in the SELECT statement to match the supplied + "The query supplied to pselectWithIndexKey() does not contain + the unique key to re-index on. Make sure to supply the + appropriate key in the SELECT statement to match the supplied parameter of this function", $query ); @@ -802,7 +802,7 @@ class Database implements LoggerAwareInterface if (isset($filteredResult[$row[$uniqueKey]])) { throw new DatabaseException( "The uniqueKey supplied to pselectWithIndexKey() does not appear - to be unique or is nullable. This function expects the key to + to be unique or is nullable. This function expects the key to be both UNIQUE and NOT NULL.", $query ); @@ -868,7 +868,7 @@ class Database implements LoggerAwareInterface $colNumber = count($row); if ($colNumber !== 1) { throw new DatabaseException( - "The pselectCol() function expects only one column in the + "The pselectCol() function expects only one column in the SELECT clause of the query, $colNumber were passed.", $query ); @@ -909,7 +909,7 @@ class Database implements LoggerAwareInterface if (is_null($uniqueKey) || empty($uniqueKey)) { throw new LorisException( "The pselectColWithIndexKey() function expects the uniqueKey - parameter to not be null or empty. If re-indexing on the primary + parameter to not be null or empty. If re-indexing on the primary key is not necessary please use the pselectCol() function instead." ); } @@ -927,9 +927,9 @@ class Database implements LoggerAwareInterface // Check first that the row contains the primary key supplied if (!array_key_exists($uniqueKey, $row) || $colNumber !== 2) { throw new DatabaseException( - "The query supplied to pselectColWithIndexKey() should only - contain the unique key and one other column in the SELECT - clause. Make sure to supply the appropriate key in the SELECT + "The query supplied to pselectColWithIndexKey() should only + contain the unique key and one other column in the SELECT + clause. Make sure to supply the appropriate key in the SELECT statement to match the supplied parameter of this function.", $query ); @@ -939,8 +939,8 @@ class Database implements LoggerAwareInterface // in the result array if (isset($filteredResult[$row[$uniqueKey]])) { throw new DatabaseException( - "The uniqueKey supplied to pselectColWithIndexKey() does not - appear to be unique or is nullable. This function expects the + "The uniqueKey supplied to pselectColWithIndexKey() does not + appear to be unique or is nullable. This function expects the key to be both UNIQUE and NOT NULL.", $query ); diff --git a/php/libraries/InstrumentDataParser.class.inc b/php/libraries/InstrumentDataParser.class.inc index 46e8966f6b1..cef80d07c0a 100644 --- a/php/libraries/InstrumentDataParser.class.inc +++ b/php/libraries/InstrumentDataParser.class.inc @@ -13,10 +13,15 @@ */ namespace LORIS; -require_once __DIR__ . "/../../tools/generic_includes.php"; // TODO: Delete line -include_once 'NDB_Factory.class.inc'; -include_once 'CSVParser.class.inc'; +require_once 'NDB_Factory.class.inc'; +require_once 'CSVParser.class.inc'; + +use _PHPStan_c875e8309\Nette\Neon\Exception; +use DatabaseException; +use NDB_BVL_Battery; +use NDB_Factory; +use NotFound; use SplFileInfo; /** @@ -33,11 +38,12 @@ use SplFileInfo; class InstrumentDataParser extends CSVParser { private string $instrumentName; + /** * Construct parser * - * @param string $instrumentName Instrument name - * @param SplFileInfo $csvFile csv file path + * @param string $instrumentName Instrument name + * @param SplFileInfo $csvFile csv file path * * @return void */ @@ -47,30 +53,101 @@ class InstrumentDataParser extends CSVParser $this->instrumentName = $instrumentName; } + /** + * Parse the csv file + * + * @param LorisInstance $loris Loris instance + * + * @return array Returns array of column names as strings + * @throws Exception + */ + public function parseCSV(LorisInstance $loris): array + { + try { + $expectedHeaders = self::getCSVHeaders($loris, $this->instrumentName); + return $this->parse($expectedHeaders); + } catch (\Exception $e) { + throw new Exception("An error occurred while parsing: {$e->getMessage()}"); + } + } + + /** + * Get expected csv columns for an instrument + * + * @param LorisInstance $loris Loris instance + * @param string $instrumentName Instrument name + * + * @return array Returns array of column names as strings + * @throws NotFound + */ + public static function getCSVHeaders(LorisInstance $loris, string $instrumentName): array + { + return array_merge( + ['PSCID', 'Visit_label'], // SessionID + // Convert to static list of flag fields + self::getFlagHeaders( // Whitelist would prevent invalid columns -- write tests + [ + 'ID', 'SessionID', 'Test_name', 'TestID', + 'CommentID', 'UserID', 'Data', 'DataID', + 'Order' + ] + ), + self::getInstrumentHeaders( + $loris, + $instrumentName, + [ + 'CommentID', 'UserID', 'Testdate', 'Examiner' + ] + ), + ); + } + + /** + * Get expected columns from `flag` table + * + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string + * + * @return array Returns columns from flag table + */ + public static function getFlagHeaders(array $excluded = ['']): array + { + return self::getDBColumns('flag', $excluded); + } + /** * Get columns from DB table * - * @param array $excluded Excluded columns. Default is - * single-item array with empty - * string + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string * * @return array Returns columns from DB table */ public static function getDBColumns(string $tableName, array $excluded = ['']): array { - $db = \NDB_Factory::singleton()->database(); + $db = NDB_Factory::singleton()->database(); - $query = "SELECT column_name + $query = "SELECT column_name FROM INFORMATION_SCHEMA.COLUMNS WHERE table_name = :table AND column_name NOT IN (" . - implode(',', array_map( - function ($col) { return "'$col'"; }, - $excluded, - )) . ")"; - $results = $db->pselect($query, [ - 'table' => $tableName, - ]); + implode( + ',', + array_map( + function ($col) { + return "'$col'"; + }, + $excluded, + ) + ) . ")"; + $results = $db->pselect( + $query, + [ + 'table' => $tableName, + ] + ); $headers = []; foreach ($results as $row) { @@ -80,62 +157,320 @@ class InstrumentDataParser extends CSVParser } /** - * Get expected columns from `flag` table + * Get expected columns from the instrument's table * - * @param array $excluded Excluded columns. Default is - * single-item array with empty - * string + * @param LorisInstance $loris Loris instance + * @param string $instrumentName Instrument name + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string * * @return array Returns columns from flag table + * @throws NotFound */ - public static function getFlagHeaders(array $excluded = ['']): array + protected static function getInstrumentHeaders(LorisInstance $loris, string $instrumentName, array $excluded = ['']): array { - return InstrumentDataParser::getDBColumns('flag', $excluded); + $instrumentType = self::checkInstrumentType($instrumentName); + + return match ($instrumentType) { +// 'SQL' => self::getDBColumns($instrumentName, $excluded), // TODO: Below should be good for all + 'PHP', 'Instrument Builder' => self::getDataDictionary($loris, $instrumentName, $excluded), + default => self::getDataDictionary($loris, $instrumentName, $excluded), + }; } /** - * Get expected columns from the instrument's table + * Validate Instrument data + * + * @return array Returns array of column names as strings + */ + public function validateData(array $instrumentData, array $injectedColumns): array + { + try { + foreach ($instrumentData as &$dataRow) { + // Validate Session + $sessionID = $this->obtainSessionID( + $dataRow['PSCID'], + $dataRow['Visit_label'] + ); + $dataRow['SessionID'] = $sessionID; + $dataRow = array_merge($dataRow, $injectedColumns); + } + return $instrumentData; + } catch (\Exception $e) { + error_log("Invalid data: {$e->getMessage()}"); + return []; + } + } + + + /** + * Get the session ID from PSCID and Visit_label * - * @param array $excluded Excluded columns. Default is - * single-item array with empty - * string + * @param string $pscID candidate PSCID + * @param string $visitLabel session Visit_label * - * @return array Returns columns from flag table + * @return int Returns SessionID + * @throws Exception */ - public static function getInstrumentHeaders(string $instrumentName, array $excluded = ['']): array + public function obtainSessionID(string $pscID, string $visitLabel): int + // TODO: Timepoint/getTimepointFormPSCIDandVisitLabel(,) { - return InstrumentDataParser::getDBColumns($instrumentName, $excluded); + $db = NDB_Factory::singleton()->database(); + + $query = "SELECT s.ID + FROM session AS s + LEFT JOIN candidate USING (CandID) + WHERE Visit_label = :visitLabel + AND PSCID = :PSCID"; + + $sessionID = $db->pselectOneInt( + $query, + [ + 'PSCID' => $pscID, + 'visitLabel' => $visitLabel, + ] + ); + + if (is_null($sessionID)) { + throw new Exception( + "Session not found for PSCID: $pscID + and Visit_label: $visitLabel" + ); + } + return $sessionID; } /** - * Get expected csv columns for an instrument + * Insert instrument data in DB * - * @return array Returns array of column names as strings + * @param LorisInstance $loris Loris instance + * @param array $instrumentData Instrument data to insert + * + * @return array Returns insertion results + * @throws NotFound */ - public static function getCSVHeaders(string $instrumentName): array + public function insertInstrumentData(LorisInstance $loris, array $instrumentData): array { - return array_merge( - ['PSCID', 'Visit_label'], - InstrumentDataParser::getFlagHeaders(['ID', 'CommentID', 'SessionID']), - InstrumentDataParser::getInstrumentHeaders($instrumentName, ['CommentID', 'UserID', 'Testdate']), + $db = NDB_Factory::singleton()->database(); + + $testID = $db->pselectOneInt( + "SELECT ID + FROM test_names + WHERE Test_name = :testName", + [ + 'testName' => $this->instrumentName, + ] ); + + $instrumentHeaders = $this->getInstrumentHeaders($loris, $this->instrumentName); + + $useInstrumentDataTable = !$db->tableExists($this->instrumentName); + // TODO: INSTEAD USE ->usesJSONData + + $errors = []; + try { + $db->beginTransaction(); + try { + foreach ($instrumentData as $data) { + try { + // TODO: addInstrument then _save() (or save() with validation/score -- checkbox) + $data['CommentID'] = NDB_BVL_Battery::generateCommentID( + new \SessionID(strval($data['SessionID'])), + $this->instrumentName, + ); + + error_log('trying to save'); + error_log(json_encode(array_reduce( + $instrumentHeaders, + function($columns, $columnName) use ($data) { + $columns[$columnName] = + strlen($data[$columnName]) > 0 + ? $data[$columnName] + : null; + return $columns; + }, + [] + )) + ); + + $dataID = null; + + if ($useInstrumentDataTable) { + // Insert as JSON string -- TODO: unsafe? + $db->unsafeinsert( + 'instrument_data', + [ + 'Data' => json_encode(array_reduce( + $instrumentHeaders, + function($columns, $columnName) use ($data) { + $columns[$columnName] = + strlen($data[$columnName]) > 0 + ? $data[$columnName] + : null; + return $columns; + }, + [] + )) + ] + ); + $dataID = $db->getLastInsertId(); + } else { + // Insert in instrument's table + $db->insert( + $this->instrumentName, + array_reduce( + $instrumentHeaders, + function($columns, $columnName) use ($data) { + $columns[$columnName] = + strlen($data[$columnName]) > 0 + ? $data[$columnName] + : null; + return $columns; + }, + [] + ), + ); + } + + // Insert flag data + $db->insert( + 'flag', + [ + 'SessionID' => $data['SessionID'], + 'TestID' => $testID, + 'CommentID' => $data['CommentID'], + 'Data_entry' => $data['Data_entry'], + 'Required_elements_completed' => $data['Required_elements_completed'], + 'Administration' => $data['Administration'], + 'Validity' => $data['Validity'], + 'Exclusion' => $data['Exclusion'], + 'UserID' => $data['UserID'], + 'Testdate' => $data['Testdate'], + 'DataID' => $dataID, + ] + ); + } catch (PDOException $e) { + error_log("Error caught: {$e->getMessage()}"); + $errors[] = [ + 'row' => $data, + 'error' => $e->getMessage(), + 'message' => "Error caught: {$e->getMessage()}", + ]; + } + } + + if (count($errors) > 0) { + error_log('Errors found:'.json_encode($errors)); + $db->rollBack(); + return [ + 'success' => false, + 'errors' => $errors, + 'message' => 'Errors found:'.json_encode($errors), + ]; + } else { + $db->commit(); + error_log("Saved " . count($instrumentData) . " row(s)"); + return [ + 'success' => true, + 'errors' => [], + 'message' => "Saved " . count($instrumentData) . " row(s)", + ]; + } + } catch (\Exception $e) { + error_log("Rolling back due to unexpected error: {$e->getMessage()}"); + $db->rollBack(); + return [ + 'success' => false, + 'errors' => [], + 'message' => "Unexpected error: {$e->getMessage()}" + ]; + } + } catch (DatabaseException $e) { + error_log("DB exception: {$e->getMessage()}"); + return [ + 'success' => false, + 'errors' => [], + 'message' => "DB Error: {$e->getMessage()}" + ]; + } } + /** - * Parse the csv file + * Get columns from NDB_BVL_Instrument * - * @return array Returns array of column names as strings + * @param LorisInstance $loris Loris instance + * @param string $instrumentName Instrument name + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string + * + * @return array Returns columns from DB table + * @throws NotFound */ - public function parseCSV(): array + public static function getDataDictionary(LorisInstance $loris, string $instrumentName, array $excluded = ['']): array { - try - { - $expectedHeaders = $this->getCSVHeaders($this->instrumentName); - return $this->parse($expectedHeaders); - } catch (\Exception $e) { - error_log("An error occurred while parsing {$e->getMessage()}"); - return []; + $instrument = \NDB_BVL_Instrument::factory($loris, $instrumentName); + $dataDict = $instrument->getDataDictionary(); + + // TODO: Find out whave _/save needs to works + + return array_reduce( + $dataDict, + function($columns, $field) use ($instrumentName, $excluded) { + $fieldName = $field->getName(); + // Strip prefix + $fieldName = str_replace("${instrumentName}_", '', $fieldName); + if (!in_array($fieldName, $excluded)) { + $columns[] = $fieldName; + } + return $columns; + }, + [] + ); + } + + + /** + * Determine the instrument type between LINST and PHP + * + * @param string $instrument The instrument name + * + * @return string The instrument type + */ + public static function checkInstrumentType(string $instrument): string + { + // TODO: COPIED FROM modules/instrument_manager/php/instrumentrow.class.inc + $db = NDB_Factory::singleton()->database(); + + if ($db->tableExists($instrument)) { + return 'SQL'; } + + $_factory = \NDB_Factory::singleton(); + $_path = $_factory->config()->getSetting("base"); + + $linst = $_path . "/project/instruments/$instrument.linst"; + if (file_exists($linst)) { + return 'Instrument Builder'; + } + + $php = $_path . + "/project/instruments/NDB_BVL_Instrument_$instrument.class.inc"; + + if (file_exists($php)) { + return 'PHP'; + } + + return 'Missing'; } - } +} + + +// TODO: Tech debt +/* + * Seems like Examiner can't be put in the csv -- so selected in dropdown and one per file + * + */ diff --git a/php/libraries/Utility.class.inc b/php/libraries/Utility.class.inc index 019472a618d..0bc1c44a986 100644 --- a/php/libraries/Utility.class.inc +++ b/php/libraries/Utility.class.inc @@ -1064,7 +1064,7 @@ class Utility $dxToUpdate = $DB->pselect( "SELECT cde.CandID, cde.DxEvolutionID - FROM candidate_diagnosis_evolution_rel cde + FROM candidate_diagnosis_evolution_rel cde JOIN diagnosis_evolution de USING (DxEvolutionID) JOIN session s ON (cde.CandID=s.CandID AND s.Visit_label=de.visitLabel) WHERE s.ID=:sid", @@ -1082,4 +1082,7 @@ class Utility ); } } + + + } From 54b55367d08e74c2581f17975e2924936a576554 Mon Sep 17 00:00:00 2001 From: Jefferson Casimir Date: Fri, 15 Nov 2024 10:52:03 -0500 Subject: [PATCH 5/8] Added tests + linter fixes --- modules/instrument_manager/jsx/uploadForm.js | 10 +- .../php/instrument_manager.class.inc | 55 ++-- php/libraries/CSVParser.class.inc | 41 +-- php/libraries/InstrumentDataParser.class.inc | 293 ++++++++---------- php/libraries/NDB_BVL_Instrument.class.inc | 29 +- test/unittests.sh | 2 +- test/unittests/CSVParserTest.php | 228 ++++++++++++++ 7 files changed, 437 insertions(+), 221 deletions(-) create mode 100644 test/unittests/CSVParserTest.php diff --git a/modules/instrument_manager/jsx/uploadForm.js b/modules/instrument_manager/jsx/uploadForm.js index 8d6fb005722..83a47909d76 100644 --- a/modules/instrument_manager/jsx/uploadForm.js +++ b/modules/instrument_manager/jsx/uploadForm.js @@ -83,7 +83,7 @@ class InstrumentUploadForm extends Component { title: 'Upload Successful!', type: 'success', text: data.message, - }).then(function() { + }).then(() => { window.location.assign(loris.BaseURL + '/instrument_manager/'); }); } @@ -125,8 +125,10 @@ class InstrumentUploadForm extends Component { title: 'Upload Successful!', type: 'success', text: data.message, - }).then(function() { - // window.location.assign(loris.BaseURL + '/instrument_manager/'); + }).then(() => { + this.setState({ + selectedDataFile: null, + }); }); } else { swal.fire({ @@ -186,8 +188,6 @@ class InstrumentUploadForm extends Component {
- {/*
*/} - {/*
*/}
instrumentExists($instrument)) { - $headers = InstrumentDataParser::getCSVHeaders($this->loris, $instrument); + $headers = InstrumentDataParser::getCSVHeaders( + $this->loris, + $instrument + ); $fileName = "${instrument}_headers.csv"; - $fileObject = new \SplFileObject(sys_get_temp_dir() . "/$fileName", 'w'); + $fileObject = new \SplFileObject( + sys_get_temp_dir() . "/$fileName", + 'w' + ); $fileObject->fputcsv($headers); $downloader = new \LORIS\FilesDownloadHandler( @@ -123,8 +129,9 @@ class Instrument_Manager extends \DataFrameworkMenu * * @return ResponseInterface The PSR15 response for the page. */ - protected function handlePOST(ServerRequestInterface $request): ResponseInterface - { + protected function handlePOST( + ServerRequestInterface $request + ): ResponseInterface { // Ensure the user is allowed to upload. if (! $request->getAttribute('user')->hasPermission( 'instrument_manager_write' @@ -155,8 +162,8 @@ class Instrument_Manager extends \DataFrameworkMenu /** - * Handle a POST request for uploading a new instrument. It moves the uploaded file - * in the project/instruments directory then call the php script from the + * Handle a POST request for uploading a new instrument. It moves the uploaded + * file in the project/instruments directory then call the php script from the * tools directory to generate sql statement required for the installation * of the instrument. Finally, it sources the sql file to complete the * installation. @@ -165,8 +172,9 @@ class Instrument_Manager extends \DataFrameworkMenu * * @return ResponseInterface The PSR15 response for the page. */ - protected function handleInstrumentUpload(ServerRequestInterface $request): ResponseInterface - { + protected function handleInstrumentUpload( + ServerRequestInterface $request + ): ResponseInterface { $uploaded_file = $request->getUploadedFiles()['install_file']; $filename = $uploaded_file->getClientFilename(); $instrument = pathinfo($filename)['filename']; @@ -175,7 +183,9 @@ class Instrument_Manager extends \DataFrameworkMenu if ($this->instrumentExists($instrument)) { // An instrument with this name already exists in the test_names // table. Abort upload. - return new \LORIS\Http\Response\JSON\Conflict(self::ALREADY_INSTALLED); + return new \LORIS\Http\Response\JSON\Conflict( + self::ALREADY_INSTALLED + ); } /* Although only .linst files should be uploaded to this module, and @@ -252,8 +262,9 @@ class Instrument_Manager extends \DataFrameworkMenu * * @return ResponseInterface The PSR15 response for the page. */ - protected function handleDataUpload(ServerRequestInterface $request): ResponseInterface - { + protected function handleDataUpload( + ServerRequestInterface $request + ): ResponseInterface { $data_file = $request->getUploadedFiles()['data_file']; $request_body = (array)$request->getParsedBody(); @@ -277,18 +288,23 @@ class Instrument_Manager extends \DataFrameworkMenu $validData = $dataParser->validateData( $data, [ - 'UserID' => $userID, + 'UserID' => $userID, 'Examiner' => $userID ] ); - $result = $dataParser->insertInstrumentData($this->loris, $validData); + $result = $dataParser->insertInstrumentData( + $this->loris, + $validData + ); return new \LORIS\Http\Response\JSON\Created($result); - } catch (\LorisException | \Exception $e) { - return new \LORIS\Http\Response\JSON\OK([ - 'message' => $e->getMessage() - ]); + } catch (\Exception $e) { + return new \LORIS\Http\Response\JSON\OK( + [ + 'message' => $e->getMessage() + ] + ); } } @@ -359,7 +375,10 @@ class Instrument_Manager extends \DataFrameworkMenu $this->_path = $this->loris->getConfiguration()->getSetting('base'); $instrument_dir = $this->_path . 'project/instruments'; - error_log($instrument_dir . ' writeable? ' . (is_writable($instrument_dir) ? 'Y':'N')); + error_log( + $instrument_dir . ' writeable? ' . + (is_writable($instrument_dir) ? 'Y':'N') + ); return is_writable($instrument_dir); } diff --git a/php/libraries/CSVParser.class.inc b/php/libraries/CSVParser.class.inc index ac899251e2f..6b966ebc2aa 100644 --- a/php/libraries/CSVParser.class.inc +++ b/php/libraries/CSVParser.class.inc @@ -51,16 +51,19 @@ class CSVParser * * @param array $headers Headers parsed from csv file * @param array $expectedHeaders Expected headers (ex: from instrument) - * @param bool $strict If true, $headers cannot be a superset of $expectedHeaders + * @param bool $strict If true, $headers cannot be a superset + * of $expectedHeaders * - * @return bool Returns true if headers are valid + * @return bool Returns true if headers are valid */ - private function _validateHeaders(array $headers, array $expectedHeaders, bool $strict = false): bool - { + private function _validateHeaders( + array $headers, array $expectedHeaders, bool $strict = false + ): bool { if ($strict && count($headers) !== count($expectedHeaders)) { return false; } + echo error_log(json_encode($headers)); $matchingHeaders = true; foreach ($expectedHeaders as $expectedHeader) { if (!in_array($expectedHeader, $headers)) { @@ -75,27 +78,25 @@ class CSVParser /** * Parses the csv file * - * @param array $expectedHeaders Expected headers. Leave empty to skip validation - * + * @param array $expectedHeaders Expected headers. Leave empty to + * skip validation * @param string $separator The field delimiter (one character only). * Defaults as a comma. - * - * @param string $enclosure The field enclosure character (one character only). + * @param string $enclosure The field enclosure character + * (one character only). * Defaults as a double quotation mark - * * @param string $escape The escape character (one character only). - * Defaults as a backslash (\) - * + * Defaults as backslash * @param int $flags Bit mask of the flags to set. * See SplFileObject constants * for the available flags. * Default is: * SplFileObject::READ_CSV | - * SplFileObject::DROP_NEW_LINE - * | SplFileObject::SKIP_EMPTY | - * SplFileObject::READ_AHEADSplFileObject::READ_AHEAD + * SplFileObject::DROP_NEW_LINE | + * SplFileObject::SKIP_EMPTY | + * SplFileObject::READ_AHEAD * - * @return array Returns array of header to row value mapping + * @return array Returns array of header->value mapping * * Example with defaults: * csv file: A,B,C @@ -112,7 +113,8 @@ class CSVParser string $separator = ',', string $enclosure = '"', string $escape = '\\', - int $flags = + int $flags + = SplFileObject::READ_CSV | SplFileObject::DROP_NEW_LINE | SplFileObject::SKIP_EMPTY | @@ -120,12 +122,15 @@ class CSVParser ): array { $fileObject = new SplFileObject($this->csvFile->getRealPath()); $fileObject->setFlags($flags); - $csvHeaders = $fileObject->fgetcsv(); + + $csvHeaders = $fileObject->fgetcsv($separator, $enclosure, $escape); // Validate expected headers, if provided if (count($expectedHeaders) > 0) { if (!$this->_validateHeaders($csvHeaders, $expectedHeaders)) { - throw new \RuntimeException('Headers do not match expected headers'); + throw new \RuntimeException( + 'File headers do not match expected headers' + ); } } diff --git a/php/libraries/InstrumentDataParser.class.inc b/php/libraries/InstrumentDataParser.class.inc index cef80d07c0a..a62118cbc2b 100644 --- a/php/libraries/InstrumentDataParser.class.inc +++ b/php/libraries/InstrumentDataParser.class.inc @@ -17,12 +17,12 @@ namespace LORIS; require_once 'NDB_Factory.class.inc'; require_once 'CSVParser.class.inc'; -use _PHPStan_c875e8309\Nette\Neon\Exception; use DatabaseException; -use NDB_BVL_Battery; +use RuntimeException; use NDB_Factory; use NotFound; use SplFileInfo; +use \LORIS\Data\Dictionary\DictionaryItem as DictionaryItem; /** * This class is used as a helper for parsing csv files @@ -42,8 +42,8 @@ class InstrumentDataParser extends CSVParser /** * Construct parser * - * @param string $instrumentName Instrument name - * @param SplFileInfo $csvFile csv file path + * @param string $instrumentName Instrument name + * @param SplFileInfo $csvFile csv file path * * @return void */ @@ -59,7 +59,7 @@ class InstrumentDataParser extends CSVParser * @param LorisInstance $loris Loris instance * * @return array Returns array of column names as strings - * @throws Exception + * @throws RuntimeException */ public function parseCSV(LorisInstance $loris): array { @@ -67,25 +67,28 @@ class InstrumentDataParser extends CSVParser $expectedHeaders = self::getCSVHeaders($loris, $this->instrumentName); return $this->parse($expectedHeaders); } catch (\Exception $e) { - throw new Exception("An error occurred while parsing: {$e->getMessage()}"); + throw new RuntimeException( + "An error occurred while parsing: {$e->getMessage()}" + ); } } /** * Get expected csv columns for an instrument * - * @param LorisInstance $loris Loris instance - * @param string $instrumentName Instrument name + * @param LorisInstance $loris Loris instance + * @param string $instrumentName Instrument name * * @return array Returns array of column names as strings * @throws NotFound */ - public static function getCSVHeaders(LorisInstance $loris, string $instrumentName): array - { + public static function getCSVHeaders( + LorisInstance $loris, string $instrumentName + ): array { return array_merge( ['PSCID', 'Visit_label'], // SessionID // Convert to static list of flag fields - self::getFlagHeaders( // Whitelist would prevent invalid columns -- write tests + self::getFlagHeaders( // Whitelist would prevent invalid columns [ 'ID', 'SessionID', 'Test_name', 'TestID', 'CommentID', 'UserID', 'Data', 'DataID', @@ -119,14 +122,16 @@ class InstrumentDataParser extends CSVParser /** * Get columns from DB table * - * @param array $excluded Excluded columns. Default is - * single-item array with empty - * string + * @param string $tableName DB table name + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string * * @return array Returns columns from DB table */ - public static function getDBColumns(string $tableName, array $excluded = ['']): array - { + public static function getDBColumns( + string $tableName, array $excluded = [''] + ): array { $db = NDB_Factory::singleton()->database(); $query = "SELECT column_name @@ -157,35 +162,70 @@ class InstrumentDataParser extends CSVParser } /** - * Get expected columns from the instrument's table + * Convert instrument data dictionary to columns * - * @param LorisInstance $loris Loris instance - * @param string $instrumentName Instrument name - * @param array $excluded Excluded columns. Default is - * single-item array with empty - * string + * @param DictionaryItem[] $dataDict Data dictionary + * @param string $instrumentName Instrument name + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string * - * @return array Returns columns from flag table - * @throws NotFound + * @return array Returns instrument columns */ - protected static function getInstrumentHeaders(LorisInstance $loris, string $instrumentName, array $excluded = ['']): array - { - $instrumentType = self::checkInstrumentType($instrumentName); + protected static function convertDictionaryToColumns( + array $dataDict, string $instrumentName, array $excluded = [''] + ): array { + return array_reduce( + $dataDict, + function ($columns, $field) use ($instrumentName, $excluded) { + $fieldName = implode( + '', // Strip first occurrence of prefix + explode("${instrumentName}_", $field->getName(), 2) + ); + if (!in_array($fieldName, $excluded)) { + $columns[] = $fieldName; + } + return $columns; + }, + [] + ); + } - return match ($instrumentType) { -// 'SQL' => self::getDBColumns($instrumentName, $excluded), // TODO: Below should be good for all - 'PHP', 'Instrument Builder' => self::getDataDictionary($loris, $instrumentName, $excluded), - default => self::getDataDictionary($loris, $instrumentName, $excluded), - }; + /** + * Get instrument columns + * + * @param LorisInstance $loris Loris instance + * @param string $instrumentName Instrument name + * @param array $excluded Excluded columns. Default is + * single-item array with empty + * string + * + * @return array Returns instrument columns + * @throws NotFound + */ + protected static function getInstrumentHeaders( + LorisInstance $loris, string $instrumentName, array $excluded = [''] + ): array { + $instrument = \NDB_BVL_Instrument::factory($loris, $instrumentName); + $dataDict = $instrument->getDataDictionary(); + return self::convertDictionaryToColumns( + $dataDict, + $instrumentName, + $excluded, + ); } /** * Validate Instrument data * + * @param array $instrumentData Instrument data from csv row + * @param array $injectedColumns Columns to inject to every row + * * @return array Returns array of column names as strings */ - public function validateData(array $instrumentData, array $injectedColumns): array - { + public function validateData( + array $instrumentData, array $injectedColumns + ): array { try { foreach ($instrumentData as &$dataRow) { // Validate Session @@ -194,27 +234,28 @@ class InstrumentDataParser extends CSVParser $dataRow['Visit_label'] ); $dataRow['SessionID'] = $sessionID; - $dataRow = array_merge($dataRow, $injectedColumns); + $dataRow = array_merge($dataRow, $injectedColumns); } return $instrumentData; } catch (\Exception $e) { - error_log("Invalid data: {$e->getMessage()}"); - return []; + throw new RuntimeException( + "Invalid data: {$e->getMessage()}" + ); } } /** * Get the session ID from PSCID and Visit_label + * TODO: Timepoint/getTimepointFormPSCIDandVisitLabel(,) * - * @param string $pscID candidate PSCID + * @param string $pscID candidate PSCID * @param string $visitLabel session Visit_label * * @return int Returns SessionID - * @throws Exception + * @throws RuntimeException */ public function obtainSessionID(string $pscID, string $visitLabel): int - // TODO: Timepoint/getTimepointFormPSCIDandVisitLabel(,) { $db = NDB_Factory::singleton()->database(); @@ -233,7 +274,7 @@ class InstrumentDataParser extends CSVParser ); if (is_null($sessionID)) { - throw new Exception( + throw new RuntimeException( "Session not found for PSCID: $pscID and Visit_label: $visitLabel" ); @@ -244,29 +285,22 @@ class InstrumentDataParser extends CSVParser /** * Insert instrument data in DB * - * @param LorisInstance $loris Loris instance - * @param array $instrumentData Instrument data to insert + * @param LorisInstance $loris Loris instance + * @param array $instrumentData Instrument data to insert * * @return array Returns insertion results * @throws NotFound */ - public function insertInstrumentData(LorisInstance $loris, array $instrumentData): array - { + public function insertInstrumentData( + LorisInstance $loris, array $instrumentData + ): array { $db = NDB_Factory::singleton()->database(); - - $testID = $db->pselectOneInt( - "SELECT ID - FROM test_names - WHERE Test_name = :testName", - [ - 'testName' => $this->instrumentName, - ] + $instrumentHeaders = $this->getInstrumentHeaders( + $loris, + $this->instrumentName, ); - $instrumentHeaders = $this->getInstrumentHeaders($loris, $this->instrumentName); - - $useInstrumentDataTable = !$db->tableExists($this->instrumentName); - // TODO: INSTEAD USE ->usesJSONData + // TODO: Conditional rollback? $errors = []; try { @@ -274,83 +308,35 @@ class InstrumentDataParser extends CSVParser try { foreach ($instrumentData as $data) { try { - // TODO: addInstrument then _save() (or save() with validation/score -- checkbox) - $data['CommentID'] = NDB_BVL_Battery::generateCommentID( - new \SessionID(strval($data['SessionID'])), - $this->instrumentName, + $battery = new \NDB_BVL_Battery(); + $battery->selectBattery( + new \SessionID(strval($data['SessionID'])) ); - - error_log('trying to save'); - error_log(json_encode(array_reduce( - $instrumentHeaders, - function($columns, $columnName) use ($data) { - $columns[$columnName] = - strlen($data[$columnName]) > 0 - ? $data[$columnName] - : null; - return $columns; - }, - [] - )) + $commentID = $battery->addInstrument( + $loris, + $this->instrumentName ); - $dataID = null; - - if ($useInstrumentDataTable) { - // Insert as JSON string -- TODO: unsafe? - $db->unsafeinsert( - 'instrument_data', - [ - 'Data' => json_encode(array_reduce( - $instrumentHeaders, - function($columns, $columnName) use ($data) { - $columns[$columnName] = - strlen($data[$columnName]) > 0 - ? $data[$columnName] - : null; - return $columns; - }, - [] - )) - ] - ); - $dataID = $db->getLastInsertId(); - } else { - // Insert in instrument's table - $db->insert( - $this->instrumentName, - array_reduce( - $instrumentHeaders, - function($columns, $columnName) use ($data) { - $columns[$columnName] = - strlen($data[$columnName]) > 0 - ? $data[$columnName] - : null; - return $columns; - }, - [] - ), - ); - } - - // Insert flag data - $db->insert( - 'flag', - [ - 'SessionID' => $data['SessionID'], - 'TestID' => $testID, - 'CommentID' => $data['CommentID'], - 'Data_entry' => $data['Data_entry'], - 'Required_elements_completed' => $data['Required_elements_completed'], - 'Administration' => $data['Administration'], - 'Validity' => $data['Validity'], - 'Exclusion' => $data['Exclusion'], - 'UserID' => $data['UserID'], - 'Testdate' => $data['Testdate'], - 'DataID' => $dataID, - ] + $instrument = \NDB_BVL_Instrument::factory( + $loris, + $this->instrumentName, + $commentID + ); + $instrument->_saveValues( + array_reduce( + $instrumentHeaders, + function ($columns, $columnName) use ($data) { + $columns[$columnName] + = strlen($data[$columnName]) > 0 + ? $data[$columnName] + : null; + return $columns; + }, + [] + ) ); - } catch (PDOException $e) { + $instrument->scoreAndPropagateData(); + } catch (\Exception $e) { error_log("Error caught: {$e->getMessage()}"); $errors[] = [ 'row' => $data, @@ -378,12 +364,14 @@ class InstrumentDataParser extends CSVParser ]; } } catch (\Exception $e) { - error_log("Rolling back due to unexpected error: {$e->getMessage()}"); + error_log( + "Rolling back due to unexpected error: {$e->getMessage()}" + ); $db->rollBack(); return [ 'success' => false, 'errors' => [], - 'message' => "Unexpected error: {$e->getMessage()}" + 'message' => "Data error: {$e->getMessage()}" ]; } } catch (DatabaseException $e) { @@ -397,41 +385,6 @@ class InstrumentDataParser extends CSVParser } - /** - * Get columns from NDB_BVL_Instrument - * - * @param LorisInstance $loris Loris instance - * @param string $instrumentName Instrument name - * @param array $excluded Excluded columns. Default is - * single-item array with empty - * string - * - * @return array Returns columns from DB table - * @throws NotFound - */ - public static function getDataDictionary(LorisInstance $loris, string $instrumentName, array $excluded = ['']): array - { - $instrument = \NDB_BVL_Instrument::factory($loris, $instrumentName); - $dataDict = $instrument->getDataDictionary(); - - // TODO: Find out whave _/save needs to works - - return array_reduce( - $dataDict, - function($columns, $field) use ($instrumentName, $excluded) { - $fieldName = $field->getName(); - // Strip prefix - $fieldName = str_replace("${instrumentName}_", '', $fieldName); - if (!in_array($fieldName, $excluded)) { - $columns[] = $fieldName; - } - return $columns; - }, - [] - ); - } - - /** * Determine the instrument type between LINST and PHP * @@ -467,10 +420,8 @@ class InstrumentDataParser extends CSVParser } } - - // TODO: Tech debt /* - * Seems like Examiner can't be put in the csv -- so selected in dropdown and one per file - * + * Seems like Examiner can't be put in the csv -- + * so selected in dropdown and one per file */ diff --git a/php/libraries/NDB_BVL_Instrument.class.inc b/php/libraries/NDB_BVL_Instrument.class.inc index 3d539b2e4ed..7e22e0b5aa7 100644 --- a/php/libraries/NDB_BVL_Instrument.class.inc +++ b/php/libraries/NDB_BVL_Instrument.class.inc @@ -287,6 +287,8 @@ abstract class NDB_BVL_Instrument extends NDB_Page $obj->displayAllFields = true; } + error_log("commentID: $commentID"); + // Set page name to testName $obj->name = $instrument; // Sets up page variables such as $this->commentID and $this->form @@ -550,6 +552,19 @@ abstract class NDB_BVL_Instrument extends NDB_Page return $defaults; } + /** + * Performs post-processing on valid data + * + * @return void + * @access public + */ + function scoreAndPropagateData() + { + $this->score(); + $this->updateRequiredElementsCompletedFlag(); + $this->propagateData(); + } + /** * Attempts to validate the form (using the defined rules) and * saves the validated data into the database @@ -561,9 +576,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page { if ($this->form->validate()) { $this->form->process([&$this, '_saveValues'], true); - $this->score(); - $this->updateRequiredElementsCompletedFlag(); - $this->propagateData(); + $this->scoreAndPropagateData(); } else { $submittedData = $this->form->getSubmitValues(); @@ -2269,7 +2282,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page $conditions = ['tn.test_name=:tn']; $params['tn'] = $this->testName; if ($commentIDs !== null) { - $conditions[] = "CommentID IN + $conditions[] = "CommentID IN (SELECT CommentID FROM load_comment_ids)"; } if ($condition !== null) { @@ -2343,7 +2356,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page . ' '; } $where = 'WHERE ' . join(' AND ', $conditions); - $query = "SELECT + $query = "SELECT session.CandID as CandID, t.CommentID as CommentID, session.Visit_Label as VisitLabel, @@ -2419,8 +2432,8 @@ abstract class NDB_BVL_Instrument extends NDB_Page // for this instrument with current session's visit & project $dxEvolutionID = $DB->pselectOne( "SELECT DxEvolutionID FROM diagnosis_evolution - WHERE instrumentName=:tn - AND ProjectID=:pid + WHERE instrumentName=:tn + AND ProjectID=:pid AND visitLabel=:vl", [ 'tn' => $this->testName, @@ -2448,7 +2461,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page // Get the fields that correspond to a configured diagnosis $sourceFields = $DB->pselectOne( - "SELECT sourceField + "SELECT sourceField FROM diagnosis_evolution WHERE DxEvolutionID=:dxEvID", ['dxEvID' => $dxEvolutionID] diff --git a/test/unittests.sh b/test/unittests.sh index eb27a1f175e..219c1521df1 100755 --- a/test/unittests.sh +++ b/test/unittests.sh @@ -28,7 +28,7 @@ sed -i \ -e "s/%DATABASE%/$database/g" \ config.xml LORIS_DB_CONFIG=$(pwd)/config.xml -export $LORIS_DB_CONFIG +export LORIS_DB_CONFIG if [ $# -eq 2 ]; then # Run specific unit test by specifying test name and file path diff --git a/test/unittests/CSVParserTest.php b/test/unittests/CSVParserTest.php new file mode 100644 index 00000000000..300dc541812 --- /dev/null +++ b/test/unittests/CSVParserTest.php @@ -0,0 +1,228 @@ + + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris + */ +require_once __DIR__ . '/../../vendor/autoload.php'; + +use PHPUnit\Framework\TestCase; +/** + * Unit test for CSV Parser class + * + * @category Tests + * @package Main + * @author Jefferson Casimir + * @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3 + * @link https://www.github.com/aces/Loris + */ +class CSVParserTest extends TestCase +{ + protected const VALID_PASSWORD = 'correct horse battery staple'; + + /** + * Test double for NDB_Config object + * + * @var \NDB_Config | PHPUnit\Framework\MockObject\MockObject + */ + private $_configMock; + + /** + * Test double for Database object + * + * @var \Database | PHPUnit\Framework\MockObject\MockObject + */ + private $_dbMock; + + /** + * NDB_Factory used in tests. + * Test doubles are injected to the factory object. + * + * @var NDB_Factory + */ + private $_factory; + + /** + * Setup + * + * @return void + */ + protected function setUp(): void + { + parent::setUp(); + + $configMock = $this->getMockBuilder('NDB_Config')->getMock(); + $dbMock = $this->getMockBuilder('Database')->getMock(); + '@phan-var \NDB_Config $configMock'; + '@phan-var \Database $dbMock'; + + $this->_configMock = $configMock; + $this->_dbMock = $dbMock; + + $this->_factory = NDB_Factory::singleton(); + $this->_factory->setConfig($this->_configMock); + $this->_factory->setDatabase($this->_dbMock); + } + + /** + * Tears down the fixture, for example, close a network connection. + * This method is called after a test is executed. + * + * @return void + */ + protected function tearDown(): void + { + parent::tearDown(); + $this->_factory->reset(); + } + + /** + * DataProviders for constructor valid CSV files + * + * @return array[] + */ + public function validCSVFiles(): array + { + $filePaths = []; + $filePath = tempnam(sys_get_temp_dir(), 'test_') . '.csv'; + $csvData = <<parse($expectedHeaders); + // Tests: All flags and Field enclosure character (") + $this->assertEquals( + [ + [ + 'header1' => 'value1', + 'header2' => 'value2', + 'header3' => 'value3' + ], + [ + 'header1' => 'value4', + 'header2' => 'value5', + 'header3' => 'value6' + ], + ], + $data + ); + } + + /** + * DataProviders for constructor valid CSV files + * + * @return array[] + */ + public function invalidCSVFiles(): array + { + $filePaths = []; + + // Test invalid file extension (not .csv) + $invalidExtensionFilePath + = tempnam(sys_get_temp_dir(), '_RuntimeException_') . '.csw'; + file_put_contents($invalidExtensionFilePath, ''); + $filePaths[] = [$invalidExtensionFilePath]; + + $incongruentData = << 0) { + $this->expectException($expectedException); + } + + $csvParser = new \LORIS\CSVParser(new SplFileInfo($invalidCSVPath)); + $expectedHeaders = ['header1', 'header2', 'header3']; + $data = $csvParser->parse($expectedHeaders); + $this->assertEquals( + [ + [ + 'header1' => 'value1', + 'header2' => 'value2', + 'header3' => 'value3' + ], + [ + 'header1' => 'value4', + 'header2' => 'value5', + 'header3' => 'value6' + ], + ], + $data + ); + } +} From 0fb265fbb658d38c81d89ee87f2806db9c97bc2a Mon Sep 17 00:00:00 2001 From: Jefferson Casimir Date: Fri, 15 Nov 2024 12:41:19 -0500 Subject: [PATCH 6/8] Remove unwanted changes --- .../jsx/instrumentManagerIndex.js | 26 ++++++------- modules/instrument_manager/jsx/uploadForm.js | 3 +- .../php/instrument_manager.class.inc | 4 -- php/libraries/CSVParser.class.inc | 1 - php/libraries/Database.class.inc | 26 ++++++------- php/libraries/NDB_BVL_Instrument.class.inc | 2 - php/libraries/Utility.class.inc | 37 +++++++------------ 7 files changed, 40 insertions(+), 59 deletions(-) diff --git a/modules/instrument_manager/jsx/instrumentManagerIndex.js b/modules/instrument_manager/jsx/instrumentManagerIndex.js index 5ba1a7f3e88..dadd31410af 100644 --- a/modules/instrument_manager/jsx/instrumentManagerIndex.js +++ b/modules/instrument_manager/jsx/instrumentManagerIndex.js @@ -241,19 +241,19 @@ class InstrumentManagerIndex extends Component { tabs.push({id: 'upload', label: 'Upload'}); } - const feedback = () => {}; - // if (!this.state.data.fieldOptions.caninstall - // && this.props.hasPermission('instrument_manager_write')) { - // return ( - //
- // Instrument installation is not possible given the current server - // configuration; the LORIS 'adminUser' is not configured properly. - // File upload is still possible but instruments will need to be - // installed manually - //
- // ); - // } - // }; + const feedback = () => { + if (!this.state.data.fieldOptions.caninstall + && this.props.hasPermission('instrument_manager_write')) { + return ( +
+ Instrument installation is not possible given the current server + configuration; the LORIS 'adminUser' is not configured properly. + File upload is still possible but instruments will need to be + installed manually +
+ ); + } + }; const uploadTab = () => { let content = null; diff --git a/modules/instrument_manager/jsx/uploadForm.js b/modules/instrument_manager/jsx/uploadForm.js index 83a47909d76..69def8104f1 100644 --- a/modules/instrument_manager/jsx/uploadForm.js +++ b/modules/instrument_manager/jsx/uploadForm.js @@ -245,8 +245,7 @@ InstrumentUploadForm.propTypes = { export default InstrumentUploadForm; /** - * Create a componenet to select permissions from a list of available - * permissions. + * Panel containing upload form * * @param {object} props - react props * @return {JSX.Element} diff --git a/modules/instrument_manager/php/instrument_manager.class.inc b/modules/instrument_manager/php/instrument_manager.class.inc index b5906bb9aca..d1fc122b3c0 100644 --- a/modules/instrument_manager/php/instrument_manager.class.inc +++ b/modules/instrument_manager/php/instrument_manager.class.inc @@ -375,10 +375,6 @@ class Instrument_Manager extends \DataFrameworkMenu $this->_path = $this->loris->getConfiguration()->getSetting('base'); $instrument_dir = $this->_path . 'project/instruments'; - error_log( - $instrument_dir . ' writeable? ' . - (is_writable($instrument_dir) ? 'Y':'N') - ); return is_writable($instrument_dir); } diff --git a/php/libraries/CSVParser.class.inc b/php/libraries/CSVParser.class.inc index 6b966ebc2aa..c1de6ba9f16 100644 --- a/php/libraries/CSVParser.class.inc +++ b/php/libraries/CSVParser.class.inc @@ -63,7 +63,6 @@ class CSVParser return false; } - echo error_log(json_encode($headers)); $matchingHeaders = true; foreach ($expectedHeaders as $expectedHeader) { if (!in_array($expectedHeader, $headers)) { diff --git a/php/libraries/Database.class.inc b/php/libraries/Database.class.inc index 87d225c925e..c348633be37 100644 --- a/php/libraries/Database.class.inc +++ b/php/libraries/Database.class.inc @@ -771,8 +771,8 @@ class Database implements LoggerAwareInterface if (is_null($uniqueKey) || empty($uniqueKey)) { throw new LorisException( - "The pselectWithIndexKey() function expects the uniqueKey parameter - to not be null or empty. If re-indexing on the primary key is + "The pselectWithIndexKey() function expects the uniqueKey parameter + to not be null or empty. If re-indexing on the primary key is not necessary please use the pselect() function instead." ); } @@ -789,9 +789,9 @@ class Database implements LoggerAwareInterface // Check first that the row contains the primary key supplied if (!array_key_exists($uniqueKey, $row)) { throw new DatabaseException( - "The query supplied to pselectWithIndexKey() does not contain - the unique key to re-index on. Make sure to supply the - appropriate key in the SELECT statement to match the supplied + "The query supplied to pselectWithIndexKey() does not contain + the unique key to re-index on. Make sure to supply the + appropriate key in the SELECT statement to match the supplied parameter of this function", $query ); @@ -802,7 +802,7 @@ class Database implements LoggerAwareInterface if (isset($filteredResult[$row[$uniqueKey]])) { throw new DatabaseException( "The uniqueKey supplied to pselectWithIndexKey() does not appear - to be unique or is nullable. This function expects the key to + to be unique or is nullable. This function expects the key to be both UNIQUE and NOT NULL.", $query ); @@ -868,7 +868,7 @@ class Database implements LoggerAwareInterface $colNumber = count($row); if ($colNumber !== 1) { throw new DatabaseException( - "The pselectCol() function expects only one column in the + "The pselectCol() function expects only one column in the SELECT clause of the query, $colNumber were passed.", $query ); @@ -909,7 +909,7 @@ class Database implements LoggerAwareInterface if (is_null($uniqueKey) || empty($uniqueKey)) { throw new LorisException( "The pselectColWithIndexKey() function expects the uniqueKey - parameter to not be null or empty. If re-indexing on the primary + parameter to not be null or empty. If re-indexing on the primary key is not necessary please use the pselectCol() function instead." ); } @@ -927,9 +927,9 @@ class Database implements LoggerAwareInterface // Check first that the row contains the primary key supplied if (!array_key_exists($uniqueKey, $row) || $colNumber !== 2) { throw new DatabaseException( - "The query supplied to pselectColWithIndexKey() should only - contain the unique key and one other column in the SELECT - clause. Make sure to supply the appropriate key in the SELECT + "The query supplied to pselectColWithIndexKey() should only + contain the unique key and one other column in the SELECT + clause. Make sure to supply the appropriate key in the SELECT statement to match the supplied parameter of this function.", $query ); @@ -939,8 +939,8 @@ class Database implements LoggerAwareInterface // in the result array if (isset($filteredResult[$row[$uniqueKey]])) { throw new DatabaseException( - "The uniqueKey supplied to pselectColWithIndexKey() does not - appear to be unique or is nullable. This function expects the + "The uniqueKey supplied to pselectColWithIndexKey() does not + appear to be unique or is nullable. This function expects the key to be both UNIQUE and NOT NULL.", $query ); diff --git a/php/libraries/NDB_BVL_Instrument.class.inc b/php/libraries/NDB_BVL_Instrument.class.inc index 7e22e0b5aa7..ba0f08d6089 100644 --- a/php/libraries/NDB_BVL_Instrument.class.inc +++ b/php/libraries/NDB_BVL_Instrument.class.inc @@ -287,8 +287,6 @@ abstract class NDB_BVL_Instrument extends NDB_Page $obj->displayAllFields = true; } - error_log("commentID: $commentID"); - // Set page name to testName $obj->name = $instrument; // Sets up page variables such as $this->commentID and $this->form diff --git a/php/libraries/Utility.class.inc b/php/libraries/Utility.class.inc index 0bc1c44a986..7fa90d90bb2 100644 --- a/php/libraries/Utility.class.inc +++ b/php/libraries/Utility.class.inc @@ -263,11 +263,10 @@ class Utility } $query = "SELECT DISTINCT Visit_label FROM session s JOIN candidate c ON (s.CandID = c.Candid) - JOIN psc ON (s.CenterID = psc.CenterID) WHERE c.Active = 'Y' AND s.Active = 'Y' AND c.Entity_type != 'Scanner' - AND psc.CenterID!= '1' + AND s.CenterID!= '1' $ExtraProject_Criteria ORDER BY Visit_label"; $result = $db->pselect($query, $qparams); // The result has several columns; we only want the visit labels. @@ -581,24 +580,18 @@ class Utility most accurate Full_name value and should be replaced by getDDEInstrumentNamesList() of the NDB_BVL_Instrument class." ); - $Factory = \NDB_Factory::singleton(); - $DB = $Factory->Database(); - $config = $Factory->config(); - $instruments_q = $DB->pselect( - "SELECT Test_name,Full_name FROM test_names", + $Factory = \NDB_Factory::singleton(); + $DB = $Factory->Database(); + $ddeInstruments = $DB->pselect( + "SELECT DISTINCT test_battery.Test_name, Full_name + FROM test_battery + JOIN test_names ON test_battery.Test_name = test_names.Test_name + WHERE DoubleDataEntryEnabled = 'Y'", [] ); - $doubleDataEntryInstruments = $config->getSetting( - 'DoubleDataEntryInstruments' - ); - - $instruments = []; - foreach ($instruments_q as $row) { - if (isset($row['Test_name']) && isset($row['Full_name'])) { - if (in_array($row['Test_name'], $doubleDataEntryInstruments)) { - $instruments[$row['Test_name']] = $row['Full_name']; - } - } + $instruments = []; + foreach ($ddeInstruments as $instrument) { + $instruments[$instrument['Test_name']] = $instrument['Full_name']; } return $instruments; } @@ -781,11 +774,10 @@ class Utility "SELECT DISTINCT t.Test_name, t.Full_name FROM session s JOIN candidate c ON (c.CandID=s.CandID) - JOIN psc ON (s.CenterID=psc.CenterID) JOIN flag f ON (f.sessionID=s.id) JOIN test_names t ON (f.test_name=t.Test_name) WHERE c.Active='Y' AND s.Active='Y' AND s.Visit_label =:vl - AND psc.CenterID != '1' AND c.Entity_type != 'Scanner' + AND s.CenterID != '1' AND c.Entity_type != 'Scanner' ORDER BY t.Full_name", ['vl' => $visit_label], 'Test_name' @@ -1064,7 +1056,7 @@ class Utility $dxToUpdate = $DB->pselect( "SELECT cde.CandID, cde.DxEvolutionID - FROM candidate_diagnosis_evolution_rel cde + FROM candidate_diagnosis_evolution_rel cde JOIN diagnosis_evolution de USING (DxEvolutionID) JOIN session s ON (cde.CandID=s.CandID AND s.Visit_label=de.visitLabel) WHERE s.ID=:sid", @@ -1082,7 +1074,4 @@ class Utility ); } } - - - } From 731b3558258b2426dc941d817b5a36f0a4c6be7c Mon Sep 17 00:00:00 2001 From: Jefferson Casimir Date: Fri, 15 Nov 2024 14:35:38 -0500 Subject: [PATCH 7/8] Aggregate validation errors + Removal of pasted code --- modules/instrument_manager/jsx/uploadForm.js | 26 +++++++--- .../php/instrument_manager.class.inc | 11 +++- php/libraries/InstrumentDataParser.class.inc | 19 ++++--- php/libraries/Utility.class.inc | 32 +++++++----- test/unittests/CSVParserTest.php | 52 +------------------ 5 files changed, 61 insertions(+), 79 deletions(-) diff --git a/modules/instrument_manager/jsx/uploadForm.js b/modules/instrument_manager/jsx/uploadForm.js index 69def8104f1..cc9cb939071 100644 --- a/modules/instrument_manager/jsx/uploadForm.js +++ b/modules/instrument_manager/jsx/uploadForm.js @@ -26,7 +26,6 @@ class InstrumentUploadForm extends Component { this.getInstrumentOptions = this.getInstrumentOptions.bind(this); this.dataFileSelected = this.dataFileSelected.bind(this); this.uploadInstrumentData = this.uploadInstrumentData.bind(this); - } /** @@ -131,10 +130,17 @@ class InstrumentUploadForm extends Component { }); }); } else { + let message = '
'; + message += `
# Errors: ${data.message.length}

`; + data.message.forEach((error) => { + message += (error + '
'); + }); + message += '
'; + swal.fire({ title: 'No data uploaded', - type: 'warn', - text: data.message, + type: 'warning', + html: message, }); } }) @@ -155,16 +161,17 @@ class InstrumentUploadForm extends Component { } - /** * Renders the React component. * - * @return {JSX} - React markup for the component + * @return {JSX.Element} - React markup for the component */ render() { - const uploadInstrumentDisabled = () => this.state.selectedInstrumentFile === null; + const uploadInstrumentDisabled + = () => this.state.selectedInstrumentFile === null; const instrumentSelected = this.state.selectedInstrument !== ''; - const uploadDataDisabled = () => !instrumentSelected || this.state.selectedDataFile === null; + const uploadDataDisabled + = () => !instrumentSelected || this.state.selectedDataFile === null; return ( <> @@ -225,7 +232,10 @@ class InstrumentUploadForm extends Component { Download Expected Template diff --git a/modules/instrument_manager/php/instrument_manager.class.inc b/modules/instrument_manager/php/instrument_manager.class.inc index d1fc122b3c0..c46019830d4 100644 --- a/modules/instrument_manager/php/instrument_manager.class.inc +++ b/modules/instrument_manager/php/instrument_manager.class.inc @@ -276,7 +276,6 @@ class Instrument_Manager extends \DataFrameworkMenu if (isset($request_body['instrument'])) { $instrument = $request_body['instrument']; if ($this->instrumentExists($instrument)) { - // Parse $dataParser = new InstrumentDataParser( $instrument, @@ -293,9 +292,17 @@ class Instrument_Manager extends \DataFrameworkMenu ] ); + if (count($validData['errors']) > 0) { + return new \LORIS\Http\Response\JSON\OK( + [ + 'message' => $validData['errors'], + ] + ); + } + $result = $dataParser->insertInstrumentData( $this->loris, - $validData + $validData['data'] ); return new \LORIS\Http\Response\JSON\Created($result); diff --git a/php/libraries/InstrumentDataParser.class.inc b/php/libraries/InstrumentDataParser.class.inc index a62118cbc2b..00a14d1a976 100644 --- a/php/libraries/InstrumentDataParser.class.inc +++ b/php/libraries/InstrumentDataParser.class.inc @@ -226,22 +226,27 @@ class InstrumentDataParser extends CSVParser public function validateData( array $instrumentData, array $injectedColumns ): array { - try { - foreach ($instrumentData as &$dataRow) { + $errors = []; + foreach ($instrumentData as &$dataRow) { + try { // Validate Session $sessionID = $this->obtainSessionID( $dataRow['PSCID'], $dataRow['Visit_label'] ); $dataRow['SessionID'] = $sessionID; + + // Inject columns $dataRow = array_merge($dataRow, $injectedColumns); + } catch (\Exception $e) { + $errors[] = $e->getMessage(); } - return $instrumentData; - } catch (\Exception $e) { - throw new RuntimeException( - "Invalid data: {$e->getMessage()}" - ); } + + return [ + 'data' => $instrumentData, + 'errors' => $errors, + ]; } diff --git a/php/libraries/Utility.class.inc b/php/libraries/Utility.class.inc index 7fa90d90bb2..019472a618d 100644 --- a/php/libraries/Utility.class.inc +++ b/php/libraries/Utility.class.inc @@ -263,10 +263,11 @@ class Utility } $query = "SELECT DISTINCT Visit_label FROM session s JOIN candidate c ON (s.CandID = c.Candid) + JOIN psc ON (s.CenterID = psc.CenterID) WHERE c.Active = 'Y' AND s.Active = 'Y' AND c.Entity_type != 'Scanner' - AND s.CenterID!= '1' + AND psc.CenterID!= '1' $ExtraProject_Criteria ORDER BY Visit_label"; $result = $db->pselect($query, $qparams); // The result has several columns; we only want the visit labels. @@ -580,18 +581,24 @@ class Utility most accurate Full_name value and should be replaced by getDDEInstrumentNamesList() of the NDB_BVL_Instrument class." ); - $Factory = \NDB_Factory::singleton(); - $DB = $Factory->Database(); - $ddeInstruments = $DB->pselect( - "SELECT DISTINCT test_battery.Test_name, Full_name - FROM test_battery - JOIN test_names ON test_battery.Test_name = test_names.Test_name - WHERE DoubleDataEntryEnabled = 'Y'", + $Factory = \NDB_Factory::singleton(); + $DB = $Factory->Database(); + $config = $Factory->config(); + $instruments_q = $DB->pselect( + "SELECT Test_name,Full_name FROM test_names", [] ); - $instruments = []; - foreach ($ddeInstruments as $instrument) { - $instruments[$instrument['Test_name']] = $instrument['Full_name']; + $doubleDataEntryInstruments = $config->getSetting( + 'DoubleDataEntryInstruments' + ); + + $instruments = []; + foreach ($instruments_q as $row) { + if (isset($row['Test_name']) && isset($row['Full_name'])) { + if (in_array($row['Test_name'], $doubleDataEntryInstruments)) { + $instruments[$row['Test_name']] = $row['Full_name']; + } + } } return $instruments; } @@ -774,10 +781,11 @@ class Utility "SELECT DISTINCT t.Test_name, t.Full_name FROM session s JOIN candidate c ON (c.CandID=s.CandID) + JOIN psc ON (s.CenterID=psc.CenterID) JOIN flag f ON (f.sessionID=s.id) JOIN test_names t ON (f.test_name=t.Test_name) WHERE c.Active='Y' AND s.Active='Y' AND s.Visit_label =:vl - AND s.CenterID != '1' AND c.Entity_type != 'Scanner' + AND psc.CenterID != '1' AND c.Entity_type != 'Scanner' ORDER BY t.Full_name", ['vl' => $visit_label], 'Test_name' diff --git a/test/unittests/CSVParserTest.php b/test/unittests/CSVParserTest.php index 300dc541812..edad1825b29 100644 --- a/test/unittests/CSVParserTest.php +++ b/test/unittests/CSVParserTest.php @@ -25,30 +25,6 @@ */ class CSVParserTest extends TestCase { - protected const VALID_PASSWORD = 'correct horse battery staple'; - - /** - * Test double for NDB_Config object - * - * @var \NDB_Config | PHPUnit\Framework\MockObject\MockObject - */ - private $_configMock; - - /** - * Test double for Database object - * - * @var \Database | PHPUnit\Framework\MockObject\MockObject - */ - private $_dbMock; - - /** - * NDB_Factory used in tests. - * Test doubles are injected to the factory object. - * - * @var NDB_Factory - */ - private $_factory; - /** * Setup * @@ -57,18 +33,6 @@ class CSVParserTest extends TestCase protected function setUp(): void { parent::setUp(); - - $configMock = $this->getMockBuilder('NDB_Config')->getMock(); - $dbMock = $this->getMockBuilder('Database')->getMock(); - '@phan-var \NDB_Config $configMock'; - '@phan-var \Database $dbMock'; - - $this->_configMock = $configMock; - $this->_dbMock = $dbMock; - - $this->_factory = NDB_Factory::singleton(); - $this->_factory->setConfig($this->_configMock); - $this->_factory->setDatabase($this->_dbMock); } /** @@ -80,7 +44,6 @@ protected function setUp(): void protected function tearDown(): void { parent::tearDown(); - $this->_factory->reset(); } /** @@ -152,7 +115,7 @@ public function invalidCSVFiles(): array file_put_contents($invalidExtensionFilePath, ''); $filePaths[] = [$invalidExtensionFilePath]; - $incongruentData = << Date: Fri, 15 Nov 2024 15:54:47 -0500 Subject: [PATCH 8/8] Removed unused function --- php/libraries/InstrumentDataParser.class.inc | 35 -------------------- 1 file changed, 35 deletions(-) diff --git a/php/libraries/InstrumentDataParser.class.inc b/php/libraries/InstrumentDataParser.class.inc index 00a14d1a976..65202286deb 100644 --- a/php/libraries/InstrumentDataParser.class.inc +++ b/php/libraries/InstrumentDataParser.class.inc @@ -388,41 +388,6 @@ class InstrumentDataParser extends CSVParser ]; } } - - - /** - * Determine the instrument type between LINST and PHP - * - * @param string $instrument The instrument name - * - * @return string The instrument type - */ - public static function checkInstrumentType(string $instrument): string - { - // TODO: COPIED FROM modules/instrument_manager/php/instrumentrow.class.inc - $db = NDB_Factory::singleton()->database(); - - if ($db->tableExists($instrument)) { - return 'SQL'; - } - - $_factory = \NDB_Factory::singleton(); - $_path = $_factory->config()->getSetting("base"); - - $linst = $_path . "/project/instruments/$instrument.linst"; - if (file_exists($linst)) { - return 'Instrument Builder'; - } - - $php = $_path . - "/project/instruments/NDB_BVL_Instrument_$instrument.class.inc"; - - if (file_exists($php)) { - return 'PHP'; - } - - return 'Missing'; - } } // TODO: Tech debt