Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paymentez: Updates success_from method #5116

Merged
merged 1 commit into from May 15, 2024

Conversation

almalee24
Copy link

@almalee24 almalee24 commented May 6, 2024

Update success_from method to take current_status for the first message to evaluate.
If the transaction type is refund the successful current_status base
on CANCELLED if status is also success

Remote
34 tests, 85 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed

Unit
33 tests, 137 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed

@almalee24 almalee24 requested a review from a team May 6, 2024 17:38
case action
when 'refund'
successful_current_status = transaction_current_status&.upcase == 'CANCELLED' && request_status&.downcase == 'success'
successful_current_status || default_response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If current_status = "APPROVED" and request_status = "failure" it looks to me like this would be marked as successful because on line 297 success status would inclued "APPROVED" but my understanding is for a refund this combo should be failed. Is that right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's a good point, let me add some validations. I'm don't think that response should be returned but if it's is it's good to add some validations to prevent some false positives

@almalee24 almalee24 force-pushed the paymentez_update_success_from branch 2 times, most recently from 48549db to 0553701 Compare May 13, 2024 14:09
@almalee24 almalee24 requested review from DustinHaefele and a team May 13, 2024 14:09
@almalee24 almalee24 force-pushed the paymentez_update_success_from branch from 0553701 to 44a8d4e Compare May 13, 2024 14:36
Copy link
Contributor

@DustinHaefele DustinHaefele left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@almalee24 almalee24 force-pushed the paymentez_update_success_from branch from 44a8d4e to f561824 Compare May 14, 2024 13:43
Update success_from method to take current_status for the first message to evaluate.
If the transaction type is refund the  successful current_status base
on CANCELLED if status is also success

Remote
34 tests, 85 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

Unit
33 tests, 137 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
@almalee24 almalee24 force-pushed the paymentez_update_success_from branch from f561824 to e85fd16 Compare May 15, 2024 14:52
@almalee24 almalee24 merged commit e85fd16 into master May 15, 2024
0 of 5 checks passed
@almalee24 almalee24 deleted the paymentez_update_success_from branch May 15, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants