{"action":"opened","issue":{"url":"https://api.github.com/repos/WordImpress/Give/issues/1141","repository_url":"https://api.github.com/repos/WordImpress/Give","labels_url":"https://api.github.com/repos/WordImpress/Give/issues/1141/labels{/name}","comments_url":"https://api.github.com/repos/WordImpress/Give/issues/1141/comments","events_url":"https://api.github.com/repos/WordImpress/Give/issues/1141/events","html_url":"https://github.com/WordImpress/Give/issues/1141","id":184598198,"number":1141,"title":"give_get_payment_status() always returns false for non-English locales","user":{"login":"kevinwhoffman","id":1938671,"avatar_url":"https://avatars.githubusercontent.com/u/1938671?v=3","gravatar_id":"","url":"https://api.github.com/users/kevinwhoffman","html_url":"https://github.com/kevinwhoffman","followers_url":"https://api.github.com/users/kevinwhoffman/followers","following_url":"https://api.github.com/users/kevinwhoffman/following{/other_user}","gists_url":"https://api.github.com/users/kevinwhoffman/gists{/gist_id}","starred_url":"https://api.github.com/users/kevinwhoffman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kevinwhoffman/subscriptions","organizations_url":"https://api.github.com/users/kevinwhoffman/orgs","repos_url":"https://api.github.com/users/kevinwhoffman/repos","events_url":"https://api.github.com/users/kevinwhoffman/events{/privacy}","received_events_url":"https://api.github.com/users/kevinwhoffman/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-10-22T00:57:10Z","updated_at":"2016-10-22T00:57:10Z","closed_at":null,"body":"## Issue Overview\r\n\r\nWhile 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*.\r\n\r\nThis 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.\r\n\r\nThe lines in question are below and you can see it in context [here](https://github.com/WordImpress/Give/blob/release/1.7/includes/payments/functions.php#L594).\r\n\r\n```\r\n// Account that our 'publish' status is labeled 'Complete'.\r\n$post_status = 'publish' == $payment->status ? 'Complete' : $payment->post_status;\r\n\r\n// Make sure we're matching cases, since they matter.\r\nreturn array_search( strtolower( $post_status ), array_map( 'strtolower', $statuses ) );\r\n```\r\n## Expected Behavior\r\n\r\n- `give_get_payment_status( $payment, false )` should return the `post_status` of the current post as it exists in the database (e.g. `pending`).\r\n- `give_get_payment_status( $payment, true )` should return a translated status label (e.g. 'Pending').\r\n\r\n## Current Behavior\r\n\r\n- `give_get_payment_status( $payment, false )` returns false on non-English sites where status labels have been translated.\r\n- `give_get_payment_status( $payment, true )` works as intended.\r\n\r\n## Possible Solution\r\n\r\nPrior 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.\r\n\r\nAt this point, I think we can change\r\n`return array_search( strtolower( $post_status ), array_map( 'strtolower', $statuses ) );`\r\nto\r\n`return $payment->status;`\r\n\r\nThis solution would eliminate the array operations that are currently causing the function to return `false`.\r\n\r\n## Possible Concerns\r\n\r\n- We should further investigate why this array manipulation is happening and if it serves a purpose before removing it.\r\n- 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.\r\n\r\n## Steps to Reproduce (for bugs)\r\n\r\n1. Execute `var_dump( give_get_payment_status( $payment, false ) );` with a valid payment ID where the current status is `pending`.\r\n2. Confirm that a valid status is returned.\r\n3. Change WP language to French in Settings > Site Language.\r\n4. Repeat step 1 and confirm the function now returns `false`.\r\n\r\nThis will be the case in any scenario where the translated status label differs from the English version.\r\n\r\n## Related PRs\r\n\r\nPR #1140 and Issue #509\r\n\r\n## Todos\r\n- [ ] 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`.\r\n- [ ] Tests"}}
{"actor":{"display_login":"kevinwhoffman"}}