refactor(core): validate payment status using common function (#461)

crates/router/src/core/payments/helpers.rs

@@ -1,6 +1,6 @@
 use std::borrow::Cow;
 
-use common_utils::ext_traits::AsyncExt;
+use common_utils::{ext_traits::AsyncExt, fp_utils};
 // TODO : Evaluate all the helper functions ()
 use error_stack::{report, IntoReport, ResultExt};
 use masking::ExposeOptionInterface;
@@ -1145,6 +1145,20 @@ pub(crate) fn authenticate_client_secret(
     }
 }
 
+pub(crate) fn validate_payment_status_against_not_allowed_statuses(
+    intent_status: &storage_enums::IntentStatus,
+    not_allowed_statuses: &[storage_enums::IntentStatus],
+    action: &'static str,
+) -> Result<(), errors::ApiErrorResponse> {
+    fp_utils::when(not_allowed_statuses.contains(intent_status), || {
+        Err(errors::ApiErrorResponse::PreconditionFailed {
+            message: format!(
+                "You cannot {action} this payment because it has status {intent_status}",
+            ),
+        })
+    })
+}
+
 pub(crate) fn validate_pm_or_token_given(
     payment_method: &Option<api_enums::PaymentMethodType>,
     payment_method_data: &Option<api::PaymentMethod>,

crates/router/src/core/payments/operations/payment_confirm.rs

@@ -1,7 +1,7 @@
 use std::marker::PhantomData;
 
 use async_trait::async_trait;
-use error_stack::{report, ResultExt};
+use error_stack::ResultExt;
 use router_derive::PaymentOperation;
 use router_env::{instrument, tracing};
 
@@ -17,7 +17,7 @@ use crate::{
     types::{
         self,
         api::{self, PaymentIdTypeExt},
-        storage::{self, enums},
+        storage::{self, enums as storage_enums},
         transformers::ForeignInto,
     },
     utils::{self, OptionExt},
@@ -51,6 +51,22 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
             .get_payment_intent_id()
             .change_context(errors::ApiErrorResponse::PaymentNotFound)?;
 
+        payment_intent = db
+            .find_payment_intent_by_payment_id_merchant_id(&payment_id, merchant_id, storage_scheme)
+            .await
+            .map_err(|error| {
+                error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
+            })?;
+
+        helpers::validate_payment_status_against_not_allowed_statuses(
+            &payment_intent.status,
+            &[
+                storage_enums::IntentStatus::Failed,
+                storage_enums::IntentStatus::Succeeded,
+            ],
+            "confirm",
+        )?;
+
         let (token, payment_method_type, setup_mandate) =
             helpers::get_token_pm_type_mandate_details(
                 state,
@@ -60,13 +76,6 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
             )
             .await?;
 
-        payment_intent = db
-            .find_payment_intent_by_payment_id_merchant_id(&payment_id, merchant_id, storage_scheme)
-            .await
-            .map_err(|error| {
-                error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
-            })?;
-
         helpers::authenticate_client_secret(
             request.client_secret.as_ref(),
             payment_intent.client_secret.as_ref(),
@@ -149,16 +158,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
         payment_intent.billing_address_id = billing_address.clone().map(|i| i.address_id);
         payment_intent.return_url = request.return_url.clone();
 
-        match payment_intent.status {
+        Ok((
-            enums::IntentStatus::Succeeded | enums::IntentStatus::Failed => {
-                Err(report!(errors::ApiErrorResponse::PreconditionFailed {
-                    message: format!(
-                        "You cannot confirm this Payment because it has already {}, after being previously confirmed.",
-                        payment_intent.status
-                    )
-                }))
-            }
-            _ => Ok((
             Box::new(self),
             PaymentData {
                 flow: PhantomData,
@@ -189,8 +189,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
                 phone: request.phone.clone(),
                 phone_country_code: request.phone_country_code.clone(),
             }),
-            )),
+        ))
-        }
     }
 }
 
