diff --git a/crates/api_models/src/payment_methods.rs b/crates/api_models/src/payment_methods.rs index 96b5bc02d0..ef87f910b7 100644 --- a/crates/api_models/src/payment_methods.rs +++ b/crates/api_models/src/payment_methods.rs @@ -1,3 +1,5 @@ +use std::collections; + use common_utils::pii; use serde::de; @@ -165,7 +167,7 @@ fn set_or_reject_duplicate( } } -#[derive(Debug, serde::Serialize, serde::Deserialize)] +#[derive(Eq, PartialEq, Hash, Debug, serde::Serialize, serde::Deserialize)] pub struct ListPaymentMethodResponse { pub payment_method: api_enums::PaymentMethodType, pub payment_method_types: Option>, @@ -183,7 +185,7 @@ pub struct ListPaymentMethodResponse { #[derive(Debug, serde::Serialize)] pub struct ListCustomerPaymentMethodsResponse { - pub enabled_payment_methods: Vec, + pub enabled_payment_methods: collections::HashSet, pub customer_payment_methods: Vec, } @@ -210,7 +212,7 @@ pub struct CustomerPaymentMethod { pub created: Option, } -#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] +#[derive(Eq, PartialEq, Hash, Clone, Debug, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "snake_case")] #[non_exhaustive] pub enum PaymentExperience { diff --git a/crates/router/src/core/errors.rs b/crates/router/src/core/errors.rs index c388046d94..53e6ace8d2 100644 --- a/crates/router/src/core/errors.rs +++ b/crates/router/src/core/errors.rs @@ -56,6 +56,8 @@ pub enum StorageError { DuplicateValue(String), #[error("KV error")] KVError, + #[error("Serialization failure")] + SerializationFailed, #[error("MockDb error")] MockDbError, } diff --git a/crates/router/src/core/errors/utils.rs b/crates/router/src/core/errors/utils.rs index a4374ad09e..5241881c16 100644 --- a/crates/router/src/core/errors/utils.rs +++ b/crates/router/src/core/errors/utils.rs @@ -42,7 +42,6 @@ pub(crate) trait ConnectorErrorExt { fn to_verify_failed_response(self) -> error_stack::Report; } -// FIXME: The implementation can be improved by handling BOM maybe? impl ConnectorErrorExt for error_stack::Report { fn to_refund_failed_response(self) -> error_stack::Report { let data = match self.current_context() { diff --git a/crates/router/src/core/payment_methods/cards.rs b/crates/router/src/core/payment_methods/cards.rs index a4d3b1cdfd..9302581c3e 100644 --- a/crates/router/src/core/payment_methods/cards.rs +++ b/crates/router/src/core/payment_methods/cards.rs @@ -84,11 +84,11 @@ pub async fn add_payment_method( metadata: req.metadata, created: Some(common_utils::date_time::now()), payment_method_issuer_code: req.payment_method_issuer_code, - recurring_enabled: false, //TODO - installment_payment_enabled: false, //TODO + recurring_enabled: false, //[#219] + installment_payment_enabled: false, //[#219] payment_experience: Some(vec![ api_models::payment_methods::PaymentExperience::RedirectToUrl, - ]), //TODO + ]), //[#219] }) } } @@ -354,7 +354,7 @@ pub async fn list_payment_methods( db: &dyn db::StorageInterface, merchant_account: storage::MerchantAccount, mut req: api::ListPaymentMethodRequest, -) -> errors::RouterResponse> { +) -> errors::RouterResponse> { let payment_intent = helpers::verify_client_secret( db, merchant_account.storage_scheme, @@ -392,8 +392,8 @@ pub async fn list_payment_methods( error.to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound) })?; - // TODO: Deduplicate payment methods - let mut response: Vec = Vec::new(); + let mut response: collections::HashSet = + collections::HashSet::new(); for mca in all_mcas { let payment_methods = match mca.payment_methods_enabled { Some(pm) => pm, @@ -420,7 +420,7 @@ pub async fn list_payment_methods( async fn filter_payment_methods( payment_methods: Vec, req: &mut api::ListPaymentMethodRequest, - resp: &mut Vec, + resp: &mut collections::HashSet, payment_intent: Option<&storage::PaymentIntent>, payment_attempt: Option<&storage::PaymentAttempt>, address: Option<&storage::Address>, @@ -465,7 +465,7 @@ async fn filter_payment_methods( }; if filter && filter2 && filter3 { - resp.push(payment_method_object); + resp.insert(payment_method_object); } } } @@ -586,8 +586,8 @@ pub async fn list_customer_payment_method( error.to_not_found_response(errors::ApiErrorResponse::MerchantAccountNotFound) })?; - // TODO: Deduplicate payment methods - let mut enabled_methods: Vec = Vec::new(); + let mut enabled_methods: collections::HashSet = + collections::HashSet::new(); for mca in all_mcas { let payment_methods = match mca.payment_methods_enabled { Some(pm) => pm, @@ -598,7 +598,7 @@ pub async fn list_customer_payment_method( if let Ok(payment_method_object) = serde_json::from_value::(payment_method) { - enabled_methods.push(payment_method_object); + enabled_methods.insert(payment_method_object); } } } @@ -854,11 +854,11 @@ pub async fn retrieve_payment_method( payment_method_issuer_code: pm .payment_method_issuer_code .map(ForeignInto::foreign_into), - recurring_enabled: false, //TODO - installment_payment_enabled: false, //TODO + recurring_enabled: false, //[#219] + installment_payment_enabled: false, //[#219] payment_experience: Some(vec![ api_models::payment_methods::PaymentExperience::RedirectToUrl, - ]), //TODO, + ]), //[#219], }, )) } diff --git a/crates/router/src/core/payments.rs b/crates/router/src/core/payments.rs index 6bb4407ae1..c63c29f64b 100644 --- a/crates/router/src/core/payments.rs +++ b/crates/router/src/core/payments.rs @@ -454,7 +454,7 @@ where pub email: Option>, } -#[derive(Debug)] +#[derive(Debug, Default)] pub struct CustomerDetails { pub customer_id: Option, pub name: Option>, diff --git a/crates/router/src/core/payments/helpers.rs b/crates/router/src/core/payments/helpers.rs index d1f25cb1f5..96466c7a69 100644 --- a/crates/router/src/core/payments/helpers.rs +++ b/crates/router/src/core/payments/helpers.rs @@ -146,8 +146,6 @@ pub async fn get_token_for_recurring_mandate( .await .map_err(|error| error.to_not_found_response(errors::ApiErrorResponse::MandateNotFound))?; - // TODO: Make currency in payments request as Currency enum - let customer = req.customer_id.clone().get_required_value("customer_id")?; let payment_method_id = { @@ -733,7 +731,7 @@ pub async fn make_pm_data<'a, F: Clone, R>( let payment_method = match (request, token) { (_, Some(token)) => Ok::<_, error_stack::Report>( if payment_method_type == Some(storage_enums::PaymentMethodType::Card) { - // TODO: Handle token expiry + // [#196]: Handle token expiry let (pm, tokenize_value2) = Vault::get_payment_method_data_from_locker(state, &token).await?; utils::when( @@ -766,7 +764,7 @@ pub async fn make_pm_data<'a, F: Clone, R>( field_name: "payment_method_type".to_owned(), }) })?; - // TODO: Implement token flow for other payment methods + // [#195]: Implement token flow for other payment methods None }, ), diff --git a/crates/router/src/core/payments/operations/payment_start.rs b/crates/router/src/core/payments/operations/payment_start.rs index eb5049ab0c..61c4e76ea4 100644 --- a/crates/router/src/core/payments/operations/payment_start.rs +++ b/crates/router/src/core/payments/operations/payment_start.rs @@ -92,13 +92,9 @@ impl GetTracker, api::PaymentsStartRequest> f payment_intent.shipping_address_id = shipping_address.clone().map(|i| i.address_id); payment_intent.billing_address_id = billing_address.clone().map(|i| i.address_id); - //TODO: get customer from db? let customer_details = CustomerDetails { customer_id: payment_intent.customer_id.clone(), - name: None, - email: None, - phone: None, - phone_country_code: None, + ..CustomerDetails::default() }; let connector_response = db diff --git a/crates/router/src/core/webhooks.rs b/crates/router/src/core/webhooks.rs index e94e7c7d81..350ef22415 100644 --- a/crates/router/src/core/webhooks.rs +++ b/crates/router/src/core/webhooks.rs @@ -173,14 +173,14 @@ async fn trigger_webhook_to_merchant( match response { Err(e) => { - // TODO: Schedule webhook for retry. + // [#217]: Schedule webhook for retry. Err(e) .into_report() .change_context(errors::WebhooksFlowError::CallToMerchantFailed)?; } Ok(res) => { if !res.status().is_success() { - // TODO: Schedule webhook for retry. + // [#217]: Schedule webhook for retry. Err(errors::WebhooksFlowError::NotReceivedByMerchant).into_report()?; } } diff --git a/crates/router/src/db/payment_intent.rs b/crates/router/src/db/payment_intent.rs index 0149c24365..740f1d94e7 100644 --- a/crates/router/src/db/payment_intent.rs +++ b/crates/router/src/db/payment_intent.rs @@ -52,7 +52,7 @@ mod storage { api, storage::{enums, kv, payment_intent::*}, }, - utils::storage_partitioning::KvStorePartition, + utils::{self, storage_partitioning::KvStorePartition}, }; #[async_trait::async_trait] @@ -155,9 +155,11 @@ mod storage { let updated_intent = payment_intent.clone().apply_changeset(this.clone()); // Check for database presence as well Maybe use a read replica here ? - let redis_value = serde_json::to_string(&updated_intent) - .into_report() - .change_context(errors::StorageError::KVError)?; + + let redis_value = + utils::Encode::::encode_to_string_of_json(&updated_intent) + .change_context(errors::StorageError::SerializationFailed)?; + let updated_intent = self .redis_conn .set_hash_fields(&key, ("pi", &redis_value)) diff --git a/crates/router/src/db/refund.rs b/crates/router/src/db/refund.rs index 6d15da921f..2aba539e05 100644 --- a/crates/router/src/db/refund.rs +++ b/crates/router/src/db/refund.rs @@ -437,7 +437,6 @@ mod storage { let updated_refund = refund.clone().apply_changeset(this.clone()); // Check for database presence as well Maybe use a read replica here ? - // TODO: Add a proper error for serialization failure let lookup = self .get_lookup_by_lookup_id(&key) @@ -451,7 +450,7 @@ mod storage { utils::Encode::::encode_to_string_of_json( &updated_refund, ) - .change_context(errors::StorageError::KVError)?; + .change_context(errors::StorageError::SerializationFailed)?; self.redis_conn .set_hash_fields(&key, (field, redis_value)) diff --git a/crates/router/src/types/storage/payment_intent.rs b/crates/router/src/types/storage/payment_intent.rs index 44497d72cc..89bcafa08e 100644 --- a/crates/router/src/types/storage/payment_intent.rs +++ b/crates/router/src/types/storage/payment_intent.rs @@ -36,7 +36,7 @@ impl PaymentIntentDbExt for PaymentIntent { let starting_after = &pc.starting_after; let ending_before = &pc.ending_before; - //TODO: Replace this with Boxable Expression and pass it into generic filter + //[#350]: Replace this with Boxable Expression and pass it into generic filter // when https://github.com/rust-lang/rust/issues/52662 becomes stable let mut filter = ::table() .filter(dsl::merchant_id.eq(merchant_id.to_owned()))