dev/core#2244 Simplify and consistently apply checking of whether financial acls are enabled #19173
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ancial acls are enabled Per https://lab.civicrm.org/dev/core/-/issues/2244 the move of some handling to this function did not consistently include an early return when the setting is toggled off. Eventually there will be no setting - just 'is this extension enabled or not' but in the transition we need this. The new version is easier to copy and paste :-) and uses a function in the extension. The function in the main code base is mostly a lot of code to handle the way the setting used to be stored & it could just check the setting too now
|
(Standard links)
|
|
Thank you @eileenmcnaughton !!! how speedy. |
|
@alifrumin thanks for finding an embarrassing bug! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
Financial acls being applied to options (& other places) when the setting is not TRUE
Per https://lab.civicrm.org/dev/core/-/issues/2244 the move of some handling to this function
did not consistently include an early return when the setting is toggled off.
Before
See https://lab.civicrm.org/dev/core/-/issues/2244 for steps to replicate - but also affects other functions
After
Each hook function checks the setting & returns straight away if not TRUE
Technical Details
Eventually there will be no setting - just 'is this extension enabled or not' but
in the transition we need this. The new version is easier to copy and paste :-)
and uses a function in the extension. The function in the main code base is
mostly a lot of code to handle the way the setting used to be stored & it
could just check the setting too now
Comments
Thanks @alifrumin