ronantakizawa/codereview-bench huggingface dataset:
Repository: juspay/hyperswitch
File: crates/common_types/src/primitive_wrappers.rs (Rust)

Code before:
                &self.0, out,
            )
        }
    }

    impl Default for DisputePollingIntervalInHours {
        /// Default for `DisputePollingIntervalInHours` is `24`
        fn default() -> Self {
            Self(DEFAULT_DISPUTE_POLLING_INTERVAL_IN_HOURS)
        }
    }
}

mod u16_wrappers {
    use std::ops::Deref;

    use serde::{de::Error, Deserialize, Serialize};

    use crate::consts::{
        CUSTOMER_LIST_DEFAULT_LIMIT, CUSTOMER_LIST_LOWER_LIMIT, CUSTOMER_LIST_UPPER_LIMIT,
    };

    /// Customer list limit wrapper with automatic validation
    #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, diesel::expression::AsExpression)]
    #[diesel(sql_type = diesel::sql_types::SmallInt)]
    pub struct CustomerListLimit(i16);

    impl Deref for CustomerListLimit {
        type Target = i16;

        fn deref(&self) -> &Self::Target {
            &self.0
        }
    }

    impl<'de> Deserialize<'de> for CustomerListLimit {
        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
        where
            D: serde::Deserializer<'de>,
        {
            let val = i16::deserialize(deserializer)?;
            if val < CUSTOMER_LIST_LOWER_LIMIT {
                Err(D::Error::custom(format!(
                    "CustomerListLimit cannot be less than {}",
                    CUSTOMER_LIST_LOWER_LIMIT
                )))
            } else if val > CUSTOMER_LIST_UPPER_LIMIT {
                Err(D::Error::custom(format!(
                    "CustomerListLimit exceeds the maximum allowed value of {}",
                    CUSTOMER_LIST_UPPER_LIMIT
                )))

Review comment (suggestion):
```suggestion
    pub struct CustomerListLimit(u16);
```

Code after:
                &self.0, out,
            )
        }
    }

    impl Default for DisputePollingIntervalInHours {
        /// Default for `DisputePollingIntervalInHours` is `24`
        fn default() -> Self {
            Self(DEFAULT_DISPUTE_POLLING_INTERVAL_IN_HOURS)
        }
    }
}

mod u16_wrappers {
    use std::ops::Deref;

    use serde::{de::Error, Deserialize, Serialize};

    use crate::consts::{
        CUSTOMER_LIST_DEFAULT_LIMIT, CUSTOMER_LIST_LOWER_LIMIT, CUSTOMER_LIST_UPPER_LIMIT,
    };

    /// Customer list limit wrapper with automatic validation
    #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize)]
    pub struct CustomerListLimit(u16);

    impl Deref for CustomerListLimit {
        type Target = u16;

        fn deref(&self) -> &Self::Target {
            &self.0
        }
    }

    impl<'de> Deserialize<'de> for CustomerListLimit {
        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
        where
            D: serde::Deserializer<'de>,
        {
            let val = u16::deserialize(deserializer)?;
            Self::new(val).map_err(D::Error::custom)
        }
    }

    impl Default for CustomerListLimit {
        /// Default for `CustomerListLimit` is `20`
        fn default() -> Self {
            Self(CUSTOMER_LIST_DEFAULT_LIMIT)
        }
    }
Repository: juspay/hyperswitch
File: crates/common_enums/src/enums.rs (Rust)

Code before:
    CollectOtp,
}

#[derive(Eq, PartialEq, Clone, Debug, serde::Deserialize, serde::Serialize, strum::Display)]
#[serde(rename_all = "lowercase")]
pub enum SamsungPayCardBrand {
    Visa,
    MasterCard,
    Amex,
    Discover,
    Unknown,
}

/// Custom T&C Message to be shown per payment method type
#[derive(
    Clone,
    Debug,
    Eq,
    PartialEq,
    serde::Serialize,
    serde::Deserialize,
    utoipa::ToSchema,
)]
pub struct CustomTermsByPaymentMethodTypes(
    #[schema(value_type = HashMap<String, Option<String>>)]
    pub Option<std::collections::HashMap<PaymentMethodType, Option<String>>>,
);

/// Indicates the sub type of payment method. Eg: 'google_pay' & 'apple_pay' for wallets.
#[derive(
    Clone,
    Copy,
    Debug,
    Eq,
    Ord,
    Hash,
    PartialOrd,
    PartialEq,
    serde::Deserialize,
    serde::Serialize,
    SmithyModel,
    strum::Display,
    strum::VariantNames,
    strum::EnumIter,
    strum::EnumString,
    ToSchema,
)]
#[router_derive::diesel_enum(storage_type = "text")]
#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "snake_case")]
#[smithy(namespace = "com.hyperswitch.smithy.types")]