@@ -225,7 +224,7 @@ impl<F: Clone + Send> Domain<F, api::PaymentsRequest> for PaymentConfirm {
         &'a self,
         state: &'a AppState,
         payment_data: &mut PaymentData<F>,
-        _storage_scheme: enums::MerchantStorageScheme,
+        _storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(
         BoxedOperation<'a, F, api::PaymentsRequest>,
         Option<api::PaymentMethod>,
@@ -276,7 +275,7 @@ impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for Paymen
         _payment_id: &api::PaymentIdType,
         mut payment_data: PaymentData<F>,
         customer: Option<storage::Customer>,
-        storage_scheme: enums::MerchantStorageScheme,
+        storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(BoxedOperation<'b, F, api::PaymentsRequest>, PaymentData<F>)>
     where
         F: 'b + Send,
@@ -286,13 +285,13 @@ impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for Paymen
 
         let (intent_status, attempt_status) = match payment_data.payment_attempt.authentication_type
         {
-            Some(enums::AuthenticationType::NoThreeDs) => (
+            Some(storage_enums::AuthenticationType::NoThreeDs) => (
-                enums::IntentStatus::Processing,
+                storage_enums::IntentStatus::Processing,
-                enums::AttemptStatus::Pending,
+                storage_enums::AttemptStatus::Pending,
             ),
             _ => (
-                enums::IntentStatus::RequiresCustomerAction,
+                storage_enums::IntentStatus::RequiresCustomerAction,
-                enums::AttemptStatus::AuthenticationPending,
+                storage_enums::AttemptStatus::AuthenticationPending,
             ),
         };
 

crates/router/src/core/payments/operations/payment_session.rs

@@ -18,7 +18,7 @@ use crate::{
     routes::AppState,
     types::{
         api::{self, enums as api_enums, PaymentIdTypeExt},
-        storage::{self, enums},
+        storage::{self, enums as storage_enums},
         transformers::ForeignInto,
     },
     utils::OptionExt,
@@ -53,6 +53,22 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsSessionRequest>
         let merchant_id = &merchant_account.merchant_id;
         let storage_scheme = merchant_account.storage_scheme;
 
+        let mut payment_intent = db
+            .find_payment_intent_by_payment_id_merchant_id(&payment_id, merchant_id, storage_scheme)
+            .await
+            .map_err(|error| {
+                error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
+            })?;
+
+        helpers::validate_payment_status_against_not_allowed_statuses(
+            &payment_intent.status,
+            &[
+                storage_enums::IntentStatus::Failed,
+                storage_enums::IntentStatus::Succeeded,
+            ],
+            "create a session token for",
+        )?;
+
         let mut payment_attempt = db
             .find_payment_attempt_by_payment_id_merchant_id(
                 &payment_id,
@@ -64,16 +80,9 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsSessionRequest>
                 error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
             })?;
 
-        let mut payment_intent = db
-            .find_payment_intent_by_payment_id_merchant_id(&payment_id, merchant_id, storage_scheme)
-            .await
-            .map_err(|error| {
-                error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
-            })?;
-
         let currency = payment_intent.currency.get_required_value("currency")?;
 
-        payment_attempt.payment_method = Some(enums::PaymentMethodType::Wallet);
+        payment_attempt.payment_method = Some(storage_enums::PaymentMethodType::Wallet);
 
         let amount = payment_intent.amount.into();
 
@@ -163,7 +172,7 @@ impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsSessionRequest> for
         _payment_id: &api::PaymentIdType,
         mut payment_data: PaymentData<F>,
         _customer: Option<storage::Customer>,
-        storage_scheme: enums::MerchantStorageScheme,
+        storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(
         BoxedOperation<'b, F, api::PaymentsSessionRequest>,
         PaymentData<F>,
@@ -255,7 +264,7 @@ where
         &'b self,
         _state: &'b AppState,
         _payment_data: &mut PaymentData<F>,
-        _storage_scheme: enums::MerchantStorageScheme,
+        _storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(
         BoxedOperation<'b, F, api::PaymentsSessionRequest>,
         Option<api::PaymentMethod>,

crates/router/src/core/payments/operations/payment_start.rs

@@ -1,7 +1,7 @@
 use std::marker::PhantomData;
 
 use async_trait::async_trait;
-use error_stack::{report, ResultExt};
+use error_stack::ResultExt;
 use router_derive::PaymentOperation;
 use router_env::{instrument, tracing};
 
@@ -17,7 +17,7 @@ use crate::{
     routes::AppState,
     types::{
         api::{self, PaymentIdTypeExt},
-        storage::{self, enums},
+        storage::{self, enums as storage_enums},
         transformers::ForeignInto,
     },
     utils::OptionExt,
@@ -58,6 +58,15 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsStartRequest> f
                 error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
             })?;
 
+        helpers::validate_payment_status_against_not_allowed_statuses(
+            &payment_intent.status,
+            &[
+                storage_enums::IntentStatus::Failed,
+                storage_enums::IntentStatus::Succeeded,
+            ],
+            "update",
+        )?;
+
         payment_attempt = db
             .find_payment_attempt_by_payment_id_merchant_id(
                 &payment_id,
@@ -111,15 +120,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsStartRequest> f
                     .attach_printable("Database error when finding connector response")
             })?;
 
-        match payment_intent.status {
+        Ok((
-            enums::IntentStatus::Succeeded | enums::IntentStatus::Failed => {
-                Err(report!(errors::ApiErrorResponse::PreconditionFailed {
-                    message: "You cannot confirm this Payment because it has already succeeded \
-                              after being previously confirmed."
-                        .into()
-                }))
-            }
-            _ => Ok((
             Box::new(self),
             PaymentData {
                 flow: PhantomData,
@@ -144,8 +145,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsStartRequest> f
                 card_cvc: None,
             },
             Some(customer_details),
-            )),
+        ))
-        }
     }
 }
 
@@ -158,7 +158,7 @@ impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsStartRequest> for P
         _payment_id: &api::PaymentIdType,
         payment_data: PaymentData<F>,
         _customer: Option<storage::Customer>,
-        _storage_scheme: enums::MerchantStorageScheme,
+        _storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(
         BoxedOperation<'b, F, api::PaymentsStartRequest>,
         PaymentData<F>,
@@ -236,7 +236,7 @@ where
         &'a self,
         state: &'a AppState,
         payment_data: &mut PaymentData<F>,
-        _storage_scheme: enums::MerchantStorageScheme,
+        _storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(
         BoxedOperation<'a, F, api::PaymentsStartRequest>,
         Option<api::PaymentMethod>,

crates/router/src/core/payments/operations/payment_update.rs

@@ -2,7 +2,7 @@ use std::marker::PhantomData;
 
 use async_trait::async_trait;
 use common_utils::ext_traits::AsyncExt;
-use error_stack::{report, ResultExt};
+use error_stack::ResultExt;
 use router_derive::PaymentOperation;
 use router_env::{instrument, tracing};
 
@@ -17,7 +17,7 @@ use crate::{
     routes::AppState,
     types::{
         api::{self, PaymentIdTypeExt},
-        storage::{self, enums},
+        storage::{self, enums as storage_enums},
         transformers::ForeignInto,
     },
     utils::OptionExt,
@@ -41,7 +41,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
         PaymentData<F>,
         Option<CustomerDetails>,
     )> {
-        let (mut payment_intent, mut payment_attempt, currency): (_, _, enums::Currency);
+        let (mut payment_intent, mut payment_attempt, currency): (_, _, storage_enums::Currency);
 
         let payment_id = payment_id
             .get_payment_intent_id()
@@ -50,6 +50,29 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
         let storage_scheme = merchant_account.storage_scheme;
 
         let db = &*state.store;
+
+        payment_intent = db
+            .find_payment_intent_by_payment_id_merchant_id(&payment_id, merchant_id, storage_scheme)
+            .await
+            .map_err(|error| {
+                error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
+            })?;
+
+        helpers::validate_payment_status_against_not_allowed_statuses(
+            &payment_intent.status,
+            &[
+                storage_enums::IntentStatus::Failed,
+                storage_enums::IntentStatus::Succeeded,
+                storage_enums::IntentStatus::RequiresCapture,
+            ],
+            "update",
+        )?;
+
+        helpers::authenticate_client_secret(
+            request.client_secret.as_ref(),
+            payment_intent.client_secret.as_ref(),
+        )?;
+
         let (token, payment_method_type, setup_mandate) =
             helpers::get_token_pm_type_mandate_details(
                 state,
@@ -81,18 +104,6 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
             .amount
             .unwrap_or_else(|| payment_attempt.amount.into());
 
-        payment_intent = db
-            .find_payment_intent_by_payment_id_merchant_id(&payment_id, merchant_id, storage_scheme)
-            .await
-            .map_err(|error| {
-                error.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)
-            })?;
-
-        helpers::authenticate_client_secret(
-            request.client_secret.as_ref(),
-            payment_intent.client_secret.as_ref(),
-        )?;
-
         if request.confirm.unwrap_or(false) {
             helpers::validate_customer_id_mandatory_cases(
                 request.shipping.is_some(),
@@ -178,24 +189,13 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
                 if request.confirm.unwrap_or(false) {
                     payment_intent.status
                 } else {
-                    enums::IntentStatus::RequiresConfirmation
+                    storage_enums::IntentStatus::RequiresConfirmation
                 }
             }
-            None => enums::IntentStatus::RequiresPaymentMethod,
+            None => storage_enums::IntentStatus::RequiresPaymentMethod,
         };
 
-        match payment_intent.status {
+        Ok((
-            enums::IntentStatus::Succeeded
-            | enums::IntentStatus::Failed
-            | enums::IntentStatus::RequiresCapture => {
-                Err(report!(errors::ApiErrorResponse::PreconditionFailed {
-                    message: format!(
-                        "You cannot update this Payment because the status of this payment is {}",
-                        payment_intent.status
-                    )
-                }))
-            }
-            _ => Ok((
             next_operation,
             PaymentData {
                 flow: PhantomData,
@@ -226,8 +226,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
                 phone: request.phone.clone(),
                 phone_country_code: request.phone_country_code.clone(),
             }),
-            )),
+        ))
-        }
     }
 }
 
@@ -262,7 +261,7 @@ impl<F: Clone + Send> Domain<F, api::PaymentsRequest> for PaymentUpdate {
         &'a self,
         state: &'a AppState,
         payment_data: &mut PaymentData<F>,
-        _storage_scheme: enums::MerchantStorageScheme,
+        _storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(
         BoxedOperation<'a, F, api::PaymentsRequest>,
         Option<api::PaymentMethod>,
@@ -299,22 +298,23 @@ impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for Paymen
         _payment_id: &api::PaymentIdType,
         mut payment_data: PaymentData<F>,
         customer: Option<storage::Customer>,
-        storage_scheme: enums::MerchantStorageScheme,
+        storage_scheme: storage_enums::MerchantStorageScheme,
     ) -> RouterResult<(BoxedOperation<'b, F, api::PaymentsRequest>, PaymentData<F>)>
     where
         F: 'b + Send,
     {
         let is_payment_method_unavailable =
             payment_data.payment_attempt.payment_method_id.is_none()
-                && payment_data.payment_intent.status == enums::IntentStatus::RequiresPaymentMethod;
+                && payment_data.payment_intent.status
+                    == storage_enums::IntentStatus::RequiresPaymentMethod;
 
         let payment_method = payment_data.payment_attempt.payment_method;
 
         let get_attempt_status = || {
             if is_payment_method_unavailable {
-                enums::AttemptStatus::PaymentMethodAwaited
+                storage_enums::AttemptStatus::PaymentMethodAwaited
             } else {
-                enums::AttemptStatus::ConfirmationAwaited
+                storage_enums::AttemptStatus::ConfirmationAwaited
             }
         };
 
@@ -340,11 +340,11 @@ impl<F: Clone> UpdateTracker<F, PaymentData<F>, api::PaymentsRequest> for Paymen
         let intent_status = {
             let current_intent_status = payment_data.payment_intent.status;
             if is_payment_method_unavailable {
-                enums::IntentStatus::RequiresPaymentMethod
+                storage_enums::IntentStatus::RequiresPaymentMethod
             } else if !payment_data.confirm.unwrap_or(true)
-                || current_intent_status == enums::IntentStatus::RequiresCustomerAction
+                || current_intent_status == storage_enums::IntentStatus::RequiresCustomerAction
             {
-                enums::IntentStatus::RequiresConfirmation
+                storage_enums::IntentStatus::RequiresConfirmation
             } else {
                 payment_data.payment_intent.status
             }