ronantakizawa/codereview-bench huggingface dataset:
Repository: juspay/hyperswitch
File: cypress-tests/cypress/e2e/configs/Payment/Utils.js (JavaScript)

Code before:
export const CONNECTOR_LISTS = {
  // Exclusion lists (skip these connectors)
  EXCLUDE: {
    CONNECTOR_AGNOSTIC_NTID: [
      "bamboraapac",
      "bankofamerica",
      "billwerk",
      "braintree",
      "facilitapay",
      "fiserv",
      "fiuu",
      "jpmorgan",
      "nexinets",
      "payload",
      "paypal",
      "stax",
      "wellsfargo",
      "worldpayxml",
    ],
    // Add more exclusion lists
  },

  // Inclusion lists (only run for these connectors)
  INCLUDE: {
    MANDATES_USING_NTID_PROXY: ["cybersource"],
    INCREMENTAL_AUTH: ["cybersource"],
    // Add more inclusion lists
  },
};

// Helper functions
export const shouldExcludeConnector = (connectorId, list) => {
  return list.includes(connectorId);
};

export const shouldIncludeConnector = (connectorId, list) => {
  return !list.includes(connectorId);
};


Review comment (security):
Added `INCREMENTAL_AUTH` as a new list of connectors that supports Incremental Authorization.

In this PR, I am just creating the include list, so I'm not adding all the supporting connectors, but as of my knowledge, it should be `archipel`(which is addressed in my next PR #8189 ), `cybersource` and `paypal` (which will be merged soon after this PR I guess).

Code after:
export const CONNECTOR_LISTS = {
  // Exclusion lists (skip these connectors)
  EXCLUDE: {
    CONNECTOR_AGNOSTIC_NTID: [
      "bamboraapac",
      "bankofamerica",
      "billwerk",
      "braintree",
      "facilitapay",
      "fiserv",
      "fiuu",
      "jpmorgan",
      "nexinets",
      "payload",
      "paypal",
      "stax",
      "wellsfargo",
      "worldpayxml",
    ],
    // Add more exclusion lists
  },

  // Inclusion lists (only run for these connectors)
  INCLUDE: {
    MANDATES_USING_NTID_PROXY: ["cybersource"],
    INCREMENTAL_AUTH: [
      // "cybersource"    // issues with MULTIPLE_CONNECTORS handling
    ],
    // Add more inclusion lists
  },
};

// Helper functions
export const shouldExcludeConnector = (connectorId, list) => {
  return list.includes(connectorId);
};

export const shouldIncludeConnector = (connectorId, list) => {
  return !list.includes(connectorId);
};
Repository: juspay/hyperswitch
File: cypress-tests/cypress/e2e/configs/Payment/Utils.js (JavaScript)

Code before:
export const CONNECTOR_LISTS = {
  // Exclusion lists (skip these connectors)
  EXCLUDE: {
    CONNECTOR_AGNOSTIC_NTID: [
      "bamboraapac",
      "bankofamerica",
      "billwerk",
      "braintree",
      "facilitapay",
      "fiserv",
      "fiuu",
      "jpmorgan",
      "nexinets",
      "payload",
      "paypal",
      "stax",
      "wellsfargo",
      "worldpayxml",
    ],
    // Add more exclusion lists
  },

  // Inclusion lists (only run for these connectors)
  INCLUDE: {
    MANDATES_USING_NTID_PROXY: ["cybersource"],
    INCREMENTAL_AUTH: ["cybersource"],
    // Add more inclusion lists
  },
};

// Helper functions
export const shouldExcludeConnector = (connectorId, list) => {
  return list.includes(connectorId);
};

export const shouldIncludeConnector = (connectorId, list) => {
  return !list.includes(connectorId);
};


Review comment (bug):
```suggestion
    INCREMENTAL_AUTH: [
      // "cybersource"    // issues with MULTIPLE_CONNECTORS handling
    ],
```

Currently, I am thinking to comment out `cybersource`, so that the incremental auth tests skip for it, because it seems there are some issues handling `MULTIPLE_CONNECTORS` in `cybersource`, which is resulting in test cases failure for `cybersource` and hence it will fail all the PRs, and prevent them from getting merged.

