fix: fixing TODO/FIXMEs from router/core (#257)

This commit is contained in:
Nishant Joshi
2022-12-29 16:05:42 +05:30
committed by GitHub
parent 7348d76cad
commit 8535cecd7d
16 changed files with 83 additions and 84 deletions

View File

@ -28,7 +28,7 @@ pub struct PaymentsRequest {
#[serde(default, deserialize_with = "amount::deserialize_option")]
pub amount: Option<Amount>,
pub connector: Option<api_enums::Connector>,
pub currency: Option<String>,
pub currency: Option<api_enums::Currency>,
pub capture_method: Option<api_enums::CaptureMethod>,
pub amount_to_capture: Option<i64>,
#[serde(default, with = "common_utils::custom_serde::iso8601::option")]

View File

@ -10,7 +10,7 @@ license = "Apache-2.0"
build = "src/build.rs"
[features]
default = ["kv_store"]
default = ["kv_store", "stripe"]
kms = ["aws-config", "aws-sdk-kms"]
basilisk = []
stripe = ["dep:serde_qs"]

View File

@ -23,14 +23,16 @@ pub async fn payment_intents_create(
) -> HttpResponse {
let payload: types::StripePaymentIntentRequest = match qs_config
.deserialize_bytes(&form_payload)
.map_err(|err| report!(errors::StripeErrorCode::from(err)))
{
Ok(p) => p,
Err(err) => {
return api::log_and_return_error_response(report!(errors::StripeErrorCode::from(err)))
}
Err(err) => return api::log_and_return_error_response(err),
};
let create_payment_req: payment_types::PaymentsRequest = payload.into();
let create_payment_req: payment_types::PaymentsRequest = match payload.try_into() {
Ok(req) => req,
Err(err) => return api::log_and_return_error_response(err),
};
wrap::compatibility_api_wrap::<
_,
@ -129,7 +131,11 @@ pub async fn payment_intents_update(
}
};
let mut payload: payment_types::PaymentsRequest = stripe_payload.into();
let mut payload: payment_types::PaymentsRequest = match stripe_payload.try_into() {
Ok(req) => req,
Err(err) => return api::log_and_return_error_response(err),
};
payload.payment_id = Some(api_types::PaymentIdType::PaymentIntentId(payment_id));
let auth_type;
@ -186,7 +192,11 @@ pub async fn payment_intents_confirm(
}
};
let mut payload: payment_types::PaymentsRequest = stripe_payload.into();
let mut payload: payment_types::PaymentsRequest = match stripe_payload.try_into() {
Ok(req) => req,
Err(err) => return api::log_and_return_error_response(err),
};
payload.payment_id = Some(api_types::PaymentIdType::PaymentIntentId(payment_id));
payload.confirm = Some(true);

View File

@ -1,4 +1,6 @@
use api_models::{payments, refunds};
use common_utils::ext_traits::StringExt;
use error_stack::ResultExt;
use serde::{Deserialize, Serialize};
use serde_json::Value;
@ -131,12 +133,20 @@ pub struct StripePaymentIntentRequest {
pub client_secret: Option<pii::Secret<String>>,
}
impl From<StripePaymentIntentRequest> for payments::PaymentsRequest {
fn from(item: StripePaymentIntentRequest) -> Self {
Self {
impl TryFrom<StripePaymentIntentRequest> for payments::PaymentsRequest {
type Error = error_stack::Report<errors::ApiErrorResponse>;
fn try_from(item: StripePaymentIntentRequest) -> errors::RouterResult<Self> {
Ok(Self {
amount: item.amount.map(|amount| amount.into()),
connector: item.connector,
currency: item.currency.as_ref().map(|c| c.to_uppercase()),
currency: item
.currency
.as_ref()
.map(|c| c.to_uppercase().parse_enum("currency"))
.transpose()
.change_context(errors::ApiErrorResponse::InvalidDataValue {
field_name: "currency",
})?,
capture_method: item.capture_method,
amount_to_capture: item.amount_capturable,
confirm: item.confirm,
@ -171,7 +181,7 @@ impl From<StripePaymentIntentRequest> for payments::PaymentsRequest {
metadata: item.metadata,
client_secret: item.client_secret.map(|s| s.peek().clone()),
..Default::default()
}
})
}
}

View File

@ -130,7 +130,7 @@ impl From<StripeSetupIntentRequest> for payments::PaymentsRequest {
fn from(item: StripeSetupIntentRequest) -> Self {
Self {
amount: Some(api_types::Amount::Zero),
currency: Some(api_enums::Currency::default().to_string()),
currency: Some(api_enums::Currency::default()),
capture_method: None,
amount_to_capture: None,
confirm: item.confirm,

View File

@ -138,7 +138,6 @@ pub async fn add_card(
let locker = &state.conf.locker;
let db = &*state.store;
let request = payment_methods::mk_add_card_request(locker, &card, &customer_id, &req)?;
// FIXME use call_api 2. Serde's handle should be inside the generic function
let response = if !locker.mock_locker {
let response = services::call_connector_api(state, request)
.await
@ -280,7 +279,6 @@ pub async fn get_card_from_legacy_locker<'a>(
let request = payment_methods::mk_get_card_request(locker, merchant_id, card_id)
.change_context(errors::ApiErrorResponse::InternalServerError)
.attach_printable("Making get card request failed")?;
// FIXME use call_api 2. Serde's handle should be inside the generic function
let get_card_result = if !locker.mock_locker {
let response = services::call_connector_api(state, request)
.await
@ -315,7 +313,6 @@ pub async fn delete_card<'a>(
let request = payment_methods::mk_delete_card_request(&state.conf.locker, merchant_id, card_id)
.change_context(errors::ApiErrorResponse::InternalServerError)
.attach_printable("Making Delete card request Failed")?;
// FIXME use call_api 2. Serde's handle should be inside the generic function
let delete_card_resp = if !locker.mock_locker {
services::call_connector_api(state, request)
.await

View File

@ -74,10 +74,10 @@ pub fn mk_add_card_request(
customer_id,
card_exp_month: card.card_exp_month.clone(),
card_exp_year: card.card_exp_year.clone(),
merchant_id: "m0010", //FIXME: Need mapping for application mid to lockeId
email_address: Some("dummy@gmail.com".to_string().into()), //FIXME: If these are mandatory need to have customer object
name_on_card: Some("juspay".to_string().into()), //FIXME
nickname: Some("orca".to_string()), //FIXME
merchant_id: "m0010", // [#253]: Need mapping for application mid to lockeId
email_address: Some("dummy@gmail.com".to_string().into()), //
name_on_card: Some("juspay".to_string().into()), // [#256]
nickname: Some("orca".to_string()), //
};
let body = utils::Encode::<AddCardRequest<'_>>::encode(&add_card_req)
.change_context(errors::CardVaultError::RequestEncodingFailed)?;
@ -99,11 +99,11 @@ pub fn mk_add_card_response(
let card = api::CardDetailFromLocker {
scheme: None,
last4_digits: Some(card_number.split_off(card_number.len() - 4)),
issuer_country: None, // TODO bin mapping
issuer_country: None, // [#256] bin mapping
card_number: Some(card.card_number),
expiry_month: Some(card.card_exp_month),
expiry_year: Some(card.card_exp_year),
card_token: Some(response.external_id.into()), // TODO ?
card_token: Some(response.external_id.into()), // [#256]
card_fingerprint: Some(response.card_fingerprint),
card_holder_name: None,
};
@ -118,11 +118,11 @@ pub fn mk_add_card_response(
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, // [#256]
installment_payment_enabled: false, // #[#256]
payment_experience: Some(vec![
api_models::payment_methods::PaymentExperience::RedirectToUrl,
]), //TODO,
]), // [#256]
}
}
@ -132,7 +132,7 @@ pub fn mk_get_card_request<'a>(
card_id: &'a str,
) -> CustomResult<services::Request, errors::CardVaultError> {
let get_card_req = GetCard {
merchant_id: "m0010", //FIXME: need to assign locker id to every merchant
merchant_id: "m0010", // [#253]: need to assign locker id to every merchant
card_id,
};

View File

@ -43,7 +43,6 @@ pub async fn get_address_for_payment_request(
merchant_id: &str,
customer_id: &Option<String>,
) -> CustomResult<Option<storage::Address>, errors::ApiErrorResponse> {
// TODO: Refactor this function for more readability (TryFrom)
Ok(match req_address {
Some(address) => {
match address_id {
@ -60,17 +59,10 @@ pub async fn get_address_for_payment_request(
.as_deref()
.get_required_value("customer_id")
.change_context(errors::ApiErrorResponse::CustomerNotFound)?;
let address_details = address.address.clone().unwrap_or_default();
Some(
db.insert_address(storage::AddressNew {
city: address.address.as_ref().and_then(|a| a.city.clone()),
country: address.address.as_ref().and_then(|a| a.country.clone()),
line1: address.address.as_ref().and_then(|a| a.line1.clone()),
line2: address.address.as_ref().and_then(|a| a.line2.clone()),
line3: address.address.as_ref().and_then(|a| a.line3.clone()),
state: address.address.as_ref().and_then(|a| a.state.clone()),
zip: address.address.as_ref().and_then(|a| a.zip.clone()),
first_name: address.address.as_ref().and_then(|a| a.first_name.clone()),
last_name: address.address.as_ref().and_then(|a| a.last_name.clone()),
phone_number: address.phone.as_ref().and_then(|a| a.number.clone()),
country_code: address
.phone
@ -78,7 +70,8 @@ pub async fn get_address_for_payment_request(
.and_then(|a| a.country_code.clone()),
customer_id: customer_id.to_string(),
merchant_id: merchant_id.to_string(),
..storage::AddressNew::default()
..address_details.foreign_into()
})
.await
.map_err(|_| errors::ApiErrorResponse::InternalServerError)?,
@ -172,7 +165,7 @@ pub async fn get_token_for_recurring_mandate(
};
verify_mandate_details(
req.amount.get_required_value("amount")?.into(),
req.currency.clone().get_required_value("currency")?,
req.currency.get_required_value("currency")?,
mandate.clone(),
)?;
@ -360,7 +353,7 @@ fn validate_recurring_mandate(req: api::MandateValidationFields) -> RouterResult
pub fn verify_mandate_details(
request_amount: i64,
request_currency: String,
request_currency: api_enums::Currency,
mandate: storage::Mandate,
) -> RouterResult<()> {
match mandate.mandate_type {
@ -392,7 +385,7 @@ pub fn verify_mandate_details(
utils::when(
mandate
.mandate_currency
.map(|mandate_currency| mandate_currency.to_string() != request_currency)
.map(|mandate_currency| mandate_currency != request_currency.foreign_into())
.unwrap_or(false),
|| {
Err(report!(errors::ApiErrorResponse::MandateValidationFailed {
@ -488,7 +481,6 @@ pub(crate) async fn call_payment_method(
Some(pm_data) => match payment_method_type {
Some(payment_method_type) => match pm_data {
api::PaymentMethod::Card(card) => {
//TODO: get it from temp_card
let card_detail = api::CardDetail {
card_number: card.card_number.clone(),
card_exp_month: card.card_exp_month.clone(),

View File

@ -539,13 +539,10 @@ impl PaymentCreate {
pub fn payments_create_request_validation(
req: &api::PaymentsRequest,
) -> RouterResult<(api::Amount, enums::Currency)> {
let currency: enums::Currency = req
let currency = req
.currency
.as_ref()
.parse_enum("currency")
.change_context(errors::ApiErrorResponse::InvalidRequestData {
message: "invalid currency".to_string(),
})?;
.map(ForeignInto::foreign_into)
.get_required_value("currency")?;
let amount = req.amount.get_required_value("amount")?;
Ok((amount, currency))
}

View File

@ -21,7 +21,7 @@ use crate::{
storage::{self, enums},
transformers::ForeignInto,
},
utils::{OptionExt, StringExt},
utils::OptionExt,
};
#[derive(Debug, Clone, Copy, PaymentOperation)]
#[operation(ops = "all", flow = "authorize")]
@ -66,11 +66,7 @@ impl<F: Send + Clone> GetTracker<F, PaymentData<F>, api::PaymentsRequest> for Pa
})?;
currency = match request.currency {
Some(ref cur) => cur.clone().parse_enum("currency").change_context(
errors::ApiErrorResponse::InvalidRequestData {
message: "invalid currency".to_string(),
},
)?,
Some(cur) => cur.foreign_into(),
None => payment_attempt.currency.get_required_value("currency")?,
};

View File

@ -33,8 +33,6 @@ where
F: Clone,
error_stack::Report<errors::ApiErrorResponse>: From<<T as TryFrom<PaymentData<F>>>::Error>,
{
//TODO: everytime parsing the json may have impact?
let (merchant_connector_account, payment_method, router_data);
let db = &*state.store;
merchant_connector_account = db
@ -58,7 +56,7 @@ where
.or(payment_data.payment_attempt.payment_method)
.get_required_value("payment_method_type")?;
//FIXME[#44]: why should response be filled during request
// [#44]: why should response be filled during request
let response = payment_data
.payment_attempt
.connector_transaction_id

View File

@ -597,8 +597,7 @@ pub async fn schedule_refund_execution(
Ok(refund)
}
api_models::refunds::RefundType::Instant => {
// FIXME: This is not possible in schedule_refund_execution as it will always be scheduled
// FIXME: as a part of refactoring
// [#255]: This is not possible in schedule_refund_execution as it will always be scheduled
// sync_refund_with_gateway(data, &refund).await
Ok(refund)
}
@ -606,7 +605,7 @@ pub async fn schedule_refund_execution(
}
}
}
// FIXME: THis is not allowed to be otherwise or all
// [#255]: This is not allowed to be otherwise or all
_ => Ok(refund),
}?;
Ok(result)

View File

@ -15,25 +15,6 @@ use crate::{
types::storage::{self, enums::ProcessTrackerStatus},
};
//TODO: move to env
pub fn fetch_upper_limit() -> i64 {
0
}
pub fn fetch_lower_limit() -> i64 {
1800
}
pub fn producer_lock_key() -> &'static str {
"PRODUCER_LOCKING_KEY"
}
pub fn producer_lock_ttl() -> i64 {
// ttl_offset = config.scheduler_lock_offset.or_else(60);
// (scheduler_looper_interval / 100) + (ttl_offset)
160 //seconds
}
#[instrument(skip_all)]
pub async fn start_producer(
state: &AppState,
@ -80,7 +61,7 @@ pub async fn run_producer_flow(
settings: &SchedulerSettings,
) -> CustomResult<(), errors::ProcessTrackerError> {
let tag = "PRODUCER_LOCK";
let lock_key = producer_lock_key();
let lock_key = &settings.producer.lock_key;
let lock_val = "LOCKED";
let ttl = settings.producer.lock_ttl;

View File

@ -353,3 +353,22 @@ impl TryFrom<F<storage::MerchantConnectorAccount>>
.into())
}
}
impl From<F<api_models::payments::AddressDetails>> for F<storage_models::address::AddressNew> {
fn from(item: F<api_models::payments::AddressDetails>) -> Self {
let address = item.0;
storage_models::address::AddressNew {
city: address.city,
country: address.country,
line1: address.line1,
line2: address.line2,
line3: address.line3,
state: address.state,
zip: address.zip,
first_name: address.first_name,
last_name: address.last_name,
..Default::default()
}
.into()
}
}

View File

@ -290,7 +290,7 @@ async fn payments_create_core() {
merchant_id: Some("jarnura".to_string()),
amount: Some(6540.into()),
connector: None,
currency: Some("USD".to_string()),
currency: Some(api_enums::Currency::USD),
capture_method: Some(api_enums::CaptureMethod::Automatic),
amount_to_capture: Some(6540),
capture_on: Some(datetime!(2022-09-10 11:12)),
@ -449,7 +449,7 @@ async fn payments_create_core_adyen_no_redirect() {
merchant_id: Some(merchant_id.clone()),
amount: Some(6540.into()),
connector: None,
currency: Some("USD".to_string()),
currency: Some(api_enums::Currency::USD),
capture_method: Some(api_enums::CaptureMethod::Automatic),
amount_to_capture: Some(6540),
capture_on: Some(datetime!(2022-09-10 10:11:12)),

View File

@ -50,7 +50,7 @@ async fn payments_create_core() {
)),
merchant_id: Some("jarnura".to_string()),
amount: Some(6540.into()),
currency: Some("USD".to_string()),
currency: Some(api_enums::Currency::USD),
capture_method: Some(api_enums::CaptureMethod::Automatic),
amount_to_capture: Some(6540),
capture_on: Some(datetime!(2022-09-10 10:11:12)),
@ -203,7 +203,7 @@ async fn payments_create_core_adyen_no_redirect() {
merchant_id: Some(merchant_id.clone()),
amount: Some(6540.into()),
connector: None,
currency: Some("USD".to_string()),
currency: Some(api_enums::Currency::USD),
capture_method: Some(api_enums::CaptureMethod::Automatic),
amount_to_capture: Some(6540),
capture_on: Some(datetime!(2022-09-10 10:11:12)),