Skip to content

feat(sidebar): add favorite databases - #2184

Open
sophiathedev wants to merge 8 commits into
TableProApp:mainfrom
sophiathedev:agent/favorite-databases
Open

feat(sidebar): add favorite databases#2184
sophiathedev wants to merge 8 commits into
TableProApp:mainfrom
sophiathedev:agent/favorite-databases

Conversation

@sophiathedev

@sophiathedev sophiathedev commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Closes #1553

Summary

  • add database favorites from the Tree sidebar with Development, Testing, Production, and Unassigned environments
  • show favorite databases first in the Favorites sidebar, grouped by environment with search, filtering, persisted disclosure, keyboard actions, and double-click switching
  • keep favorites local and connection-scoped, clean them up when a connection is deleted, and document the behavior

Preview

Favorite databases grouped by environment in the Favorites sidebar

Verification

  • generated the Xcode project successfully
  • passed an unsigned Debug build of the TablePro scheme
  • passed 85 affected storage, grouping, menu, selection, AppKit, connection cleanup, and persistence tests in the deep pass
  • passed the focused FavoritesOutlineSelectionTests again after the final persistence regression was added
  • passed strict SwiftLint on all 34 changed Swift files with 0 violations
  • verified the native flow against a disposable MySQL server in OrbStack: all groups render, the environment filter works, state survives relaunch, and double-click switches the active database

UI automation is covered at the AppKit coordinator boundary because the database Tree layout requires a live database server. The manual native pass used an isolated app sandbox and disposable server data.

@sophiathedev
sophiathedev marked this pull request as ready for review August 18, 2026 14:22
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@datlechin

Copy link
Copy Markdown
Member

Pushed a review pass onto this branch: main merged in (the branch no longer compiled against it), 15 review findings fixed, and the two gaps that needed design work closed. Thanks for the feature, the model underneath it was sound.

The branch did not compile

DatabaseContainerRef.database became String? on main in #2286, after this branch forked. favoriteDatabaseItems still took it non-optional. It now guards, so an engine with no database dimension offers no favorite items at all rather than favoriting an empty name.

Reach

The only entry point was the Tree-layout database context menu, and SharedSidebarState.defaultLayout is .flat, so a new user could not reach the feature. The HIG is explicit about this: "Always make context menu items available in the main interface, too."

  • Toolbar database switcher: favorite and environment items on its context menu, and a star on each favorited row. Gated on switchTarget == .database, because in schema mode those rows are schemas and a schema name in the database-favorites store names nothing.
  • Tree layout: the star is now a Button on the TableRowView pattern (hover-revealed, .help, row-level accessibilityAction) instead of a static Image that did nothing when clicked.
  • Menu bar: Database > Favorite Database, delegate-filled on the SchemaMenuDelegate shape. A hover star and a context menu are both pointer-only, so without this the feature had no keyboard path.

No flat-layout database row: flatRootNodes() builds none, and adding one would give the layout a database dimension it deliberately does not have. The switcher and the menu bar cover it.

Sync

Database favorites were the only favorites that did not sync. The full path is wired (SyncRecordType.favoriteDatabase, FavoriteDatabaseSyncField, mapper, collect/apply/delete, SyncSettings.syncDatabaseFavorites with a Settings toggle and a docs row) and inert: the type is not in verifiedInProduction, so SyncSchemaGate withholds every record until the CloudKit schema is deployed. SyncSchemaGateTests.allRecordTypesAreWritable now compares against an explicit pending-deployment allowlist, which fails both when a type is gated without being listed and when a listed type has since been deployed.

Three things the sync path needed that are easy to get wrong:

  • syncId hashes connectionId|database and never the environment. Hashing a mutable payload orphans the record on every re-tag.
  • The apply path upserts. addFavoriteWithoutSync skips an entry that already exists, which would silently drop a remotely changed environment.
  • A connection marked Local only does not push its database favorites. (collectDirtyTableFavorites does not do this for table favorites; that is pre-existing and left alone here.)

The other findings

  • Localization: 10 keys added to Localizable.xcstrings with ko, tr, vi, zh-Hans and zh-Hant. scripts/localization.py merge() skips keys absent from the catalog, so they were unreachable by any translator.
  • Multi-selection: dropped the targets.count == 1 gate. A right-click inside a selection acts on the whole selection, which is what NSTableView.clickedRow documents and what FieldDrivenList already does. Mixed selections offer "Add to Favorites" and check no environment.
  • Group expansion: isFilteringDatabases became isNarrowingDatabases, so picking an environment expands its group the way search already did.
  • Empty state: FavoritesEmptyState (pure, tested) separates a filter miss from a search miss. A filter miss used to render ContentUnavailableView.search(text: ""), which is Apple's "check the spelling" advice for a query the user never typed.
  • Search: no longer matches the localized environment title. Typing "product" to find a products table listed every Production database.
  • Naming: .none is now .unassigned and menuTitle is gone. One bucket had two names on screen at once, and .none collided with Optional.none in inference.
  • Storage: rebuilt on the FavoriteTablesStorage shape (one key, TrackedAction, injected SyncChangeTracker, …WithoutSync variants). Re-picking the environment a database already has is now a no-op instead of rebuilding every visible tree row in every window.
  • Cleanup: one ConnectionLocalState.purge(connectionIds:origin:) replaces the three open-coded copies. It also clears the seven SidebarPersistenceKey families and recent tables that no delete path had ever removed, and calls SharedSidebarState.removeConnection first so a live registry entry cannot rewrite the keys underneath it. The remote-deletion path passes .remote so it leaves no tombstone to push back at the device that sent the deletion.
  • Expansion state: the collapsed-groups map decodes per value. One unknown environment used to discard every connection's group state permanently.
  • Dead environment(for:connectionId:) removed; the three-init ladder collapsed to one defaulted init; the SharedSidebarStateTests key is now removed rather than rewritten.

Verified

  • build PASS, lint PASS (0 violations, 261 doc references clean)
  • 177 app test cases pass across the 14 touched suites
  • 87 TableProSyncTests cases pass, including the production-schema parity suite

No TableProUITests case for the new entry points: exercising them needs a live connection with a database dimension, and the bundled sample database is SQLite, whose containerSwitchTarget is not .database. The menu-bar path is covered by unit tests instead (MainMenuBuilderTests).

Before merge

The CloudKit FavoriteDatabase record type still has to be created and deployed to Production, then scripts/export-cloudkit-schema.sh run and both verifiedInProduction sets flipped in the same commit as the refreshed .ckdb. Until then the toggle in Settings is visible and syncs nothing.

@datlechin

Copy link
Copy Markdown
Member

Hi @sophiathedev, could you please take the screenshots both light & dark mode for the docs page?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Favorite Databases (Right‑click on Database Name + Environment Grouping)

2 participants