dev/core#1630 Partial fix on renew membership missing

[eileenmcnaughton]

Overview

Fixes a bug which looks like a year-old-regression where price-set auto-renew memberships do not show the auto-renew flag

Before

Repro steps:
Configure a membership type with "auto-renew option" = "Give option, but not required", e.g., "General type A"
Create a membership price set with a "membership type" field (radio) including at least the "General type A" membership type.
Create a contribution page using this price set;
Just in case, ensure the contribution page uses a payment processor that supports recurring contributions (not sure if this step is necessary for repro, but it would be in the real world)
Open the contribution page in "live" mode
Below the "Membership type" price field, observe the message "Membership will renew automatically."; also observe there is no checkbox labeled "Please renew my membership automatically." (which would be expected for optional auto-renewal).

After

The checkbox now shows when the above steps are followed.

Note however this is partial - if the price set was created using the version routine then the civicrm_membership_block.membership_type field will be populated & that will be respected. I've left worrying about that out of scope

Technical Details

I'm pretty comfortable that this regressed at least a year ago - possibly much longer - so it's not
a recent regression. However, the issue is that when a price set is configured it is still 'listening to'
the presence or otherwise of auto_renew in civicrm_membership_block.membership_types. This makes it
ignore that if not present (which is the case when creating a new price set from scratch).
When converting a price set the membership_types field is still populated and still kicks in

• I think we should really empty that out when a block is converted but I've left it out of scope as this
  seems like an edge case bug & it was only the possibity it was a regression that caused me to dig into it

Comments

@twomice

[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]

@mattwire

[twomice]

Thanks @eileenmcnaughton ! PR review coming in a day or two.

[twomice]

Jenkins test this please. (Regenerating test site for review.)

[twomice]

(CiviCRM Review Template WORD-1.2)

• General standards
  • (r-explain) pass
  • (r-user) pass
  • (r-doc) pass
  • (r-run) pass (see below)
• Developer standards
  • (r-tech) pass
  • (r-code) pass
  • (r-maint) pass
  • (r-test) pass

How I tested this for r-run, using the testing site built by Jenkins: Using the repro steps in the original ticket, I observed correct behavior in all cases:

Repro steps:

1. Configure a membership type with "auto-renew option" = "Give option, but not required", e.g., "General type A"
2. Create a membership price set with a "membership type" field (radio) including at least the "General type A" membership type.
3. Create a contribution page using this price set;
4. Just in case, ensure the contribution page uses a payment processor that supports recurring contributions (not sure if this step is necessary for repro, but it would be in the real world)
5. Open the contribution page in "live" mode
6. Below the "Membership type" price field, observe the message "Membership will renew automatically."; also observe there is no checkbox labeled "Please renew my membership automatically." (which would be expected for optional auto-renewal). **Instead of that bad behavior, I observed: ** checkbox labeled "Please renew my membership automatically." appears correctly.

Bonus repro:

1. Edit the configuration for "General type A" membership type, so that "auto-renew option" = "No auto-renew option".
2. Reload the contribution page and observe that auto-renew is not required. This correct behavior was also observed in the testing site.

Bonus repro 2:

1. Create another contribution page, this time offering the "General type A" membership type without using a price set (i.e., in the "quick config" membership type options under the Membership tab), and set the "Auto-renew" option to "Give option" for this membership type.
2. Open this contribution page in "Live" mode and observe that the checkbox labeled "Please renew my membership automatically." appears as expected. This correct behavior was also observed in the testing site.

Summary:
For all combinations of price set vs "quick config", all values of "auto-renew option", and all values of quick-config "auto-renew" settings, the auto-renew behavior on the contribution page appears correctly.

@colemanw This is a passing review. Please merge or take other appropriate next steps.

[mattwire]

This is pretty toxic code that really needs cleaning up. The change in this PR certainly makes the code easier to understand / read even if it requires further improvement later. And it's been reviewed/tested by @twomice So I am going to merge.

[twomice]

FYI, one more thought: I found today -- as Eileen hinted -- that this seems to work for price sets created after the patch, but not those that pre-exist it. Fortunately, using the "Copy price set" functionality seems to yield a price set that works properly.