fix(payments_create): save the customer_id in payments create (#5262)

crates/api_models/src/payments.rs

@@ -518,6 +518,139 @@ pub struct PaymentsRequest {
     pub merchant_order_reference_id: Option<String>,
 }
 
+/// Checks if the inner values of two options are equal
+/// Returns true if values are not equal, returns false in other cases
+fn are_optional_values_invalid<T: PartialEq>(
+    first_option: Option<&T>,
+    second_option: Option<&T>,
+) -> bool {
+    match (first_option, second_option) {
+        (Some(first_option), Some(second_option)) => first_option != second_option,
+        _ => false,
+    }
+}
+
+impl PaymentsRequest {
+    /// Get the customer id
+    ///
+    /// First check the id for `customer.id`
+    /// If not present, check for `customer_id` at the root level
+    pub fn get_customer_id(&self) -> Option<&id_type::CustomerId> {
+        self.customer_id
+            .as_ref()
+            .or(self.customer.as_ref().map(|customer| &customer.id))
+    }
+
+    /// Checks if the customer details are passed in both places
+    /// If they are passed in both places, check for both the values to be equal
+    /// Or else, return the field which has inconsistent data
+    pub fn validate_customer_details_in_request(&self) -> Option<Vec<&str>> {
+        if let Some(CustomerDetails {
+            id,
+            name,
+            email,
+            phone,
+            phone_country_code,
+        }) = self.customer.as_ref()
+        {
+            let invalid_fields = [
+                are_optional_values_invalid(self.customer_id.as_ref(), Some(id))
+                    .then_some("customer_id and customer.id"),
+                are_optional_values_invalid(self.email.as_ref(), email.as_ref())
+                    .then_some("email and customer.email"),
+                are_optional_values_invalid(self.name.as_ref(), name.as_ref())
+                    .then_some("name and customer.name"),
+                are_optional_values_invalid(self.phone.as_ref(), phone.as_ref())
+                    .then_some("phone and customer.phone"),
+                are_optional_values_invalid(
+                    self.phone_country_code.as_ref(),
+                    phone_country_code.as_ref(),
+                )
+                .then_some("phone_country_code and customer.phone_country_code"),
+            ]
+            .into_iter()
+            .flatten()
+            .collect::<Vec<_>>();
+
+            if invalid_fields.is_empty() {
+                None
+            } else {
+                Some(invalid_fields)
+            }
+        } else {
+            None
+        }
+    }
+}
+
+#[cfg(test)]
+mod payments_request_test {
+    use common_utils::generate_customer_id_of_default_length;
+
+    use super::*;
+
+    #[test]
+    fn test_valid_case_where_customer_details_are_passed_only_once() {
+        let customer_id = generate_customer_id_of_default_length();
+        let payments_request = PaymentsRequest {
+            customer_id: Some(customer_id),
+            ..Default::default()
+        };
+
+        assert!(payments_request
+            .validate_customer_details_in_request()
+            .is_none());
+    }
+
+    #[test]
+    fn test_valid_case_where_customer_id_is_passed_in_both_places() {
+        let customer_id = generate_customer_id_of_default_length();
+
+        let customer_object = CustomerDetails {
+            id: customer_id.clone(),
+            name: None,
+            email: None,
+            phone: None,
+            phone_country_code: None,
+        };
+
+        let payments_request = PaymentsRequest {
+            customer_id: Some(customer_id),
+            customer: Some(customer_object),
+            ..Default::default()
+        };
+
+        assert!(payments_request
+            .validate_customer_details_in_request()
+            .is_none());
+    }
+
+    #[test]
+    fn test_invalid_case_where_customer_id_is_passed_in_both_places() {
+        let customer_id = generate_customer_id_of_default_length();
+        let another_customer_id = generate_customer_id_of_default_length();
+
+        let customer_object = CustomerDetails {
+            id: customer_id.clone(),
+            name: None,
+            email: None,
+            phone: None,
+            phone_country_code: None,
+        };
+
+        let payments_request = PaymentsRequest {
+            customer_id: Some(another_customer_id),
+            customer: Some(customer_object),
+            ..Default::default()
+        };
+
+        assert_eq!(
+            payments_request.validate_customer_details_in_request(),
+            Some(vec!["customer_id and customer.id"])
+        );
+    }
+}
+
 /// Fee information to be charged on the payment being collected
 #[derive(Debug, serde::Deserialize, serde::Serialize, Clone, ToSchema)]
 #[serde(rename_all = "snake_case")]

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

@@ -451,7 +451,7 @@ pub async fn get_token_pm_type_mandate_details(
     merchant_account: &domain::MerchantAccount,
     merchant_key_store: &domain::MerchantKeyStore,
     payment_method_id: Option<String>,
-    customer_id: &Option<id_type::CustomerId>,
+    payment_intent_customer_id: Option<&id_type::CustomerId>,
 ) -> RouterResult<MandateGenericData> {
     let mandate_data = request.mandate_data.clone().map(MandateData::foreign_from);
     let (
@@ -505,14 +505,15 @@ pub async fn get_token_pm_type_mandate_details(
                             .to_not_found_response(
                                 errors::ApiErrorResponse::PaymentMethodNotFound,
                             )?;
-                        let customer_id = get_customer_id_from_payment_request(request)
+                        let customer_id = request
+                            .get_customer_id()
                             .get_required_value("customer_id")?;
 
                         verify_mandate_details_for_recurring_payments(
                             &payment_method_info.merchant_id,
                             &merchant_account.merchant_id,
                             &payment_method_info.customer_id,
-                            &customer_id,
+                            customer_id,
                         )?;
 
                         (
@@ -552,10 +553,9 @@ pub async fn get_token_pm_type_mandate_details(
                         || request.payment_method_type
                             == Some(api_models::enums::PaymentMethodType::GooglePay)
                     {
-                        let payment_request_customer_id =
+                        let payment_request_customer_id = request.get_customer_id();
-                            get_customer_id_from_payment_request(request);
                         if let Some(customer_id) =
-                            &payment_request_customer_id.or(customer_id.clone())
+                            payment_request_customer_id.or(payment_intent_customer_id)
                         {
                             let customer_saved_pm_option = match state
                                 .store
@@ -711,10 +711,10 @@ pub async fn get_token_for_recurring_mandate(
         .map(|pi| pi.amount.get_amount_as_i64());
     let original_payment_authorized_currency =
         original_payment_intent.clone().and_then(|pi| pi.currency);
-    let customer = get_customer_id_from_payment_request(req).get_required_value("customer_id")?;
+    let customer = req.get_customer_id().get_required_value("customer_id")?;
 
     let payment_method_id = {
-        if mandate.customer_id != customer {
+        if &mandate.customer_id != customer {
             Err(report!(errors::ApiErrorResponse::PreconditionFailed {
                 message: "customer_id must match mandate customer_id".into()
             }))?
@@ -1459,25 +1459,6 @@ pub async fn get_customer_from_details<F: Clone>(
     }
 }
 
-// Checks if the inner values of two options are not equal and throws appropriate error
-fn validate_options_for_inequality<T: PartialEq>(
-    first_option: Option<&T>,
-    second_option: Option<&T>,
-    field_name: &str,
-) -> Result<(), errors::ApiErrorResponse> {
-    fp_utils::when(
-        first_option
-            .zip(second_option)
-            .map(|(value1, value2)| value1 != value2)
-            .unwrap_or(false),
-        || {
-            Err(errors::ApiErrorResponse::PreconditionFailed {
-                message: format!("The field name `{field_name}` sent in both places is ambiguous"),
-            })
-        },
-    )
-}
-
 pub fn validate_max_amount(
     amount: api_models::payments::Amount,
 ) -> CustomResult<(), errors::ApiErrorResponse> {
@@ -1496,44 +1477,21 @@ pub fn validate_max_amount(
     }
 }
 
-// Checks if the customer details are passed in both places
+/// Check whether the customer information that is sent in the root of payments request
-// If so, raise an error
+/// and in the customer object are same, if the values mismatch return an error
-pub fn validate_customer_details_in_request(
+pub fn validate_customer_information(
     request: &api_models::payments::PaymentsRequest,
-) -> Result<(), errors::ApiErrorResponse> {
+) -> RouterResult<()> {
-    if let Some(customer_details) = request.customer.as_ref() {
+    if let Some(mismatched_fields) = request.validate_customer_details_in_request() {
-        validate_options_for_inequality(
+        let mismatched_fields = mismatched_fields.join(", ");
-            request.customer_id.as_ref(),
+        Err(errors::ApiErrorResponse::PreconditionFailed {
-            Some(&customer_details.id),
+            message: format!(
-            "customer_id",
+                "The field names `{mismatched_fields}` sent in both places is ambiguous"
-        )?;
+            ),
-
+        })?
-        validate_options_for_inequality(
+    } else {
-            request.email.as_ref(),
-            customer_details.email.as_ref(),
-            "email",
-        )?;
-
-        validate_options_for_inequality(
-            request.name.as_ref(),
-            customer_details.name.as_ref(),
-            "name",
-        )?;
-
-        validate_options_for_inequality(
-            request.phone.as_ref(),
-            customer_details.phone.as_ref(),
-            "phone",
-        )?;
-
-        validate_options_for_inequality(
-            request.phone_country_code.as_ref(),
-            customer_details.phone_country_code.as_ref(),
-            "phone_country_code",
-        )?;
-    }
-
         Ok(())
+    }
 }
 
 /// Get the customer details from customer field if present
@@ -1542,12 +1500,7 @@ pub fn validate_customer_details_in_request(
 pub fn get_customer_details_from_request(
     request: &api_models::payments::PaymentsRequest,
 ) -> CustomerDetails {
-    let customer_id = request
+    let customer_id = request.get_customer_id().map(ToOwned::to_owned);
-        .customer
-        .as_ref()
-        .map(|customer_details| &customer_details.id)
-        .or(request.customer_id.as_ref())
-        .map(ToOwned::to_owned);
 
     let customer_name = request
         .customer
@@ -1582,16 +1535,6 @@ pub fn get_customer_details_from_request(
     }
 }
 
-fn get_customer_id_from_payment_request(
-    request: &api_models::payments::PaymentsRequest,
-) -> Option<id_type::CustomerId> {
-    request
-        .customer
-        .as_ref()
-        .map(|customer| customer.id.clone())
-        .or(request.customer_id.clone())
-}
-
 pub async fn get_connector_default(
     _state: &SessionState,
     request_connector: Option<serde_json::Value>,
@@ -4142,8 +4085,8 @@ pub fn validate_customer_access(
     auth_flow: services::AuthFlow,
     request: &api::PaymentsRequest,
 ) -> Result<(), errors::ApiErrorResponse> {
-    if auth_flow == services::AuthFlow::Client && request.customer_id.is_some() {
+    if auth_flow == services::AuthFlow::Client && request.get_customer_id().is_some() {
-        let is_same_customer = request.customer_id == payment_intent.customer_id;
+        let is_same_customer = request.get_customer_id() == payment_intent.customer_id.as_ref();
         if !is_same_customer {
             Err(errors::ApiErrorResponse::GenericUnauthorized {
                 message: "Unauthorised access to update customer".to_string(),

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

@@ -126,7 +126,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Co
             merchant_account,
             key_store,
             payment_attempt.payment_method_id.clone(),
-            &payment_intent.customer_id,
+            payment_intent.customer_id.as_ref(),
         )
         .await?;
         let customer_acceptance: Option<CustomerAcceptance> = request
@@ -188,12 +188,15 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Co
         currency = payment_attempt.currency.get_required_value("currency")?;
         amount = payment_attempt.get_total_amount().into();
 
-        helpers::validate_customer_id_mandatory_cases(
+        let customer_id = payment_intent
-            request.setup_future_usage.is_some(),
-            payment_intent
             .customer_id
             .as_ref()
-                .or(request.customer_id.as_ref()),
+            .or(request.get_customer_id())
+            .cloned();
+
+        helpers::validate_customer_id_mandatory_cases(
+            request.setup_future_usage.is_some(),
+            customer_id.as_ref(),
         )?;
 
         let shipping_address = helpers::create_or_update_address_for_payment_by_request(
@@ -337,7 +340,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Co
         };
 
         let customer_details = Some(CustomerDetails {
-            customer_id: request.customer_id.clone(),
+            customer_id,
             name: request.name.clone(),
             email: request.email.clone(),
             phone: request.phone.clone(),

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

@@ -508,7 +508,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
                     &m_merchant_account,
                     &m_key_store,
                     None,
-                    &payment_intent_customer_id,
+                    payment_intent_customer_id.as_ref(),
                 )
                 .await
             }
@@ -1353,7 +1353,8 @@ impl<F: Send + Clone> ValidateRequest<F, api::PaymentsRequest> for PaymentConfir
         BoxedOperation<'b, F, api::PaymentsRequest>,
         operations::ValidateResult<'a>,
     )> {
-        helpers::validate_customer_details_in_request(request)?;
+        helpers::validate_customer_information(request)?;
+
         if let Some(amount) = request.amount {
             helpers::validate_max_amount(amount)?;
         }

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

@@ -136,7 +136,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
             merchant_account,
             merchant_key_store,
             None,
-            &request.customer_id,
+            None,
         )
         .await?;
 
@@ -684,7 +684,8 @@ impl<F: Send + Clone> ValidateRequest<F, api::PaymentsRequest> for PaymentCreate
         BoxedOperation<'b, F, api::PaymentsRequest>,
         operations::ValidateResult<'a>,
     )> {
-        helpers::validate_customer_details_in_request(request)?;
+        helpers::validate_customer_information(request)?;
+
         if let Some(amount) = request.amount {
             helpers::validate_max_amount(amount)?;
         }
@@ -749,11 +750,7 @@ impl<F: Send + Clone> ValidateRequest<F, api::PaymentsRequest> for PaymentCreate
 
             helpers::validate_customer_id_mandatory_cases(
                 request.setup_future_usage.is_some(),
-                request
+                request.get_customer_id(),
-                    .customer
-                    .as_ref()
-                    .map(|customer| &customer.id)
-                    .or(request.customer_id.as_ref()),
             )?;
         }
 
@@ -1066,7 +1063,7 @@ impl PaymentCreate {
             .await;
 
         // Derivation of directly supplied Customer data in our Payment Create Request
-        let raw_customer_details = if request.customer_id.is_none()
+        let raw_customer_details = if request.get_customer_id().is_none()
             && (request.name.is_some()
                 || request.email.is_some()
                 || request.phone.is_some()
@@ -1115,7 +1112,7 @@ impl PaymentCreate {
             ),
             order_details,
             amount_captured: None,
-            customer_id: None,
+            customer_id: request.get_customer_id().cloned(),
             connector_id: None,
             allowed_payment_method_types,
             connector_metadata,
@@ -1149,10 +1146,10 @@ impl PaymentCreate {
         state: &SessionState,
         merchant_account: &domain::MerchantAccount,
     ) -> Option<ephemeral_key::EphemeralKey> {
-        match request.customer_id.clone() {
+        match request.get_customer_id() {
             Some(customer_id) => helpers::make_ephemeral_key(
                 state.clone(),
-                customer_id,
+                customer_id.clone(),
                 merchant_account.merchant_id.clone(),
             )
             .await

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

@@ -151,7 +151,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
             merchant_account,
             key_store,
             None,
-            &payment_intent.customer_id,
+            payment_intent.customer_id.as_ref(),
         )
         .await?;
         helpers::validate_amount_to_capture_and_capture_method(Some(&payment_attempt), request)?;
@@ -807,7 +807,8 @@ impl<F: Send + Clone> ValidateRequest<F, api::PaymentsRequest> for PaymentUpdate
         BoxedOperation<'b, F, api::PaymentsRequest>,
         operations::ValidateResult<'a>,
     )> {
-        helpers::validate_customer_details_in_request(request)?;
+        helpers::validate_customer_information(request)?;
+
         if let Some(amount) = request.amount {
             helpers::validate_max_amount(amount)?;
         }