Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(router): Add support for storing multiple surcharge configurations at profile level #7570

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

spritianeja03
Copy link
Contributor

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

This PR adds support for storing multiple surcharge configurations at profile level by adding new endpoints to list, create, activate and retrieve active surcharge configurations.

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

Closes #7569

How did you test it?

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

@spritianeja03 spritianeja03 added C-feature Category: Feature request or enhancement M-database-changes Metadata: This PR involves database schema changes M-api-contract-changes Metadata: This PR involves API contract changes labels Mar 19, 2025
@spritianeja03 spritianeja03 added this to the February 2025 Release milestone Mar 19, 2025
@spritianeja03 spritianeja03 self-assigned this Mar 19, 2025
@spritianeja03 spritianeja03 requested review from a team as code owners March 19, 2025 11:29
Copy link

semanticdiff-com bot commented Mar 19, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  crates/diesel_models/src/enums.rs  96% smaller
  crates/api_models/src/events/routing.rs  5% smaller
  crates/router/src/core/surcharge_decision_config.rs  2% smaller
  api-reference/openapi_spec.json  0% smaller
  crates/api_models/src/admin.rs  0% smaller
  crates/api_models/src/routing.rs  0% smaller
  crates/api_models/src/surcharge_decision_configs.rs  0% smaller
  crates/common_enums/src/enums.rs  0% smaller
  crates/common_utils/src/id_type.rs  0% smaller
  crates/common_utils/src/id_type/routing.rs  0% smaller
  crates/diesel_models/src/business_profile.rs  0% smaller
  crates/diesel_models/src/payment_attempt.rs  0% smaller
  crates/diesel_models/src/query/routing_algorithm.rs  0% smaller
  crates/diesel_models/src/routing_algorithm.rs  0% smaller
  crates/diesel_models/src/schema.rs  0% smaller
  crates/diesel_models/src/schema_v2.rs  0% smaller
  crates/diesel_models/src/user/sample_data.rs  0% smaller
  crates/hyperswitch_domain_models/src/business_profile.rs  0% smaller
  crates/hyperswitch_domain_models/src/payments/payment_attempt.rs  0% smaller
  crates/router/src/core/admin.rs  0% smaller
  crates/router/src/core/payments/helpers.rs  0% smaller
  crates/router/src/core/payments/operations/payment_create.rs  0% smaller
  crates/router/src/core/payments/retry.rs  0% smaller
  crates/router/src/core/routing.rs  0% smaller
  crates/router/src/core/routing/helpers.rs  0% smaller
  crates/router/src/db/kafka_store.rs  0% smaller
  crates/router/src/db/routing_algorithm.rs  0% smaller
  crates/router/src/routes/app.rs  0% smaller
  crates/router/src/routes/routing.rs  0% smaller
  crates/router/src/services/authorization/permissions.rs  0% smaller
  crates/router/src/types/api/admin.rs  0% smaller
  crates/router/src/types/storage/payment_attempt.rs  0% smaller
  crates/router/src/utils/user/sample_data.rs  0% smaller
  crates/storage_impl/src/mock_db/payment_attempt.rs  0% smaller
  crates/storage_impl/src/payments/payment_attempt.rs  0% smaller
  migrations/2025-03-05-093403_add_algorithm_type_to_routing_algorithm/down.sql Unsupported file format
  migrations/2025-03-05-093403_add_algorithm_type_to_routing_algorithm/up.sql Unsupported file format
  migrations/2025-03-05-105913_add_active_surcharge_algorithm_id_to_business_profile/down.sql Unsupported file format
  migrations/2025-03-05-105913_add_active_surcharge_algorithm_id_to_business_profile/up.sql Unsupported file format
  migrations/2025-03-06-075521_add_surcharge_algorithm_id_to_payment_attempt/down.sql Unsupported file format
  migrations/2025-03-06-075521_add_surcharge_algorithm_id_to_payment_attempt/up.sql Unsupported file format

@hyperswitch-bot hyperswitch-bot bot removed the M-api-contract-changes Metadata: This PR involves API contract changes label Mar 19, 2025
@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Mar 19, 2025
ToSchema,
Default,
)]
#[router_derive::diesel_enum(storage_type = "db_enum")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[router_derive::diesel_enum(storage_type = "db_enum")]
#[router_derive::diesel_enum(storage_type = "text")]

Making this db_enum would make it difficult to add new enum variant in future.

/// A wrapper type for `RoutingId` that can be used for surcharge routing ids
pub struct SurchargeRoutingId(pub RoutingId);