Code after:
export const CONNECTOR_LISTS = {
  // Exclusion lists (skip these connectors)
  EXCLUDE: {
    CONNECTOR_AGNOSTIC_NTID: [
      "bamboraapac",
      "bankofamerica",
      "billwerk",
      "braintree",
      "facilitapay",
      "fiserv",
      "fiuu",
      "jpmorgan",
      "nexinets",
      "payload",
      "paypal",
      "stax",
      "wellsfargo",
      "worldpayxml",
    ],
    // Add more exclusion lists
  },

  // Inclusion lists (only run for these connectors)
  INCLUDE: {
    MANDATES_USING_NTID_PROXY: ["cybersource"],
    INCREMENTAL_AUTH: [
      // "cybersource"    // issues with MULTIPLE_CONNECTORS handling
    ],
    // Add more inclusion lists
  },
};

// Helper functions
export const shouldExcludeConnector = (connectorId, list) => {
  return list.includes(connectorId);
};

export const shouldIncludeConnector = (connectorId, list) => {
  return !list.includes(connectorId);
};
Repository: juspay/hyperswitch
File: crates/common_enums/src/enums.rs (Rust)

Code before:
)]
#[router_derive::diesel_enum(storage_type = "text")]
#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "snake_case")]
pub enum CtpServiceProvider {
    Visa,
    Mastercard,
}

#[derive(
    Clone,
    Copy,
    Debug,
    Default,
    Eq,
    Hash,
    PartialEq,
    strum::Display,
    strum::EnumString,
    strum::EnumIter,
    serde::Serialize,
    serde::Deserialize,
)]
#[router_derive::diesel_enum(storage_type = "db_enum")]
#[strum(serialize_all = "snake_case")]
#[serde(rename_all = "snake_case")]
pub enum RefundStatus {
    #[serde(alias = "Failure")]
    Failure,
    #[serde(alias = "ManualReview")]
    ManualReview,
    #[default]
    #[serde(alias = "Pending")]
    Pending,
    #[serde(alias = "Success")]
    Success,
    #[serde(alias = "TransactionFailure")]
    TransactionFailure,
}

#[derive(
    Clone,
    Copy,
    Debug,
    Default,
    Eq,
    Hash,
    PartialEq,
    strum::Display,
    strum::EnumString,
    strum::EnumIter,

Review comment (bug):
```suggestion
    TelephoneOrder,
    #[serde(untagged)]
    #[strum(default)]
    Other(String),
```

This will avoid failure of parsing the new enum variant during the staggered release

Code after:
#[derive(
    Clone,
    Debug,
    Default,
    Eq,
    PartialOrd,
    Ord,
    Hash,
    PartialEq,
    serde::Deserialize,
    serde::Serialize,
    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")]
/// Describes the channel through which the payment was initiated.
pub enum PaymentChannel {
    #[default]
    Ecommerce,
    MailOrder,
    TelephoneOrder,
    #[serde(untagged)]
    #[strum(default)]
    Other(String),
}

#[derive(
    Clone,
    Copy,
    Debug,
    Eq,
    PartialEq,
    serde::Deserialize,
    serde::Serialize,
    strum::Display,
    strum::EnumString,
    ToSchema,
)]
#[router_derive::diesel_enum(storage_type = "text")]
#[serde(rename_all = "snake_case")]
#[strum(serialize_all = "snake_case")]
pub enum CtpServiceProvider {
    Visa,
    Mastercard,
}
Repository: juspay/hyperswitch
File: cypress-tests/cypress/support/redirectionHandler.js (JavaScript)

Code before:
/* eslint-disable cypress/unsafe-to-chain-command */

import jsQR from "jsqr";

// Detect CI environment and apply appropriate timeout multipliers (same logic as cypress.config.js)
const isCI = process.env.CI === "true" || process.env.GITHUB_ACTIONS === "true";

const timeoutMultiplier = isCI ? 1.5 : 1;

