Skip to content

Conversation

@skarya22
Copy link
Contributor

@skarya22 skarya22 commented Jul 9, 2025

Brief summary of changes

  • Created a function in the User class to getVisits that the user can access based on the Projects they have access to
  • I noted that the getFieldOptions function in candidate_list/php/candidate_list.class.inc has no affect on anything but the version in options.class.inc is used instead. I deleted the contents of the function because it was code duplication and was confusing
  • Note: There is an issue in raisinbread where aosi has TestID 6 while it should have TestID 5. When you switch to this branch you will not see aosi unless if you refresh raisinbread or run update test_names set ID = 5 where Test_name = 'aosi'

A user with just Project Rye
image
image
image

Testing instructions (if applicable)

  1. Create a User with access to one project
  2. Open the dataquery module and select Modify Fields
  3. Go to select a category and see that you can no longer see instruments that are not relevant to your user
  4. Select an instrument and see that the default visits is only populated by visits that are in projects your user has access to
  5. Do the same in the dictionary module to confirm you only see instruments you have access to by selecting Instruments in Modules, and then go to the Category filter
  6. Confirm the same behaviour in the filters for Candidate List and Behavioural_QC module

Link(s) to related issue(s)

@github-actions github-actions bot added Language: PHP PR or issue that update PHP code Module: behavioural_qc PR or issue related behavioural_qc module Module: candidate_list PR or issue related to candidate_list module Module: dictionary PR or issue related to (new) dictionary module Module: dataquery PR or issue related to (new) dataquery module labels Jul 9, 2025
@github-actions github-actions bot added Language: SQL PR or issue that update SQL code RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset labels Jul 9, 2025
@skarya22 skarya22 changed the title [dataquery] Remove instruments and visits that the user doesnt have access to [dataquery/dictionary] Remove instruments and visits that the user doesnt have access to Jul 9, 2025
* @return array of study visits in the format array('VL' => 'VL')
* where VL is the visit label
*/
public function getVisits(): array
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skarya22 this is purely out of personal preference and not at all blocking:

I would use this opportunity to create a getVisits() function in the Project class which knows how to get the visits itself instead of doing these loops through projects...cohorts...visits.

That being said, the efficiency of the code below is basically one query per project per cohort, if you choose to add the function to the project class, you could choose to use getVisitsProjectCohort() from the VisitController class to make it an efficiency of one query per project. ORR if you choose not to create the Projec::getvisits() function you could use getVisitsProjectCohort() from the VisitController class here directly to make it an efficiency of just 1 query!

I'll leave it up to you

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ridz1208
Copy link
Collaborator

@skarya22 code looks good!

Instruments are fetched from the session/flag table so that's the correct list
Visits are fetched from visit_project_cohort_rel - all good

One comment would be that I'm not sure any of your changes address the visit list displayed next to the field itself when it's chosen. I think the scenario I'm thinking of is, an instrument is administered at multiple visits by different projects (even think of categories like imaging that apply to all visits) when the user selects the category, even though the default visits will be narrowed down to the user's list, the visits for each field still show the entire list of visits for which data exists. If I'm not mistaken ofcourse

@skarya22 skarya22 self-assigned this Jul 30, 2025
@skarya22 skarya22 added the State: Needs work PR awaiting additional work by the author to proceed label Jul 30, 2025
@github-actions github-actions bot added Language: Javascript PR or issue that update Javascript code Module: statistics PR or issue related to statistics module Module: candidate_parameters PR or issue related to candidate_parameters module Module: electrophysiology_uploader PR or issue related to electrophysiology_uploader Module: imaging_browser PR or issue related to imaging_browser module Module: imaging_uploader PR or issue related to imaging_uploader module Module: instruments PR or issue related to instruments module labels Aug 13, 2025
@skarya22 skarya22 removed their assignment Aug 18, 2025
@skarya22 skarya22 removed the State: Needs work PR awaiting additional work by the author to proceed label Aug 18, 2025
Copy link
Contributor

@GeorgeMurad GeorgeMurad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@driusan driusan merged commit 7c5810a into aces:main Oct 30, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language: Javascript PR or issue that update Javascript code Language: PHP PR or issue that update PHP code Language: SQL PR or issue that update SQL code Module: behavioural_qc PR or issue related behavioural_qc module Module: candidate_list PR or issue related to candidate_list module Module: candidate_parameters PR or issue related to candidate_parameters module Module: dataquery PR or issue related to (new) dataquery module Module: dictionary PR or issue related to (new) dictionary module Module: electrophysiology_uploader PR or issue related to electrophysiology_uploader Module: imaging_browser PR or issue related to imaging_browser module Module: imaging_uploader PR or issue related to imaging_uploader module Module: instruments PR or issue related to instruments module Module: statistics PR or issue related to statistics module RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[dataquery] Real user Feedback

5 participants