Added more unit tests for process_membership job: testProcessMembershipUpdateStatus

[davejenx]

Overview

Added more unit tests for process_membership job, checking that statuses are updated correctly in various cases where they should be and left alone in cases where they shouldn't. Adds new function testProcessMembershipUpdateStatus.

Before

The only tests for process_membership are for deceased contacts/status.

After

Unit test added for process_membership job, checking that statuses are updated correctly in various cases where they should be and left alone in cases where they shouldn't.

Technical Details

Adds new function testProcessMembershipUpdateStatus. Renames existing function testProcessMembership to testProcessMembershipDeceased, as the name didn't reflect the test.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[davejenx]

@eileenmcnaughton Did you want to look at this one? 😃

[eileenmcnaughton]

I sooo want to look at this one!

[eileenmcnaughton]

@davejenx looks great! What do you think is still not covered by tests? Inherited? Or are we looking pretty good now?

[davejenx]

@eileenmcnaughton Good point about inherited memberships. The job skips these, updating them should I believe be handled by the BAO membership code when the primary membership is saved. Could add a test for that - in the same function, d'you think or is there enough going on in there?

[eileenmcnaughton]

@davej I could go either way on same function or new function - maybe same because having more memberships being updated at once my expose any 'leakage' issues

[davejenx]

@eileenmcnaughton OK. I'm wondering about an edge case where a membership is inherited, let's say from Individual to Individual via Child Of. Forgive the unpleasant examples but suppose...

(a) Parent dies: parent's membership status should get set to Deceased. What should happen to child's membership? Normally status propagates from primary to inherited membership. Questionable what should happen here: do we want child's membership to show as deceased?

(b) Child dies: should child's membership status get set to Deceased? Normally status propagates from primary to inherited membership so they have the same status, but primary will retain its existing status. I'm thinking child's membership could reasonably keep its existing status: as it's secondary, the child won't be sent renewal reminders.

[davej]

@eileenmcnaughton: Wrong DaveJ :)

[davejenx]

Discussed with @eileenmcnaughton, decided that the test should fit whatever the current behaviour is, with a comment in the test to say this behaviour could be questioned.

Behaviour observed on current dmaster:

(a) When parent contact is set to deceased, parent's membership status is set to Deceased. Child's membership is not immediately updated. When process_membership job runs, child's membership is updated to deceased.

(b) In a separate family: when child contact is set to deceased, child's membership status remains as it was, as does parent's. When process_membership job runs, child's membership is updated to deceased. Parent's membership remains as it was.

My feeling is that the behaviour observed for (a) is unfortunate. The parent has taken out a membership for a year, say, with the intention that the child will receive the benefit of this. It doesn't seem right that the child's benefit should cease: would seem better if it continued for the duration of the membership. That would be awkward to handle, involving tracking dates of the parent's membership as though it were still current, even though its status is deceased.

The observed behaviour for (b) seems OK to me.

I'll add tests based on the current behaviour for now.

[eileenmcnaughton]

Merging this now as test fails are unrelated. Other tests can be a follow up