fix(mssql, oracle): enable schema switching from Cmd+K quick switcher#1050
Merged
fix(mssql, oracle): enable schema switching from Cmd+K quick switcher#1050
Conversation
Also pin plugin-level supportsSchemaSwitching = true for MSSQL and Oracle so plugin metadata, registry default, and Quick Switcher allowlist all agree.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Summary
rpt) from Cmd+K Quick Switcher on a SQL Server or Oracle connection silently did nothing. Schemas appeared in the search list, selection was a no-op, and there was no log or alert. Reported in r/macapps feedback byPedro_Prevost.QuickSwitcherViewModelhad a hardcoded list[.postgresql, .redshift, .oracle, .mssql]that gatedfetchSchemas(), so schemas appeared as searchable items.PluginMetadataRegistry+RegistryDefaultshadsupportsSchemaSwitching: falsefor SQL Server and Oracle, and the matching plugin sources (MSSQLPlugin.swift,OraclePlugin.swift) inherited the defaultfalsefrom theDriverPluginextension.MainContentCoordinator+Navigation.switchSchema(to:)hadguard PluginManager.shared.supportsSchemaSwitching(for: connection.type) else { return }— a silent early-return with no log, no alert, no user feedback.switchSchema(to:)correctly (MSSQL updates_currentSchemafor the table-listing filter; Oracle runsALTER SESSION SET CURRENT_SCHEMA), andFakeMSSQLPluginin tests already hadsupportsSchemaSwitching = true— i.e. the bug was test/prod divergence, not missing implementation.What changes
MSSQLPlugin.swift— addsupportsSchemaSwitching = true, append.selectSchemaFromLastSessiontopostConnectActionsso the chosen schema persists across reconnects.OraclePlugin.swift— same.PluginMetadataRegistry+RegistryDefaults.swift— flipsupportsSchemaSwitching: truefor both SQL Server and Oracle, and alignpostConnectActionsso the registry default matches what the plugin reports (defense in depth: the plugin metadata wins viaPluginManager+Registration, but the registry default is what's served before the plugin loads).QuickSwitcherViewModel.swift— drop the hardcoded[postgresql, redshift, oracle, mssql]list, derive the gate fromPluginManager.shared.supportsSchemaSwitching(for:)so future engines pick up the correct behavior automatically.MainContentCoordinator+Navigation.swift— replace the silent guard with anavigationLogger.warningand a "Schema Switching Not Supported" alert, so future regressions surface visibly instead of as a dead Cmd+K shortcut. After this PR the guard should be unreachable for MSSQL/Oracle, but the alert path is the safety net for any other engine that ever appears in the Quick Switcher schema list without supporting the switch.PluginMetadataRegistrySchemaSwitchingTests.swift(new) — regression tests assertingsupportsSchemaSwitching == truefor SQL Server, Oracle, and PostgreSQL;falsefor MySQL and SQLite;selectSchemaFromLastSessionis inpostConnectActionsfor the schema-aware engines; and a cross-component consistency test that lists every engine the Quick Switcher claims is schema-aware and asserts the registry agrees. Without this test the next regression of the same shape would slip through.Test plan
dbo,rpt, etc.). Press⌘K, search forrpt, hit Enter. The schema switches: sidebar refreshes to that schema's tables, toolbar reflects the switch, no silent no-op.selectSchemaFromLastSession).⌘K→ schema name → confirmsALTER SESSION SET CURRENT_SCHEMAran (sidebar refreshes).⌘Kdoes not show schemas (theif PluginManager.shared.supportsSchemaSwitching(for:)gate filters them out at fetch time).switchSchema(to:)is somehow called for an unsupported type (URL deeplink, programmatic call), the alert "Schema Switching Not Supported" surfaces instead of silent return.swiftlint lint --strict TableProTests/Core/Plugins/PluginMetadataRegistrySchemaSwitchingTests.swift(already passing).PluginMetadataRegistrySchemaSwitchingTestsshould pass.