Upgrade When package to the lastest version

[seamuslee001]

Overview

This picks up the work from @agileware-pengyi in #14903 and goes for more of a straight composer way with this PR. Note that this PR is expected to fail testing

Note to @agh1 please make sure @agileware-pengyi is credited

Before

Old version of When package is used

After

Newer version of When package used

In terms of tests i am expecting that CRM/Core/BAO/RecurringEntityTest is going to fail as it is locally which suggests we do have some unit tests already covering this code

ping @jusfreeman @eileenmcnaughton

Agileware ref: CIVICRM-1223

[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).

[eileenmcnaughton]

fail relates

[jusfreeman]

@seamuslee001 thanks and thanks for adding the Agileware ref! Legend!

[seamuslee001]

@jusfreeman i'm not sure how to fix the test failures but given that Pengyi has looked into it already maybe he can give some guidance here

[jusfreeman]

@seamuslee001 we won't be able to get to this one today, however should have time next week. It's on our board to action.

[agileware-pengyi]

I have reviewed the fails.
Should I submit a new PR against this branch or the master branch?

[agileware-pengyi]

I have made the changes in this commit

[seamuslee001]

Thanks @agileware-pengyi i have brought that commit in, i will need another reviewer to review this now @eileenmcnaughton @monishdeb

[eileenmcnaughton]

@seamuslee001 I think this is mergeable it you and @agileware-pengyi have reached agreement - without having gone into the code to the level you two have nothing jumps out as concerning

[agileware-pengyi]

@seamuslee001 thanks

[seamuslee001]

@eileenmcnaughton @agileware-pengyi i have also just pushed one minor change which is setting the ignoring of exceptions to only happen when in UnitTests UF because i figure we still want Execptions to be thrown so they show up as status messages

[agileware-pengyi]

@eileenmcnaughton @agileware-pengyi i have also just pushed one minor change which is setting the ignoring of exceptions to only happen when in UnitTests UF because i figure we still want Execptions to be thrown so they show up as status messages

That makes sense to me. Just noting that this exception with message The start date must be the first occurrence. is not checked by the old version.

[agileware-pengyi]

Working on exclude dates issue. It needs some changes for the new version.

[agileware-pengyi]

Following the old way to generate the occurrences. see agileware@410b858

More confidence with this one because of fewer changes.

One thing I notice is that error is not defined in both old and new version. Should we keep it?

civicrm-core/CRM/Core/BAO/RecurringEntity.php

Line 992 in e058b26

$r = new When();

civicrm-core/CRM/Core/BAO/RecurringEntity.php

Line 1018 in e058b26

$r->errors[] = 'Repeats every: is a required field';

[jusfreeman]

@seamuslee001 ping just in case you missed it

[monishdeb]

Awesome. And after merging this PR we need to remove the When package thus there should be a followup PR that removes it from packages/*

[seamuslee001]

thanks @agileware-pengyi incorporated now

[seamuslee001]

@monishdeb packages PR is here civicrm/civicrm-packages#264

[eileenmcnaughton]

Ug - needs a rebase

[seamuslee001]

rebased now @eileenmcnaughton

[mattwire]

Merging so we get plenty of testing during the 5.20 release cycle. @seamuslee001 Do we need a PR to remove from packages?