feat(*): revert support v2 events as well as v3 events (#394)" #395
Conversation
This reverts commit 8aa0af1.
WalkthroughThis pull request standardizes event publishing and payload handling across the codebase. The changes remove variadic syntax in calls to the Publish method and consolidate legacy version-specific event types and payload structures into unified, single-parameter versions. Multiple workflow functions and associated tests have been updated to simplify parameters—including replacing full objects with identifiers and adjusting child workflow execution calls—while error messages and variable names have been refined. No exported public APIs were altered. Changes
Sequence Diagram(s)sequenceDiagram
participant A as Activity/Workflow
participant E as Event Creator
participant P as Publish Method
A->>E: Call NewEventSavedX(...)
E-->>A: Return single event message
A->>P: Publish(event message)
P-->>A: Acknowledgment/Result
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #395 +/- ##
=======================================
Coverage 59.71% 59.71%
=======================================
Files 538 538
Lines 26680 26680
=======================================
Hits 15933 15933
Misses 9907 9907
Partials 840 840 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
internal/events/bank_account.go (3)
18-23: Potential confusion between empty string and truly missing data.
Switching from pointer types to plain strings (withomitempty) causes a nil pointer in the model to become an empty string in the JSON payload. If preserving the distinction between empty and nil is important, consider pointers or alternative solutions. If not, this approach remains valid.
33-33: Function returns only one event but is still named in plural.
RenamingNewEventSavedBankAccountstoNewEventSavedBankAccountwould better reflect that it now returns a singlepublish.EventMessage.
62-62: Variable shadowing inside loop.
Re-declaringrelatedAccountwithin the loop reuses the same name as the loop variable, increasing the risk of confusion. Consider renaming one of them (e.g.,raPayload) to avoid shadowing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
internal/connectors/engine/activities/events_send_account.go(1 hunks)internal/connectors/engine/activities/events_send_balance.go(1 hunks)internal/connectors/engine/activities/events_send_bank_account.go(1 hunks)internal/connectors/engine/activities/events_send_connector_reset.go(1 hunks)internal/connectors/engine/activities/events_send_payment.go(1 hunks)internal/connectors/engine/activities/events_send_payment_initiation.go(1 hunks)internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go(1 hunks)internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go(1 hunks)internal/connectors/engine/activities/events_send_pool_creation.go(1 hunks)internal/connectors/engine/activities/events_send_pool_deletion.go(1 hunks)internal/connectors/engine/engine.go(1 hunks)internal/connectors/engine/workflow/create_bank_account.go(1 hunks)internal/connectors/engine/workflow/create_bank_account_test.go(1 hunks)internal/connectors/engine/workflow/create_payout.go(2 hunks)internal/connectors/engine/workflow/create_payout_test.go(9 hunks)internal/connectors/engine/workflow/create_transfer.go(2 hunks)internal/connectors/engine/workflow/create_transfer_test.go(8 hunks)internal/connectors/engine/workflow/poll_payout.go(2 hunks)internal/connectors/engine/workflow/poll_payout_test.go(12 hunks)internal/connectors/engine/workflow/poll_transfer.go(2 hunks)internal/connectors/engine/workflow/poll_transfer_test.go(12 hunks)internal/connectors/engine/workflow/reverse.go(1 hunks)internal/connectors/engine/workflow/reverse_payout.go(1 hunks)internal/connectors/engine/workflow/reverse_payout_test.go(6 hunks)internal/connectors/engine/workflow/reverse_transfer.go(1 hunks)internal/connectors/engine/workflow/reverse_transfer_test.go(6 hunks)internal/connectors/engine/workflow/send_events.go(3 hunks)internal/connectors/engine/workflow/send_events_test.go(6 hunks)internal/connectors/engine/workflow/utils.go(4 hunks)internal/events/account.go(3 hunks)internal/events/balance.go(3 hunks)internal/events/bank_account.go(3 hunks)internal/events/connector.go(1 hunks)internal/events/events.go(1 hunks)internal/events/payment.go(3 hunks)internal/events/payment_initiation.go(6 hunks)internal/events/pool.go(2 hunks)internal/models/connectors.go(0 hunks)pkg/events/event.go(1 hunks)test/e2e/api_accounts_test.go(2 hunks)test/e2e/api_bank_accounts_test.go(2 hunks)test/e2e/api_payment_initiations_test.go(5 hunks)test/e2e/api_payments_test.go(4 hunks)test/e2e/api_pools_test.go(9 hunks)
💤 Files with no reviewable changes (1)
- internal/models/connectors.go
🧰 Additional context used
🧬 Code Definitions (24)
internal/events/events.go (1)
pkg/events/event.go (1)
TopicPayments(4-4)
test/e2e/api_pools_test.go (2)
pkg/events/event.go (3)
EventTypeSavedPool(9-9)EventTypeDeletePool(10-10)EventTypeSavedAccounts(12-12)pkg/testserver/api.go (2)
RemovePool(95-97)RemovePoolAccount(103-105)
test/e2e/api_payments_test.go (2)
pkg/testserver/matchers.go (1)
Event(142-147)pkg/events/event.go (2)
EventTypeSavedPayments(11-11)EventTypeSavedAccounts(12-12)
internal/connectors/engine/engine.go (2)
internal/models/payment_initiations.go (1)
PaymentInitiation(35-66)internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)
internal/connectors/engine/workflow/reverse_transfer_test.go (5)
internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)internal/connectors/engine/workflow/main_test.go (1)
UnitTestSuite(20-48)internal/connectors/engine/workflow/send_events.go (1)
RunSendEvents(210-210)internal/models/plugin.go (1)
ReverseTransferResponse(162-164)internal/connectors/engine/workflow/reverse_transfer.go (2)
RunReverseTransfer(156-156)ReverseTransfer(9-13)
internal/connectors/engine/workflow/send_events_test.go (5)
internal/connectors/engine/activities/events_send_pool_deletion.go (1)
EventsSendPoolDeletionActivity(14-14)internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go (1)
EventsSendPaymentInitiationAdjustmentActivity(14-14)internal/connectors/engine/workflow/send_events.go (2)
RunSendEvents(210-210)SendEvents(15-26)internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go (1)
EventsSendPaymentInitiationRelatedPaymentActivity(14-14)
internal/connectors/engine/workflow/reverse.go (1)
internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)
internal/connectors/engine/workflow/send_events.go (4)
internal/models/payment_initiations.go (1)
PaymentInitiation(35-66)internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go (1)
EventsSendPaymentInitiationAdjustment(16-18)internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go (1)
EventsSendPaymentInitiationRelatedPayment(16-18)
test/e2e/api_payment_initiations_test.go (2)
pkg/testserver/matchers.go (2)
Event(142-147)WithPayloadSubset(92-97)pkg/events/event.go (4)
EventTypeSavedPaymentInitiation(16-16)EventTypeSavedPaymentInitiationAdjustment(17-17)EventTypeSavedPayments(11-11)EventTypeSavedPaymentInitiationRelatedPayment(18-18)
internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go (2)
internal/connectors/engine/activities/activity.go (1)
Activities(17-26)internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)
internal/events/balance.go (2)
internal/events/events.go (1)
Events(11-15)pkg/events/event.go (2)
EventVersion(6-6)EventTypeSavedBalances(13-13)
internal/connectors/engine/workflow/create_bank_account_test.go (3)
internal/connectors/engine/workflow/main_test.go (1)
UnitTestSuite(20-48)internal/models/plugin.go (1)
CreateBankAccountResponse(116-118)internal/connectors/engine/workflow/create_bank_account.go (1)
CreateBankAccount(11-15)
test/e2e/api_bank_accounts_test.go (3)
pkg/testserver/matchers.go (2)
Event(142-147)WithPayload(84-89)pkg/events/event.go (1)
EventTypeSavedBankAccount(14-14)internal/events/bank_account.go (2)
BankAccountMessagePayload(11-24)BankAccountRelatedAccountsPayload(26-31)
internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go (2)
internal/connectors/engine/activities/activity.go (1)
Activities(17-26)internal/models/payment_initation_related_payments.go (1)
PaymentInitiationRelatedPayments(3-9)
internal/connectors/engine/workflow/reverse_payout_test.go (5)
internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)internal/connectors/engine/workflow/main_test.go (1)
UnitTestSuite(20-48)internal/connectors/engine/workflow/send_events.go (1)
RunSendEvents(210-210)internal/models/plugin.go (1)
ReversePayoutResponse(197-199)internal/connectors/engine/workflow/reverse_payout.go (2)
RunReversePayout(156-156)ReversePayout(9-13)
internal/events/payment.go (3)
internal/events/events.go (1)
Events(11-15)internal/models/payments.go (1)
Payment(57-97)pkg/events/event.go (2)
EventVersion(6-6)EventTypeSavedPayments(11-11)
test/e2e/api_accounts_test.go (3)
pkg/testserver/matchers.go (2)
Event(142-147)WithCallback(45-50)pkg/events/event.go (2)
EventTypeSavedAccounts(12-12)EventTypeSavedBalances(13-13)internal/events/balance.go (1)
BalanceMessagePayload(12-20)
internal/connectors/engine/workflow/create_transfer_test.go (5)
internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)internal/connectors/engine/workflow/main_test.go (1)
UnitTestSuite(20-48)internal/connectors/engine/workflow/send_events.go (1)
RunSendEvents(210-210)internal/models/plugin.go (1)
CreateTransferResponse(150-157)internal/connectors/engine/workflow/create_transfer.go (2)
RunCreateTransfer(221-221)CreateTransfer(15-19)
internal/connectors/engine/workflow/utils.go (2)
internal/models/payment_initation_related_payments.go (1)
PaymentInitiationRelatedPayments(3-9)internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)
internal/events/bank_account.go (3)
internal/events/events.go (1)
Events(11-15)internal/models/bank_accounts.go (1)
BankAccount(26-39)pkg/events/event.go (2)
EventVersion(6-6)EventTypeSavedBankAccount(14-14)
internal/events/account.go (3)
internal/events/events.go (1)
Events(11-15)internal/models/accounts.go (1)
Account(29-55)pkg/events/event.go (2)
EventVersion(6-6)EventTypeSavedAccounts(12-12)
internal/events/pool.go (3)
internal/events/events.go (1)
Events(11-15)internal/models/pools.go (1)
Pool(10-16)pkg/events/event.go (4)
EventVersion(6-6)EventTypeSavedPool(9-9)EventApp(7-7)EventTypeDeletePool(10-10)
internal/events/connector.go (2)
internal/events/events.go (1)
Events(11-15)pkg/events/event.go (3)
EventApp(7-7)EventVersion(6-6)EventTypeConnectorReset(15-15)
internal/events/payment_initiation.go (5)
internal/events/events.go (1)
Events(11-15)internal/models/payment_initiations.go (1)
PaymentInitiation(35-66)pkg/events/event.go (4)
EventVersion(6-6)EventTypeSavedPaymentInitiation(16-16)EventTypeSavedPaymentInitiationAdjustment(17-17)EventTypeSavedPaymentInitiationRelatedPayment(18-18)internal/models/payment_initiation_adjustments.go (1)
PaymentInitiationAdjustment(12-28)internal/models/payment_initation_related_payments.go (1)
PaymentInitiationRelatedPayments(3-9)
🪛 GitHub Check: codecov/patch
internal/connectors/engine/activities/events_send_balance.go
[warning] 11-12: internal/connectors/engine/activities/events_send_balance.go#L11-L12
Added lines #L11 - L12 were not covered by tests
internal/connectors/engine/activities/events_send_payment_initiation.go
[warning] 11-11: internal/connectors/engine/activities/events_send_payment_initiation.go#L11
Added line #L11 was not covered by tests
internal/connectors/engine/activities/events_send_account.go
[warning] 11-11: internal/connectors/engine/activities/events_send_account.go#L11
Added line #L11 was not covered by tests
internal/connectors/engine/activities/events_send_payment.go
[warning] 16-16: internal/connectors/engine/activities/events_send_payment.go#L16
Added line #L16 was not covered by tests
internal/connectors/engine/activities/events_send_connector_reset.go
[warning] 12-12: internal/connectors/engine/activities/events_send_connector_reset.go#L12
Added line #L12 was not covered by tests
internal/connectors/engine/activities/events_send_pool_creation.go
[warning] 11-11: internal/connectors/engine/activities/events_send_pool_creation.go#L11
Added line #L11 was not covered by tests
internal/connectors/engine/engine.go
[warning] 480-481: internal/connectors/engine/engine.go#L480-L481
Added lines #L480 - L481 were not covered by tests
internal/connectors/engine/activities/events_send_bank_account.go
[warning] 14-14: internal/connectors/engine/activities/events_send_bank_account.go#L14
Added line #L14 was not covered by tests
[warning] 16-16: internal/connectors/engine/activities/events_send_bank_account.go#L16
Added line #L16 was not covered by tests
internal/connectors/engine/activities/events_send_pool_deletion.go
[warning] 10-11: internal/connectors/engine/activities/events_send_pool_deletion.go#L10-L11
Added lines #L10 - L11 were not covered by tests
[warning] 16-17: internal/connectors/engine/activities/events_send_pool_deletion.go#L16-L17
Added lines #L16 - L17 were not covered by tests
internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go
[warning] 10-11: internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go#L10-L11
Added lines #L10 - L11 were not covered by tests
[warning] 16-17: internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go#L16-L17
Added lines #L16 - L17 were not covered by tests
internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go
[warning] 10-11: internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go#L10-L11
Added lines #L10 - L11 were not covered by tests
[warning] 16-17: internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go#L16-L17
Added lines #L16 - L17 were not covered by tests
internal/events/bank_account.go
[warning] 35-35: internal/events/bank_account.go#L35
Added line #L35 was not covered by tests
🔇 Additional comments (106)
internal/events/payment.go (3)
14-14: Simplified event payload structureRenaming from
V3PaymentMessagePayloadtopaymentMessagePayloadremoves version-specific naming, aligning with the standardization on v3 events only.
36-37: Simplified event publication signatureFunction now returns a single
publish.EventMessageinstead of a slice, eliminating support for multiple event versions and simplifying the API.
83-85: Standardized event versioning and typeUsing
events.EventVersionandevents.EventTypeSavedPaymentsconstants ensures consistent versioning across the codebase, supporting the migration to v3 events only.internal/events/connector.go (3)
12-15: Simplified connector message payloadRenamed from
V3ConnectorMessagePayloadtoconnectorMessagePayload, removing version-specific naming and streamlining the payload structure.
17-17: Simplified event creation function signatureFunction now returns a single
publish.EventMessageinstead of a slice, eliminating support for multiple event versions and simplifying the API.
22-24: Standardized event versioning and payload constructionUsing
events.EventVersionandevents.EventTypeConnectorResetconstants ensures consistent versioning across the codebase, supporting the migration to v3 events only. The payload construction is now simplified with the consolidated payload type.internal/events/account.go (3)
12-12: Simplified account message payload structureRenaming from
V3AccountMessagePayloadtoAccountMessagePayloadremoves version-specific naming, aligning with the standardization on v3 events only.
28-29: Simplified event creation function signature and implementationFunction now returns a single
publish.EventMessageinstead of a slice, and directly constructs the payload using the consolidatedAccountMessagePayloadtype, eliminating support for multiple event versions.
52-54: Standardized event versioning and typeUsing
events.EventVersionandevents.EventTypeSavedAccountsconstants ensures consistent versioning across the codebase, supporting the migration to v3 events only.internal/connectors/engine/workflow/utils.go (6)
16-20: Simplified function signature using ID instead of full objectChanged parameter type from a pointer to the full
PaymentInitiationobject to just thePaymentInitiationID, which simplifies the function signature and reduces unnecessary data passing.
31-39: Updated related payments storage to use ID directlyNow directly using the
paymentInitiationIDparameter instead of accessing it through thepiobject, which is more direct and eliminates potential null pointer issues.
54-58: Simplified payment initiation related payment structureDirectly initializing the
PaymentInitiationRelatedPaymentstruct with the appropriate ID values, which is cleaner and more straightforward than before.
60-60: Streamlined child workflow executionSimplified by directly calling
Get(ctx, nil)instead of using an intermediate method, reducing unnecessary code complexity.
64-68: Simplified PI adjustment ID creationNow directly using the
paymentInitiationIDparameter when creating a newPaymentInitiationAdjustmentID, which follows the pattern of using IDs rather than full objects.
119-122: Simplified event sending for payment initiation adjustmentStreamlined the
SendEventsstructure initialization and workflow execution, making the code more readable and consistent with the pattern of simplification throughout the file.internal/events/bank_account.go (5)
11-11: Use of a unified naming convention for bank account payload is clean.
This rename fromV3BankAccountMessagePayloadtoBankAccountMessagePayloadsuccinctly indicates the intended usage without cluttering the code with version references.
26-26: Renamed struct for consistency.
BankAccountRelatedAccountsPayloadaligns well with the shift away from versioned naming.
35-35: Uncovered error branch in tests.
Line 35 returns an emptypublish.EventMessagealongside an error, and static analysis indicates it's not covered by tests. Consider adding a negative test case that triggers an error inbankAccount.Obfuscate()to validate this branch.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 35-35: internal/events/bank_account.go#L35
Added line #L35 was not covered by tests
38-38: Constructing the unified bank account payload appears correct.
No issues found; the field assignments align with the structure and naming conventions.
76-79: Creation of the single event message is aligned with unified versioning.
Returning a singlepublish.EventMessagewithEventVersionandEventTypeSavedBankAccountis consistent with the removal of v2 events.pkg/events/event.go (2)
6-7: Single version and application constants simplify event handling.
DeclaringEventVersion = "v3"andEventApp = "payments"clarifies the default version and app references across the code.
9-18: Unified event type constants look consistent.
Removing versioned prefixes aligns with the decision to drop v2 events, preventing confusion and reducing complexity in referencing event types.internal/events/payment_initiation.go (6)
13-13: Renaming the paymentInitiationMessagePayload ensures clarity.
Replacing the old version-based name (V3PaymentInitiationMessagePayload) goes hand in hand with a single-version strategy.
32-32: Adjustment message payload rename is consistent.
DroppingV3fromV3PaymentInitiationAdjustmentMessagePayloadhelps maintain the new unified naming convention.
45-45: New name clarifies related payment message payload.
Transitioning from a versioned payload name topaymentInitiationRelatedPaymentMessagePayloadmaintains alignment with the updated schema.
50-51: Returning a single event for PaymentInitiation is consistent with the new approach.
The function no longer produces multiple messages—this matches the simplified logic for event creation in a single step.
77-78: Adjustment event creation streamlined.
Providing a singlepublish.EventMessagefor adjustments aligns with the unified approach, removing legacy version logic.
104-105: Related payment event updated properly.
This shift to a single event version and type for related payments ensures consistency with other event creation functions.internal/connectors/engine/activities/events_send_account.go (1)
11-11:❓ Verification inconclusive
Code change aligns with the PR objective to standardize on v3 events
The modification to remove the variadic syntax in the
Publishmethod call matches the PR objective of removing support for v2 events and standardizing on v3 events. This simplifies the event publishing structure.However, the static analysis indicates this line isn't covered by tests. Consider verifying test coverage for this event publishing functionality, especially since this is a breaking change in how events are handled.
🏁 Script executed:
#!/bin/bash # Checking test coverage for the events_send_account functionality echo "Searching for test files related to account events..." fd -e go -g "*test*" events_send_account echo "\nChecking test coverage for Events package..." fd -e go -g "*test*" events | xargs grep -l "NewEventSavedAccounts" || echo "No direct tests found for NewEventSavedAccounts" echo "\nChecking test coverage for Publish method..." fd -e go -g "*test*" events | xargs grep -l "Publish" || echo "No direct tests found for Publish method"Length of output: 978
Action: Verify Test Coverage for Event Publishing
The code change properly removes the variadic syntax in the
Publishcall, aligning with the goal of standardizing on v3 events. However, static analysis suggests that the functionality at:
- File:
internal/connectors/engine/activities/events_send_account.go(line 11)...is not covered by tests. The initial search for tests related to:
- The account events functionality (
events_send_account)- The
NewEventSavedAccountsevent constructor under the Events package- The
Publishmethoddid not return any results. This absence may be due to the tests being located in different directories, or it could indicate that tests are indeed missing from these areas.
Request:
Please verify whether there is test coverage elsewhere for this event publishing functionality. If not, consider adding tests to ensure that the refactored event handling behaves as expected.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 11-11: internal/connectors/engine/activities/events_send_account.go#L11
Added line #L11 was not covered by testsinternal/connectors/engine/activities/events_send_pool_creation.go (1)
11-11: Updated event publishing to use non-variadic approachThe call to
a.events.Publishhas been changed to pass a single event message instead of using variadic syntax. This aligns with the PR objective to standardize event publishing across the codebase by simplifying the parameters.The static analysis shows this line is not covered by tests. Consider adding unit tests for this function to ensure proper event publishing behavior.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 11-11: internal/connectors/engine/activities/events_send_pool_creation.go#L11
Added line #L11 was not covered by testsinternal/connectors/engine/activities/events_send_bank_account.go (3)
12-12: Simplified variable naming frombastobaThe variable has been renamed from what was likely
bas(suggesting a plural form for bank accounts) toba(singular), which better represents the single event message now being returned byNewEventSavedBankAccounts.
14-14: Updated error message to reflect singular event handlingThe error message has been changed from referring to "bank account events" (plural) to "bank account" (singular), which is consistent with the move away from handling multiple events in a single call.
This line lacks test coverage according to static analysis. Consider adding error case tests to validate error handling behavior.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 14-14: internal/connectors/engine/activities/events_send_bank_account.go#L14
Added line #L14 was not covered by tests
16-16: Updated event publishing to use non-variadic approachThe call to
a.events.Publishnow passes a single event message instead of using variadic syntax withbas..., aligning with the standardized event publishing approach implemented in this PR.This line lacks test coverage according to static analysis. Consider adding unit tests to ensure proper event publishing behavior.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-16: internal/connectors/engine/activities/events_send_bank_account.go#L16
Added line #L16 was not covered by testsinternal/connectors/engine/activities/events_send_payment.go (1)
16-16: Updated event publishing to use non-variadic approachThe call to
a.events.Publishnow passes a single event message directly fromNewEventSavedPaymentsinstead of expanding it with variadic syntax. This is consistent with the standardization of the event publishing mechanism across the codebase.This line lacks test coverage according to static analysis. Consider adding unit tests to ensure proper event publishing behavior for payment events.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-16: internal/connectors/engine/activities/events_send_payment.go#L16
Added line #L16 was not covered by testsinternal/events/events.go (1)
24-27: Simplified the Publish method to handle a single event messageThe
Publishmethod signature has been changed from accepting variadic arguments (...publish.EventMessage) to a single event message parameter. This simplifies the event publishing logic and is part of the PR's objective to standardize event handling across the codebase.The implementation has been streamlined to directly publish the provided message to the
TopicPaymentstopic without needing to iterate through multiple messages.This is a significant change that affects all event publishing throughout the application. Ensure comprehensive integration tests are in place to verify that all event publishing continues to work correctly with this new signature.
test/e2e/api_payments_test.go (4)
95-95: Standardized event type in test assertions.The event type constant has been updated from version-specific to a unified event type, aligning with the PR goal of transitioning away from v2 events support.
149-149: Standardized event type in test assertions.The event type constant has been updated from version-specific to a unified event type, consistent with other changes in this PR.
183-183: Standardized event type for account events.The account event type constant has been updated from version-specific to a unified event type, providing consistency across the test suite.
196-196: Standardized event type in test assertions.The event type constant has been updated from version-specific to a unified event type, completing the standardization across all test cases.
internal/connectors/engine/workflow/create_bank_account.go (1)
112-112: Simplified child workflow execution call.The code now directly calls
Get(ctx, nil)on the child workflow execution instead of the more verboseGetChildWorkflowExecution().Get(ctx, nil). This maintains the same functionality while making the code more concise.internal/connectors/engine/workflow/poll_transfer.go (2)
11-17: Replaced object reference with ID in struct definition.Changed the
PaymentInitiationfield (which was a full object) toPaymentInitiationID(just the ID). This is a better approach that reduces memory usage and simplifies the data structure.
73-73: Updated function call to use ID instead of full object.The call to
storePIPaymentWithStatusnow correctly uses the payment initiation ID instead of the full object, consistent with the struct field changes.internal/connectors/engine/workflow/create_transfer_test.go (8)
31-32: Updated field name in test assertion.The field name has been updated from
SendEventPaymentInitiationAdjustmenttoPaymentInitiationAdjustment, consistent with the event structure changes in this PR.
52-60: Updated field name in test assertion.The field name has been updated from
SendEventPaymentInitiationRelatedPaymenttoPaymentInitiationRelatedPayment, consistent with the event structure changes.
72-73: Updated field name in test assertion.Field name has been standardized to match other changes in this PR.
108-109: Updated field name in test assertion.Field name has been standardized to match other changes in this PR.
124-125: Updated field name in test assertion.Field name has been standardized to match other changes in this PR.
164-165: Updated field name in test assertion.Field name has been standardized to match other changes in this PR.
198-199: Updated field name in test assertion.Field name has been standardized to match other changes in this PR.
375-404: Added test case for RunSendEvents error scenario.Good addition of a test case that verifies proper error handling when the RunSendEvents workflow fails. This improves test coverage by ensuring error scenarios are properly handled.
internal/connectors/engine/workflow/reverse_transfer.go (1)
96-103: Clean parameter simplification.Good improvement to pass only the payment initiation ID rather than the entire object to
storePIPaymentWithStatus. This reduces unnecessary data passing and aligns with the broader effort to streamline parameter handling across the codebase.internal/connectors/engine/workflow/create_payout_test.go (2)
31-31: Clean field name standardization.Good refactoring of the test assertions to use the new direct field names (
PaymentInitiationAdjustmentandPaymentInitiationRelatedPayment) instead of the previous nested structure. This change is consistent with the broader effort to simplify the event payload structure throughout the codebase.Also applies to: 53-53, 72-72, 107-107, 122-122, 144-144, 163-163
374-403: Good addition of error case test coverage.The new test method properly tests the error handling path when
RunSendEventsreturns a non-retryable application error. This improves the test coverage for error scenarios and ensures that tasks are properly marked as failed when events cannot be sent.internal/connectors/engine/workflow/reverse.go (1)
97-99: Improved child workflow execution pattern.Good simplification of the SendEvents structure and child workflow execution pattern:
- Directly using
PaymentInitiationAdjustmentinstead of a nested structure- Streamlining the workflow execution call by using
Get(ctx, nil)directlyThis change is consistent with the overall effort to standardize event structure and simplify workflow execution patterns throughout the codebase.
internal/connectors/engine/workflow/reverse_payout.go (1)
99-99: Good refactoring: Passing ID instead of full objectThis change follows the pattern of passing the payment initiation ID rather than the full payment initiation object to
storePIPaymentWithStatus. This simplifies the method signature and improves code maintainability.internal/connectors/engine/workflow/create_transfer.go (2)
122-122: Good refactoring: Passing ID instead of full objectThe code now passes
createTransfer.PaymentInitiationIDinstead of the fullpiobject tostorePIPaymentWithStatus. This matches similar changes across the codebase and reduces unnecessary parameter passing.
167-171: Good refactoring: Simplified struct initializationThe PollTransfer struct initialization now directly uses fields from createTransfer rather than relying on the
piobject. This is more straightforward and creates consistency with other parts of the codebase.test/e2e/api_payment_initiations_test.go (6)
83-83: Good standardization: Using unified event typesReplaced version-specific event type with a unified
EventTypeSavedPaymentInitiationtype, aligning with the PR objective of removing v2 event support and standardizing on v3.
89-89: Good standardization: Using unified event typesReplaced version-specific event type with a unified
EventTypeSavedPaymentInitiationAdjustmenttype, maintaining consistency with the PR's goal.
120-126: Good standardization: Using unified event types for multiple assertionsSuccessfully updated multiple assertions to use unified event types instead of version-specific ones, which streamlines the test code and supports the transition to v3 events only.
191-191: Good standardization: Using unified event typeReplaced version-specific event type with a unified
EventTypeSavedPaymentstype, providing consistent event handling throughout tests.
284-284: Good standardization: Using unified event typeReplaced version-specific event type with a unified
EventTypeSavedPaymentstype in the payout test section, maintaining consistency with other test changes.
349-349: Good standardization: Using unified event typeReplaced version-specific event type with a unified
EventTypeSavedPaymentstype in the payout reversal test, completing the standardization across all test cases.internal/connectors/engine/workflow/create_payout.go (2)
121-121: Good refactoring: Passing ID instead of full objectChanged to pass
createPayout.PaymentInitiationIDinstead of the fullpiobject tostorePIPaymentWithStatus, which aligns with the changes made in other workflow files.
165-170: Good refactoring: Simplified struct initializationUpdated PollPayout struct initialization to directly use fields from createPayout object, which creates a more maintainable and consistent codebase.
internal/connectors/engine/workflow/poll_payout.go (2)
11-17: Approve transition from using full object to ID.The change in the
PollPayoutstruct from storing a fullPaymentInitiationobject to just storing thePaymentInitiationIDis a good architectural improvement. This approach reduces memory usage and simplifies data passing between functions.
70-77: Refactoring aligns with the ID-based approach.The parameter passed to
storePIPaymentWithStatusnow correctly uses thePaymentInitiationIDfield, consistent with the struct changes. This modification supports the transition from passing full objects to just passing identifiers.10000 test/e2e/api_pools_test.go (1)
71-71: Standardized event types across versions.Event types have been consolidated from version-specific constants (
V2EventTypeSavedPool,V3EventTypeSavedPool, etc.) to unified constants (EventTypeSavedPool,EventTypeDeletePool, etc.). This change aligns with the PR objective to fully transition to v3 events and eliminate support for v2 events, reducing complexity in the event handling system.Also applies to: 106-106, 110-110, 149-149, 184-185, 188-188, 232-232, 238-238, 255-255, 303-303, 309-309, 326-326, 372-372
internal/connectors/engine/workflow/reverse_payout_test.go (2)
60-61: Simplified field names in SendEvents struct.The field names in assertions have been cleaned up by removing the redundant "SendEvent" prefix. This change improves code readability while maintaining the same functionality. The updated field names (
PaymentInitiationAdjustment,PaymentInitiationRelatedPayment) are more concise and consistent with the object types they reference.Also applies to: 93-93, 108-109, 173-174, 196-197
823-903: Added comprehensive error handling test for SendEvents workflow.This new test case properly verifies the error handling path when the
RunSendEventsworkflow fails. The test ensures that tasks are correctly marked as failed when event publishing encounters errors, which improves test coverage for error scenarios.test/e2e/api_accounts_test.go (1)
79-79: Unified event types and message payload structures.The changes standardize event handling by:
- Replacing version-specific event types with unified constants (
EventTypeSavedAccounts,EventTypeSavedBalances)- Using a unified message payload structure (
events.BalanceMessagePayloadinstead ofevents.V3BalanceMessagePayload)These modifications align with the PR goal of standardizing on v3 events and simplify the event handling logic by eliminating the need to differentiate between versions.
Also applies to: 104-104, 106-106, 108-108
internal/connectors/engine/workflow/reverse_transfer_test.go (4)
60-60: Renamed field improves consistency in event handlingField
SendEventPaymentInitiationAdjustmenthas been renamed toPaymentInitiationAdjustment, removing the redundantSendEventprefix. This change makes the field naming more consistent throughout the codebase.
93-93: Renamed field improves consistency in event handlingField
SendEventPaymentInitiationRelatedPaymenthas been renamed toPaymentInitiationRelatedPayment, which follows the same pattern of removing redundant prefixes and standardizing field names.
108-108: Standardized field naming in remainder of test codeAdditional occurrences of renamed field, maintaining consistent naming throughout the test file.
Also applies to: 173-173, 196-196
823-903: Added important test for error handling in the SendEvents workflowThis new test verifies that errors from the
RunSendEventsworkflow are properly propagated and handled in theReverseTransferworkflow. This is important for ensuring robust error handling in the payment processing pipeline, particularly since this PR is simplifying the event publishing mechanism.internal/connectors/engine/workflow/poll_payout_test.go (4)
38-38: Renamed field for consistency across codebaseField
SendEventPaymentInitiationRelatedPaymenthas been renamed toPaymentInitiationRelatedPaymentas part of the standardization effort to remove redundant prefixes from field names.
50-50: Consistent field renaming in assertionUsing the renamed field
PaymentInitiationAdjustmentin assertion, ensuring consistent naming throughout the test.
66-68: Changed parameter type from object to IDChanged
PaymentInitiationtoPaymentInitiationID, indicating a shift from passing complete objects to just using identifiers. This is a good practice that reduces unnecessary data passing and makes the code more maintainable.Also applies to: 88-91, 123-125, 151-153, 183-185, 220-223, 253-256, 289-291, 325-328, 363-366, 399-402
193-224: Added test for handling errors from RunSendEvents workflowThis new test ensures that errors from the
RunSendEventsworkflow are properly handled in thePollPayoutworkflow, completing the error handling test coverage for this component. This is particularly important as the PR is standardizing the event publishing mechanism.test/e2e/api_bank_accounts_test.go (2)
157-165: Standardized event type and payload structureReplaced version-specific event types and payloads (
V2EventTypeSavedBankAccount/V3EventTypeSavedBankAccountand their corresponding payload structures) with a unifiedEventTypeSavedBankAccountandBankAccountMessagePayload. This change aligns with the PR objective to standardize on a single event version, simplifying the codebase.
212-212: Simplified event verificationUpdated event assertion to use the unified event type without specifying payload details. This is consistent with the goal of standardizing on a single event version.
internal/connectors/engine/activities/events_send_pool_deletion.go (2)
10-11: Simplified function signature by removing unnecessary time parameterRemoved the
at time.Timeparameter from theEventsSendPoolDeletionmethod, simplifying the function signature. The method now directly publishes the event using the event factory method without passing the time parameter.The static analysis indicates these lines are not covered by tests. Consider adding test coverage for this function to ensure it works correctly with the simplified signature.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 10-11: internal/connectors/engine/activities/events_send_pool_deletion.go#L10-L11
Added lines #L10 - L11 were not covered by tests
16-17: Updated workflow function to match activity signatureUpdated the
EventsSendPoolDeletionworkflow function to match the simplified activity signature, removing the time parameter. This ensures consistency between the activity and workflow function.The static analysis indicates these lines are not covered by tests. Consider adding test coverage to ensure the workflow correctly calls the activity with the updated signature.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-17: internal/connectors/engine/activities/events_send_pool_deletion.go#L16-L17
Added lines #L16 - L17 were not covered by testsinternal/connectors/engine/workflow/create_bank_account_test.go (2)
167-192: Well-structured test for event handling error pathThis new test properly verifies workflow error handling when the runSendEvents workflow fails. It follows the same pattern as existing tests and ensures complete test coverage for the error path.
200-201: Updated mocks to align with event publishing changesThese changes correctly update the test mocks to match the modified behavior in StorageBankAccountsAddRelatedAccountActivity and runSendEvents workflow. The test now returns nil and a temporal error respectively, consistent with the new error handling structure.
internal/connectors/engine/workflow/poll_transfer_test.go (4)
38-38: Updated field reference for consistencyThe field name change from
sendEvents.SendEventPaymentInitiationRelatedPaymenttosendEvents.PaymentInitiationRelatedPaymentaligns with the PR objective to standardize event handling across the codebase.
50-50: Updated field reference for consistencyThe field name change from
req.SendEventPaymentInitiationAdjustmenttoreq.PaymentInitiationAdjustmentmaintains consistency with the new standardized event handling structure.
66-68: Standardized PollTransfer struct field namingThe field name change from
PaymentInitiationtoPaymentInitiationIDbetter reflects that this field stores an ID rather than the full payment initiation object, improving code clarity and type safety.Also applies to: 89-91, 123-125, 151-153, 183-185, 220-223, 254-256, 289-291, 326-328, 364-366, 399-402
193-229: Added comprehensive test for RunSendEvents error handlingThis new test properly verifies the error handling path when RunSendEvents workflow fails. It follows the established testing pattern and ensures proper task status updates and error propagation.
internal/connectors/engine/workflow/send_events_test.go (4)
439-439: Simplified activity mock setupRemoved unnecessary
mock.Anythingparameter from EventsSendPoolDeletionActivity mock, aligning with the simplified function signature that now only requires the context and pool ID.Also applies to: 456-456
512-512: Simplified PaymentInitiationAdjustment activity mockRemoved unnecessary parameters from EventsSendPaymentInitiationAdjustmentActivity mock to match the updated function signature that now only accepts the context and adjustment object.
Also applies to: 530-530
548-548: Simplified PaymentInitiationRelatedPayment activity mockRemoved unnecessary parameters from EventsSendPaymentInitiationRelatedPaymentActivity mock to match the updated function signature that now only accepts the context and related payment object.
Also applies to: 566-566
516-516: Standardized SendEvents struct field usageUpdated field references in SendEvents struct to use the simplified direct field names (PaymentInitiationAdjustment, PaymentInitiationRelatedPayment) instead of the previous nested structure, improving code clarity and consistency.
Also applies to: 533-533, 552-552, 569-569
internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go (2)
16-17: Simplified workflow activity function signatureUpdated the workflow activity function signature to match the simplified method signature, maintaining consistency between the two. This change properly propagates the parameter removal throughout the codebase.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-17: internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go#L16-L17
Added lines #L16 - L17 were not covered by tests
10-11:❓ Verification inconclusive
Simplified EventsSendPaymentInitiationAdjustment method signature
Removed the unnecessary
pi models.PaymentInitiationparameter, simplifying the function and its usage. The PaymentInitiationAdjustment object already contains all necessary information for event publishing.While this change appears safe, the static analysis indicates these lines aren't covered by tests. Consider adding direct unit tests for this specific method:
🏁 Script executed:
#!/bin/bash # Check if there are tests that invoke this method either directly or indirectly rg -A 1 "EventsSendPaymentInitiationAdjustment" --type goLength of output: 2201
Simplified Signature Approved; Consider Adding Direct Unit Tests
The refactoring to remove the extra
pi models.PaymentInitiationparameter is clear and simplifies the function by relying solely on thePaymentInitiationAdjustmentobject for publishing events. We noted that while the method is indirectly exercised via workflow tests (e.g., ininternal/connectors/engine/workflow/send_events_test.go), there isn’t a direct unit test specifically targetingEventsSendPaymentInitiationAdjustment. For improved isolation and confidence in this functionality, please consider adding direct unit tests that verify:
- The correct invocation of
a.events.Publishwith the expected parameters when given a validPaymentInitiationAdjustment.- The behavior under failure conditions if applicable.
Keep an eye on ensuring that these tests adequately cover the method’s behavior in isolation, even if the current invocation paths (via workflow tests) provide some level of integration coverage.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 10-11: internal/connectors/engine/activities/events_send_payment_initiation_adjustment.go#L10-L11
Added lines #L10 - L11 were not covered by testsinternal/events/balance.go (2)
12-20: Clean consolidation of V2/V3 payload typesThe renaming from
V3BalanceMessagePayloadtoBalanceMessagePayloadsimplifies the codebase by removing version-specific naming, aligning with the PR objective to standardize on v3 events only.
22-41: Simplified event publishing patternThe function signature change from returning
[]publish.EventMessageto a singlepublish.EventMessagestreamlines the event creation process. The code correctly uses the constants from the events package for version and type fields, ensuring consistency across the codebase.internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go (2)
16-18:✅ Verification successful
Consistent parameter updates in workflow function
The signature update in the workflow function matches the implementation changes in the Activities struct method, ensuring consistency throughout the event sending process.
🏁 Script executed:
#!/bin/bash # Check if there are any calls to this function that might need updating rg -p "EventsSendPaymentInitiationRelatedPayment\(" --type go | grep -v "activities.go"Length of output: 583
Verified: Consistent Parameter Updates Across Workflow & Activities
The updated signature for
EventsSendPaymentInitiationRelatedPaymentis consistent. The function call ininternal/connectors/engine/workflow/send_events.go(line 196) correctly uses the updated parameters, and the implementation ininternal/connectors/engine/activities/events_send_payment_initiation_related_payments.goconfirms this consistency.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-17: internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go#L16-L17
Added lines #L16 - L17 were not covered by tests
10-12:❓ Verification inconclusive
Simplified function signature improves code clarity
The function has been updated to accept only the essential parameter
relatedPayment, removing unnecessary parameters. This change aligns with the PR objective of standardizing event handling.
🏁 Script executed:
#!/bin/bash # Check if there are any tests covering this functionality rg -p "TestEventsSendPaymentInitiationRelatedPayment" --type goLength of output: 63
Simplified function signature verified – please double-check test coverage
The changes simplify the function by accepting only the essential parameter (
relatedPayment), which aligns with standardizing event handling. However, our automated search didn’t locate any tests covering this functionality. Please manually verify that the behavior is adequately tested.
- File: internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go (Lines 10–12)
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 10-11: internal/connectors/engine/activities/events_send_payment_initiation_related_payments.go#L10-L11
Added lines #L10 - L11 were not covered by testsinternal/connectors/engine/workflow/send_events.go (3)
15-26: Simplified struct removes redundant type definitionsThe
SendEventsstruct has been streamlined by directly including thePaymentInitiationAdjustmentfield and removing unnecessary nested type definitions. This simplification improves code readability and maintainability.
173-188: Consistent event handling for payment initiation adjustmentsThe event sending logic has been updated to work directly with the
PaymentInitiationAdjustmentfield. The code correctly accesses the necessary fields for the idempotency key and connector ID.
190-205: Simplified related payment handling maintains functionalityThe event sending logic for payment initiation related payments has been updated to match the simplified struct structure. The changes preserve the existing functionality while making the code more consistent with the overall event handling pattern.
internal/events/pool.go (4)
12-17: Clean consolidation of pool message payload typesThe introduction of a single
poolMessagePayloadtype replacing version-specific payloads (V2PoolMessagePayloadandV3PoolMessagePayload) simplifies the codebase and aligns with the PR objective to standardize on v3 events.
19-39: Simplified pool event creationThe
NewEventSavedPoolfunction now returns a singlepublish.EventMessageinstead of a slice, streamlining the event creation process. The code correctly uses constants from the events package for version and type fields, maintaining consistency with other event types.
41-44: Consolidated delete pool message payloadThe new
deletePoolMessagePayloadtype replaces version-specific payloads, simplifying the code structure and improving maintainability.
46-58:❓ Verification inconclusive
Simplified delete pool event creation
The
NewEventDeletePoolfunction has been updated to match the pattern used for other event types. The function now generates the timestamp internally rather than accepting it as a parameter, which simplifies the API but may reduce flexibility.
🏁 Script executed:
#!/bin/bash # Check if there are any callers that pass a specific timestamp that might be affected rg -p "NewEventDeletePool\(.*," --type goLength of output: 43
I'll run a broader search for any references to
NewEventDeletePoolin our codebase to see if any caller is passing additional parameters (like a custom timestamp) that might be affected by this change.
🏁 Script executed:
#!/bin/bash # Search for all instances where NewEventDeletePool is called in the codebase. rg "NewEventDeletePool(" --type goLength of output: 125
Delete Pool Event Creation Simplified
The updatedNewEventDeletePoolfunction now generates its own timestamps viatime.Now().UTC(), aligning with the pattern used in other event types. Initial searches for callers supplying a custom timestamp didn’t return any results, but please manually verify in the broader codebase to ensure no edge cases rely on an externally provided timestamp.
| @@ -9,7 +9,7 @@ | |||
| ) | |||
|
|
|||
| func (a Activities) EventsSendConnectorReset(ctx context.Context, connectorID models.ConnectorID, at time.Time) error { | |||
| return a.events.Publish(ctx, a.events.NewEventResetConnector(connectorID, at)...) | |||
| return a.events.Publish(ctx, a.events.NewEventResetConnector(connectorID, at)) | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Standardized event publishing pattern for v3 events
The removal of the variadic syntax in this event publishing call is consistent with the changes in other files and aligns with the PR objective to standardize on v3 events.
Similar to the other files, static analysis shows this line isn't covered by tests. Consider adding or extending tests to verify this functionality works as expected after removing v2 event support.
🏁 Script executed:
#!/bin/bash
# Checking test coverage for connector reset events
echo "Searching for test files related to connector reset events..."
fd -e go -g "*test*" events_send_connector_reset
echo "\nChecking test coverage for NewEventResetConnector..."
fd -e go -g "*test*" events | xargs grep -l "NewEventResetConnector" || echo "No direct tests found for NewEventResetConnector"Length of output: 687
Standardized V3 Event Publishing – Test Coverage Needed
The updated event publishing in
internal/connectors/engine/activities/events_send_connector_reset.go (line 12):
return a.events.Publish(ctx, a.events.NewEventResetConnector(connectorID, at))now aligns with the PR objective to standardize on v3 events. However, our investigation using static analysis indicates that there aren’t any direct tests covering the functionality of NewEventResetConnector (as confirmed by the errors when searching for related test files). To ensure this change behaves as expected after dropping v2 event support, please add or extend tests to cover this event publishing path—similar to what’s done (or needed) in other parts of the codebase.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-12: internal/connectors/engine/activities/events_send_connector_reset.go#L12
Added line #L12 was not covered by tests
| @@ -8,8 +8,8 @@ | |||
| ) | |||
|
|
|||
| func (a Activities) EventsSendBalance(ctx context.Context, balance models.Balance) error { | |||
| return a.events.Publish(ctx, a.events.NewEventSavedBalances(balance)...) | |||
| return a.events.Publish(ctx, a.events.NewEventSavedBalances(balance)) | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consistent event publishing pattern for balance events
This change follows the same pattern as other event publishing modifications, removing the variadic syntax to align with the v3 events standardization goal.
The static analysis indicates this line isn't covered by tests. Consider ensuring test coverage for the updated event publishing functionality, particularly since this represents a fundamental change in how events are structured.
🏁 Script executed:
#!/bin/bash
# Checking test coverage for balance events
echo "Searching for test files related to balance events..."
fd -e go -g "*test*" events_send_balance
echo "\nChecking test coverage for NewEventSavedBalances..."
fd -e go -g "*test*" events | xargs grep -l "NewEventSavedBalances" || echo "No direct tests found for NewEventSavedBalances"
# Check for integration tests that might cover this functionality
echo "\nChecking for integration tests that might cover balance events..."
fd -e go -g "*integration*test*" | xargs grep -l "Balance" || echo "No integration tests found for balance events"Length of output: 955
Action Required: Enhance Test Coverage for the Updated Events Publishing Functionality
The updated event publishing code follows the v3 events standardization, aligning with the approach used in other parts of the code. However, our analysis indicates that there are currently no tests covering the newly implemented NewEventSavedBalances functionality. This is concerning given that it represents a fundamental change in event structure. Please add appropriate unit and/or integration tests to verify that the balance events are published and structured correctly.
- File to review:
internal/connectors/engine/activities/events_ 8000 send_balance.go - Action: Add test cases that target the
NewEventSavedBalancesevent publishing path. Consider both direct unit tests and integration tests to ensure comprehensive coverage.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 11-12: internal/connectors/engine/activities/events_send_balance.go#L11-L12
Added lines #L11 - L12 were not covered by tests
| @@ -8,7 +8,7 @@ | |||
| ) | |||
|
|
|||
| func (a Activities) EventsSendPaymentInitiation(ctx context.Context, pi models.PaymentInitiation) error { | |||
| return a.events.Publish(ctx, a.events.NewEventSavedPaymentInitiation(pi)...) | |||
| return a.events.Publish(ctx, a.events.NewEventSavedPaymentInitiation(pi)) | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Standardized event handling for payment initiation events
This modification removes the variadic syntax from the Publish method call, consistent with the other event publishing changes and the PR objective to standardize on v3 events.
As with the other files, static analysis indicates this line isn't covered by tests. Consider adding test coverage to verify this critical functionality works correctly after the event structure changes.
🏁 Script executed:
#!/bin/bash
# Checking test coverage for payment initiation events
echo "Searching for test files related to payment initiation events..."
fd -e go -g "*test*" events_send_payment_initiation
echo "\nChecking test coverage for NewEventSavedPaymentInitiation..."
fd -e go -g "*test*" events | xargs grep -l "NewEventSavedPaymentInitiation" || echo "No direct tests found for NewEventSavedPaymentInitiation"
# Check if there are broader integration tests covering event publishing
echo "\nChecking integration tests that might cover payment events..."
fd -e go -g "*integration*test*" | xargs grep -l "Payment" || echo "No integration tests found for payment events"Length of output: 1036
🏁 Script executed:
#!/bin/bash
# Checking for changes in the event creation functions
echo "Looking at implementation of event creation functions that are being called..."
rg -A 5 -B 5 "func .* NewEventSavedPaymentInitiation" || echo "Function definition not found"
rg -A 5 -B 5 "func .* NewEventSavedBalances" || echo "Function definition not found"
rg -A 5 -B 5 "func .* NewEventSavedAccounts" || echo "Function definition not found"
rg -A 5 -B 5 "func .* NewEventResetConnector" || echo "Function definition not found"Length of output: 5292
Standardized event handling for payment initiation events – please add tests
This change correctly updates the call to Publish by removing the variadic syntax, aligning it with the standardized v3 events approach. However, static analysis (and our test search efforts) indicate that there is currently no direct unit or integration test covering the NewEventSavedPaymentInitiation functionality. Please consider adding appropriate tests to ensure that any impacts from the event structure changes are properly verified.
- Location:
internal/connectors/engine/activities/events_send_payment_initiation.go(line 11) - Affected Function:
NewEventSavedPaymentInitiationininternal/events/payment_initiation.go - Action: Add or enhance test coverage (unit/integration) for this event publishing functionality
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 11-11: internal/connectors/engine/activities/events_send_payment_initiation.go#L11
Added line #L11 was not covered by tests
| PaymentInitiation: &pi, | ||
| PaymentInitiationAdjustment: &adj, | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Simplified event payload structure.
Good refactoring to flatten the SendEvents structure by directly including the PaymentInitiation and PaymentInitiationAdjustment fields rather than using a nested approach. This makes the code more readable and consistent with the standardized event structure used throughout the codebase.
Static analysis indicates these lines lack test coverage. Consider adding a test case for CreateFormancePaymentInitiation that verifies events are properly structured:
🏁 Script executed:
#!/bin/bash
# Check if there are any tests covering the CreateFormancePaymentInitiation method
echo "Searching for tests that cover CreateFormancePaymentInitiation..."
rg -l "Test.*CreateFormancePaymentInitiation" --type goLength of output: 198
Simplified Event Payload Structure: Approval with Test Coverage Needed
Great job on flattening the SendEvents structure. Directly embedding the PaymentInitiation and PaymentInitiationAdjustment fields improves readability and consistency across the codebase.
However, our search for tests covering the CreateFormancePaymentInitiation method returned no results. To ensure that this refactoring works as expected and to guard against regressions, please add a test case that verifies the event payload—including both PaymentInitiation and PaymentInitiationAdjustment—is structured correctly.
For example, you could start by double-checking the test coverage with:
#!/bin/bash
echo "Searching for tests that cover CreateFormancePaymentInitiation..."
rg -l "Test.*CreateFormancePaymentInitiation" --type goOnce the tests are in place, it will strengthen our confidence in this refactoring.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 480-481: internal/connectors/engine/engine.go#L480-L481
Added lines #L480 - L481 were not covered by tests
This PR reverts commit 8aa0af1.
Atfer discussion, we will completely switch the events to v3 and not handle v2 events also to not induce pressure on the pipes and not adding complexity
the two services impacted will be webhooks and flows:
We're gonna update also the docs to have a full page about migrations to stack v3