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(debug): hide dummypay from configs response when not in debug mode #316

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

laouji
Copy link
Contributor

@laouji laouji commented Feb 6, 2025

Fixes: ENG-1587

Copy link
Contributor

coderabbitai bot commented Feb 6, 2025

Walkthrough

The pull request updates numerous service initialization functions across the codebase to include an additional boolean parameter, commonly representing a debug flag. This flag is now passed to various service constructors and test cases, altering the configuration behavior. Additionally, the changes standardize the dummy PSP name by introducing a constant and modifying the configuration retrieval functions to conditionally hide dummy PSP settings when debug mode is off. A new debug option is also added in the test server configuration.

Changes

File(s) or Grouped Files Change Summary
internal/api/module.go, internal/api/services/services.go Updated constructor signatures to include an extra boolean parameter; added a new debug field to the Service struct.
internal/api/services/*_test.go (all test files) Updated service instantiation calls from two to three parameters, adding false for the new debug parameter.
internal/connectors/plugins/public/dummypay/plugin.go Modified plugin registration to use registry.DummyPSPName constant instead of a string literal.
internal/connectors/plugins/registry/plugins.go Introduced DummyPSPName constant and modified GetConfigs to accept a debug parameter; conditional logic added to hide dummy PSP configuration when debug is false.
internal/connectors/plugins/registry/plugins_test.go Added test cases to validate that the dummy PSP configuration is hidden when debug mode is off, and shown when debug mode is on.
test/e2e/api_connectors_test.go Added a Debug boolean field to the test server configuration, enabling the new debug mode for testing.

Sequence Diagram(s)

sequenceDiagram
    participant C as Caller/Test
    participant S as Service
    participant R as Registry

    C->>S: New(store, eng, debug)
    Note right of S: Service initialized with debug flag
    S->>R: ConnectorsConfigs() calls GetConfigs(debug)
    R-->>S: Return Configs (dummy PSP hidden if debug is false)
    S-->>C: Return Configs
Loading

Assessment against linked issues

Objective (ENG-1587) Addressed Explanation
Hide Dummypay configuration from UI connectors/configs request

Poem

I'm a bunny with code so neat,
Hopping through functions on swift feet.
A little debug flag lights my lane,
Hiding dummypay configs with no strain.
Code carrots crunch—oh what a treat! 🐇💻

Tip

🌐 Web search-backed reviews and chat
  • We have enabled web search-based reviews and chat for all users. This feature allows CodeRabbit to access the latest documentation and information on the web.
  • You can disable this feature by setting web_search: false in the knowledge_base settings.
  • Please share any feedback in the Discord discussion.
✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.91%. Comparing base (4c4844e) to head (36dc12e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #316      +/-   ##
==========================================
+ Coverage   72.89%   72.91%   +0.01%     
==========================================
  Files         541      541              
  Lines       27335    27338       +3     
==========================================
+ Hits        19927    19934       +7     
+ Misses       6335     6333       -2     
+ Partials     1073     1071       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@laouji laouji marked this pull request as ready for review February 6, 2025 12:00
@laouji laouji requested a review from a team as a code owner February 6, 2025 12:00
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (8)
internal/api/services/connector_configs_test.go (1)

20-20: Add test case for debug mode.

While the current test verifies basic functionality, consider adding a test case with debug=true to ensure the debug flag correctly affects the configs response.

 func TestConnectorConfigs(t *testing.T) {
 	t.Parallel()
+	testCases := []struct {
+		name     string
+		debug    bool
+		validate func(*testing.T, registry.Configs)
+	}{
+		{
+			name:  "debug mode off",
+			debug: false,
+			validate: func(t *testing.T, configs registry.Configs) {
+				require.NotNil(t, configs)
+				// Add assertions to verify dummypay is not present
+			},
+		},
+		{
+			name:  "debug mode on",
+			debug: true,
+			validate: func(t *testing.T, configs registry.Configs) {
+				require.NotNil(t, configs)
+				// Add assertions to verify dummypay is present
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+			store := storage.NewMockStorage(ctrl)
+			eng := engine.NewMockEngine(ctrl)
+
+			s := New(store, eng, tc.debug)
+			tc.validate(t, s.ConnectorsConfigs())
+		})
+	}
 }
internal/api/services/payments_get_test.go (2)

23-23: Consider creating a test helper for service initialization.

Given the consistent pattern of service initialization across test files, consider creating a test helper function that encapsulates the service creation logic with debug mode parameter. This would make it easier to maintain and modify the initialization behavior across all test files.

+func newTestService(t *testing.T, debugMode bool) *Service {
+	ctrl := gomock.NewController(t)
+	t.Cleanup(func() { ctrl.Finish() })
+	store := storage.NewMockStorage(ctrl)
+	eng := engine.NewMockEngine(ctrl)
+	return New(store, eng, debugMode)
+}

 func TestPaymentGet(t *testing.T) {
 	t.Parallel()
-
-	ctrl := gomock.NewController(t)
-	defer ctrl.Finish()
-	store := storage.NewMockStorage(ctrl)
-	eng := engine.NewMockEngine(ctrl)
-
-	s := New(store, eng, false)
+	s := newTestService(t, false)

23-23: Consider table-driven tests for debug mode configurations.

The changes consistently add a debug flag parameter across all test files, but there's an opportunity to improve the test architecture:

  1. Create a shared test helper for service initialization
  2. Add table-driven tests to verify behavior in both debug and non-debug modes
  3. Consider creating a shared test suite for debug-mode-specific behaviors that can be reused across test files

This would improve maintainability and ensure consistent coverage of debug mode behavior across all service tests.

internal/api/services/accounts_get_test.go (1)

24-24: Add test cases for debug mode.

While setting debug=false aligns with production behavior, consider adding test cases with debug=true to ensure both modes work as expected.

Example test table modification:

 tests := []struct {
     name          string
     accountID     models.AccountID
     err           error
     expectedError error
+    debug         bool
 }{
     {
         name:          "success without debug",
         accountID:     id,
         err:           nil,
         expectedError: nil,
+        debug:         false,
     },
+    {
+        name:          "success with debug",
+        accountID:     id,
+        err:           nil,
+        expectedError: nil,
+        debug:         true,
+    },
internal/api/services/bank_accounts_forward_to_connector_test.go (1)

64-65: Document the debug parameter's purpose.

The debug parameter is being passed through multiple layers. Consider adding a comment explaining its purpose and impact on the ForwardBankAccount operation.

+// debug flag controls whether dummy/test configurations are included in responses
 eng.EXPECT().ForwardBankAccount(gomock.Any(), test.bankAccountID, connectorID, false).Return(models.Task{}, test.err)
internal/api/services/connectors_uninstall_test.go (1)

23-23: Consider parameterizing the debug flag in tests.

The debug flag is hardcoded to false. Consider parameterizing it through test cases to verify behavior in both modes.

 tests := []struct {
     name          string
     engineErr     error
     storageErr    error
     expectedError error
     typedError    bool
+    debug         bool
 }{
     {
         name:          "success",
         engineErr:     nil,
         storageErr:    nil,
         expectedError: nil,
+        debug:         false,
     },
+    {
+        name:          "success with debug mode",
+        engineErr:     nil,
+        storageErr:    nil,
+        expectedError: nil,
+        debug:         true,
+    },
internal/api/services/payment_initiation_adjustments_get_last_test.go (1)

25-25: Add test coverage for debug mode configuration.

The service is initialized with debug=false, but there's no test coverage for debug mode behavior. Consider adding test cases to verify both configurations.

Example test setup:

 s := New(store, eng, false)
+s_debug := New(store, eng, true)
 
 tests := []struct {
     name            string
     storageResponse []models.PaymentInitiationAdjustment
     err             error
     expectedError   error
+    useDebugService bool
 }{
     {
         name:            "success",
         err:             nil,
         storageResponse: []models.PaymentInitiationAdjustment{{}},
         expectedError:   nil,
+        useDebugService: false,
     },
+    {
+        name:            "success with debug mode",
+        err:             nil,
+        storageResponse: []models.PaymentInitiationAdjustment{{}},
+        expectedError:   nil,
+        useDebugService: true,
+    },
internal/connectors/plugins/registry/plugins.go (1)

127-137: LGTM with a documentation suggestion.

The implementation correctly controls the visibility of the dummy PSP based on the debug mode. Consider adding a comment explaining why the dummy PSP should be hidden in non-debug mode for better maintainability.

 func GetConfigs(debug bool) Configs {
 	confs := make(Configs)
 	for key, info := range pluginsRegistry {
-		// hide dummy PSP outside of debug mode
+		// Hide dummy PSP outside of debug mode to prevent its usage in production
+		// as it's intended only for testing and development purposes
 		if !debug && key == DummyPSPName {
 			continue
 		}
 		confs[key] = info.config
 	}
 	return confs
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c4844e and 36dc12e.

📒 Files selected for processing (49)
  • internal/api/module.go (1 hunks)
  • internal/api/services/accounts_create_test.go (1 hunks)
  • internal/api/services/accounts_get_test.go (1 hunks)
  • internal/api/services/accounts_list_test.go (1 hunks)
  • internal/api/services/balances_list_test.go (1 hunks)
  • internal/api/services/bank_accounts_create_test.go (1 hunks)
  • internal/api/services/bank_accounts_forward_to_connector_test.go (1 hunks)
  • internal/api/services/bank_accounts_get_test.go (1 hunks)
  • internal/api/services/bank_accounts_list_test.go (1 hunks)
  • internal/api/services/bank_accounts_update_metadata_test.go (1 hunks)
  • internal/api/services/connector_configs.go (1 hunks)
  • internal/api/services/connector_configs_test.go (1 hunks)
  • internal/api/services/connectors_config_update_test.go (1 hunks)
  • internal/api/services/connectors_handle_webhooks_test.go (1 hunks)
  • internal/api/services/connectors_install_test.go (1 hunks)
  • internal/api/services/connectors_list_test.go (1 hunks)
  • internal/api/services/connectors_reset_test.go (1 hunks)
  • internal/api/services/connectors_uninstall_test.go (1 hunks)
  • internal/api/services/payment_initiation_adjustments_get_last_test.go (1 hunks)
  • internal/api/services/payment_initiation_adjustments_list_test.go (2 hunks)
  • internal/api/services/payment_initiation_related_payments_list_test.go (2 hunks)
  • internal/api/services/payment_initiations_approve_test.go (1 hunks)
  • internal/api/services/payment_initiations_create_test.go (1 hunks)
  • internal/api/services/payment_initiations_delete_test.go (1 hunks)
  • internal/api/services/payment_initiations_get_test.go (1 hunks)
  • internal/api/services/payment_initiations_list_test.go (1 hunks)
  • internal/api/services/payment_initiations_reject_test.go (1 hunks)
  • internal/api/services/payment_initiations_retry_test.go (1 hunks)
  • internal/api/services/payment_initiations_reversal_create_test.go (1 hunks)
  • internal/api/services/payments_create_test.go (1 hunks)
  • internal/api/services/payments_get_test.go (1 hunks)
  • internal/api/services/payments_list_test.go (1 hunks)
  • internal/api/services/payments_update_metadata_test.go (1 hunks)
  • internal/api/services/pools_add_account_test.go (1 hunks)
  • internal/api/services/pools_balances_at_test.go (1 hunks)
  • internal/api/services/pools_create_test.go (1 hunks)
  • internal/api/services/pools_delete_test.go (1 hunks)
  • internal/api/services/pools_get_test.go (1 hunks)
  • internal/api/services/pools_list_test.go (1 hunks)
  • internal/api/services/pools_remove_account_test.go (1 hunks)
  • internal/api/services/schedules_get_test.go (1 hunks)
  • internal/api/services/schedules_list_test.go (1 hunks)
  • internal/api/services/services.go (1 hunks)
  • internal/api/services/tasks_get_test.go (1 hunks)
  • internal/api/services/workflows_instances_list_test.go (1 hunks)
  • internal/connectors/plugins/public/dummypay/plugin.go (1 hunks)
  • internal/connectors/plugins/registry/plugins.go (2 hunks)
  • internal/connectors/plugins/registry/plugins_test.go (7 hunks)
  • test/e2e/api_connectors_test.go (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • internal/connectors/plugins/public/dummypay/plugin.go
🔇 Additional comments (44)
internal/api/services/services.go (1)

11-11: LGTM! Clean addition of debug flag.

The addition of the debug field and its constructor initialization is well-structured and follows Go conventions.

Also applies to: 14-19

internal/api/services/connector_configs.go (1)

11-13: LGTM! Clean implementation of debug-controlled config visibility.

The modification correctly passes the debug flag to control config visibility, implementing the PR objective.

internal/api/module.go (1)

35-35: LGTM! Proper propagation of debug flag.

The change correctly propagates the debug flag from module initialization to service creation.

internal/api/services/payments_list_test.go (1)

22-22: Add test coverage for debug mode.

Given that this PR modifies dummy payment provider behavior based on debug mode, consider adding test cases with debug=true to verify the service behaves correctly in both modes.

internal/api/services/balances_list_test.go (1)

22-22: Add test coverage for debug mode.

Since balances may be associated with dummy payment providers, consider adding test cases with debug=true to ensure the service behaves correctly in both modes.

internal/api/services/schedules_list_test.go (1)

22-22: LGTM! Consider adding debug mode test cases.

The addition of the debug flag parameter is consistent with the PR objective. Since this change introduces a new configuration option, consider adding test cases that verify the behavior in debug mode.

internal/api/services/connectors_list_test.go (1)

22-22: Add test coverage for connector config visibility.

Since this PR deals with hiding "dummypay" from configs response, and this test file handles connector listing, consider adding test cases that verify:

  1. Connector configs are properly filtered in non-debug mode
  2. All configs (including dummypay) are visible in debug mode
internal/api/services/workflows_instances_list_test.go (1)

22-22: LGTM!

The addition of the debug flag parameter is consistent with the codebase changes.

internal/api/services/bank_accounts_list_test.go (2)

22-22: LGTM!

The addition of the debug flag parameter is consistent with the codebase changes.


22-22: Verify consistent debug flag usage across test files.

Let's verify that all test files consistently handle the debug flag parameter.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Description: Check for consistent debug flag usage in test files

# Find all test files in the services package and check New function calls
echo "Checking debug flag usage in test files..."
rg -U "s := New\([^)]+\)" "internal/api/services/*_test.go"

# Find any test files that might be testing debug-specific behavior
echo -e "\nChecking for debug-specific test cases..."
rg -i "debug.*mode|mode.*debug" "internal/api/services/*_test.go"

Length of output: 467


🏁 Script executed:

#!/bin/bash
set -e

echo "Listing all Go test files in the repository..."
fd -e go '_test\.go$'

echo -e "\nSearching for 'New(' calls in test files with assignment to variable 's'..."
fd -e go '_test\.go$' -x rg -U "s := New\([^)]*\)"

echo -e "\nSearching for any 'New(' calls in test files that pass debug flag as true..."
fd -e go '_test\.go$' -x rg -U "New\([^,]+,\s*[^,]+,\s*true\)"

Length of output: 16651


Consistent Debug Flag Usage Confirmed

All test files invoking the New() function are consistently passing the debug flag as false, including the instance in internal/api/services/bank_accounts_list_test.go. There were no occurrences of the debug flag being set to true.

internal/api/services/tasks_get_test.go (1)

23-23: Consider adding test cases for debug mode behavior.

While the addition of the debug flag parameter is consistent, consider adding test cases that verify the service behavior in both debug and non-debug modes to ensure complete test coverage.

 func TestTasksGet(t *testing.T) {
 	t.Parallel()
 
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 	store := storage.NewMockStorage(ctrl)
 	eng := engine.NewMockEngine(ctrl)
 
-	s := New(store, eng, false)
+	tests := []struct {
+		name      string
+		debugMode bool
+	}{
+		{
+			name:      "with debug mode off",
+			debugMode: false,
+		},
+		{
+			name:      "with debug mode on",
+			debugMode: true,
+		},
+	}
 
-	tests := []struct {
+	for _, mode := range tests {
+		s := New(store, eng, mode.debugMode)
internal/api/services/payment_initiations_list_test.go (1)

22-22: Similar feedback applies to debug mode test coverage.

internal/api/services/bank_accounts_create_test.go (1)

23-23: Similar feedback applies to debug mode test coverage.

internal/api/services/pools_get_test.go (1)

24-24: LGTM! Verify debug mode behavior.

The addition of the debug flag parameter aligns with the PR objectives. Since this test covers non-debug mode scenarios, consider adding test cases for debug mode to ensure complete coverage.

internal/api/services/bank_accounts_update_metadata_test.go (1)

23-23: LGTM!

The addition of the debug flag parameter is consistent with other test files and maintains existing test coverage.

internal/api/services/connectors_config_update_test.go (1)

25-25: Add test coverage for debug mode configuration.

While the change aligns with PR objectives, consider adding test cases to verify that:

  1. Dummypay config is hidden when debug is false
  2. Dummypay config is visible when debug is true

This ensures the core functionality of hiding dummypay in non-debug mode is properly tested.

internal/api/services/payments_update_metadata_test.go (1)

23-23: LGTM!

The addition of the debug flag parameter is consistent with other test files and maintains existing test coverage.

internal/api/services/schedules_get_test.go (1)

23-23: Consider adding test cases for debug mode behavior.

While the addition of the debug parameter is consistent, consider adding test cases that verify how the debug flag affects the service behavior, if applicable to schedule operations.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Check if debug flag affects schedule-related operations
ast-grep --pattern 'func (s *Service) SchedulesGet($$$) {
  $$$
  if s.debug {
    $$$
  }
  $$$
}'

Length of output: 101


Debug Mode Behavior for SchedulesGet Is Not Currently Exercised

Our investigation confirms that the SchedulesGet functionality does not conditionally branch on the debug flag—even though the New method now accepts a debug parameter, there isn’t any logic within SchedulesGet (or its related flows) that leverages the value of s.debug. As such, adding tests specifically for debug-mode behavior isn’t necessary at this moment. If future changes introduce debug-specific handling in schedule operations, then test coverage should be revisited.

internal/api/services/bank_accounts_get_test.go (1)

24-24: LGTM! Consider adding debug mode test cases.

The change is consistent with the service constructor update. If bank account operations are affected by debug mode, consider adding test cases to verify the behavior in both debug and non-debug modes.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Check if debug flag affects bank account operations
ast-grep --pattern 'func (s *Service) BankAccountsGet($$$) {
  $$$
  if s.debug {
    $$$
  }
  $$$
}'

Length of output: 104


Attention: No Debug Mode Handling Found

Our verification indicates that the BankAccountsGet implementation does not conditionally check or modify its behavior based on the service’s debug flag. The updated test case correctly reflects the updated constructor signature. If future changes introduce debug-dependent behavior for bank account operations, then adding debug mode test cases would be beneficial.

internal/api/services/payment_initiations_get_test.go (1)

23-23: LGTM! Consider adding debug mode test cases.

The change is consistent with the service constructor update. If payment initiation operations are affected by debug mode, consider adding test cases to verify the behavior in both debug and non-debug modes.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Check if debug flag affects payment initiation operations
ast-grep --pattern 'func (s *Service) PaymentInitiationsGet($$$) {
  $$$
  if s.debug {
    $$$
  }
  $$$
}'

Length of output: 110


I'll generate an additional shell script to search for the PaymentInitiationsGet function definition and check for any usage of the s.debug flag within that function's file. This will help us verify whether the debug mode affects the payment initiation operations.


🏁 Script executed:

#!/bin/bash
# Find the file(s) that define the PaymentInitiationsGet function in the services package.
FILE=$(rg -l "func (s\\*Service) PaymentInitiationsGet" internal/api/services/)
echo "File(s) with PaymentInitiationsGet function:"
echo "$FILE"

# If a file is found, search within it for any usage of the debug flag.
if [ -n "$FILE" ]; then
    echo -e "\nSearching for debug flag usage in the PaymentInitiationsGet function file(s):"
    rg -n "s\.debug" $FILE
else
    echo "PaymentInitiationsGet function file not found."
fi

Length of output: 310


I'll run an additional script to search for any reference to PaymentInitiationsGet in the repository to better understand its presence and whether it inspects the debug flag.


🏁 Script executed:

#!/bin/bash
# Search for any occurrence of 'PaymentInitiationsGet' in the repository.
rg -n "PaymentInitiationsGet" .

Length of output: 20016


Payment Initiations Get Operation Unaffected by Debug Mode

Our investigation shows that the PaymentInitiationsGet function in internal/api/services/payment_initiations_get.go does not utilize the debug flag (i.e., there’s no branch condition or behavior change based on s.debug). This confirms that while the constructor update to include the false parameter is consistent, there isn’t any debug-specific behavior to test for in this function at present.

  • The code in internal/api/services/payment_initiations_get.go simply calls s.storage.PaymentInitiationsGet without checking the debug flag.
  • No alternative paths or conditional logic based on debug mode is detected.

The current tests are comprehensive for the error scenarios, and additional debug mode test cases aren’t necessary unless future changes introduce debug-sensitive behavior.

internal/api/services/pools_delete_test.go (2)

23-24: Verify the broader impact of debug mode.

While the changes consistently add the debug parameter across all service tests, it would be beneficial to verify how this flag affects the overall system behavior, particularly around dummy PSP visibility as mentioned in the PR objectives.


23-23: LGTM! Consider adding debug mode test cases.

The change is consistent with the service constructor update. If pool operations are affected by debug mode, consider adding test cases to verify the behavior in both debug and non-debug modes.

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Check if debug flag affects pool operations
ast-grep --pattern 'func (s *Service) PoolsDelete($$$) {
  $$$
  if s.debug {
    $$$
  }
  $$$
}'

Length of output: 100


Code Consistency Verified – Debug Mode Not Currently Impacting Pool Deletion

After verifying the code and checking for any conditional behavior involving the debug flag in the pool deletion logic, no branches (e.g., “if s.debug …”) were found in the PoolsDelete implementation. This confirms that the updated constructor parameter (false in this test) does not affect pool deletion behavior.

The current tests correctly cover error cases without needing to exercise separate debug mode behavior. If, in the future, pool operations are meant to behave differently when debug mode is enabled, then adding corresponding test cases would be advisable.

internal/api/services/pools_create_test.go (1)

23-23: Consider adding test cases for debug mode behavior.

While the debug flag is added, consider adding test cases that validate the service behavior in both debug and non-debug modes to ensure complete test coverage.

internal/api/services/accounts_create_test.go (1)

23-23: Same debug mode test coverage concern as in pools_create_test.go.

internal/api/services/payments_create_test.go (1)

23-23: Same debug mode test coverage concern as in pools_create_test.go.

internal/api/services/connectors_reset_test.go (1)

23-23: 🛠️ Refactor suggestion

Add connector-specific debug mode test cases.

Since this PR aims to hide "dummypay" from configs in non-debug mode, consider adding specific test cases to validate:

  1. Connector reset behavior in debug mode
  2. Connector reset behavior in non-debug mode, especially regarding dummypay visibility

Add test cases by extending the test struct:

 tests := []struct {
 	name          string
+	debug         bool
 	err           error
 	expectedError error
 	typedError    bool
 }{
 	{
 		name:          "success",
+		debug:         false,
 		err:           nil,
 		expectedError: nil,
 	},
+	{
+		name:          "success with debug mode",
+		debug:         true,
+		err:           nil,
+		expectedError: nil,
+	},
 	// ... other test cases
 }

Then update the test execution:

 for _, test := range tests {
 	t.Run(test.name, func(t *testing.T) {
+		s := New(store, eng, test.debug)
 		eng.EXPECT().ResetConnector(gomock.Any(), models.ConnectorID{}).Return(models.Task{}, test.err)
internal/api/services/connectors_handle_webhooks_test.go (1)

23-23: Add test coverage for debug mode.

The test only covers non-debug mode (false). Consider adding test cases with debug mode enabled (true) to ensure the service behaves correctly in both modes.

internal/api/services/connectors_install_test.go (1)

23-23: Add test coverage for debug mode.

The test only covers non-debug mode (false). Consider adding test cases with debug mode enabled (true) to ensure the service behaves correctly in both modes.

internal/api/services/pools_add_account_test.go (1)

24-24: Add test coverage for debug mode.

The test only covers non-debug mode (false). Consider adding test cases with debug mode enabled (true) to ensure the service behaves correctly in both modes.

internal/api/services/pools_remove_account_test.go (1)

24-24: Add test coverage for debug mode.

The test only covers non-debug mode (false). Consider adding test cases with debug mode enabled (true) to ensure the service behaves correctly in both modes.

internal/api/services/pools_balances_at_test.go (1)

26-26: LGTM!

The addition of the debug flag parameter is consistent with the PR objectives to control the visibility of dummy PSP configurations.

internal/api/services/payment_initiation_adjustments_list_test.go (1)

24-24: LGTM!

The addition of the debug flag parameter in both test functions is consistent with the PR objectives to control the visibility of dummy PSP configurations.

Also applies to: 72-72

internal/api/services/payment_initiation_related_payments_list_test.go (1)

24-24: LGTM!

The addition of the debug flag parameter in both test functions is consistent with the PR objectives to control the visibility of dummy PSP configurations.

Also applies to: 72-72

internal/connectors/plugins/registry/plugins.go (1)

16-16: LGTM!

The constant definition follows Go naming conventions and improves code maintainability.

internal/api/services/payment_initiations_create_test.go (1)

24-24: LGTM!

The service constructor is correctly updated to include the debug flag parameter set to false, which aligns with the PR objective.

internal/api/services/payment_initiations_delete_test.go (1)

25-25: LGTM!

The service constructor is correctly updated to include the debug flag parameter set to false, which aligns with the PR objective.

internal/api/services/payment_initiations_reject_test.go (1)

25-25: LGTM!

The service constructor is correctly updated to include the debug flag parameter set to false, which aligns with the PR objective.

internal/connectors/plugins/registry/plugins_test.go (3)

51-51: LGTM!

The DummyPSPName plugin is correctly registered with the same capabilities and configuration as other plugins.


53-113: LGTM!

All existing test cases are correctly updated to explicitly pass debug=false, maintaining existing behavior while supporting the new debug flag functionality.


122-132: LGTM!

The new test cases effectively verify the dummypay visibility behavior:

  • Hidden when debug mode is off
  • Visible when debug mode is on
internal/api/services/payment_initiations_reversal_create_test.go (1)

23-23: LGTM!

The addition of the debug flag parameter (set to false) aligns with the PR objective of controlling dummypay visibility.

internal/api/services/payment_initiations_retry_test.go (1)

26-26: LGTM!

The addition of the debug flag parameter (set to false) is consistent with other test files and supports the PR objective.

internal/api/services/payment_initiations_approve_test.go (1)

26-26: LGTM!

The addition of the debug flag parameter (set to false) maintains consistency with other test files and supports the PR objective.

test/e2e/api_connectors_test.go (1)

39-39: LGTM! Verify test coverage for debug mode behavior.

The addition of Debug: true in the test server configuration is appropriate as these e2e tests need access to dummypay connector functionality.

Consider adding test cases to verify that:

  1. Dummypay is visible in the configs response when debug mode is true
  2. Dummypay is hidden in the configs response when debug mode is false

This can be achieved by adding a new test case in the "fetching connector configurations" section with debug mode set to false.

internal/api/services/pools_list_test.go Show resolved Hide resolved
internal/api/services/accounts_list_test.go Show resolved Hide resolved
@laouji laouji merged commit f6f0cc9 into main Feb 6, 2025
9 checks passed
@laouji laouji deleted the fix/hide-dummypay-from-configs-response branch February 6, 2025 14:00
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.

2 participants