fix(connections): preserve tags on reconnect when no tags are provided#5723
Closed
hassan254-prog wants to merge 1 commit intomasterfrom
Closed
fix(connections): preserve tags on reconnect when no tags are provided#5723hassan254-prog wants to merge 1 commit intomasterfrom
hassan254-prog wants to merge 1 commit intomasterfrom
Conversation
Contributor
There was a problem hiding this comment.
Suggested changes highlight a potential regression in tag-clearing behavior when reconnecting with empty tags.
Status: Changes Suggested | Risk: Medium
Issues Identified & Suggestions
- Clarify or add explicit tag-clearing mechanism for empty tags
connection.service.ts
Review Details
📁 2 files reviewed | 💬 1 comments
Instruction Files
└── .claude/
├── agents/
│ └── nango-docs-migrator.md
└── skills
👍 / 👎 individual comments to help improve reviews for you
| deleted: false, | ||
| deleted_at: null, | ||
| tags: tags ?? exists?.tags ?? {} | ||
| tags: tags && Object.keys(tags).length > 0 ? tags : (exists?.tags ?? {}) |
Contributor
There was a problem hiding this comment.
[Logic] This change treats {} as “preserve existing tags.” That makes it impossible to clear tags by reconnecting with an empty object, which previously resulted in tags being cleared. If clients rely on clearing tags via {}, we need an explicit way to clear (e.g., tags: null or a separate preserveTags/clearTags flag) or update callers/docs accordingly. Is clearing tags still supported after this change?
Context for Agents
This change treats `{}` as “preserve existing tags.” That makes it impossible to clear tags by reconnecting with an empty object, which previously resulted in tags being cleared. If clients rely on clearing tags via `{}`, we need an explicit way to clear (e.g., `tags: null` or a separate `preserveTags`/`clearTags` flag) or update callers/docs accordingly. Is clearing tags still supported after this change?
File: packages/shared/lib/services/connection.service.ts
Line: 219
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.
Describe the problem and your solution
Fix reconnect tag preservation in
upsertAuthConnectionThis PR updates
upsertAuthConnectionto preserve existing connection tags when reconnecting with emptytags, while still replacing tags when non-empty values are provided. It also adds integration tests to validate both behaviors.This summary was automatically generated by @propel-code-bot