-
Couldn't load subscription status.
- Fork 186
[instrument_manager] Parse Instrument Data from CSV file #9472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[instrument_manager] Parse Instrument Data from CSV file #9472
Conversation
|
From Nov 19 LORIS call:
|
b50171d to
73f26b9
Compare
e334448 to
dfecef0
Compare
87fd447 to
1982dff
Compare
578a57f to
9a9d471
Compare
a34a08c to
46ee819
Compare
fb3ad42 to
ebf43b4
Compare
| if (is_null($dictionary)) { | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a bug fix ?
If I got it correctly, it seems to be handling for cases where we have select / radio buttons with no option. The whole function is rather cryptic and lacking in comments IMO, and could benefit from a rewrite, but if this simple fix works, then that is also fine with me.
Anyway, if this is a bug fix as I said, could you split this into its own PR ?
| @@ -0,0 +1,216 @@ | |||
| <?php declare(strict_types=1); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(commenting code to allow thread discussion)
I only reviewed the REDCap-related parts of the PR, the whole part about parsing REDCap instrument CSV files and parsing them to LINST seems very similar to Regis' redcap2linst.php script, and I am worried this introduces significant duplication which will be hard to maintain and may display inconsistencies.
Also, this PR introduces a RedcapCSVParser.class.inc class in libraries, I would prefer that all the REDCap-related stuff, even libraries, go into the REDCap module, as to not bloat the shared libraries and keep the architecture more predictable and modular.
Closes #9341
Brief summary of changes
Contains a new
CSVParserclass, extended by anInstrumentDataParserclass.A template file with the expected headers for the selected instrument can be downloaded from the new "Upload Instrument Data" panel's form in the Upload tab of the Instrument Manager module.
The uploaded .csv file will be saved and tracked in a new table called
instrument_data_files.The file's headers must match the selected instrument's expected headers.
For each row, an instrument is created using the
NDB_BVL_Instrument::factoryand is then saved to the database.If an error is detected, the database transaction will be rolled back, and the list of errors will be returned to the user.
If the uploaded file exceeds a certain size (currently hard-coded to 512 MB), the task will be performed in the background as a new
server_process_managertype calledparse_instrument_data. This process runs theparse_instrument_data.phpscript. It is expected that themonitor_instrument_data.phpscript is run periodically to check the status of running tasks. When a task is done executing, theserver_process_managertable is updated and the user who uploaded the file receives an email containing the "computed exit text".Testing instructions
MAX_FILE_BYTESMriUploadServerProcessstill works as expectedScreenshots:






New features since writing:
redcap{@}truein .meta)Still unresolved: