give_get_payment_status() sometimes returns false for non-English locales

[kevinwhoffman]

Issue Overview

While working on #509, I noticed that give_get_payment_status() was validating the payment status against an array of translated status labels, rather than an array of the status keys.

This is a problem because status labels change based on the site's current language, while the status keys remain the same regardless of language. For this reason, we should validate against the keys and not labels.

The lines in question are below and you can see it in context here.

// Account that our 'publish' status is labeled 'Complete'.
$post_status = 'publish' == $payment->status ? 'Complete' : $payment->post_status;

// Make sure we're matching cases, since they matter.
return array_search( strtolower( $post_status ), array_map( 'strtolower', $statuses ) );

Expected Behavior

• give_get_payment_status( $payment, false ) should return the post_status of the current post as it exists in the database (e.g. pending).
• give_get_payment_status( $payment, true ) should return a translated status label (e.g. 'Pending').

Current Behavior

• give_get_payment_status( $payment, false ) returns false on non-English sites where status labels have been translated.
• give_get_payment_status( $payment, true ) works as intended.

Possible Solution

Prior to the return line in question, we already have the payment status available via $payment->status. We've also confirmed that $payment->status exists as a key in the $statuses array. So we have the status and have validated it against a set of acceptable statuses. The purpose of further validating the status again in the return line is unclear.

At this point, I think we can change
return array_search( strtolower( $post_status ), array_map( 'strtolower', $statuses ) );
to
return $payment->status;

This solution would eliminate the array operations that are currently causing the function to return false.

Possible Concerns

• We should further investigate why this array manipulation is happening and if it serves a purpose before removing it.
• Note that just above the return line, the publish status gets changed to Complete. If that value is being used elsewhere, we should be careful of changing this behavior.

Steps to Reproduce (for bugs)

1. Execute var_dump( give_get_payment_status( $payment, false ) ); with a valid payment ID where the current status is pending.
2. Confirm that a valid status is returned.
3. Change WP language to French in Settings > Site Language.
4. Repeat step 1 and confirm the function now returns false.

This will be the case in any scenario where the translated status label differs from the English version.

Related PRs

PR #1140 and Issue #509

Todos

• Determine if proposed solution would cause breaking changes for Give core or any extension that uses give_get_payment_status() specifically when the second parameter is false.
• Tests

[kevinwhoffman]

@DevinWalker Please see the above Todo:

Determine if proposed solution would cause breaking changes for Give core or any extension that uses give_get_payment_status() specifically when the second parameter is false

From what I can tell, give_get_payment_status() is used in a several places in Give core, but not when the second parameter is false, which is the use case affected by the solution. Can you do a search of the add-ons and let me know if you find that use case anywhere else.?