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

Fix Fatal error on membership detail report (with ACLs enabled) dev/core/#100 #12150

Merged
merged 1 commit into from
May 16, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

@MegaphoneJon per #12144 backport of #12094

@seamuslee001
Copy link
Contributor

are we back porting to 5.2 or 5.1?

@MegaphoneJon
Copy link
Contributor

Thank you Eileen! Do you want me to test this? Also I had thought this merited inclusion in the RC but not a point release, so 5.2 seems appropriate.

Also, this is very informative regarding how to test a report for ACLs, I had no idea!

@eileenmcnaughton
Copy link
Contributor Author

@MegaphoneJon it seems we have to do a second point release as there was a bug in the previous. You are right - this is a bit borderline but I like the idea of have no known recent regressions in our latest published version & get to that baseline

@eileenmcnaughton
Copy link
Contributor Author

& yes @MegaphoneJon if you are able to test it - I had the fix about 2 months ago TBh but no-one to test if for me :-(

@MegaphoneJon
Copy link
Contributor

Well - the good news is that this works when applied to a 5.1.0 site. I guess I'm a little mystified by it - I can see that when from() is called, $this->_aclFrom isn't defined, so I'm not sure why it's still referenced. Still - this fixes the problem and ACLs are respected, so I can't complain.

@eileenmcnaughton
Copy link
Contributor Author

:-) - I think the buildACL is burried a bit deeper now

@eileenmcnaughton eileenmcnaughton merged commit 09b4502 into civicrm:5.2 May 16, 2018
@eileenmcnaughton eileenmcnaughton deleted the memdetail52 branch May 16, 2018 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants