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

Fix Membership.create in BAO to respect passed in status_id #20976

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 30, 2021

Overview

Fix Membership.create in BAO to respect passed in status_id

Before

  • Membership::create overwrites passed in status_id
  • Order create forces membership status id for new memberships (good) and renewals (bad)

After

'status_id' respected if it reaches create, renewals not reset to pending

Technical Details

I didn't touch the handling of status_id when I fixed the handling of dates because it caused tests to fail - however, it appears the tests were incorrect. The failure was in the setup and they were creating Memberships with a 'grace' status before the payment was received. So, I did another round of fixing the tests

Comments

@monishdeb I'd really like your help to get this merged before the rc is cut as @artfulrobot has been having issues around this & also because I think there are a few membership bao changes that should ideally be in the same release.

@civibot
Copy link

civibot bot commented Jul 30, 2021

(Standard links)

@monishdeb
Copy link
Member

I have tested the patch on membership create and renewal and confirmed that it updates the status correctly. The updated unit test captures the workflow accurately. Hence merging

  • General standards
    • (r-explain) PASS
    • (r-run) PASS: tested the patch on membership create and renewal and confirmed that it updates the status correctly
  • Developer standards

@monishdeb monishdeb merged commit c359334 into civicrm:master Aug 9, 2021
@eileenmcnaughton eileenmcnaughton deleted the mem_status branch August 9, 2021 06:44
@eileenmcnaughton
Copy link
Contributor Author

Yay - thanks @monishdeb !

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.

2 participants