[bvl_feedback] Feedback summary and thread list only viewable with access_all_profiles permission #7190
Assignees
Labels
24.0.0-bugs
Issues or bug fix PRs that were raised during the testing of release 24.0.0
Bug
PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label)
Comments
|
I'm confused now whether this is intentional ? It seems like only someone with across all sites should be able to view summary of threads: as well as list of threads: |
|
I don't know.. I just did a git blame to try and see the history of it and that line dates from the initial commit of LORIS.. |
|
@jesscall yes, but anyone without that permission should be able to view threads created at their own site. I don' t know much about the module but is there a way to compute what thread was created by who? or at which site ? in that case I would say current behaviour is the expected one... and we can make an improvement by addind a check for the logged in user's sites Alternatively we can amend the testpln to specifically mention the required permission @driusan ? |
|
The thread has a UserID that created it and one of: 1. A CandID 2. a SessionID 3. A CommentID (the rest of which are null). I would say the CandID/SessionID/CommentID is more accurate for determining permissions than the user |
|
So would we proceed as follows: If user has site affiliation to at least one cand site, they should have access? |
|
I think this risks getting convoluted since it's either the CandID OR the SessionID OR the CommentID set. You're also going to lose project permissions if you just modify the query . I think the way to do it right would be to implement the |
|
This seems more like a new feature than a bugfix considering it's been using If it's okay with you both, I'll remove this from |
|
Fine with me. |
|
fine by me as well (not that you waited for my approval) |
14 tasks
1 task
|
working on this one. the solution given seems to be working. Will do a bit more of testing. |
zaliqarosli
added a commit
to zaliqarosli/Loris
that referenced
this issue
Jun 15, 2023
* [Issue tracker] - fix can't Delete attachment (aces#8337) Allow users to delete their own attachment. If a user does not have Issue Tracker: Close/Edit/Re-assign/Comment on All Issues, they cannot currently delete their own attachment. Resolves aces#8006 * [configuration] Fix error messages & saving with null Alias (aces#8349) This prevents the user from saving a Project configuration with a null Alias in a new project as well as existing projects. * [webpack] remove util shortcut as it is unusedand causes conflicts (aces#8634) The util shortcut seems to be completely unused in the code, the only place using the content of that directory references the whole path of the directory Resolves aces#8577 * [Tools] double_escape_report tool breaking on non-string values (aces#8484) This tool tries to run pregmatch on every value pulled from the instrument Data. there is a possibility that values coming from json_decode() are decoded into floats or arrays if that is how they have been saved. if it's the case the script should just skip them and not fail * [issue tracker] Populate reporter dropdown (aces#8469) In the issue tracker module, the reporter dropdown is populated with the assignee users instead of the reporter users. This fills the reporter dropdown with the right reporter values. Fixes aces#8429 * [examiner] Column for 'Instrument' in certification menu appears narrow (aces#8453) This is changing the CSS class used in the examiner page. The former class had a small percentage of the width causing the first field become narrow, by changing it to another class, the problem is solved. Fixes aces#8026 * [tools] support JSON instruments in fix_candidate_age.php (aces#8286) Fixes aces#8090 * [bvl_feedback] Fix Permissions for Feedback Summary & Thread List (aces#7826) Currently, only users with the access_all_profiles permission can see Feedback Threads and Open Thread Summary at the profile level. Even if a user adds a feedback entry, they won't be able to see their own feedback thread unless they have the access_all_profiles permission. This change allows users to see threads and the summary that exists for candidates that they have access to (i.e., if a user is affiliated with MTL, they can now see the feedback threads & summary of MTL candidates). Fixes aces#7190 * [new_profile/sex] Remove strtolower() and force uppercase (aces#8633) The keys in the new_profile module were lower case which forces the Sex library class to use a strtolower() function to validate the value and then submits the lowercase value in the SQL insert statement where SQL implicitly converts it to uppercase. This workflow is very risky as different versions of SQL or different databases may not recognise the lowercase and uppercase as the same word and treat it as a truncation. This is also simply bad practice and unnecessary here. * [NDB_BVL_Feedback] Fixing 500 error on instrument list page (aces#8694) A recent change (aces#7826) to the NDB_BVL_Feedback class causes the following 500 Error to appear when the instrument_list page is loaded. This is because references are made to Timepoint::singleton instead of TimePoint::singleton. Fix casing. * [Login] Case insensitive comparison of Authentication header The HTTP spec says the header name is case-insensitive. Some clients send it as "authentication" (lowercase). This makes our check for the header case-insensitive by lower-casing the headers before doing the comparison. * Fix PHPCS (aces#8719) * Fix nonaggregated column error in mri_violation provisioner (aces#8716) Fixes aces#8705, Fixes aces#8697 * [EEG Uploader] Handle Checksum value (aces#8729) Include checksum in value saved to database. * [instruments] Fix properly disabling/enabling 'Delete instrument data' button display (aces#8686) Currently, if the 'InstrumentResetting' config is set to 'No', the 'Delete instrument data' button is displayed and the user is able to clear the instrument. This fixes that by letting the if statement check a true boolean instead of string "true" by calling 'settingEnabled'. The PR also modifies the testing plan so this is tested in the future. * [electrophysiology_uploader] Added 'Help' content for the elctrophysiology_uploader module (aces#8728) Added 'Help' content for electrophysiology_uploader module. Fixes aces#8597 * [module_manager] Visibility of 'My Preferences' in menu reflects state (aces#8726) The 'My Preferences' menu item is no longer visible if the my_preferences module is not Active. Fixes aces#8695 * [Media] Fix Input callback not set (aces#8720) Add a onUserInput prop to the "Update File" ButtonElement Fixes aces#8700 * [battery_manager] React warning messages in console (aces#8724) A few number values were being passed as Strings in the props to Form elements. Change them to be numbers. Fixes aces#8703 * [user_accounts] Update TestPlan wording (aces#8718) This changes the wording slightly in the TestPlan of user_accounts to indicate that an email is only sent to a new user if the "Send email to user" button is selected. Fixes aces#6971 * 24.1 to 25.0 release patch (aces#8715) Combine files in the New_patches into one release file. * [document_repository] Include steps to test for multiple files (aces#8758) Updated some steps to include the newly added support for uploading multiple files at once. For testing assignment aces#8519. * [document_repository] Update help content (aces#8761) Added reference to new feature to support uploading multiple files at once. * [user_accounts] Update TestPlan.md (aces#8754) The pwned password example was not complex enough to pass the complexity check and trigger the pwned check, so change the example to another one which is more secure but also in a pwned database. * [acknowledgements] Update test plan (aces#8763) Correct step about clearing filters wording and add step to check the citation policy works. Fixes aces#8736 * [instrument & instrument_manager] test plans update (aces#8765) Transfer the permission test step from instruments module to instrument_manager module, since it is now managed there instead of in the config.xml * [dashboard] Load project description from ajax and run through DOMPurify (aces#8762) This makes sure the Project Description on the dashboard runs through DOMPurify. In order to do that, it was also necessary to move the description from being loaded in a smarty template to being loaded from an AJAX call (so that we can import the DOMPurify module.) Fixes aces#8750 * [configuration] Use unsafeInsert/unsafeUpdate for saving values (aces#8759) The values get double-escaped when modified now if they contain HTML. Use unsafe variants of database calls so that the values to not get modified when re-saved. Fixes aces#8748 * [new_profile] Fix date requirements and formats with EDC (aces#8767) Fixed NewProfileIndex to use the correct variable containing the EDC date To create a candidate, the date of birth for a candidate is now required only when useEDC = no in Candidate.class.inc. If useEDC = yes, DoB is null unless it's specified. Added the validation of the EDC date format If the Ym format is selected in the configuration, added a '-15' to the end of the EDC date to be compatible with the SQL type date Resolves aces#8742 * [Media] Fix warning invalid prop options (aces#8722) Update the prop option hiddenOptions for the fileVisibility SelectElement to be an object instead of an array. Resolves aces#8696 * [Survey] Fix loading of survey instruments (aces#8647) The module search path for survey instruments was incorrect. Set to the same search path as in tools/generic_includes.php Resolves aces#8546 * [new_profile] Fix Date Picker (aces#8775) Removed the legacy JQuery month picker, without it the issue is gone. Resolves aces#8753 * [api] Test multiple versions (aces#8778) This attempts to add tests for multiple versions of the API. The tests from v0.0.4-dev are copied to v0.0.3 classes and the version in the new classes changed to v0.0.3. * [survey_accounts] Fix help text (aces#8789) Replace `<br>` tag with ` \n` as per markdown spec in order to fix rendering of help text. * [Timepoint List] Hide visits that are from user unaffiliated projects (aces#8723) Adjusted the Timepoint_List module to also filter out the visits of projects that are not affiliated to the current user when they have permission to all sites. Resolves aces#8710 * [Candidate] Fix wrong comments to avoid confusion in future (aces#8791) The middle day of the month is used when date format is Y-M, not the first. --------- Co-authored-by: Shen <[email protected]> Co-authored-by: CamilleBeau <[email protected]> Co-authored-by: Rida Abou-Haidar <[email protected]> Co-authored-by: regis <[email protected]> Co-authored-by: miladheshmati <[email protected]> Co-authored-by: Suzanne Lee <[email protected]> Co-authored-by: Alexandra Livadas <[email protected]> Co-authored-by: Dave MacFarlane <[email protected]> Co-authored-by: Laetitia Fesselier <[email protected]> Co-authored-by: jeffersoncasimir <[email protected]> Co-authored-by: charlottesce <[email protected]> Co-authored-by: Saagar Arya <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Issue originally reported by @racostas on CCNA Repo during candidate release testing of
LORIS 23.0.2aces/CCNA#4269
Not consistent behaviour of bvl_feedback at the profile level when the user has the permission Behavioural QC but not the Across all sites access candidate profiles one.
How to test:
(take a
user having the permission Behavioural QCbutnot the Across all sites access candidate profiles (neither the superadmin one))MainMenu->Canditate-> Access Profile.bvl_feedbackpanel clicking on the notepad icon in the left part of the Main Menu bar (at the left of the help button sign)New profile level feedback tab(middle one)(The new thread has been submitted!)but the upperFeedback Threadsand bottomFeedback Threadstabs remains unchanged . (if the same process is done with asuperuseraccount or auser having at the time Behavioural QC but and Across all sites access candidate profiles these two tabs are populated. [Please note that the not shown info is nevertheless stored]Behavioural Feedback Notifications(the same not shown in step 5)bvl_feedback button(the notepad icon) the feedback is not shown (step 5).NOTE: Please note that this issue is not observed when in the same conditions the user is providing the feedback at the
Visit or Instrument levelThe text was updated successfully, but these errors were encountered: