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-20498, removed code from run() since its been invoked in parent f… #10278

Merged
merged 1 commit into from
Apr 28, 2017

Conversation

pradpnayak
Copy link
Contributor

…unction


…unction

----------------------------------------
* CRM-20498: buildform hook invoked twice
  https://issues.civicrm.org/jira/browse/CRM-20498
@pradpnayak
Copy link
Contributor Author

Removed code since this same functions are invoked again from parent class i.e CRM_Core_Page_Basic::run();

Copy link
Member

@monishdeb monishdeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and confirmed that this patch removes the duplicate code as earlier it overrides the parent run() function which is already covered in https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Page/Basic.php#L134

@monishdeb
Copy link
Member

monishdeb commented Apr 28, 2017

@eileenmcnaughton @colemanw can we merge this PR?

@colemanw colemanw merged commit 411b68f into civicrm:master Apr 28, 2017
@monishdeb monishdeb deleted the CRM-20498 branch April 28, 2017 16:06
@eileenmcnaughton
Copy link
Contributor

cool @agh1 actually did a pr to add tests for when buildForm is called twice - I think I was supposed to be helping him sort out some test issues on it (looks sheepish)

@agh1
Copy link
Contributor

agh1 commented Apr 28, 2017

@eileenmcnaughton @Coleman Yeah i think this actually causes a conflict for #10068 it does basically the same thing but without addressing the other places or adding the test

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

Successfully merging this pull request may close these issues.

5 participants