Review comment (suggestion):
```suggestion
    pub Option<std::collections::HashMap<PaymentMethodType, String>>,
```

Code after:
    CollectOtp,
}

#[derive(Eq, PartialEq, Clone, Debug, serde::Deserialize, serde::Serialize, strum::Display)]
#[serde(rename_all = "lowercase")]
pub enum SamsungPayCardBrand {
    Visa,
    MasterCard,
    Amex,
    Discover,
    Unknown,
}

/// Custom T&C Message to be shown per payment method type
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize, utoipa::ToSchema)]
pub struct CustomTermsByPaymentMethodTypes(
    #[schema(value_type = HashMap<String, Option<String>>)]
    pub  Option<std::collections::HashMap<PaymentMethodType, String>>,
);

/// Indicates the sub type of payment method. Eg: 'google_pay' & 'apple_pay' for wallets.
#[derive(
    Clone,
    Copy,
    Debug,
    Eq,
    Ord,
    Hash,
    PartialOrd,
    PartialEq,
    serde::Deserialize,
    serde::Serialize,
    SmithyModel,
    strum::Display,
    strum::VariantNames,
    strum::EnumIter,
    strum::EnumString,
    ToSchema,
)]
#[router_derive::diesel_enum(storage_type = "text")]
#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "snake_case")]
#[smithy(namespace = "com.hyperswitch.smithy.types")]
pub enum PaymentMethodType {
    Ach,
    Affirm,
    AfterpayClearpay,
    Alfamart,
    AliPay,
    AliPayHk,
    Alma,
Repository: juspay/hyperswitch
File: crates/hyperswitch_connectors/src/connectors/nexixpay.rs (Rust)

Code before:
                auth_type,
                ..
            } => payment_method == Some(enums::PaymentMethod::Card) && auth_type.is_three_ds(),
            api::CurrentFlowInfo::SetupMandate { .. } => false,
        }
    }

    fn get_supported_payment_methods(&self) -> Option<&'static SupportedPaymentMethods> {
        Some(&*NEXIXPAY_SUPPORTED_PAYMENT_METHODS)
    }

    fn get_supported_webhook_flows(&self) -> Option<&'static [enums::EventClass]> {
        Some(&*NEXIXPAY_SUPPORTED_WEBHOOK_FLOWS)
    }

    fn is_pre_authentication_flow_required(&self, current_flow: api::CurrentFlowInfo<'_>) -> bool {
        match current_flow {
            api::CurrentFlowInfo::Authorize {
                request_data,
                auth_type,
            } => {
                // Mandate payments should skip pre-authentication and go directly to authorize
                if request_data.is_mandate_payment() {
                    return false;
                }
                self.is_3ds_setup_required(request_data, *auth_type)
            }
            // No alternate flow for complete authorize
            api::CurrentFlowInfo::CompleteAuthorize { .. } => false,
            api::CurrentFlowInfo::SetupMandate { .. } => false,
        }
    }
}


Review comment (security):
avoid early returns
```suggestion
                if request_data.is_mandate_payment() {
                    false
                }else {
                    self.is_3ds_setup_required(request_data, *auth_type)
                }
```

Code after:
                payment_method,
                auth_type,
                ..
            } => payment_method == Some(enums::PaymentMethod::Card) && auth_type.is_three_ds(),
            api::CurrentFlowInfo::SetupMandate { .. } => false,
        }
    }

    fn get_supported_payment_methods(&self) -> Option<&'static SupportedPaymentMethods> {
        Some(&*NEXIXPAY_SUPPORTED_PAYMENT_METHODS)
    }

    fn get_supported_webhook_flows(&self) -> Option<&'static [enums::EventClass]> {
        Some(&*NEXIXPAY_SUPPORTED_WEBHOOK_FLOWS)
    }

    fn is_pre_authentication_flow_required(&self, current_flow: api::CurrentFlowInfo<'_>) -> bool {
        match current_flow {
            api::CurrentFlowInfo::Authorize {
                request_data,
                auth_type,
            } => {
                // Mandate payments should skip pre-authentication and go directly to authorize
                if request_data.is_mandate_payment() {
                    false
                } else {
                    self.is_3ds_setup_required(request_data, *auth_type)
                }
            }
            // No alternate flow for complete authorize
            api::CurrentFlowInfo::CompleteAuthorize { .. } => false,
            api::CurrentFlowInfo::SetupMandate { .. } => false,
        }
    }
}
Repository: juspay/hyperswitch
File: crates/hyperswitch_interfaces/src/consts.rs (Rust)

