[instruments] Save value into UserID field

[zaliqarosli]

Brief summary of changes

Currently, nothing is ever saved into the UserID field of instruments. This PR fixes that by inserting into the field the user's Real_name.

Context: Project require to render the Name of the Data Entry personnel on the instrument page on 21.0-release.

Testing instructions (if applicable)

1. On main branch, go to any instrument page. Complete an instrument and save. Check the Data column of flag table and the UserID column of the instrument table. You will see that the UserID data is empty / doesn't exist. There is a UserID column of flag that has data in it, however that is specific to the session, not the instrument data entry.

2. On this PR branch, repeat the above step and notice the UserID in the instrument table will now be populated with your user account username, as well as the UserID key in the JSON data of flag table Data column.

3. On this PR branch, run php tools/single_use/SaveUserIDToInstrumentData.php to back populate all the UserID fields with data queried from the history table. Run without the confirm flag to check that the correct data has been queried and printed. Run php tools/single_use/SaveUserIDToInstrumentData.php confirm to apply the changes.

Link(s) to related issue(s)

• Resolves # (Reference the issue this fixes, if any.)

[ridz1208]

but why???? it's tradition !!

[driusan]

Why would the real name go in the UserID column?

[zaliqarosli]

UserID column doesn't have a reference to the user table. it's set as just a varchar. Putting the full name and not the user table id will let the user's actual name to be rendered as is, without having to query the user table. i find this useful for linst instruments where you can just grab the value of the UserID directly. it's the same case with the UserID column of the flag table, except that the value there only contains the user's first name. in this case, its more information to have the full name

[driusan]

The users table has a unique key named "UserID". The column is clearly supposed to be a reference to that and using a different column is confusing and will lead almost certainly lead to bugs down the line.

[xlecours]

UserID

The issue I am foreseeing would occur if a user's Full Name changes. We would need to update all the values in the flag table as well. But no one will remember that so we would need to guess who that was; hopefully the names will be similar.

[zaliqarosli]

UserID

The issue I am foreseeing would occur if a user's Full Name changes. We would need to update all the values in the flag table as well. But no one will remember that so we would need to guess who that was; hopefully the names will be similar.

that is a good point!

[zaliqarosli]

hey @driusan @ridz1208 i made a script for myelineurogene to back populate the missing userIDs with data queried from the history table. is this something you guys think would be useful for loris core / other projects? if yes, i can add it to this PR. i wouldn't mind some help from loris core reviewing it either

[driusan]

Yes, if we have the data and can restore it from the history table it seems like it would be a good idea to include the script

[PapillonMcGill]

@ridz1208 @driusan Could you review? This is blocking fixes on Myelineurogene.
Thanks

[zaliqarosli]

@PapillonMcGill its actually waiting on my to update, and then review by xavier

[xlecours]

@zaliqarosli I did not tested it yet but I really like the way it looks. :)

[zaliqarosli]

i made a couple of changes i noticed needed fixing @xlecours.

ive tested this on myelineurogene staging and it works as intended/expected. could we see if someone can test this on raisinbread data or maybe another project's data? @ridz1208 @driusan

[ridz1208]

@pierre-p-s please test asap

[zaliqarosli]

@pierre-p-s let me know if you have questions or run into any trouble

[pierre-p-s]

On CCNA DB: The first part works but for the back population of the UserID field I am getting the error below:

Any idea of what I am doing wrong please?

[ridz1208]

@pierre-p-s can you provide your SQL version ?

[pierre-p-s]

@pierre-p-s can you provide your SQL version ?

Server version: 10.1.46-MariaDB MariaDB Server

[pierre-p-s]

The first part works correctly.

The script is taking a long time to run on the history table of CCNA

[pierre-p-s]

Sorry it took a really long time to run the script.

It seems to be working for the most part, however I ran into an issue where the script stops because in a table on CCNA there is no UserID field. Otherwise I checked other instruments and everything seemed to be fine with the UserID entered correctly.

[ridz1208]

@zaliqarosli can you add the columnExists check ?

[zaliqarosli]

@ridz1208 is there a way to check whether UserID key in Data column should exist? for instrument that don't use SQL tables

[ridz1208]

@zaliqarosli you could by loading the instrument and parsing its form content but I would say thats an overkill. I think you can just add it to the data element by default.

[ridz1208]

@pierre-p-s you can test again

[pierre-p-s]

Tested again on CCNA. No visible PHP errors this time.

However I noticed that on some candidates, the UserID in flag does not correspond to the one in the instrument table (I have 2 instances where UserID is lorisadmin in the instrument table but it is [email protected] in the flag table).

Side note: on CCNA DB it takes around 14 hours to run the script. Would it be possible to add a comment at the begining of the script for the projects to be aware of this please?

[ridz1208]

@pierre-p-s have you noticed any other users saved incorrectly ? or is it only sometimes lorisadmin ? have you made sure the entry in history for that instrument is not lorisadmin itself ?

[pierre-p-s]

No, I have noticed this on 2 users.

Wouldnt it save lorisadmin as well in the flag table if the UserID in the history table was lorisadmin?

[zaliqarosli]

this is weird to me because the NDB_BVL_Instrument _save() function should be saving the same value to the instrument and flag table. could it be that the UserID value in flag was already there and the value wasn't overwritten? just thinking about possiblities

[ridz1208]

@zaliqarosli... highly unlikely, there is no logic for that anywhere. its very weird

[zaliqarosli]

@pierre-p-s is the UserID key in the Data object of flag table at the very end of the JSON?

[pierre-p-s]

this is weird to me because the NDB_BVL_Instrument _save() function should be saving the same value to the instrument and flag table. could it be that the UserID value in flag was already there and the value wasn't overwritten? just thinking about possiblities

You are right we have an override on CCNA to save to the flag table.

[zaliqarosli]

@pierre-p-s can you confirm that the CCNA override is changing the script's function in a way that is expected? i.e. this script works as intended given no overrides? if you have any other suggestions for this script that you think might be beneficial for projects, please let me know! i'll add the comment warning about runtime

[zaliqarosli]

@ridz1208 @driusan is this still going to the correct branch?

[ridz1208]

@driusan The fix here is functional and @zaliqarosli did her best to make the "recovery" script be compatible with other projects. I don't think we should delay this more. I think any project with overrides should make the effort to accommodate the script to their codebase/database

[zaliqarosli]

@ridz1208 @driusan i found that I have to change the query a bit when running this script on myelineurogene https://github.com/aces/myelineurogene/pull/250/files

is this worth pushing to loris core?

[driusan]

That repo isn't public and the link is a 404.

[zaliqarosli]

@driusan

[ridz1208]

before SQL