// Define constants for wait times with adaptive scaling
const CONSTANTS = {
  TIMEOUT: Math.round(90000 * timeoutMultiplier), // 90s local, 135s (2.25min) CI
  WAIT_TIME: Math.round(30000 * timeoutMultiplier), // 30s local, 45s CI
  ERROR_PATTERNS: [
    /^(4|5)\d{2}\s/, // HTTP error status codes
    /\berror occurred\b/i,
    /\bpayment failed\b/i,
    /\binvalid request\b/i,
    /\bserver error\b/i,
    /\btransaction failed\b/i,
    /\bpayment declined\b/i,
    /\bauthorization failed\b/i,
  ],
  VALID_TERMINAL_STATUSES: [
    "failed",
    "processing",
    "requires_capture",
    "succeeded",
  ],
};

export function handleRedirection(
  redirectionType,

Review comment (refactor):
may be, put this in `cypress-tests/cypress/utils/RequestBodyUtils.js` or connector config utils and export the timeout multiplier instead of having it duplicated in both the files?

Code after:
/* eslint-disable cypress/unsafe-to-chain-command */

import jsQR from "jsqr";
import { getTimeoutMultiplier } from "../utils/RequestBodyUtils.js";

const timeoutMultiplier = getTimeoutMultiplier();

const CONSTANTS = {
  TIMEOUT: Math.round(90000 * timeoutMultiplier), // 90s local, 135s (2.25min) CI
  WAIT_TIME: Math.round(30000 * timeoutMultiplier), // 30s local, 45s CI
  ERROR_PATTERNS: [
    /^(4|5)\d{2}\s/, // HTTP error status codes
    /\berror occurred\b/i,
    /\bpayment failed\b/i,
    /\binvalid request\b/i,
    /\bserver error\b/i,
    /\btransaction failed\b/i,
    /\bpayment declined\b/i,
    /\bauthorization failed\b/i,
  ],
  VALID_TERMINAL_STATUSES: [
    "failed",
    "processing",
    "requires_capture",
    "succeeded",
  ],
};

export function handleRedirection(
  redirectionType,
  urls,
  connectorId,
  paymentMethodType,
Repository: juspay/hyperswitch
File: crates/common_enums/src/transformers.rs (Rust)

Code before:
            Some(true) => Self::Enable,
            _ => Self::Skip,
        }
    }
}

/// Get the boolean value of the `External3dsAuthenticationRequest`.
impl super::External3dsAuthenticationRequest {
    pub fn as_bool(&self) -> bool {
        match self {
            Self::Enable => true,
            Self::Skip => false,
        }
    }
}

/// Get the boolean value of the `SplitTxnsEnabled`.
impl super::SplitTxnsEnabled {
    pub fn as_bool(&self) -> bool {
        match self {
            Self::True => true,
            Self::False => false,
            Self::Default => false,
        }
    }
}

impl super::EnablePaymentLinkRequest {
    pub fn as_bool(&self) -> bool {
        match self {
            Self::Enable => true,
            Self::Skip => false,
        }
    }
}

impl From<Option<bool>> for super::EnablePaymentLinkRequest {
    fn from(value: Option<bool>) -> Self {
        match value {
            Some(true) => Self::Enable,
            _ => Self::Skip,
        }
    }
}

impl From<Option<bool>> for super::MitExemptionRequest {
    fn from(value: Option<bool>) -> Self {
        match value {
            Some(true) => Self::Apply,
            _ => Self::Skip,
        }

Review comment (suggestion):
If only true and false is needed, collect in boolean form as well and default can be handled by default implementation

Code after:
            Some(true) => Self::Enable,
            _ => Self::Skip,
        }
    }
}

/// Get the boolean value of the `External3dsAuthenticationRequest`.
impl super::External3dsAuthenticationRequest {
    pub fn as_bool(&self) -> bool {
        match self {
            Self::Enable => true,
            Self::Skip => false,
        }
    }
}

impl super::EnablePaymentLinkRequest {
    pub fn as_bool(&self) -> bool {
        match self {
            Self::Enable => true,
            Self::Skip => false,
        }
    }
}

