CRM-20787: For a repeating Event series. Changing price set of main event, it should be copied to all repeating events if we select 'Every event' mode.

[agilewarealok]

Overview

For a repeating Event series. If we change the Price Set for a paid Event then this Price Set selection is not applied to all Events in the series even when apply to Every Event is selected.

Steps to reproduce:

1. Create Price set A
2. Create Price set B
3. Create event A with price set A
4. Open event A settings and Repeating tab
5. Create couple of repeating events.
6. Open event A settings and Fees tab
7. Change price set to B and select 'Every Event' mode.

Before

Updated price set is not applied to the Repeating events which is unexpected behaviour.

After

It now works as expected. Price set is changed for repeating events too.

Agileware Ref CIVICRM-152

---

• CRM-20787: CIVICRM-152 For a repeating Event series. If change the Price Set for a paid Event then this Price Set selection is not applied to all Events in the series even when apply to Every Event is selected

[agilewarealok]

@eileenmcnaughton
We want to write Unit test for this case, any example we can look into ?

[eileenmcnaughton]

In terms of a unit test I think it makes most sense to extract the guts of the function (everything except the bit which outputs the json) into another function (probably on the BAO class. Probably extract the parameters from the REQUEST in the AJAX function. Then your unit test can test the extracted function & sit in CRM_Core_BAO_RecurringEntityTest & you can test the result without having to bypass the json exit in the function.

I terms of this PR my assessment is that the code looks good. I don't feel able to comment on the appropriateness of this change as I don't work with that area of the codebase but @deepak-srivastava @totten or @colemanw might be able to - it's certainly not raising any red flags for me

[agilewarealok]

@eileenmcnaughton Understood, We will try to write UT for this.

[agilewarealok]

@eileenmcnaughton
We've added a UT by refactoring the code.

[MegaphoneJon]

@eileenmcnaughton I give a +1 for the appropriateness of the change. Personally, I would have blocked merging recurring events at all unless this (and CRM-16902) were present.

[eileenmcnaughton]

Just a quick note to say I'm happy with this from a code point of view & concept - per @MegaphoneJon. The code concerns I have (by-passing hooks) are pre-existing and this just moves them around.

This is good to merge pending someone doing a UI test of it (that might be me but if someone else gets to it first....)

[jusfreeman]

Can we get this PR merged? @eileenmcnaughton or @seamuslee001

[eileenmcnaughton]

I did some UI testing on this & can confirm it works per the description of the PR. With the patch applies I can still alter only one event's price set but I can alter all. Merging per my tests & prior discussion

[jusfreeman]

Thanks very much @eileenmcnaughton

[eileenmcnaughton]

Well I thought I'd better make good after Francis reviewed my deadlock PR yesterday :-)

[jusfreeman]

Hell yeah! @eileenmcnaughton

[eileenmcnaughton]

:-)

[eileenmcnaughton]

Noting that this caused a regression https://issues.civicrm.org/jira/browse/CRM-21764

[jusfreeman]

@eileenmcnaughton thanks for the heads up, I've made a note to discuss what we can do here.