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

fix(event): Updated the ApiEventMetric #5126

Merged
merged 13 commits into from
Jul 2, 2024

Conversation

tanbirali
Copy link
Contributor

@tanbirali tanbirali commented Jun 26, 2024

Type of Change

  • Bugfix

Description

Updated the ApiEventMetric trait to return ApiEventsType::Payment { payment_id : String} variant.
Fixes #5059

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I reviewed the submitted code

@tanbirali tanbirali requested a review from a team as a code owner June 26, 2024 10:56
@tanbirali tanbirali changed the title bug(event): Updated the ApiEventMetric fix(event): Updated the ApiEventMetric Jun 26, 2024
@lsampras
Copy link
Contributor

Hey @tanbirali
you'll need to update the ApiEventMetric trait implementation for PaymentsSessionResponse which is defined here

you can refer to this implementation

impl ApiEventMetric for PaymentsCompleteAuthorizeRequest {
fn get_api_event_type(&self) -> Option<ApiEventsType> {
Some(ApiEventsType::Payment {
payment_id: self.payment_id.clone(),
})
}
}

@tanbirali
Copy link
Contributor Author

Let me know if I got this right this time

@lsampras
Copy link
Contributor

lsampras commented Jun 26, 2024

@tanbirali yup,
can you move it to this file crates/api_models/src/events/payment.rs.
and remove the older implementation here

… the crates/api_models/src/events/payment.rs file
… the crates/api_models/src/events/payment.rs file
@tanbirali
Copy link
Contributor Author

Let me know if I need to do anything else

@lsampras
Copy link
Contributor

Hey @tanbirali, you'll need to remove this line I think since it causes duplicate implementations of this trait

PaymentsSessionResponse,

@lsampras
Copy link
Contributor

Can you also add screenshots for local testing?

you can run the data pipeline and this event should show up in the dashboard events & logs section when you make an sessions api call.

…yperswitch into updateApiEventMetric

Merging the remote changes as the local changes are creating conflicts
and not allowing me to push.
@tanbirali
Copy link
Contributor Author

Sorry I can't do the local testing right now I had issues setting it up with docker, hyperswitch server was failing, I had a talk with @SanchithHegde about this, if I can I will add a ss of the local test

lsampras
lsampras previously approved these changes Jul 1, 2024
SanchithHegde
SanchithHegde previously approved these changes Jul 1, 2024
@lsampras
Copy link
Contributor

lsampras commented Jul 1, 2024

@tanbirali Would you be able to fix the CI failures here?

@tanbirali
Copy link
Contributor Author

Let me give it a try

@tanbirali tanbirali dismissed stale reviews from SanchithHegde and lsampras via c146b9b July 1, 2024 09:41
@tanbirali
Copy link
Contributor Author

@lsampras All done I guess

@likhinbopanna likhinbopanna added this pull request to the merge queue Jul 2, 2024
Merged via the queue into juspay:main with commit 1bb2ae8 Jul 2, 2024
11 checks passed
pixincreate added a commit that referenced this pull request Jul 5, 2024
…ify-cypress

* 'main' of github.com:juspay/hyperswitch: (22 commits)
  refactor: Adding millisecond to Kafka timestamp (#5202)
  chore(version): 2024.07.05.0
  fix(user_auth_method): make id option in auth select (#5213)
  Docs: Updated API - ref for payments (#5172)
  feat(core): add merchant order reference id (#5197)
  feat(analytics): Refund status serialization issue for ckh analytics (#5199)
  fix(router): `override setup_future_usage` filed to on_session based on merchant config (#5195)
  feat(cypress): make tests forcefully skippable (#5176)
  feat(core): Added integrity framework for Authorize and Sync flow with connector as Stripe (#5109)
  ci(cypress): Update card number for adyen and status for paypal (#5192)
  refactor(cypress): error handling and add sync refunds in places where missing (#5128)
  feat(analytics): FRM Analytics (#4880)
  chore(version): 2024.07.04.0
  feat(pm_auth): Added balance check for PM auth bank account (#5054)
  refactor(payment_methods): add appropriate missing logs (#5190)
  refactor(migrations): add commands to make file to run migrations for api v2 (#5169)
  chore(version): 2024.07.03.0
  fix(event): Updated the ApiEventMetric (#5126)
  feat(router): add refunds manual-update api (#5094)
  refactor(payment_link): logs payment links logs coverage (#4918)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(events): update the ApiEventsType for PaymentsSessionResponse to use payments flow
4 participants