impl From<Option<bool>> for super::EnablePaymentLinkRequest {
    fn from(value: Option<bool>) -> Self {
        match value {
            Some(true) => Self::Enable,
            _ => Self::Skip,
        }
    }
}

impl From<Option<bool>> for super::MitExemptionRequest {
    fn from(value: Option<bool>) -> Self {
        match value {
            Some(true) => Self::Apply,
            _ => Self::Skip,
        }
    }
}

impl super::MitExemptionRequest {
    pub fn as_bool(&self) -> bool {
        match self {
            Self::Apply => true,
            Self::Skip => false,
        }
    }
}
Repository: juspay/hyperswitch
File: crates/router/src/routes/subscription.rs (Rust)

Code before:
#[cfg(all(feature = "olap", feature = "v1"))]
#[instrument(skip_all)]
pub async fn create_subscription(
    state: web::Data<AppState>,
    req: HttpRequest,
    json_payload: web::Json<subscription_types::CreateSubscriptionRequest>,
) -> impl Responder {
    let flow = Flow::RoutingCreateConfig;
    Box::pin(oss_api::server_wrap(
        flow,
        state,
        &req,
        json_payload.into_inner(),
        |state, auth: auth::AuthenticationData, payload, _| {
            let merchant_context = domain::MerchantContext::NormalMerchant(Box::new(
                domain::Context(auth.merchant_account, auth.key_store),
            ));
            subscription::create_subscription(state, merchant_context, payload.clone())
        },
        auth::auth_type(
            &auth::HeaderAuth(auth::ApiKeyAuth {
                is_connected_allowed: false,
                is_platform_allowed: false,
            }),
            &auth::JWTAuth {
                permission: Permission::ProfileRoutingWrite,
            },
            req.headers(),
        ),
        api_locking::LockAction::NotApplicable,
    ))
    .await
}


Review comment (security):
```suggestion
                permission: Permission::ProfileSubscriptionCreate,
```

Code after:
};

#[cfg(all(feature = "oltp", feature = "v1"))]
#[instrument(skip_all)]
pub async fn create_subscription(
    state: web::Data<AppState>,
    req: HttpRequest,
    json_payload: web::Json<subscription_types::CreateSubscriptionRequest>,
) -> impl Responder {
    let flow = Flow::CreateSubscription;
    let profile_id = match req.headers().get(X_PROFILE_ID) {
        Some(val) => val.to_str().unwrap_or_default().to_string(),
        None => {
            return HttpResponse::BadRequest().json(
                errors::api_error_response::ApiErrorResponse::MissingRequiredField {
                    field_name: "x-profile-id",
                },
            );
        }
    };
    Box::pin(oss_api::server_wrap(
        flow,
        state,
        &req,
        json_payload.into_inner(),
        move |state, auth: auth::AuthenticationData, payload, _| {
            let merchant_context = domain::MerchantContext::NormalMerchant(Box::new(
                domain::Context(auth.merchant_account, auth.key_store),
            ));
            subscription::create_subscription(
                state,
                merchant_context,
                profile_id.clone(),
                payload.clone(),
            )
        },
        auth::auth_type(
            &auth::HeaderAuth(auth::ApiKeyAuth {
                is_connected_allowed: false,
                is_platform_allowed: false,
            }),
            &auth::JWTAuth {
                permission: Permission::ProfileSubscriptionWrite,
            },
            req.headers(),
        ),
        api_locking::LockAction::NotApplicable,
    ))
    .await
}
Repository: juspay/hyperswitch
File: crates/api_models/src/subscription.rs (Rust)

Code before:
    pub client_secret: Option<String>,
    pub merchant_id: String,
    pub mca_id: Option<String>,
    pub coupon_code: Option<String>,
    pub customer: Option<CustomerDetailsResponse>,
    pub invoice: Option<Invoice>,
}

#[derive(Debug, Clone, serde::Serialize)]
pub struct Subscription {
    pub id: String,
    pub status: SubscriptionStatus,
    pub plan_id: Option<String>,
}

#[derive(Debug, Clone, serde::Serialize, strum::EnumString, strum::Display)]
pub enum SubscriptionStatus {
    Created,
    Active,
    InActive,
}

