feat: Cleanup conflicting Slack integrations

[johnjcsmith]

User description

Update Slack integration's onActivate handler to clean up any existing Slack integrations with the same teamId.

---

PR Type

Enhancement

---

Description

• Added functionality to clean up existing Slack integrations with the same teamId during activation to prevent duplicate integrations
• Improved function naming for better code readability and maintenance:
  • getIntegrationForTeamId → integrationByTeam
  • getIntegrationForClusterId → integrationByCluster
  • uninstall → deleteNangoConnection
• Added proper error handling and early returns in onActivate and onDeactivate handlers
• Implemented proper null checks for Slack connection IDs

---

Changes walkthrough 📝

Relevant files

Enhancement

index.ts Enhance Slack integration activation and cleanup process  control-plane/src/modules/integrations/slack/index.ts Added cleanup of existing Slack integrations during activation to prevent duplicates Renamed functions for better clarity (e.g., getIntegrationForTeamId to integrationByTeam) Improved error handling in onDeactivate and onActivate handlers Split uninstall function into more specific deleteNangoConnection +27/-9

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The cleanup of existing integrations during activation should handle potential errors when calling upsertIntegrations to ensure the operation is atomic and won't leave the system in an inconsistent state await upsertIntegrations({ clusterId: clusterId, config: { slack: null } }) } Null Check The teamId access on newConfig.slack?.teamId may be undefined if newConfig.slack is null, which could cause runtime errors const existingIntegration = await integrationByTeam(newConfig.slack?.teamId);

[qodo-merge-pro]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Fix function parameter mismatch to ensure correct handling of deactivation process The onDeactivate handler has mismatched parameters compared to its usage. The second parameter '_' is unused and should be removed, while 'existingConfig' should be the second parameter. control-plane/src/modules/integrations/slack/index.ts [31-35] onDeactivate: async ( clusterId: string, - _: z.infer<typeof integrationSchema>, existingConfig: z.infer<typeof integrationSchema> ) => { Apply this suggestion Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a significant issue where the function signature has an unnecessary unused parameter that could cause runtime errors. Fixing this ensures proper function usage and prevents potential bugs.	9
	Add null check for required parameter to prevent potential runtime errors The onActivate handler should verify the teamId exists before attempting to find existing integrations to prevent potential null reference errors. control-plane/src/modules/integrations/slack/index.ts [49] -const existingIntegration = await integrationByTeam(newConfig.slack?.teamId); +if (!newConfig.slack?.teamId) { + logger.warn("Can not install Slack integration with no teamId"); + return; +} +const existingIntegration = await integrationByTeam(newConfig.slack.teamId); Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion addresses a critical null safety issue that could cause runtime errors. Adding the teamId check before using it is essential for robust error handling.	8
General	Enhance error logging by including relevant context information The error message in the authorize callback is missing proper error context. Add the teamId to the error message for better debugging. control-plane/src/modules/integrations/slack/index.ts [119-122] if (!teamId) { - logger.warn("Slack event is missing teamId"); - throw new Error("Slack event is missing teamId"); + const errorMsg = "Slack event is missing teamId"; + logger.warn(errorMsg, { enterpriseId }); + throw new Error(errorMsg); } Apply this suggestion Suggestion importance[1-10]: 4 Why: While the suggestion improves error logging by adding context, it's a minor enhancement that doesn't affect functionality. The existing error handling is already functional.	4