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

dev/core#2717 Use order api for new membership create in batch #21152

Merged
merged 1 commit into from
Sep 19, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 16, 2021

Overview

dev/core#2717 Use order api for new membership create in batch

Before

Batch membership create calls membership bao directly

After

As with the back office membership form it calls the order api

Technical Details

@kcristiano this is really how I think the code should wind up looking for this flow (although the renew flow still needs more work.) It really does surface the setting of the end date though as the order flow expectst that all 3 dates will be set when it is in pending status. I have fixed it to that in the BAO - but it does mean it's not possible to avoid setting it if I do it this way.

I guess I could only pass in date fields if they are populated - which would cause the BAO to otherwise set them, That might be where I end up as I think we can treat the failure to set end_date as a bug.

Comments

@civibot
Copy link

civibot bot commented Aug 16, 2021

(Standard links)

@kcristiano
Copy link
Member

@eileenmcnaughton I've done a number more r-run In all cases this PR is an improvement. It works as expected and anything I'd want o 'fix' is an improvement. Mostly the missing Transaction ID field and the fact that source only updates the membership entity not the contribution entity. The areas I want improved are not regressions, they work the same way on 5.39-5.41.

This looks good to merge.

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 can you merge based on ^^

@seamuslee001 seamuslee001 merged commit 0cd4e55 into civicrm:master Sep 19, 2021
@seamuslee001 seamuslee001 deleted the batch_order branch September 19, 2021 20:29
@eileenmcnaughton
Copy link
Contributor Author

thanks @kcristiano @seamuslee001

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.

3 participants