refactor(blocklist): separate utility function & kill switch for validating data in blocklist #3360
Conversation
prajjwalkumar17
added
A-core
…ng>> for connector
…witch into refactor_blocklist
|
Hey @vspecky, I feel that the error should be handled gracefully by passing a field or a flag saying that the payment method is blocked and all the further steps should read this value and skip the step if payment method is blocked, the status should then be updated in the |
@Narayanbhat166 I don't think there's an issue with early return. If we wanted to do it gracefully, we'd have to add guards everywhere in the subsequent flows in the form of |
|
@vspecky the problem lies in the way this error is raised, in fact this is not an application error, but we are made to handle it as an application error to short circuit the flow and save some of the dev effort. |
@Narayanbhat166 That makes sense. In the first place, do we even want to send an error? We should probably be sending a |
| Err(inner) => { | ||
| if !inner.current_context().is_db_not_found() { | ||
| logger::error!("Error fetching guard blocklist enabled config {:?}", inner); | ||
| } | ||
| false | ||
| } |
There was a problem hiding this comment.
is this logic doing what you intend to do? There is a negation of the condition, and the return value is outside the if block.
There was a problem hiding this comment.
raise database error if any other error other than DbNotFound is found
There was a problem hiding this comment.
@Narayanbhat166 we already have major refactorings ongoing for the above feature, which will result in most of the code becoming depricated. So we can possibly do all refactorings in the upcoming PRs, as this is critical to go.
| Err(inner) => { | ||
| if !inner.current_context().is_db_not_found() { | ||
| logger::error!("Error fetching guard blocklist enabled config {:?}", inner); | ||
| } | ||
| false | ||
| } |
There was a problem hiding this comment.
raise database error if any other error other than DbNotFound is found
|
Here are all the testing procedures to be followed. |
prajjwalkumar17
removed
S-waiting-on-review
Type of Change
Description
Refactoring the way blocklist is written making the code separated into utility functions rather that being on the core flow itself.
Moreover this will allow us to enable and disable the blocklist feature based on each merchant's requirements.
The steps to test out this feature can be found in the mentioned issue.
Additional Changes
Motivation and Context
How did you test it?
Checklist
cargo +nightly fmt --allcargo clippy