impl Deref for SurchargeRoutingId {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Comment on lines +1059 to +1062
)
.service(
web::resource("/{algorithm_id}/surcharge/activate")
.route(web::post().to(routing::surcharge_link_config)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
)
.service(
web::resource("/{algorithm_id}/surcharge/activate")
.route(web::post().to(routing::surcharge_link_config)),
)
.service(
web::resource("decision/surcharge/{algorithm_id}/activate")
.route(web::post().to(routing::surcharge_link_config)),

req,
payload,
&TransactionType::Payment,
AlgorithmType::Surcharge,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create a const for this and pass to this function.

@@ -0,0 +1,4 @@
-- Your SQL goes here
CREATE TYPE "AlgorithmType" AS ENUM ('routing', 'surcharge', '3ds');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets not create a DB enum right now.

-- Your SQL goes here
CREATE TYPE "AlgorithmType" AS ENUM ('routing', 'surcharge', '3ds');

ALTER TABLE routing_algorithm ADD COLUMN IF NOT EXISTS algorithm_type "AlgorithmType" NOT NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ALTER TABLE routing_algorithm ADD COLUMN IF NOT EXISTS algorithm_type "AlgorithmType" NOT NULL;
ALTER TABLE routing_algorithm ADD COLUMN IF NOT EXISTS algorithm_type VARCHAR(64) NOT NULL;

Comment on lines +240 to +246
let profile_id = profile_id
.ok_or_else(|| errors::ApiErrorResponse::MissingRequiredField {
field_name: "profile_id",
})
.change_context(errors::ApiErrorResponse::MissingRequiredField {
field_name: "profile_id",
})?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets do this validation in the parent function get profile_id as non optional value int this function.

Comment on lines +307 to +313
let profile_id = profile_id
.ok_or_else(|| errors::ApiErrorResponse::MissingRequiredField {
field_name: "profile_id",
})
.change_context(errors::ApiErrorResponse::MissingRequiredField {
field_name: "profile_id",
})?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets do this validation in the parent function get profile_id as non optional value int this function.

Comment on lines +280 to +303
let name = request
.name
.get_required_value("name")
.change_context(errors::ApiErrorResponse::MissingRequiredField { field_name: "name" })
.attach_printable("Name of config not given")?;

let description = request
.description
.get_required_value("description")
.change_context(errors::ApiErrorResponse::MissingRequiredField {
field_name: "description",
})
.attach_printable("Description of config not given")?;

let surcharge_algorithm_data = SurchargeDecisionManagerConfig {
merchant_surcharge_configs: request.merchant_surcharge_configs.clone(),
algorithm: request
.algorithm
.get_required_value("algorithm")
.change_context(errors::ApiErrorResponse::MissingRequiredField {
field_name: "algorithm",
})
.attach_printable("Algorithm of config not given")?,
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

introduce a new request type where are these values will be required.

.await
.to_not_found_response(errors::ApiErrorResponse::ResourceIdNotFound)?;

let new_record = SurchargeRecord {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return description as well

let profile_id = query.profile_id;

let final_response;
if let Some(profile_id) = profile_id.clone() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

profile_id can be amde required in SurchargeRetrieveLinkQuery

Comment on lines +411 to +413
return Err(errors::ApiErrorResponse::ResourceIdNotFound)
.change_context(errors::ApiErrorResponse::ResourceIdNotFound)
.attach_printable("Active surcharge algorithm ID not found");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return Err(errors::ApiErrorResponse::ResourceIdNotFound)
.change_context(errors::ApiErrorResponse::ResourceIdNotFound)
.attach_printable("Active surcharge algorithm ID not found");
return report!(errors::ApiErrorResponse::ResourceIdNotFound)
.attach_printable("Active surcharge algorithm ID not found");

state: SessionState,
merchant_account: domain::MerchantAccount,
key_store: domain::MerchantKeyStore,
auth_profile_id: Option<id_type::ProfileId>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auth_profile_id: Option<id_type::ProfileId>,
auth_profile_id: id_type::ProfileId,

merchant_account: domain::MerchantAccount,
key_store: domain::MerchantKeyStore,
auth_profile_id: Option<id_type::ProfileId>,
algorithm_id: id_type::RoutingId,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
algorithm_id: id_type::RoutingId,
algorithm_id: id_type::SurchargeRoutingId,

active_surcharge_algorithm_id: Some(surcharge_algorithm_id),
};

db.update_profile_by_profile_id(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
db.update_profile_by_profile_id(
let updated_profile = db.update_profile_by_profile_id(

Comment on lines +475 to +478
let updated_profile = db
.find_business_profile_by_profile_id(key_manager_state, &key_store, &profile_id)
.await
.to_not_found_response(errors::ApiErrorResponse::ResourceIdNotFound)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let updated_profile = db
.find_business_profile_by_profile_id(key_manager_state, &key_store, &profile_id)
.await
.to_not_found_response(errors::ApiErrorResponse::ResourceIdNotFound)?;

Comment on lines +484 to +487
let record = db
.find_routing_algorithm_by_profile_id_algorithm_id(&profile_id, &active_algorithm_id)
.await
.to_not_found_response(errors::ApiErrorResponse::ResourceIdNotFound)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should do this validation before updating business_profile.
steps

  1. validate business_profile_id
  2. validate surcharge_routing_id
  3. update business profile with active surcharge id.

@@ -120,6 +120,7 @@ impl RoutingAlgorithmUpdate {
created_at: timestamp,
modified_at: timestamp,
algorithm_for: transaction_type,
algorithm_type: enums::AlgorithmType::Routing,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use const wherever this is hardcoded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature Category: Feature request or enhancement M-api-contract-changes Metadata: This PR involves API contract changes M-database-changes Metadata: This PR involves database schema changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add support for multiple surcharge configurations at profile level
3 participants