Skip to content

Extract getCurrentMembership #20882

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

Merged
merged 1 commit into from
Jul 23, 2021
Merged

Extract getCurrentMembership #20882

merged 1 commit into from
Jul 23, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Extract getCurrentMembership

Before

currentMembership not reliably accessable from anywhere in class

After

Extracted

Technical Details

Note I've moved the fixmembershipStatusBeforeRenew here - it is just compensating for the possibility of
the job not being run. On the Membership form we run it in preProcess but this feels like our first
chance in this form

Coments

The goal is to decommission the legacyProcessMembership function

@civibot
Copy link

civibot bot commented Jul 16, 2021

(Standard links)

@civibot civibot bot added the master label Jul 16, 2021
@colemanw
Copy link
Member

CRM_Batch_Form_EntryTest::testMembershipRenewalDates
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'2021-07-16'
+'2016-04-01'

@eileenmcnaughton
Copy link
Contributor Author

Interesting - tests are working then :-) I suspect the test might need fixing - will look

Note I've moved the fixmembershipStatusBeforeRenew here - it is just compensating for the possibility of
the job not being run. On the Membership form we run it in preProcess but this feels like our first
chance in this form
@eileenmcnaughton
Copy link
Contributor Author

@colemanw this is passing now - it was a loop-leakage issue

@monishdeb
Copy link
Member

Tested in local, r-run passed. Reviewed code looks good.

@monishdeb monishdeb merged commit c11002d into civicrm:master Jul 23, 2021
@eileenmcnaughton eileenmcnaughton deleted the hh branch July 23, 2021 20:28
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