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

Permit modified_id as a parameter for membership create api #16166

Merged
merged 1 commit into from
Jan 2, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Adds modified_id as a parameter to CRM_Member_BAO_Membership::create (this is available via apiv3 & can be available for apiv4 too with some discussion about how best to do so).

Before

Setting modified_id only possible by calling the BAO create (or add) function & passing in the deprecated $ids array

After

modified_id is a key in $params

Technical Details

The key goal here is really about deprecating the $ids array so we can remove it as a parameter from the add function and work to remove it from create. I will follow on with that once this is merged

There is quite a bit of inconsistency with member.create & for apiv3 there is stuff in the api layer that should be in the BAO. There is also some stuff that should go.

Note this PR does nothing at the api level to make it available - it just takes advantage of apiv3 passing params through to the BAO.

I think we should add formal apiv4 support for modified_id
as a parameter for Membership.create - only issue is that of course it's not a 'real' param so we need to discuss /
understand how that needs to look. In general I think there are a few BAO that store modified_id

Comments

@civibot
Copy link

civibot bot commented Dec 31, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

test this please

Adding a membership create api parameter makes this accessible via the api and also allows us to deprecate & remove
the  array. Note this PR does nothing at the api level to make it available - it just takes advantage of apiv3 passing params
through to the BAO. The support is added in the BAO. However, I think we should add formal apiv4 support for modified_id
as a parameter for Membership.create - only issue is that of course it's not a 'real' param so we need to discuss /
understand how that needs to look. In general I think there are a few BAO that store modified_id
if ($session->get('userID')) {
$membershipLog['modified_id'] = $session->get('userID');
elseif (CRM_Core_Session::singleton()->get('userID')) {
$membershipLog['modified_id'] = CRM_Core_Session::singleton()->get('userID');
}
elseif (!empty($ids['userId'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton just wondering if we could do something like this earlier up

if (empty($params['modified_id'] && !empty($ids['userId'])) {
  $params['modified_id'] = $ids['userId'];
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 so next step is to stop passing $ids into add at all & pass modified_id in from create

Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton We should replace get('userID') with getLoggedInContactID()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mattwire yes - there is a bunch of follow on tidy up we should do - goal one is just to get $ids out of the params for the add function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ie once there is an alternative we can eliminate $ids['userID']

@seamuslee001
Copy link
Contributor

agree with @mattwire about changes but i think this is a step forward and has a unit test. merging

@seamuslee001 seamuslee001 merged commit 5fd37df into civicrm:master Jan 2, 2020
@seamuslee001 seamuslee001 deleted the mod_member branch January 2, 2020 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants