-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat(revenue_recovery): Introducing new calculate job for card switching and invoice queueing #8848
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
base: redis_manipulation
Are you sure you want to change the base?
Conversation
Changed Files
|
@@ -42,6 +48,24 @@ use crate::{ | |||
workflows::revenue_recovery as revenue_recovery_flow, | |||
}; | |||
|
|||
// function to extract customer ID from payment intent | |||
pub fn extract_customer_id_from_intent( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already this function in written above
// update the status of token in redis | ||
|
||
// Update CALCULATE_WORKFLOW to complete status on payment success | ||
if let Err(e) = update_calculate_workflow_on_success(state, payment_intent).await { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we are updating the calculate workflow on success of a payment?, on completion of calculate workflow only a execute task itself created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are updating the business status of CALCULATE_WORKFLOW from scheduled to CALCULATE_WORKFLOW_COMPLETE, these business status will help in monitoring of invoice status for dashboard which will be picked up.
) | ||
.await; | ||
.await?; | ||
// get a reschedule time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why these code where commented if not required remove those commented lines, other places also
@@ -354,6 +397,30 @@ impl Action { | |||
); | |||
}; | |||
|
|||
// Update CALCULATE_WORKFLOW to complete status on payment success | |||
if let Err(e) = | |||
update_calculate_workflow_on_success(state, payment_intent).await |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we are updating the calculate workflow on success of a payment?, on completion of calculate workflow only a execute task itself created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are updating the business status of CALCULATE_WORKFLOW from scheduled to CALCULATE_WORKFLOW_COMPLETE, these business status will help in monitoring of invoice status for dashboard which will be picked up.
state, | ||
&connector_customer_id, | ||
&token_id, | ||
"-1".to_string() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of passing a hardcoded value, have a increment and decrement function separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 is an error code which basically means success, i am sending this to redis so that it can update the status of token, handled in this PR (#8846)
} | ||
|
||
RevenueRecoveryPaymentsAttemptStatus::Processing => { | ||
update_calculate_workflow_psync_status( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we are updating the calculate workflow on processing of a payment?, on completion of calculate workflow only a execute task itself created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are only updating the business status of CALCULATE_WORKFLOW
to from scheduled to processing, these business status will help in monitoring of invoice status for dashboard which will be picked up.
@@ -563,18 +645,94 @@ impl Action { | |||
match response { | |||
Ok(_payment_data) => match payment_attempt.status.foreign_into() { | |||
RevenueRecoveryPaymentsAttemptStatus::Succeeded => { | |||
let payment_intent = db |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't read payment related database calls, which against the modularity, refer with @Aprabhat19
Ok(Self::SuccessfulPayment(payment_attempt)) | ||
} | ||
RevenueRecoveryPaymentsAttemptStatus::Failed => { | ||
Self::decide_retry_failure_action( | ||
let payment_intent = db | ||
.find_payment_intent_by_id( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't read payment related database calls, which against the modularity, refer with @Aprabhat19
// 1. Extract customer_id and token_list from tracking_data | ||
let customer_id = payment_intent | ||
.extract_connector_customer_id_from_payment_intent() | ||
.map_err(|_| sch_errors::ProcessTrackerError::MissingRequiredField)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't discard error,
change context of the error to MissingRequiredField
let new_schedule_time = scheduled_token | ||
.scheduled_at | ||
.unwrap_or(common_utils::date_time::now()) | ||
+ time::Duration::minutes(15); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't hard code time like this, make this as configurable one
); | ||
|
||
// Update the process tracker to reopen the calculate workflow | ||
// 1. Change status from "finish" to "new" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
retrying should be a pending
Type of Change
Description
CALCULATE WORKFLOW
CALCULATE WORKFLOW
When we receive failed webhook and its intent retry count is more than the threshold, it will inserted into calculate job with the schedule time with 1 hour from now, before inserting it will first check the DB for existing entry for CALCULATE_WORKFLOW_{payment_intent},
If found and business_status is CALCULATE_WORKFLOW_QUEUED, then we just update the existing entry with scheduled time with plus hour.
If found and business_status is not CALCULATE_WORKFLOW_QUEUED, we do not insert the entry
If not found, make a new entry with scheduled time as 1 hour plus from now time and feed token list from payment_intent to tracking_data
Introduced one new field in tracking_data
Invoice_scheduled_time Option (used as scheduling time in execute workflow and as well as for dashboard purpose)
At the scheduled time, consumer picks it up and perform tasks like:-
Call a function(best_token_with_time() ) that returns Option<{(token, scheduled_time)}>
We pass scheduled time in tracking_data as well as pass in insert_execute_task_to_pt so that we can schedule EXECUTE WORKFLOW at that time, we change calculate flow's the business_status to CALCULATE_WORKFLOW_SCHEDULED and status as Finish also lock the customer of that active token with redis fn call.
If we receive None from best_token_with_time(), then we call get_payment_processor_token_with_schedule_time() whose return type is also Option<{PSPstatusdetails}>, if this is Some() then we just requeue it with wait time return by this fn, and add 15 minutes as buffer time and update the business_status as CALCULATE_WORKFLOW_QUEUED, else if it returns None then we mark the invoice as finish and finish the calculate workflow since this is a HARD DECLINE.
EXECUTE WORKFLOW
When the consumer picks up the execute task, it tries to do payment by calling proxy_api at the scheduled_time, here we have three possibilities for response of payment:-
Success:-
Update the token with success status and also update calculate_workflow entry’s business_status to CALCULATE_WORKFLOW_COMPLETED and status to Finish and also unlock the customer with redis fn call and change the status of token to success in redis.
Failure:-
Finish the task by updating the status to Finish and business status to EXECUTE_WORKFLOW_FINISH, increment the retry count by 1 and also unlock the customer with redis fn call, then find the calculate job for the id, and update its the business status to CALCULATE_WORKFLOW_QUEUED and scheduled time to now() + 1 hr
Pending:-
Will call PSYNC, update the business status of calculate job to CALCULATE_WORKFLOW_PROCESSING
PSYNC WORKFLOW
When the consumer picks up the psync task, here we have three possibilities:-
Success:-
Update the token with success status and also update calculate_workflow entry’s business_status to CALCULATE_WORKFLOW_COMPLETED and status to Finish.
Failure:-
Finish the task by updating the status to Finish and business status to EXECUTE_WORKFLOW_FINISH and also increment the retry count by 1, find the calculate job for the id, and update the business status to CALCULATE_WORKFLOW_QUEUED and scheduled time to now() + 1 hr
Pending:-
Will call PSYNC, update the business status of calculate job to CALCULATE_WORKFLOW_PROCESSING
Additional Changes
Motivation and Context
How did you test it?
Checklist
cargo +nightly fmt --all
cargo clippy