Code before:
/// error message for timed out request
pub const REQUEST_TIMEOUT_ERROR_MESSAGE: &str = "Connector did not respond in specified time";

/// Header value indicating that signature-key-based authentication is used.
pub const UCS_AUTH_SIGNATURE_KEY: &str = "signature-key";

/// Header value indicating that body-key-based authentication is used.
pub const UCS_AUTH_BODY_KEY: &str = "body-key";

/// Header value indicating that header-key-based authentication is used.
pub const UCS_AUTH_HEADER_KEY: &str = "header-key";

/// Header value indicating that currency-auth-key-based authentication is used.
pub const UCS_AUTH_CURRENCY_AUTH_KEY: &str = "currency-auth-key";

/// Header value for content type JSON
pub const CONTENT_TYPE: &str = "Content-Type";

/// Header name for flow name
pub const X_FLOW_NAME: &str = "x-flow";

/// Header name for request ID
pub const X_REQUEST_ID: &str = "x-request-id";

/// Default webhook setup capabilities for connectors
pub static WEBHOOK_SETUP_CAPABILITIES:
    common_types::connector_webhook_configuration::WebhookSetupCapabilities =
    common_types::connector_webhook_configuration::WebhookSetupCapabilities {
        is_webhook_auto_configuration_supported: false,
        requires_webhook_secret: None,
        config_type: None,
    };


Review comment (suggestion):
```suggestion
pub static DEFAULT_WEBHOOK_SETUP_CAPABILITIES:
```

Code after:
/// error message for timed out request
pub const REQUEST_TIMEOUT_ERROR_MESSAGE: &str = "Connector did not respond in specified time";

/// Header value indicating that signature-key-based authentication is used.
pub const UCS_AUTH_SIGNATURE_KEY: &str = "signature-key";

/// Header value indicating that body-key-based authentication is used.
pub const UCS_AUTH_BODY_KEY: &str = "body-key";

/// Header value indicating that header-key-based authentication is used.
pub const UCS_AUTH_HEADER_KEY: &str = "header-key";

/// Header value indicating that currency-auth-key-based authentication is used.
pub const UCS_AUTH_CURRENCY_AUTH_KEY: &str = "currency-auth-key";

/// Header value for content type JSON
pub const CONTENT_TYPE: &str = "Content-Type";

/// Header name for flow name
pub const X_FLOW_NAME: &str = "x-flow";

/// Header name for request ID
pub const X_REQUEST_ID: &str = "x-request-id";

/// Default webhook setup capabilities for connectors
pub static DEFAULT_WEBHOOK_SETUP_CAPABILITIES:
    common_types::connector_webhook_configuration::WebhookSetupCapabilities =
    common_types::connector_webhook_configuration::WebhookSetupCapabilities {
        is_webhook_auto_configuration_supported: false,
        requires_webhook_secret: None,
        config_type: None,
    };
Repository: juspay/hyperswitch
File: crates/api_models/src/payment_methods.rs (Rust)

Code before:
#[derive(Debug, serde::Serialize, ToSchema)]
pub struct NetworkTokenStatusCheckFailureResponse {
    /// Error message describing what went wrong
    pub error_message: String,
}

#[cfg(feature = "v2")]
#[derive(Debug, serde::Serialize, ToSchema)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum NetworkTokenStatusCheckResponse {
    /// Successful network token status check response
    SuccessResponse(NetworkTokenStatusCheckSuccessResponse),
    /// Error response for network token status check
    FailureResponse(NetworkTokenStatusCheckFailureResponse),
}

#[cfg(feature = "v2")]
#[derive(Debug, serde::Serialize, ToSchema)]
pub struct PaymentMethodGetTokenDetailsResponse {
    /// The payment method ID associated with the token
    #[schema(value_type = String, example = "12345_pm_019959146f92737389eb6927ce1eb7dc")]
    pub id: id_type::GlobalPaymentMethodId,

    /// The tokenization id associated with the payment method
    #[schema(value_type = String, example = "token_CSum555d9YxDOpGwYq6q")]
    pub token: String,
}


Review comment (suggestion):
```suggestion
    pub payment_method_token: String,
```
also please modify the doc comment. 
we have a different tokenization id in our service. this will create confusion

