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-21216 Remove api limit, make deprecated function more deprecated #11029

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Sep 26, 2017

Overview

Fixes based on comments from previous PR #11020.

Assuming this passes tests it should be merged as the previous patch inadvertently introduces a limit on 25 membership types.

@eileenmcnaughton thanks for the previous merge. This one fixes the issues identified in comments.


}
return $membershipTypes;
Civi::log()->warning('Deprecated function getMembershipTypesByOrg, please user membership_type api', array('civi.tag' => 'deprecated'));
$memberTypesSameParentOrg = civicrm_api3('MembershipType', 'get', array(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think some copy & pasting did go wrong here. I think you wanted to paste this into: CRM_Member_Page_Tab::browse() and leave the to be deprecated function as it is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine - it does the same thing as the previous function but in a way that is more clearly 'the same as the api'

@eileenmcnaughton
Copy link
Contributor

I think there is one more location - CRM/Member/Page/Tab.php where the limit is an issue

@mattwire mattwire force-pushed the CRM-21216_getMembershipTypesByOrg_to_api branch from d8683f7 to c8eada2 Compare September 26, 2017 20:59
@mattwire
Copy link
Contributor Author

@eileenmcnaughton Thanks for spotting the file I missed... Now updated limits in all places!

@eileenmcnaughton
Copy link
Contributor

Looks good now - thanks @mattwire @tschuettler

@eileenmcnaughton eileenmcnaughton merged commit a85b09c into civicrm:master Sep 26, 2017
@mattwire mattwire deleted the CRM-21216_getMembershipTypesByOrg_to_api branch November 3, 2017 23:07
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