#[derive(Debug, Clone, serde::Serialize)]
pub struct Invoice {
    pub id: String,
    pub total: u64,
}

impl Subscription {
    pub fn new(id: impl Into<String>, status: SubscriptionStatus, plan_id: Option<String>) -> Self {
        Self {
            id: id.into(),
            status,
            plan_id,
        }
    }
}

impl Invoice {
    pub fn new(id: impl Into<String>, total: u64) -> Self {
        Self {
            id: id.into(),
            total,
        }
    }
}
impl CreateSubscriptionResponse {
    #[allow(clippy::too_many_arguments)]
    pub fn new(
        subscription: Subscription,
        merchant_id: impl Into<String>,

Review comment (suggestion):
```suggestion
    pub total: u64,
    pub currency: Currency,
```

Code after:
#[derive(Debug, Clone, serde::Serialize, ToSchema)]
pub struct CreateSubscriptionResponse {
    /// Unique identifier for the subscription.
    pub id: common_utils::id_type::SubscriptionId,

    /// Merchant specific Unique identifier.
    pub merchant_reference_id: Option<String>,

    /// Current status of the subscription.
    pub status: SubscriptionStatus,

    /// Identifier for the associated subscription plan.
    pub plan_id: Option<String>,

    /// Associated profile ID.
    pub profile_id: common_utils::id_type::ProfileId,

    /// Optional client secret used for secure client-side interactions.
    pub client_secret: Option<Secret<String>>,

    /// Merchant identifier owning this subscription.
    pub merchant_id: common_utils::id_type::MerchantId,

    /// Optional coupon code applied to this subscription.
    pub coupon_code: Option<String>,

    /// Optional customer ID associated with this subscription.
    pub customer_id: common_utils::id_type::CustomerId,
}

/// Possible states of a subscription lifecycle.
///
/// - `Created`: Subscription was created but not yet activated.
/// - `Active`: Subscription is currently active.
/// - `InActive`: Subscription is inactive (e.g., cancelled or expired).
#[derive(Debug, Clone, serde::Serialize, strum::EnumString, strum::Display, ToSchema)]
pub enum SubscriptionStatus {
    /// Subscription is active.
    Active,
    /// Subscription is created but not yet active.
    Created,
    /// Subscription is inactive.
    InActive,
    /// Subscription is in pending state.
    Pending,
}

impl CreateSubscriptionResponse {
    /// Creates a new [`CreateSubscriptionResponse`] with the given identifiers.
    ///
    /// By default, `client_secret`, `coupon_code`, and `customer` fields are `None`.
Repository: juspay/hyperswitch
File: crates/api_models/src/subscription.rs (Rust)

Code before:
use common_utils::{events::ApiEventMetric, pii};

use crate::{
    customers::{CustomerRequest, CustomerResponse},
    payments::CustomerDetailsResponse,
};

pub const SUBSCRIPTION_ID_PREFIX: &str = "sub";

#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
pub struct CreateSubscriptionRequest {
    pub subscription_id: Option<String>,
    pub profile_id: common_utils::id_type::ProfileId,
    pub plan_id: Option<String>,
    pub coupon_code: Option<String>,
    pub mca_id: Option<String>,
    pub confirm: bool,
    pub customer_id: Option<common_utils::id_type::CustomerId>,
    pub customer: Option<CustomerRequest>,
}

impl CreateSubscriptionRequest {
    pub fn get_customer_id(&self) -> Option<&common_utils::id_type::CustomerId> {
        self.customer_id
            .as_ref()
            .or_else(|| self.customer.as_ref()?.customer_id.as_ref())
    }
}

#[derive(Debug, Clone, serde::Serialize)]
pub struct CreateSubscriptionResponse {
    pub subscription: Subscription,
    pub profile_id: common_utils::id_type::ProfileId,
    pub client_secret: Option<String>,
    pub merchant_id: String,
    pub mca_id: Option<String>,
    pub coupon_code: Option<String>,
    pub customer: Option<CustomerDetailsResponse>,
    pub invoice: Option<Invoice>,
}


Review comment (suggestion):
`pub merchant_connector_account_id: Option<common_utils::id_type::MerchantConnectorAccountId>`

Code after:
use common_utils::events::ApiEventMetric;
use masking::Secret;
use utoipa::ToSchema;

// use crate::{
//     customers::{CustomerRequest, CustomerResponse},
//     payments::CustomerDetailsResponse,
// };

/// Request payload for creating a subscription.
///
/// This struct captures details required to create a subscription,
/// including plan, profile, merchant connector, and optional customer info.
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize, ToSchema)]
pub struct CreateSubscriptionRequest {
    /// Merchant specific Unique identifier.
    pub merchant_reference_id: Option<String>,

