Skip to content
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

[Instrument] Remove exception from NDB_BVL_Instrument::factory #8290

Merged
merged 3 commits into from Feb 21, 2023

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Dec 20, 2022

This removes the exception from NDB_BVL_Instrument::factory(), so that it's possible to call hasAccess on an instrument. Currently the hasAccess or Factory call mix concerns, making it difficult for a module to check if a user has access to an instrument.

@driusan driusan added Cleanup PR or issue introducing/requiring at least one clean-up operation php Pull requests that update Php code labels Dec 20, 2022
@driusan driusan force-pushed the InstrumentSeparateAccessCheck branch from be27b1f to 783e387 Compare December 20, 2022 14:53
@driusan driusan added the Blocking PR should be prioritized because it is blocking the progress of another task label Dec 21, 2022
@ridz1208 ridz1208 self-assigned this Feb 14, 2023
@driusan driusan force-pushed the InstrumentSeparateAccessCheck branch from 783e387 to 473bfea Compare February 15, 2023 18:52
@@ -252,6 +252,11 @@ class Unresolved extends Endpoint implements ETagCalculator
'',
true
);
if ($instrument->_hasAccess($user) == false) {
return new \LORIS\Http\Response\JSON\NotFound(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not Found?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

This removes the exception from NDB_BVL_Instrument::factory(), so that
it's possible to call hasAccess on an instrument. Currently the hasAccess
or Factory call mix concerns, making it difficult for a module to check
if a user has access to an instrument.
@driusan driusan force-pushed the InstrumentSeparateAccessCheck branch from 473bfea to a4784be Compare February 21, 2023 19:12
Copy link
Collaborator

@ridz1208 ridz1208 left a comment

Choose a reason for hiding this comment

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

one last comment

@driusan driusan merged commit 3929f99 into aces:main Feb 21, 2023
@ridz1208 ridz1208 added this to the 25.0.0 milestone Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocking PR should be prioritized because it is blocking the progress of another task Cleanup PR or issue introducing/requiring at least one clean-up operation php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants