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

fix(frm): update FRM manual review flow #3176

Merged
merged 7 commits into from
Jan 19, 2024
Merged

fix(frm): update FRM manual review flow #3176

merged 7 commits into from
Jan 19, 2024

Conversation

kashif-m
Copy link
Contributor

@kashif-m kashif-m commented Dec 20, 2023

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

#3175

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Tested locally using postman collection

https://galactic-capsule-229427.postman.co/workspace/My-Workspace~2b563e0d-bad3-420f-8c0b-0fd5b278a4fe/collection/9906252-e8ad195c-90f5-4527-a40b-1a25bc9bedd2?action=share&creator=9906252

Test Cases

  • Payments <> FRM manual flow (pre + post) - Approve + Rejection should be working

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@kashif-m kashif-m added the C-bug Category: Bug label Dec 20, 2023
@kashif-m kashif-m self-assigned this Dec 20, 2023
@kashif-m kashif-m requested review from a team as code owners December 20, 2023 09:15
jagan-jaya
jagan-jaya previously approved these changes Dec 22, 2023
Copy link
Contributor

@jagan-jaya jagan-jaya left a comment

Choose a reason for hiding this comment

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

@kashif-m
I'm good with the changes, we need to have someone from core to check payment_response.rs file just to mitigate unknown issues. I understand that the status is changed only for FRM case but payment_response is currently used in every payment call so just need more eyes to verify.

vspecky
vspecky previously approved these changes Dec 22, 2023
Copy link
Member

@vspecky vspecky left a comment

Choose a reason for hiding this comment

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

Got some nitpicks but approving since these are not blockers.

Copy link
Member

Choose a reason for hiding this comment

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

Should probably rename the file as well. Can be taken up later.

Comment on lines +523 to +529
(
enums::AttemptStatus::Authorized,
enums::AttemptStatus::Unresolved,
Some(frm_message),
) => match frm_message.frm_status {
enums::FraudCheckStatus::Fraud | enums::FraudCheckStatus::ManualReview => {
attempt_status
Copy link
Member

@vspecky vspecky Dec 22, 2023

Choose a reason for hiding this comment

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

Nit:

Suggested change
(
enums::AttemptStatus::Authorized,
enums::AttemptStatus::Unresolved,
Some(frm_message),
) => match frm_message.frm_status {
enums::FraudCheckStatus::Fraud | enums::FraudCheckStatus::ManualReview => {
attempt_status
(
enums::AttemptStatus::Authorized,
enums::AttemptStatus::Unresolved,
Some(FrmMessage { frm_status: enums::FraudCheckStatus::Fraud | enums::FraudCheckStatus::ManualReview }),
) => attempt_status,

Narayanbhat166
Narayanbhat166 previously approved these changes Jan 5, 2024
@kashif-m kashif-m dismissed stale reviews from Narayanbhat166, vspecky, and jagan-jaya via acb2fec January 9, 2024 09:44
vspecky
vspecky previously approved these changes Jan 9, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Jan 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 18, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Jan 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 18, 2024
@kashif-m kashif-m changed the title fix(frm): update FRM's manual review flow fix(frm): update FRM manual review flow Jan 19, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Jan 19, 2024
Merged via the queue into main with commit 5255ba9 Jan 19, 2024
11 checks passed
@likhinbopanna likhinbopanna deleted the frm_manual_fix branch January 19, 2024 06:58
@kashif-m kashif-m restored the frm_manual_fix branch April 12, 2024 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Bug
Projects
No open projects
Status: Candidate
Development

Successfully merging this pull request may close these issues.

[BUG] No provision for approving payments kept in ManualReview state for FRM flows
5 participants