    /// Identifier for the subscription plan.
    pub plan_id: Option<String>,

    /// Optional coupon code applied to the subscription.
    pub coupon_code: Option<String>,

    /// customer ID associated with this subscription.
    pub customer_id: common_utils::id_type::CustomerId,
}

/// Response payload returned after successfully creating a subscription.
///
/// Includes details such as subscription ID, status, plan, merchant, and customer info.
#[derive(Debug, Clone, serde::Serialize, ToSchema)]
pub struct CreateSubscriptionResponse {
    /// Unique identifier for the subscription.
    pub id: common_utils::id_type::SubscriptionId,

    /// Merchant specific Unique identifier.
    pub merchant_reference_id: Option<String>,

    /// Current status of the subscription.
    pub status: SubscriptionStatus,
Repository: juspay/hyperswitch
File: crates/router/src/core/subscription.rs (Rust)

Code before:
        || customer.phone.is_some()
        || customer.phone_country_code.is_some()
    {
        let customer = get_or_create_customer(state, request.customer, merchant_context.clone())
            .await
            .map_err(|e| e.change_context(errors::ApiErrorResponse::CustomerNotFound))
            .attach_printable("subscriptions: unable to process customer")?;

        let customer_table_response = match &customer {
            ApplicationResponse::Json(inner) => {
                Some(subscription_types::map_customer_resp_to_details(inner))
            }
            _ => None,
        };
        response.customer = customer_table_response;
        response
            .customer
            .as_ref()
            .and_then(|customer| customer.id.clone())
    } else {
        request.customer_id.clone()
    }
    .ok_or(errors::ApiErrorResponse::InternalServerError)
    .attach_printable("subscriptions: unable to create a customer")?;

    // If provided we can strore plan_id, coupon_code etc as metadata
    let mut subscription = SubscriptionNew::new(
        id,
        SubscriptionStatus::Created.to_string(),
        None,
        None,
        request.merchant_connector_account_id,
        None,
        None,
        merchant_context.get_merchant_account().get_id().clone(),
        customer_id,
        None,
        request.profile_id,
    );
    response.client_secret = Some(subscription.generate_and_set_client_secret());
    db.insert_subscription_entry(subscription)
        .await
        .change_context(errors::ApiErrorResponse::InternalServerError)
        .attach_printable("subscriptions: unable to insert subscription entry to database")?;

    Ok(ApplicationResponse::Json(response))
}


Review comment (suggestion):
```suggestion
    // If provided we can store plan_id, coupon_code etc as metadata
```

Code after:
        merchant_context.get_merchant_account().get_id().clone(),
        request.customer_id.clone(),
        None,
        profile_id,
        request.merchant_reference_id,
    );

    subscription.generate_and_set_client_secret();
    let subscription_response = db
        .insert_subscription_entry(subscription)
        .await
        .change_context(errors::ApiErrorResponse::InternalServerError)
        .attach_printable("subscriptions: unable to insert subscription entry to database")?;

    let response = CreateSubscriptionResponse::new(
        subscription_response.id.clone(),
        subscription_response.merchant_reference_id,
        SubscriptionStatus::from_str(&subscription_response.status)
            .unwrap_or(SubscriptionStatus::Created),
        None,
        subscription_response.profile_id,
        subscription_response.merchant_id,
        subscription_response.client_secret.map(Secret::new),
        request.customer_id,
    );

    Ok(ApplicationResponse::Json(response))
}