fix: (v2) ensure connector install v2 response is compatible to fctl #235
Conversation
WalkthroughThis pull request introduces a new Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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? 🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
test/e2e/api_accounts_test.go (1)
102-102: Maintain test consistencySimilar to line 68, confirm whether installing with version 3 is intentional or if the
vervariable should be used to maintain consistency across test entries (e.g., “with v2”, “with v3”).test/e2e/api_connectors_test.go (2)
42-43: Descriptive variable names
idandworkflowIDare straightforward, but consider naming them more explicitly (e.g.,connectorUUID,installWorkflowID) to align with domain context across the tests.
89-89: Consider grouping variable declarationsGrouping
id uuid.UUIDwith other related variables (likeworkflowID) can improve clarity. Currently, the line stands alone, which might be slightly less organized.test/e2e/api_bank_accounts_test.go (1)
205-205: Confirm ConnectorID validity before forwarding.
While the new usage ofconnectorRes.Data.ConnectorIDis correct, consider verifying that the returnedConnectorIDis not empty or malformed to prevent unexpected failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
internal/api/v2/handler_connectors_install.go(2 hunks)test/e2e/api_accounts_test.go(2 hunks)test/e2e/api_bank_accounts_test.go(2 hunks)test/e2e/api_connectors_test.go(4 hunks)test/e2e/api_payments_test.go(2 hunks)test/e2e/api_pools_test.go(4 hunks)
🔇 Additional comments (18)
internal/api/v2/handler_connectors_install.go (2)
15-19: Maintain backward compatibility with a descriptive type
Introducing ConnectorInstallResponse is a good approach for returning more structured data while preserving backward compatibility. The comments accurately describe the intent, and the ConnectorID field is well-defined and clear.
50-52: Ensure consistent JSON response format
Returning ConnectorInstallResponse via api.Created() aligns well with the new structured response. Verify all consuming clients are updated to expect this structure instead of a raw string.
test/e2e/api_accounts_test.go (1)
68-68: Confirm consistent version usage
Hardcoding the API version to 3 while the test is parameterized with version 2 and 3 can cause confusion. If you intend to test version 2 with the same connector installation logic, consider using the ver variable or clarifying that installation must occur at a specific version.
test/e2e/api_payments_test.go (2)
107-107: Use of v2.ConnectorInstallResponse is correct
Replacing the generic data struct with v2.ConnectorInstallResponse ensures that tests accurately reflect the new, structured API response.
130-133: Correct usage of ConnectorID field
Accessing connectorRes.Data.ConnectorID exemplifies proper usage of the new response structure. This ensures clarity when referencing the connector’s unique ID in the request body.
test/e2e/api_connectors_test.go (7)
13-13: New import statement for v2
This import for v2 is necessary to reference the newly introduced types in the tests. Confirm all references to v2 are consistent throughout the file.
51-51: Still returning plain string for v3
Here, connectorRes.Data remains a plain string for v3. Check if v3 also needs to align with any new structured response or if the plain string is intentional.
75-75: Use of v2.ConnectorInstallResponse
This is consistent with the changes in other test files, capturing ConnectorID for further calls.
81-81: Accurate retrieval of connector configuration
Accessing connectorRes.Data.ConnectorID here is correct per the new struct. Good job verifying the connector configuration after installation.
97-97: Maintaining existing approach for v3 uninstallation
For v3, the connector ID is returned as a plain string. Verify that this difference between v2 and v3 is intentional.
118-118: Leveraging the new v2.ConnectorInstallResponse for v2 uninstallation
This matches the updated approach of referencing ConnectorID.
123-125: Uninstall workflow completeness
Calling blockTillWorkflowComplete with connectorRes.Data.ConnectorID is consistent; it ensures the uninstall process is fully tested.
test/e2e/api_bank_accounts_test.go (2)
181-181: Switching to a structured response type is a welcome improvement.
The newly introduced v2.ConnectorInstallResponse type improves clarity and cements forward compatibility.
209-209: Validation of ConnectorID consistency looks good.
Ensuring that the fetched ConnectorID matches the installed connector is a solid check.
test/e2e/api_pools_test.go (4)
122-122: Adopting the structured response type aligns with the new API design.
This improvement ensures that the connector response is more explicit and less error-prone.
134-134: Readability improved by referencing ConnectorID directly.
Assigning connectorID = connectorRes.Data.ConnectorID is clear, ensuring that future code remains self-explanatory.
271-271: Consistent use of v2.ConnectorInstallResponse fosters maintainability.
This structured approach optimizes test readability and code clarity.
288-288: Directly setting the connectorID from the structured response is correct.
No changes needed; the assignment is straightforward and follows the updated response schema.
fixes: ENG-1575