aces / Loris

LORIS is a web-accessible database solution for longitudinal multi-site studies.
GNU General Public License v3.0
143 stars 172 forks source link

[SQL][Instrument List][Battery Manager] Move DDE Enabled to Test_Battery #9264

Open skarya22 opened 4 months ago

skarya22 commented 4 months ago

Brief summary of changes

image

Testing instructions (if applicable)

  1. Confirm that the sql patch copies over the selected DDE instruments to the test_battery field properly
  2. Test all affected tools and confirm they still work as intended
  3. Confirm that the correct tests show the DDE option in instrument list for the visit-instrument tuple as defined in test_battery
ridz1208 commented 4 months ago

@driusan removing from release unless you wanted it for 26 ?

driusan commented 4 months ago

No, I thought I had already removed it from the release.

skarya22 commented 2 months ago

Move from Instrument_Manager to Battery_Manager

driusan commented 4 weeks ago

That stack trace has a project/libraries/NDB_BVL_Battery.class.inc file in it. Are you sure you don't have overrides that are breaking @skarya22's change?

skarya22 commented 4 weeks ago

@shonibare from the stack trace it looks like the problem is that there is likely an override like @driusan mentioned in NDB_BVL_Battery.class.inc, specifically here:

image

$config->getSetting("DoubleDataEntryInstruments") should no longer be present in the function, but the stack trace has "Config setting DoubleDataEntryInstruments does not exist in database"

shonibare commented 4 weeks ago

@skarya22 could you take a look at this function https://github.com/aces/Loris/blob/b5e10cabd89fa6efed76a0b529f446a414c9b641/php/libraries/NDB_Config.class.inc#L430

skarya22 commented 3 weeks ago

@shonibare That function is alright, other parts of LORIS need it, however it should not be called with "DoubleDataEntryInstruments" as the name since that was removed from Config

shonibare commented 3 weeks ago

@shonibare That function is alright, other parts of LORIS need it, however it should not be called with "DoubleDataEntryInstruments" as the name since that was removed from Config

@skarya22 is it not breaking on your VM? cos I did fetch your PR and ran the patch. I also searched for this DoubleDataEntryInstruments in the files in your PR in my VM but didn't find it.

skarya22 commented 3 weeks ago

Yeah it works for me

image
CamilleBeau commented 1 week ago

Confirmed that this is working when I check out Saagar's PR, run the patch, and compile: image

I also tested the other relevant modules and those all loaded also

skarya22 commented 1 week ago

@driusan I think I made the necessary adjustments. I could not find anything in the swagger documentation to update however. 0.0.4-dev: image

0.0.3: image