Code after:
#[derive(Debug, serde::Serialize, ToSchema)]
pub struct NetworkTokenStatusCheckFailureResponse {
    /// Error message describing what went wrong
    pub error_message: String,
}

#[cfg(feature = "v2")]
#[derive(Debug, serde::Serialize, ToSchema)]
#[serde(tag = "type", rename_all = "snake_case")]
pub enum NetworkTokenStatusCheckResponse {
    /// Successful network token status check response
    SuccessResponse(NetworkTokenStatusCheckSuccessResponse),
    /// Error response for network token status check
    FailureResponse(NetworkTokenStatusCheckFailureResponse),
}

#[cfg(feature = "v2")]
#[derive(Debug, serde::Serialize, ToSchema)]
pub struct PaymentMethodGetTokenDetailsResponse {
    /// The payment method ID associated with the token
    #[schema(value_type = String, example = "12345_pm_019959146f92737389eb6927ce1eb7dc")]
    pub id: id_type::GlobalPaymentMethodId,

    /// The token associated with the payment method
    #[schema(value_type = String, example = "token_CSum555d9YxDOpGwYq6q")]
    pub payment_method_token: String,
}
Repository: juspay/hyperswitch
File: crates/router/src/routes/payment_methods.rs (Rust)

Code before:
        },
        api_locking::LockAction::NotApplicable,
    ))
    .await
}

#[cfg(feature = "v2")]
#[instrument(skip_all, fields(flow = ?Flow::PaymentMethodGetTokenDetails))]
pub async fn payment_method_get_token_details_api(
    state: web::Data<AppState>,
    req: HttpRequest,
    path: web::Path<String>,
) -> HttpResponse {
    let flow = Flow::PaymentMethodGetTokenDetails;
    let temporary_token = path.into_inner();

    Box::pin(api::server_wrap(
        flow,
        state,
        &req,
        (),
        |state, auth: auth::AuthenticationData, _, _| {
            let temporary_token = temporary_token.clone();
            async move {
                let platform: domain::Platform = auth.platform;
                payment_methods_routes::payment_method_get_token(
                    state,
                    platform.get_provider().clone(),
                    temporary_token,
                )
                .await
            }
        },
        &auth::V2ApiKeyAuth {
            is_connected_allowed: false,
            is_platform_allowed: false,
        },
        api_locking::LockAction::NotApplicable,
    ))
    .await
}


Review comment (suggestion):
nit:
```suggestion
                payment_methods_routes::payment_method_get_token_details_core(
```

Code after:
        },
        api_locking::LockAction::NotApplicable,
    ))
    .await
}

#[cfg(feature = "v2")]
#[instrument(skip_all, fields(flow = ?Flow::PaymentMethodGetTokenDetails))]
pub async fn payment_method_get_token_details_api(
    state: web::Data<AppState>,
    req: HttpRequest,
    path: web::Path<String>,
) -> HttpResponse {
    let flow = Flow::PaymentMethodGetTokenDetails;
    let temporary_token = path.into_inner();

    Box::pin(api::server_wrap(
        flow,
        state,
        &req,
        (),
        |state, auth: auth::AuthenticationData, _, _| {
            let temporary_token = temporary_token.clone();
            async move {
                let platform: domain::Platform = auth.platform;
                payment_methods_routes::payment_method_get_token_details_core(
                    state,
                    platform.get_provider().clone(),
                    temporary_token,
                )
                .await
            }
        },
        &auth::V2ApiKeyAuth {
            is_connected_allowed: false,
            is_platform_allowed: false,
        },
        api_locking::LockAction::NotApplicable,
    ))
    .await
}
Repository: juspay/hyperswitch
File: cypress-tests/cypress/e2e/configs/Payment/Mollie.js (JavaScript)

Code before:
  },
};

const billingAddressNL = {
  address: {
    line1: "1467",
    line2: "Harrison Street",
    city: "San Francisco",
    state: "California",
    zip: "94122",
    country: "NL",
    first_name: "John",
    last_name: "Doe",
  },
  phone: {
    number: "9123456789",
    country_code: "+31",
  },
};

const billingAddressAT = {
  address: {
    line1: "1467",
    line2: "Harrison Street",
    city: "Vienna",
    state: "Vienna",
    zip: "1010",
    country: "AT",
    first_name: "John",
    last_name: "Doe",
  },
  phone: {
    number: "9123456789",
    country_code: "+43",
  },
};

