fix(connector): send valid sdk information in authentication flow netcetera #4474
Conversation
hrithikesh026
added
C-bug
hrithikesh026
changed the title
crates/router/src/connector/utils.rs
Outdated
| @@ -1228,6 +1233,7 @@ pub trait AddressDetailsData { | |||
| fn get_country(&self) -> Result<&api_models::enums::CountryAlpha2, Error>; | |||
| fn get_combined_address_line(&self) -> Result<Secret<String>, Error>; | |||
| fn to_state_code(&self) -> Result<Secret<String>, Error>; | |||
| fn to_state_code_option(&self) -> Result<Option<Secret<String>>, Error>; | |||
There was a problem hiding this comment.
| fn to_state_code_option(&self) -> Result<Option<Secret<String>>, Error>; | |
| fn to_state_code_as_optional(&self) -> Result<Option<Secret<String>>, Error>; |
crates/router/src/connector/utils.rs
Outdated
| @@ -1186,6 +1186,7 @@ pub trait PhoneDetailsData { | |||
| fn get_country_code(&self) -> Result<String, Error>; | |||
| fn get_number_with_country_code(&self) -> Result<Secret<String>, Error>; | |||
| fn get_number_with_hash_country_code(&self) -> Result<Secret<String>, Error>; | |||
| fn get_country_code_without_plus(&self) -> Result<String, Error>; | |||
There was a problem hiding this comment.
Please find a better naming than without_plus
| home_phone: billing_address | ||
| .phone | ||
| .clone() | ||
| .map(TryInto::try_into) |
There was a problem hiding this comment.
Rather define the try_from here so code navigation is easy
| mobile_phone: billing_address | ||
| .phone | ||
| .clone() | ||
| .map(TryInto::try_into) |
There was a problem hiding this comment.
Rather define the try_from here so code navigation is easy
| work_phone: billing_address | ||
| .phone | ||
| .clone() | ||
| .map(TryInto::try_into) |
There was a problem hiding this comment.
Rather define the try_from here so code navigation is easy
crates/router/src/connector/utils.rs
Outdated
| fn to_state_code_option(&self) -> Result<Option<Secret<String>>, Error> { | ||
| self.state | ||
| .as_ref() | ||
| .map(|_| self.to_state_code()) | ||
| .transpose() | ||
| } |
There was a problem hiding this comment.
Don't return a result, do a map_err and return None in case of error.
Return Option<<Secret> and then update wherever you are using transpose().transpose()?
There was a problem hiding this comment.
Unfortunately we cannot return None if Err occurs. to_state_code() will return Err when state is an invalid string. We cannot ignore this Err.
| @@ -716,11 +721,24 @@ impl | |||
| bill_addr_state: billing_address | |||
| .address | |||
| .as_ref() | |||
| .and_then(|add| add.state.clone()), | |||
| .and_then(|add| add.to_state_code_as_optional().transpose()) | |||
There was a problem hiding this comment.
We need not call to_state_code_as_optional here if it's already sent as state code, we should check and call this fn only if required (as state in our request is a String and any value can be sent there)
| api_models::payments::DeviceChannel::App => { | ||
| Some(netcetera_types::DeviceRenderingOptionsSupported { | ||
| // hard-coded until core provides these values. | ||
| sdk_interface: netcetera_types::SdkInterface::Both, |
There was a problem hiding this comment.
This should always be Native for us
There was a problem hiding this comment.
As per netcetera we are supposed to send this value.
sai-harsha-vardhan
changed the title
SanchithHegde
removed
the
S-waiting-on-review
Type of Change
Description
Two required params in SDK information were not serde renamed properly. This was causing SDK based authentication call to fail. Fixed it.
Other changes:
error_messagecolumn inauthenticationtable toTEXTfromVARCHAR(64). Because when length of error_message was greater than 64 chars, update operation was failing.+prefixAdditional Changes
Motivation and Context
How did you test it?
Manual
Checklist
cargo +nightly fmt --allcargo clippy