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

CRM-20351 Multiple pages repeat stuff in the page run() method, invoking hooks multiple times #10068

Merged
merged 2 commits into from
Jul 10, 2017

Conversation

agh1
Copy link
Contributor

@agh1 agh1 commented Mar 28, 2017


@agh1 agh1 changed the title CRM-20351 ACL: Don't repeat stuff in the page run() method CRM-20351 Multiple pages repeat stuff in the page run() method, invoking hooks multiple times Mar 29, 2017
@agh1
Copy link
Contributor Author

agh1 commented Mar 29, 2017

Test failures are unrelated, as best as I can tell. I'm getting the CRM_Dedupe_MergerTest problems on master, and I can't reproduce the CRM_Event_BAO_AdditionalPaymentTest failure locally.

@jitendrapurohit
Copy link
Contributor

test this please

@seamuslee001
Copy link
Contributor

@agh1 are you able to rebase this?

@agh1
Copy link
Contributor Author

agh1 commented Apr 28, 2017

@seamuslee yeah not a prob. #10278 was opened and merged all in the course of the day and broke this--go figure.

@colemanw
Copy link
Member

colemanw commented May 1, 2017

Sorry @agh1 I didn't see this one before merging he other.

@colemanw
Copy link
Member

colemanw commented May 1, 2017

@agh1 @eileenmcnaughton
looks like there are some relevant test failures.

@monishdeb
Copy link
Member

@agh1 @colemanw @eileenmcnaughton I have squashed multiple commits and fixed related test failures. Also reviewed the patch and its working fine. Since the changes here are vital to basic page, so I need if someone can eyeball the patch to ensure that the fix won't cause any uninteded regression, before merge.

@colemanw
Copy link
Member

@civicrm-builder retest this please

@monishdeb
Copy link
Member

Jenkins test this please

@monishdeb
Copy link
Member

@colemanw I have fixed the related test build failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants