CiviSEPA incompatible with new future CiviCRM versions?

[bjendres]

@eileenmcnaughton writes in her dev digest:

I’m working on NOT adding ‘Overdue’ and ‘In progress’ statuses to the contribution_status options on new installs - these are not contribution statuses but hang overs from when recurring contributions and pledges shared the same option value group. However, I’ve hit a few gotchas (e.g paypal ipn) where code is still looking in the wrong option group so I’m working through removing those - check your custom code to ensure that references to ‘In progress’ and ‘Overdue’ and looking in the right option group.

This would, however, clash with CiviSEPA's status model, where a SEPA contribution is in status in Progress when submitted to the bank, and will change to status Completed (or possibly Cancelled) only when we get the confirmation from the bank that it's been collected - which can be an arbitrary time period later.

@eileenmcnaughton Could you please give us a hand assessing the impact? Would it be enough for the extension to simply create the in Progress status if not present? Do you have an idea when this change would be released?

Thanks to @pfigel and @Detsieber for bringing this to my attention.

[eileenmcnaughton]

@bjendres - yep - the function CRM_Core_BAO_OptionValue::ensureOptionValueExists is not officially supported for use outside core - but it is unlikely to ever change..... & it does what it says.

Also note that there is no current plan to remove the statuses proactively - but at some point we might encourage people to disable or delete them if they don't use them (e.g a system check or upgrade message)

[bjendres]

This is already happening on the current master and we need to get a workaround for this ASAP.

[bjendres]

It looks like this is happening with the next release (5.54.0)

[bjendres]

@jensschuppe would you agree to release this with 1.7?

[jensschuppe]

Sure, this looks solid, although we should revisit the mitigation when Core tries to proactively remove those status. We should then just create the status on installation of the extension and not call ensureOptionValueExists() every time we need to access them.

[bjendres]

Thanks for the review, @jensschuppe.

we should revisit the mitigation when Core tries to proactively remove those status.

I agree, but my understanding was, that this is not the plan of the core team. They just want to phase it out for new installations.

We should then just create the status on installation of the extension and not call ensureOptionValueExists() every time we need to access them.

I disagree, because then it would still be broken when you first install an older version of CiviSEPA on the next CiviCRM release, and then do an upgrade to CiviSEPA.

We could add it to the CiviSEPA installer and the upgrade steps, so we cover both scenarios, but I would tend to leave it as it is, just to be on the safe side for weird edge cases. Plus, the lookup in ensureOptionValueExists should be using the same cache, anyway.

@jensschuppe agreed?

[jensschuppe]

Installing old stable versions when there are newer ones doesn't make sense and we should not be actively supporting this, but you're most probably right with the cache argument, so let's leave it as is.

[bjendres]

Installing old stable versions when there are newer ones doesn't make sense

I could think of scenarios where you're stuck with old extensions that are important for your workflows.

... so let's leave it as is

Great, I'll merge it. Thanks!

[eileenmcnaughton]

Regarding removing for existing installs - I think it would be presumptive of us to assume sites are not using them in any way so we might in future do something to encourage / help people remove them but not auto-remove