chore: refactor integrations.settings column and extract nango mappings (CM-963)#3866
Open
chore: refactor integrations.settings column and extract nango mappings (CM-963)#3866
Conversation
Signed-off-by: Uroš Marolt <uros@marolt.me>
backend/src/database/migrations/V1771839722__refactorIntegrationsTable.sql
Outdated
Show resolved
Hide resolved
backend/src/database/migrations/V1771839722__refactorIntegrationsTable.sql
Show resolved
Hide resolved
Signed-off-by: Uroš Marolt <uros@marolt.me>
| `Repository not found, nango_mapping.repositoryId remains NULL`, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Repository link uses non-normalized URL
Medium Severity
linkNangoMappingToRepo looks up repositories using an exact url = $(url) match, but repository URLs may be normalized (case, trailing slash, .git, protocol). This can cause the lookup to miss an existing repository and leave integration.nango_mapping.repositoryId as NULL, breaking the intended repo-linking behavior.
| } | ||
| output.settings = { ...output.settings, nangoMapping } | ||
| } | ||
| } |
There was a problem hiding this comment.
N+1 queries when populating nango mapping
Medium Severity
_populateRelations now runs an extra SQL query per github-nango integration to rebuild settings.nangoMapping. Any endpoint returning lists of integrations can degrade into an N+1 query pattern, increasing latency and DB load proportional to the number of returned integrations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Note
Medium Risk
Includes a schema migration moving GitHub-Nango connection mappings into a new table and removing several integration columns/endpoints; risks are mainly around data migration correctness and any missed code paths still expecting
settings.nangoMappingor removed import/limit fields.Overview
Refactors integration persistence by dropping deprecated integration columns (
limitCount,limitLastResetAt,emailSentAt,importHash) and removing the/integration/importAPI + related permission/service logic.Extracts GitHub-Nango
settings.nangoMappinginto a newintegration.nango_mappingtable, migrates existing JSONB data into it, and updates Nango cron jobs/workers + data-access helpers to read/write mappings via SQL. Backend responses forgithub-nangointegrations continue to exposesettings.nangoMappingby hydrating it from the new table, and the nango worker now links mapping rows torepositoriesviarepositoryIdwhen possible.Unifies integration settings typing by moving platform-specific settings interfaces into
@crowd/types(integrationSettings.ts) and makingIIntegrationgeneric/typed byPlatformType.Written by Cursor Bugbot for commit ad940fb. This will update automatically on new commits. Configure here.