dev/core#1588 Fix regression where empty string is passed to PropertyBag

[eileenmcnaughton]

Overview

Fixes a regression where a fatal error results when:

If you enabled the recurring option "Support recurring intervals" on a contribution page you can no longer submit the contribution form unless you enter a value for the recurring interval, even if you have not selected to make a recurring contribution.

Before

Fatal
Error: recurFrequencyInterval must be a positive integer

After

No error

Technical Details

We have a scenario where the checkbox is presented but is optional. This is then in 'params' which is passed through to
PropertyBag. The value is equal to '' so it fails to validate when we use it to set the value on the PropertyBag.

I don't think we lose anything meaningful by not setting an empty string and it avoids this error so we should
merge & release as a regression fix IMHO. If we want to revist then we should do that in master

https://lab.civicrm.org/dev/core/issues/1588

Comments

@artfulrobot @mattwire

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

[artfulrobot]

I would prefer this: #16538

[artfulrobot]

Is there ever a case when a property wants to get set to ZLS?

[eileenmcnaughton]

is ZLS an acronym? I see Zimmermann–Laband syndrome

Zero Length String?

[eileenmcnaughton]

I would be very surprised - given where we are coming from - if the difference between NULL & '' is material for receiving functions

[seamuslee001]

merging as the discussion suggests that this is an ok way to go forward

[artfulrobot]

late reply, but yes, I meant "" for zero length string (ZLS). My bad for the TLA, IDK! LOL.