const billingAddressDE = {
  address: {
    line1: "1467",
    line2: "Harrison Street",
    city: "Berlin",
    state: "Berlin",
    zip: "10115",
    country: "DE",
    first_name: "John",
    last_name: "Doe",
  },
  phone: {
    number: "9123456789",
    country_code: "+49",

Review comment (suggestion):
Can we make the changes like this to avoid duplication

```
const billingAddressAT = {
  ...baseBillingAddress,
  address: {
    ...billingAddressNL.address,
    country: "AT",
  },
};
```

Code after:
    country_code: "+1",
  },
};

const billingAddress = {
  ...baseBillingAddress,
  address: {
    ...baseBillingAddress.address,
    first_name: "Test",
    last_name: "User",
  },
};

const billingAddressNL = {
  ...baseBillingAddress,
  address: {
    ...baseBillingAddress.address,
    country: "NL",
  },
  phone: {
    ...baseBillingAddress.phone,
    country_code: "+31",
  },
};

const billingAddressAT = {
  ...baseBillingAddress,
  address: {
    ...baseBillingAddress.address,
    country: "AT",
  },
  phone: {
    ...baseBillingAddress.phone,
    country_code: "+43",
  },
};

const billingAddressDE = {
  ...baseBillingAddress,
  address: {
    ...baseBillingAddress.address,
    country: "DE",
  },
  phone: {
    ...baseBillingAddress.phone,
    country_code: "+49",
  },
};

const billingAddressPL = {
  ...baseBillingAddress,
Repository: juspay/hyperswitch
File: crates/common_utils/src/new_type.rs (Rust)

Code before:
pub struct MaskedRoutingNumber(Secret<String>);
impl From<String> for MaskedRoutingNumber {
    fn from(src: String) -> Self {
        let masked_value = apply_mask(src.as_ref(), 3, 3);
        Self(Secret::from(masked_value))
    }
}
impl From<Secret<String>> for MaskedRoutingNumber {
    fn from(secret: Secret<String>) -> Self {
        Self::from(secret.expose())
    }
}

/// Masked bank account
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
pub struct MaskedBankAccount(Secret<String>);
impl From<String> for MaskedBankAccount {
    fn from(src: String) -> Self {
        let masked_value = apply_mask(src.as_ref(), 4, 4);
        Self(Secret::from(masked_value))
    }
}

impl MaskedBankAccount {
    /// Expose the inner secret
    pub fn expose_inner(&self) -> String {
        self.0.clone().expose()
    }
}
impl From<Secret<String>> for MaskedBankAccount {
    fn from(secret: Secret<String>) -> Self {
        Self::from(secret.expose())
    }
}

/// Masked IBAN
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
pub struct MaskedIban(Secret<String>);
impl From<String> for MaskedIban {
    fn from(src: String) -> Self {
        let masked_value = apply_mask(src.as_ref(), 5, 5);
        Self(Secret::from(masked_value))
    }
}
impl From<Secret<String>> for MaskedIban {
    fn from(secret: Secret<String>) -> Self {
        Self::from(secret.expose())
    }
}

/// Masked IBAN

Review comment (suggestion):
expose function can take ownership of the value. Peek would take reference.
```suggestion
    pub fn expose_inner(self) -> String {
```

Code after:
pub struct MaskedRoutingNumber(Secret<String>);
impl From<String> for MaskedRoutingNumber {
    fn from(src: String) -> Self {
        let masked_value = apply_mask(src.as_ref(), 3, 3);
        Self(Secret::from(masked_value))
    }
}
impl From<Secret<String>> for MaskedRoutingNumber {
    fn from(secret: Secret<String>) -> Self {
        Self::from(secret.expose())
    }
}

/// Masked bank account
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
pub struct MaskedBankAccount(Secret<String>);
impl From<String> for MaskedBankAccount {
    fn from(src: String) -> Self {
        let masked_value = apply_mask(src.as_ref(), 4, 4);
        Self(Secret::from(masked_value))
    }
}

impl MaskedBankAccount {
    /// Expose the inner secret
    pub fn expose_inner(self) -> String {
        self.0.expose()
    }
}
impl From<Secret<String>> for MaskedBankAccount {
    fn from(secret: Secret<String>) -> Self {
        Self::from(secret.expose())
    }
}

/// Masked IBAN
#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)]
pub struct MaskedIban(Secret<String>);
impl From<String> for MaskedIban {
    fn from(src: String) -> Self {
        let masked_value = apply_mask(src.as_ref(), 5, 5);
        Self(Secret::from(masked_value))
    }
}
impl From<Secret<String>> for MaskedIban {
    fn from(secret: Secret<String>) -> Self {
        Self::from(secret.expose())
    }
}

/// Masked IBAN