feat(connections): support multiple tags per connection with tag filtering (#744)#1753
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c675898213
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sslEnabled: type == .mssql ? (mssqlSSLMode != .disable) : sslEnabled, | ||
| groupId: groupId, | ||
| tagId: tagId | ||
| tagIds: tagId.map { [$0] } ?? [] |
There was a problem hiding this comment.
When a connection arrives from sync/import with more than one tag, ConnectionFormViewModel.init still stores only conn.tagId (the first tag) in the single tagId field. Saving any edit then rebuilds the model here as tagIds: tagId.map { [$0] } ?? [], so all secondary tags are silently removed and can be synced/exported as a deletion even if the user did not touch tags. Keep existingConnection.tagIds when the tag field is unchanged, or make the iOS form write the full tag array.
Useful? React with 👍 / 👎.
| sslConfig: sslConfig, | ||
| color: value("color"), | ||
| tagName: value("tagName"), | ||
| tagNames: values("tagName").isEmpty ? nil : values("tagName"), |
There was a problem hiding this comment.
For a deeplink such as tablepro://import?...&tagName=prod&tagName=readonly on a device where readonly does not already exist, this captures both names in tagNames, but DeeplinkImportSheet still builds the import envelope with tags: connection.tagName.map { ... }, which includes only the first tag. performImport only creates tags listed in that envelope, so buildDatabaseConnection cannot resolve the second name and the imported connection loses it unless the tag was pre-existing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80b268378c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !connection.tagIds.isEmpty { | ||
| let tagIdStrings = connection.tagIds.map { $0.uuidString } | ||
| record["tagIds"] = tagIdStrings as CKRecordValue | ||
| record["tagId"] = tagIdStrings[0] as CKRecordValue | ||
| } |
There was a problem hiding this comment.
When a connection loses its last tag, this branch writes neither the new tagIds field nor the legacy tagId field. The macOS push path builds a fresh record here and saves it via CloudKitSyncEngine with .changedKeys, which the local wrapper documents as overwriting only fields that are set, so iCloud can retain the stale tag values and other devices can resurrect the deleted association. Use an update path that includes field deletions or otherwise explicitly clear both tag fields for the empty-array case.
Useful? React with 👍 / 👎.
| if !vm.availableTags.isEmpty { | ||
| TagFilterBar(tagFilter: $vm.tagFilter, availableTags: vm.availableTags) | ||
| Divider() |
There was a problem hiding this comment.
If the active filter is the only tag in use and the last matching connection has that tag removed or deleted, vm.availableTags becomes empty while vm.tagFilter remains active. This guard then hides TagFilterBar, including its Clear button, so the list stays filtered to an empty state with no visible way to recover. Show the bar whenever a filter is active, or clear selected IDs that are no longer available.
Useful? React with 👍 / 👎.
| if !tagIds.isEmpty { | ||
| try container.encode(tagIds, forKey: .tagIds) | ||
| } |
There was a problem hiding this comment.
TableProMobile persists [DatabaseConnection] by JSON-encoding this public model, but tagged connections now serialize only tagIds. The previous mobile model decoded only tagId, so opening the same on-disk data with an older build drops even the first tag, unlike the macOS stored model and sync/export paths that dual-write the legacy field for compatibility. Encode tagIds[0] under .tagId here when the array is non-empty.
Useful? React with 👍 / 👎.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
1 participant
Closes #744.
Problem
Connections could hold at most one tag (
DatabaseConnection.tagId: UUID?), so tags behaved like mutually exclusive groups. There was no way to assign several tags or filter by them.Change
Tags are now a
[UUID]set per connection, end to end:tagId→tagIdswith a one-way Codable migration that promotes an existing single tag into the array.+Noverflow that opens the full list.StoredConnection, CloudKitSyncRecordMapper(macOS + iOS),ExportableConnection, and thetablepro://deep link all carry multiple tags, dual-writing the first tag for backward compatibility with older app versions.tagIdkept as a computed{ tagIds.first }); no iOS multi-tag UI in this change.Tests
Added unit tests for the Codable migration (tagId → tagIds, encode drops the legacy key),
StoredConnectionround-trip + back-compat dual-write,SyncRecordMapperround-trip + legacy fallback, andTagFiltermatch-any/all + tree filtering. Updated existing import/sharing tests for the new init signatures.Notes
--strictclean. CHANGELOG and the URL-scheme doc updated.