gocodebox / lifterlms

LifterLMS, a WordPress LMS Solution: Easily create, sell, and protect engaging online courses.
https://lifterlms.com
GNU General Public License v3.0
180 stars 134 forks source link

Proposal: Changes to "Pending" Orders #1495

Open thomasplevy opened 3 years ago

thomasplevy commented 3 years ago

Recently some issues with the way initial transaction failures for orders are handled, this proposal attempts to (A) outline current behaviors and explain the engineering / architectural reasons behind current behavior, (B) expose problems and pitfalls encountered as a result of these current behaviors, and (C) develop a set of changes which can be implemented to improve the existing system to both maintain existing behavior without regression and solve the issues with this current system.

@eri-trabiccolo and I have talked about this quite a bit and this proposal is a summation of our discussion(s). We'd like to open this up to the team (and any watchers) to discuss before we commit to development

A. Current Behaviors

  1. A new order is created/resumed when an end user (student/learner) submits the checkout form 1A. If a pending orders exists for the user/access plan, this plan will be used 1B. If no pending orders exists a new (pending) order is created
  2. A user is created
  3. Data is passed to the payment gateway for a transaction to be attempted (or in the case of async gateways like paypal the user is redirected offsite)
  4. If an error is encountered at this phase, it is not recorded on the order and the status of the order will stay as "Pending" indefinitely

The main reasoning for item 4 is to enable the functionality found in item 1A. A failed transaction automatically fails the associated order, and a failed order cannot be resumed through this functionality.

The reason that failed orders cannot be resumed is mainly in order to prevent recurring orders that have reached the end of their automatic retry cycle from being resumed later. In most scenarios it may not matter if a failed recurring order is resumed, but, in some scenarios it would be preferable that a recurring order cannot be resumed.

As an example, consider a recurring payment plan with 4 monthly payments that allows access for one year. Should the 2nd payment fail, 3 months pass, and then a user was allowed to resume the plan, LifterLMS would resume access, although technically they would owe 3 more payments. LifterLMS does not treat this as a true payment plan so it does not track or attempt to charge "back payments" like this. Instead of doing this LifterLMS prevents the orders from being resumable at all. A user who wishes to regain access in this scenario must start over or contact the site administartor for special accommodations.

B. Problems with Current Behavior

  1. The primary issue we're being made aware of is that both end users and site administrators believe that "Pending" means that the payment has been executed and that we are awaiting confirmation from a bank / merchant for the order to be completed. This is a problem, I believe, mostly in language and assumptions.
  2. The end user is displayed an error message upon transaction failure but this is not recorded in the database. If the end user ignores / misses the notice displayed on screen they will never see this again and may not realize that the transaction failed. 2.A. This issue may not be solvable by LifterLMS alone. We reviewed a site who's primary theme color is red, the same color used to outline / emphasize an error message on checkout. It's possible that users are missing this because it doesn't have enough visual contrast with the rest of the site and doesn't immediately present itself as an error message. 2.B. It seems unreasonable for LifterLMS to add special CSS to make error messages a different color when the primary theme color is red. This would be a very difficult programmatic task anyway.
  3. End users who visit their order history on the student dashboard will see a pending order with no notice of the failed transaction
  4. This morning I noticed our documentation defines "Pending" as "No payment has been attempted" which is true expect when it's not. The documentation is technically incorrect.

C. Changes We Can Make

  1. On initial checkout, record all failed transactions & mark the order as failed per our normal logic
  2. Currently, on checkout, we look for an existing pending order for the given user/access plan and, if found, we resume that order in favor of creating a new one. We should preserve this behavior but extend it to allow (some) failed orders to be resumed also 2A. An order can be resumed if:
    • It is a one-time order with the status of failed or pending
    • It is a recurring order with the failed or pending status that has not yet received at least one successful transaction.
      • We can run a transaction query to determine this, querying for transactions with the refunded/success status. As long as the query returns 0 transactions the order can be resumed 2.B.Add a button on the order-review page (for students) to allow them to resume resumable orders (This is just a link to the checkout page for the given access plan). Recurring orders can already do this, one time needs to be added (thomas has a patch stashed on his local)
  3. Create a scheduled job (cron) that automatically moves "Pending" orders to "Timeout" 7 days (or some other timeframe configurable by a filter) after the order is created 3.A. This is a new order status which is used exclusively for orders in this scenario 3.B. Gateways should implement a new gateway feature "pending-timeout" which is used to determine whether or not orders processed by the gateway should automatically timeout. The manual, gateway, for example, should not implement pending-timeouts by default 3.C. New order notifications (likely just to the admin) can be added for timeouts?
  4. Create a transaction status code field 4.A. When a transaction fails we can record the gateway's error code / status here 4.B. The gateway can then output information to both the admin and end user about why the transaction failed, essentially outputting the same error message shown on the frontend during checkout again
  5. Add status help text on the front- and back-end of the site next to the payment status label describing in additional detail what the status means. For example:
    • Pending (?) Pending orders are recently created orders which have not yet received any transaction attempts
    • Failed (?) Failed orders have received one or more failed transaction attempts
    • Success (?) The order was successfully completed
    • Active (?) The recurring order is in good standing
    • etc...
thomasplevy commented 3 years ago

@eri-trabiccolo can you have a second look at this and let me know if you have any suggestions or if I've missed anything before we open this up for ~debate~ civil discussion

eri-trabiccolo commented 3 years ago

@thomasplevy:

2A. An order can be resumed if:

  • It is a one-time order with the status of failed or pending
  • It is a recurring order with the failed or pending status that has not yet received at least one successful transaction.

Can't we just say unify as: It is an order with failed or pending status with no transaction or if its last transaction status has failed

I'm very positive for all the rest.

thomasplevy commented 3 years ago

I don't think that will work...

And I think I actually need to make it more complicated.

A recurring order that has 1 successful payment and then fails will, therefore, be resumable, but we don't want it to be.

We only want a recurring order that has 0 successful transactions and the "Failed" status to be resumable.

We also should consider the "On Hold" recurring status to be resumable.

So the language for this second bullet should actually read

chrisbadgett commented 3 years ago

Looks like a solid payments enhancement set. The new "timeout" status, resume option, and allowing the user to see failed transaction error reports from their dashboard after the fact makes a lot of sense for UX improvement.

In the case of a red site, could theme authors choose to provide color style options for checkout error messages? For example, a theme could choose to provide the ability for a red site (or any color site) to change red background error messages to dark yellow (or any other color) background color error messages?

thomasplevy commented 3 years ago

In the case of a red site, could theme authors choose to provide color style options for checkout error messages? For example, a theme could choose to provide the ability for a red site (or any color site) to change red background error messages to dark yellow (or any other color) background color error messages?

Yes, of course. It's all CSS and actually in the site in question there's already CSS from the theme that's changing the style of the error message, but they haven't changed the color. Of course theme author's could do something about this. My point is that it's complicated to automatically identify visual problems like this from within our codebase. IT would actually probably be difficult for theme authors to do this too.

An author could give theme options for notice colors though. This would keep the "challenge" in the hands of the site owner (or their dev) but still give them a simple way to solve it.

I haven't seen any theme offer this kind of setting before but it would be a good and useful one (IMO). Most platform plugins like ours have notices so the functionality doesn't even have to be LifterLMS specific. WooCommerce has notices too.

We have 4 kinds of notices:

https://github.com/gocodebox/lifterlms/blob/6821b10b4a851acc128eb3cfdcc2862c6658841b/includes/functions/llms.functions.notice.php#L57

https://github.com/gocodebox/lifterlms/blob/trunk/assets/scss/frontend/_notices.scss