Orphaned or Un-purchasable Access Plans

[pondermatic]

Description

Do not display the checkout form if the access plan does not have an existing product, or the product has restrictions that prevent it from being purchasable, e.g. the course enrollment is at capacity.

Refactored LLMS_Shortcode_Checkout::checkout() by moving code that printed notices to their own methods and moving check logic to their model classes.

Fixes gocodebox/private-issues#8. Fixes gocodebox/private-issues#9. Fixes #994.

How has this been tested?

Manually and with existing and new unit tests.

Types of changes

Bugfix (non-breaking change which fixes an issue)

Checklist:

• [x] My code has been tested.
• [x] My code passes all existing automated tests.
• [x] My code follows the LifterLMS Coding & Documentation Standards.

[pondermatic]

@gocodebox/engineering,

I have refactored all of the logic that verifies if the checkout form should be displayed and prints notices if not. I feel it is much better than my previous attempt, but I am a little bit unhappy that the "verify_X" methods not only return true or false but can also print a notice. Generally, a function that does two things should be split, but then the logic that decides whether to print the notice or not must be duplicated. In this case, it's in the shortcode checkout class and the not-purchasable template.

Do you think the "not purchasable" template should also verify if the user is enrolled, that the product exists and that the plan is available to the user (user is in a required membership)? Right now it only checks the course enrollment period start/stop and the course capacity. If so, the template can call LLMS_Shortcode_Checkout::verify_checkout_form_is_displayable() and all the little verify methods can be changed to private.

I changed this PR to draft mode while I work on issue #994.

[pondermatic]

To help answer the question, "Do you think the not-purchasable template should also ...":

Flow

The not-purchasable template is displayed from the pricing-table template, which is displayed from the content-single-course-after and content-single-membership-after templates (and by the possibly unused LLMS_Shortcodes::pricing_table() method).

Verify User Is Not Enrolled

The pricing-table template does not load the not-purchasable template if the user is already enrolled. This check is needed by the not-purchasable template, but it wouldn't cause a problem if is performed.

Verify Product Exists

If for some strange reason, lifterlms_template_pricing_table() can not get a post ID that exists, we get PHP Warning: Attempt to read property "post_type" on null in C:\wordpress-develop\src\wp-content\plugins\lifterlms\includes\abstracts\abstract.llms.post.model.php on line 452 when the non-purchasble template tries to get the product type. This check is not normally needed by the not-purchasable template, but it wouldn't hurt to have it.

Plan Is Available

The pricing-table template does not load the not-purchasable template if the product is purchasable (and the user is not enrolled and the plan has no restrictions). Without this check in the not-purchasable template, the enroll button links to the membership page. This check is not needed by the not-purchasable template.

My conclusion is that the extra checks are not really needed by the not-purchasable template.

[eri-trabiccolo]

Oh another thing, I wonder if admins should always see the checkout form, even when the course is not purchasable because it's closed ecc. Like if a course enrollment is not opened yet, should the admin be able to reach the checkout anyways? I guess so, then the view manager can be used to switch context.

[eri-trabiccolo]

Do you think the "not purchasable" template should also verify if the user is enrolled, that the product exists and that the plan is available to the user (user is in a required membership)? Right now it only checks the course enrollment period start/stop and the course capacity. If so, the template can call LLMS_Shortcode_Checkout::verify_checkout_form_is_displayable() and all the little verify methods can be changed to private.

To help answer the question, "Do you think the not-purchasable template should also ...":

Flow

The not-purchasable template is displayed from the pricing-table template, which is displayed from the content-single-course-after and content-single-membership-after templates (and by the possibly unused LLMS_Shortcodes::pricing_table() method).

Verify User Is Not Enrolled

The pricing-table template does not load the not-purchasable template if the user is already enrolled. This check is needed by the not-purchasable template, but it wouldn't cause a problem if is performed.

Verify Product Exists

If for some strange reason, lifterlms_template_pricing_table() can not get a post ID that exists, we get PHP Warning: Attempt to read property "post_type" on null in C:\wordpress-develop\src\wp-content\plugins\lifterlms\includes\abstracts\abstract.llms.post.model.php on line 452 when the non-purchasble template tries to get the product type. This check is not normally needed by the not-purchasable template, but it wouldn't hurt to have it.

Plan Is Available

The pricing-table template does not load the not-purchasable template if the product is purchasable (and the user is not enrolled and the plan has no restrictions). Without this check in the not-purchasable template, the enroll button links to the membership page. This check is not needed by the not-purchasable template.

My conclusion is that the extra checks are not really needed by the not-purchasable template.

I'm not sure. I think that the logic, until now, was to made the required checks before calling the template. So this is the status-quo, we can change it of course, but why? Once you know the rules, if you want to call the function lifterlms_template_pricing_table(), you have to make sure to run the required checks before.

[pondermatic]

@eri-trabiccolo wrote,

Oh another thing, I wonder if admins should always see the checkout form, even when the course is not purchasable because it's closed ecc. Like if a course enrollment is not opened yet, should the admin be able to reach the checkout anyways? I guess so, then the view manager can be used to switch context.

I believe that admins should be able to preview the checkout form, even when the checkout form is not displayable because of restrictions.

@thomasplevy, should we roll this feature into this PR and or create a new feature request issue?

[eri-trabiccolo]

@pondermatic is this ready for the review or not? I guess you're waiting for thomas' feedback on the admins privileges right? If so shall we turn this into Draft until it's ready?

[thomasplevy]

I'm far out of context of this PR and I won't have time to fully review or make decisions around this for a bit. If you two can't come up with a solution together let's put this on the back burner and I'll revisit in a few weeks.