Membership Detail Report fails when ACLs are in effect #12144
Closed
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
Also remove use of CRM_Utils_Array call since we're sure it's set
…error if the price fields are disabled.
This is a partial of #12065 which is a very simple extraction - only one additional change of moving a variable to a class property
Despite seeming like a lot of change this is mostly just extraction to run via the tests. I can break out into a couple of refactor commits for reviewablility. I have tested this on mysql 5.6 with full group by mode enabled.
When a custom table is removed from logged tables by hook (e.g a calculated table like a summary table) the logging report still tries to include it but fails with a fatal error. This patch excludes it from the list of customDataEnabled logging tables
…r than primary throw DB error
…i version and CMS name
CRM-20459: Actively deprecate CRM_Core_OptionGroup::getValue[Sub PR 2]
(dev/core/98) FGB Searching by any Address fields with location type other than primary throw DB error
5.1.0 release notes: updated synopsis
"Payflo" is a different payment processor.
Payflow Pro not payflo
(dev/drupal#10) Make civicrm-version.php self-sufficient
Use getter function for entity id as on some forms is protected.
Remove some functions that were not testing what they said they were & clarify others. Improve formatting & declaration of unused vars
dev/core#105 Manage PCP URL Wrong for the notification email under wo…
dev/core#30 export of contact master_id name only if there is a master_id given (rebase)
…-multiple-change CRM-21853: Do not change 'is_multiple' unless included in the params
…g to be problematic on different MySQL versions and add in warning in regards to the usage of CRM_Utils_SQL::supportStorageOfAccents() Disable another flacky test
(NFC) Disable tests performing quick search with no orderby as provin…
CRM-21598 - Case Activity issues with custom Completed Status Type
Add labels to membership type metadata, allowing for addField method to be used
[NFC] Cleanup on ActivityTest class.
(NFC) Update to latest versions of karma and jasmine used only in tes…
core/issues/74 - 'Price Set Details for Event Participants' gives DB …
(NFC) Update view issues and report bugs link in footer to point to the lab
(dev/drupal/17) Add Drupal8 support for getUFLocale()
Fix file perms on files back to 664
(NFC) fix file permissions on civicrm-version.php
added unit test
|
@MegaphoneJon there is an existing fix in master to backport if you think it should go in the rc. I hadn't been treating this as a recent regression as it was first logged against 4.7.31 - https://issues.civicrm.org/jira/browse/CRM-21852 |
|
See #12094 |
|
@MegaphoneJon I'm gonna close this for now because it is chocka full of commits that should not be in it - you can re-open when it's just a backport of the patch per above |
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
Membership Detail Report fails with a DB error when a user has ACLs applied.
Before
Membership Detail fails when a user has ACLs/can't view all contacts.
After
Report runs successfully.
Technical Details
During a tidy-up on the Membership Detail Report, the from() method was modified to use CRM_Report_Form::setFromBase(), which concatenates $this->aclFrom. However, concatenating $this->aclFrom was never removed from the child class, so it's added to the SQL statement twice.
Comments
I'm submitting this against the RC because a) this is a recent regression (worked in 4.7.31), b) it's localized to a single report, c) The bug and solution are more obvious than usual, which makes it easy to review.