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 payment incoming webhooks support for v2 #6551

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

Conversation

sai-harsha-vardhan
Copy link
Contributor

@sai-harsha-vardhan sai-harsha-vardhan commented Nov 13, 2024

Type of Change

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

Description

add payment incoming webhooks support for v2

Additional Changes

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

Motivation and Context

How did you test it?

Tested Manually

  1. Sanity payment webhooks in v1

  2. Incoming payment webhooks (source_verified = true)
    image
    image

  3. Incoming payment webhooks (source_verified = false)
    image
    image

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

Narayanbhat166 and others added 30 commits October 29, 2024 15:18
Copy link

semanticdiff-com bot commented Nov 13, 2024

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/core/payments/operations/payment_create_intent.rs  80% smaller
  crates/router/src/core/payments/operations/payment_get_intent.rs  80% smaller
  crates/router/src/core/payments/operations/payment_get.rs  79% smaller
  crates/router/src/core/payments/operations/payment_confirm_intent.rs  79% smaller
  crates/router/src/lib.rs  66% smaller
  crates/router/src/core/payments/operations.rs  30% smaller
  api-reference-v2/openapi_spec.json  25% smaller
  crates/common_utils/src/types.rs  7% smaller
  crates/router/src/core/payments.rs  6% smaller
  crates/router/src/routes/app.rs  4% smaller
  crates/storage_impl/src/payments/payment_attempt.rs  4% smaller
  crates/api_models/src/payments.rs  1% smaller
  crates/api_models/src/admin.rs  0% smaller
  crates/api_models/src/webhooks.rs  0% smaller
  crates/common_utils/src/consts.rs  0% smaller
  crates/common_utils/src/events.rs  0% smaller
  crates/common_utils/src/id_type/global_id/payment.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/payment_attempt.rs  0% smaller
  crates/hyperswitch_domain_models/src/business_profile.rs  0% smaller
  crates/hyperswitch_domain_models/src/payments.rs  0% smaller
  crates/hyperswitch_domain_models/src/payments/payment_attempt.rs  0% smaller
  crates/hyperswitch_domain_models/src/router_data.rs  0% smaller
  crates/router/src/analytics.rs  0% smaller
  crates/router/src/core.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/transformers.rs  0% smaller
  crates/router/src/core/user.rs  0% smaller
  crates/router/src/core/webhooks.rs  0% smaller
  crates/router/src/core/webhooks/incoming_v2.rs  0% smaller
  crates/router/src/core/webhooks/webhook_events.rs  0% smaller
  crates/router/src/db/kafka_store.rs  0% smaller
  crates/router/src/events/api_logs.rs  0% smaller
  crates/router/src/events/outgoing_webhook_logs.rs  0% smaller
  crates/router/src/routes.rs  0% smaller
  crates/router/src/routes/dummy_connector.rs  0% smaller
  crates/router/src/routes/dummy_connector/core.rs  0% smaller
  crates/router/src/routes/payments.rs  0% smaller
  crates/router/src/routes/user.rs  0% smaller
  crates/router/src/routes/webhook_events.rs  0% smaller
  crates/router/src/routes/webhooks.rs  0% smaller
  crates/router/src/services/api.rs  0% smaller
  crates/router/src/services/authentication.rs  0% smaller
  crates/router/src/types.rs  0% smaller
  crates/router/src/types/api/payments.rs  0% smaller
  crates/storage_impl/src/mock_db/payment_attempt.rs  0% smaller

@hyperswitch-bot hyperswitch-bot bot added M-database-changes Metadata: This PR involves database schema changes M-api-contract-changes Metadata: This PR involves API contract changes labels Nov 13, 2024
@hyperswitch-bot hyperswitch-bot bot removed M-database-changes Metadata: This PR involves database schema changes M-api-contract-changes Metadata: This PR involves API contract changes labels Nov 13, 2024
crates/api_models/src/payments.rs Show resolved Hide resolved
crates/diesel_models/src/payment_attempt.rs Outdated Show resolved Hide resolved
@@ -188,6 +188,33 @@ impl PaymentAttempt {
.await
}

#[cfg(feature = "v2")]
pub async fn find_by_merchant_id_connector_txn_id(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub async fn find_by_merchant_id_connector_txn_id(
pub async fn find_by_merchant_id_connector_transaction_id(

#[cfg(feature = "v2")]
pub async fn find_by_merchant_id_connector_txn_id(
conn: &PgPooledConn,
merchant_id: &common_utils::id_type::MerchantId,
Copy link
Member

Choose a reason for hiding this comment

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

this should be a profile based

@@ -102,6 +102,16 @@ pub trait PaymentAttemptInterface {
storage_scheme: storage_enums::MerchantStorageScheme,
) -> error_stack::Result<PaymentAttempt, errors::StorageError>;

#[cfg(feature = "v2")]
async fn find_payment_attempt_by_merchant_id_connector_txn_id(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
async fn find_payment_attempt_by_merchant_id_connector_txn_id(
async fn find_payment_attempt_by_merchant_id_connector_transaction_id(

}
}

async fn get_payment_id(
Copy link
Member

Choose a reason for hiding this comment

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

return objects from the function, just to get a id we shouldn't do database call, if we do so also, return the object so that, the objects can be used later. change the function name too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are not fetching the object in all the cases, in case of PaymentIdType::PaymentIntentId we are directly returning the id

Copy link
Member

Choose a reason for hiding this comment

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

Can we make this consistent, we should avoid reading same object twice in the flow. Like in redirection, we eliminated double reads

@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Nov 14, 2024
@sai-harsha-vardhan sai-harsha-vardhan changed the base branch from main to add_finish_redirection November 14, 2024 09:26
@sai-harsha-vardhan sai-harsha-vardhan changed the base branch from add_finish_redirection to main November 14, 2024 09:26
} else if connector.is_webhook_source_verification_mandatory() {
// if webhook consumption is mandatory for connector, fail webhook
// so that merchant can retrigger it after updating merchant_secret
return Err(errors::ApiErrorResponse::WebhookAuthenticationFailed.into());
Copy link
Member

Choose a reason for hiding this comment

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

Avoid return from middle of the flow

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

Successfully merging this pull request may close these issues.

4 participants