fix(router): better positioning for mandatory customer id conditions (#364)

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

@@ -306,33 +306,19 @@ fn validate_new_mandate_request(req: api::MandateValidationFields) -> RouterResu
     Ok(())
 }
 
-pub fn validate_customer_id_mandatory_cases_api(
+pub fn validate_customer_id_mandatory_cases(
-    shipping: &Option<api::Address>,
+    has_shipping: bool,
-    billing: &Option<api::Address>,
+    has_billing: bool,
-    setup_future_usage: &Option<api_enums::FutureUsage>,
+    has_setup_future_usage: bool,
     customer_id: &Option<String>,
 ) -> RouterResult<()> {
-    match (shipping, billing, setup_future_usage, customer_id) {
+    match (
-        (Some(_), _, _, None) | (_, Some(_), _, None) | (_, _, Some(_), None) => {
+        has_shipping,
-            Err(errors::ApiErrorResponse::PreconditionFailed {
+        has_billing,
-                message: "customer_id is mandatory when shipping or billing \
+        has_setup_future_usage,
-                address is given or when setup_future_usage is given"
+        customer_id,
-                    .to_string(),
+    ) {
-            })
+        (true, _, _, None) | (_, true, _, None) | (_, _, true, None) => {
-            .into_report()
-        }
-        _ => Ok(()),
-    }
-}
-
-pub fn validate_customer_id_mandatory_cases_storage(
-    shipping: &Option<storage::Address>,
-    billing: &Option<storage::Address>,
-    setup_future_usage: &Option<storage_enums::FutureUsage>,
-    customer_id: &Option<String>,
-) -> RouterResult<()> {
-    match (shipping, billing, setup_future_usage, customer_id) {
-        (Some(_), _, _, None) | (_, Some(_), _, None) | (_, _, Some(_), None) => {
             Err(errors::ApiErrorResponse::PreconditionFailed {
                 message: "customer_id is mandatory when shipping or billing \
                 address is given or when setup_future_usage is given"

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

@@ -106,6 +106,16 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
         currency = payment_attempt.currency.get_required_value("currency")?;
         amount = payment_attempt.amount.into();
 
+        helpers::validate_customer_id_mandatory_cases(
+            request.shipping.is_some(),
+            request.billing.is_some(),
+            request.setup_future_usage.is_some(),
+            &payment_intent
+                .customer_id
+                .clone()
+                .or_else(|| request.customer_id.clone()),
+        )?;
+
         let shipping_address = helpers::get_address_for_payment_request(
             db,
             request.shipping.as_ref(),
@@ -123,18 +133,6 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
         )
         .await?;
 
-        helpers::validate_customer_id_mandatory_cases_storage(
-            &shipping_address,
-            &billing_address,
-            &payment_intent
-                .setup_future_usage
-                .or_else(|| request.setup_future_usage.map(ForeignInto::foreign_into)),
-            &payment_intent
-                .customer_id
-                .clone()
-                .or_else(|| request.customer_id.clone()),
-        )?;
-
         connector_response = db
             .find_connector_response_by_payment_id_merchant_id_attempt_id(
                 &payment_attempt.payment_id,

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

@@ -378,6 +378,13 @@ impl<F: Send + Clone> ValidateRequest<F, api::PaymentsRequest> for PaymentCreate
                 &mandate_type,
                 &request.payment_token,
             )?;
+
+            helpers::validate_customer_id_mandatory_cases(
+                request.shipping.is_some(),
+                request.billing.is_some(),
+                request.setup_future_usage.is_some(),
+                &request.customer_id,
+            )?;
         }
 
         Ok((

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

@@ -93,6 +93,18 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
             payment_intent.client_secret.as_ref(),
         )?;
 
+        if request.confirm.unwrap_or(false) {
+            helpers::validate_customer_id_mandatory_cases(
+                request.shipping.is_some(),
+                request.billing.is_some(),
+                request.setup_future_usage.is_some(),
+                &payment_intent
+                    .customer_id
+                    .clone()
+                    .or_else(|| request.customer_id.clone()),
+            )?;
+        }
+
         let shipping_address = helpers::get_address_for_payment_request(
             db,
             request.shipping.as_ref(),
@@ -114,20 +126,6 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
         payment_intent.billing_address_id = billing_address.clone().map(|x| x.address_id);
         payment_intent.return_url = request.return_url.clone();
 
-        if request.confirm.unwrap_or(false) {
-            helpers::validate_customer_id_mandatory_cases_storage(
-                &shipping_address,
-                &billing_address,
-                &payment_intent
-                    .setup_future_usage
-                    .or_else(|| request.setup_future_usage.map(ForeignInto::foreign_into)),
-                &payment_intent
-                    .customer_id
-                    .clone()
-                    .or_else(|| request.customer_id.clone()),
-            )?;
-        }
-
         let token = token.or_else(|| payment_attempt.payment_token.clone());
 
         if request.confirm.unwrap_or(false) {