fix(sidebar): refresh table tree after dropping a table#1714
Conversation
|
Thank you for your contribution! Before we can merge this PR, you need to sign our Contributor License Agreement. To sign, please comment below with:
I have read the CLA Document and I hereby sign the CLA. xuhengyu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
|
I have read the CLA Document and I hereby sign the CLA. |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
|
Pushed a maintainer update to this branch:
The red |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54b9816b32
ℹ️ 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".
| } catch { | ||
| Self.logger.warning( | ||
| "tables refresh failed db=\(key.database, privacy: .public) schema=\(key.schema ?? "nil", privacy: .public) error=\(error.localizedDescription, privacy: .public)" | ||
| ) |
There was a problem hiding this comment.
Surface refresh failures instead of keeping stale tables
When the metadata refetch fails after a successful drop or truncate, this catch only logs and leaves the old .loaded table list in tablesState; because loadTables returns immediately for loaded states, the sidebar continues to present the stale table with no error until the user manually forces an object refresh. Mark this key failed or clear it before refetching so refresh failures do not silently preserve deleted objects.
Useful? React with 👍 / 👎.
Problem
Dropping a table from the sidebar (right-click Delete, then Save Changes) did not remove it from the tree on multi-database servers like MySQL and PostgreSQL. The dropped table stayed visible until you refreshed the schema by hand.
Root cause
reloadSchemaonly reloadedSchemaService. The tree for.bySchemaand.byDatabaseservers (DatabaseTreeView) reads fromDatabaseTreeMetadataService, andloadTablesshort-circuits once a list is loaded. Nothing refreshed that store after a drop, so the stale table list stayed put.Fix
DatabaseTreeMetadataService.refreshLoadedTables(connectionId:): cancel dedup, clear, and refetch every expanded database/schema's tables for the connection.MainContentCoordinator.reloadSchemacalls it right afterschemaService.reload, so both data sources update.Scope is tables only (no routines).
Tests
Two new tests in
DatabaseTreeMetadataServiceTests:Both pass. App and test targets compile.
Notes
connectionObjectKeysis nownonisolated. It is a pure function with no actor state, and the old MainActor isolation broke the existing tests under Xcode 27 beta strict concurrency. Stable Xcode accepts the annotation too.Verified by hand on a MySQL connection: drop a table from the sidebar, Save Changes, and it disappears from the tree without a manual refresh.