Code cleanup - remove extraneous permissions clause #13645
Merged
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
As the test demonstrates this permission is applied without this line. It is applied even when the skipPermission is TRUE but that has been code-commented for now rather than resolved
|
(Standard links)
|
eileenmcnaughton
changed the title
| @@ -237,9 +237,6 @@ public static function whereClauseSingle(&$values, &$query) { | |||
| return; | |||
|
|
|||
| case 'financial_type_id': | |||
| // @todo we need to make this resemble a hook approach. | |||
| CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($financialTypes); | |||
| $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("civicrm_contribution.$name", 'IN', array_keys($financialTypes), 'String'); | |||
There was a problem hiding this comment.
removing this line is the actual change - test shows it doesn't stop the ACL being applied - clause is being added twice
|
@pradpnayak maybe you can check this - it's pretty simple - just adds a test to prove the removed line is just a duplicate clause |
|
Merging based on code review & passing tests. |
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
Minor code cleanup - removes redundant clause when financial acls in play
Before
financial type clause added by buildPermissions AND when it is a critieria
After
Clause not added twice in that scenario (still added twice when the default clause is hit which I have not addressed in this PR
Technical Details
As the test demonstrates this permission is applied without this line. It is applied
even when the skipPermission is TRUE but that has been code-commented for now rather
than resolved
Comments
@monishdeb this one is a safe & minor cleanup - respecting skipPermissions might need some more testing but we will need to do that at some point