CRM-19600: Membership Renewal activity not created through online contribution page

[monishdeb]

@eileenmcnaughton it's regression due to refactoring changes done in 4.7 and issue arises when we call
CRM_Member_BAO_Membership::renewMembership(...) here before its rightful place i.e. is after completeTransaction(...) here As a result of which additional info is not updated as in here 'Membership Renewal' activity. Although there are other alternatives like handling :

1. under Membership.update api
2. or under Membership::create(...) BAO fn by fetching current membership info
   but I think this patch is more appropriate and safe

Tried to add unit test to assert membership-renewal activity via online contribution but currently its not possible.

---

• CRM-19600: Membership Renewal activity not created through online contribution page

[eileenmcnaughton]

Let's try to get a unit test on this - I have been trying pretty hard on that function to ensure we have unit tests. Also, I like putting the job of adding tests for functions like that to people learning tests because there are clear precedents like https://github.com/davecivicrm/civicrm-core/blob/257ab382cb93c70223247f03d0b6a353279e96a0/tests/phpunit/api/v3/ContributionTest.php#L1619-L1619 - I'm going to tag it in JIRA so potential GSoCers can find it since there are a few looking for something to get to grips with Civi with at the moment - I've linked to a search of things like this from the wiki

https://wiki.civicrm.org/confluence/display/CRM/Preparing+for+GSOC

Not to say it has to be a GSoC one - but this is a really good 'write your first test' opportunity

(As an aside I suspect the action really should be inside the membership.create function rather than here)

[eileenmcnaughton]

Actually I should get you to clarify 'Tried to add unit test to assert membership-renewal activity via online contribution but currently its not possible.'before I tag it as an 'easy test to write' - it should also be an option in ContributionPageTest class to copy one of those- what is the gap ?

[monishdeb]

@eileenmcnaughton my bad :( There's already a unit test to assert membership registration via online contribution page and I have extended it 0445e08

[eileenmcnaughton]

Ok - I'm not sure the create shouldn't be deeper - but I'm happy that the test would ensure that if that were changed later & it no longer created exactly one activity the test would pick it up