fix(editor): write an inline cell edit to the table its rows came from, and ten more reported defects - #954
Open
yusuf-gundogdu wants to merge 10 commits into
Open
yusuf-gundogdu wants to merge 10 commits into
yusuf-gundogdu wants to merge 10 commits into
Conversation
An inline cell edit picked its table from the tab title, falling back to a regex over the editor buffer. Both outlive the rows on screen: renaming a tab, or typing a new query without running it, pointed the UPDATE at a different table than the one being edited and the write landed there. A tab now records the statement that produced the result it is showing (`resultQuery`), and the edit resolves its table from that statement, read under the connection's own dialect by a new reader in `src/lib/sql/`. A query whose rows have no single base table - a join, a comma-separated FROM, a subquery in FROM or in the select list, a CTE, a set operation - is refused with a reason instead of guessed at. The reader uses the folder's existing span scanner, so quoting and comment rules follow the dialect rather than a second hand-rolled one. Three further defects on the same path: - The grid passed `row.index` to the update. That is the index into the filtered array, so with a column filter active the UPDATE was built from one row's primary key and another row's value. - `executeQuery` returned void, so the apply loop counted every statement as applied. A refused UPDATE was reported as saved and its edit discarded, with no way to retry it. - The post-apply refresh re-ran the last UPDATE rather than the SELECT, so the grid went empty after a successful save. Closes #881 Closes #882 Closes #883
Recharts writes the radial background and the tooltip surface as inline styles, so a hardcoded white track survives the theme switch. In the light theme the gauge's unfilled arc was white on white and the reading was impossible to place. Both gauges now take the track colour from the active chart theme, the same source the rest of the chart already uses. Closes #856
…lbars An icon-only button with no text and no label reaches a screen reader as "button", which is all a keyboard-only user gets to tell them apart. The sidebar's collapse, refresh, settings and connection controls, the schema explorer, and the toolbars above the results grid were all in that state. Each now carries an accessible name and a matching tooltip, so the pointer and the screen reader read the same thing. Refs #919
…word The one-click Koyeb deploy passed a real JWT_SECRET and a real admin password as default values. Every deployment made from that button shared them, and both are readable by anyone who opens the README. They are placeholders now, and the JWT_SECRET placeholder is deliberately shorter than the 32-character minimum the server enforces, so a deployment that leaves it as it stands refuses to boot rather than running on a published secret. A test pins both, so a future edit cannot quietly put a usable value back. Refs #943
The liveness path is /api/db/health, which is not where an operator looks first. Our own Azure guide, install.sh and the Helm chart each hard-code it precisely because the obvious paths returned the login redirect, and a probe pointed at /health reported the container unhealthy while it was serving. Both paths now answer with the same fixed body as /api/db/health, from one shared handler, and both are on the proxy's public list. They depend on nothing and disclose nothing beyond liveness. Refs #909
Every other release artifact ships a .sha256 sidecar. The two snap packages and the CycloneDX SBOM did not, so the one file a downstream packager is meant to verify a build against had nothing to verify it with. The release workflow now writes a sidecar for each, and the SBOM sidecar joins the required-asset list so a release missing it fails rather than ships. Refs #913
With AUTH_BOOTSTRAP off and no JWT_SECRET set, the server started, reported itself healthy, and answered every single login with a 503. The health probe said the deployment was fine while nobody could get in, which is the hardest shape of this to diagnose. In production, that combination now prints what is missing and exits instead of starting. A container that cannot serve a login says so at boot. Refs #908
The Data Profiler drew the same hash icon beside every column heading, including text and date columns, so the one signal telling column types apart said "number" for all of them. The type is already named in the line below. Closes #880
Schema Diff read the schema held in memory from the last explorer load. Run a DDL statement and diff, and it compared the snapshot against a copy of the database as it was before the change: no differences, which is the one answer that tool must never give wrongly. It now reads the schema from the connection when it opens, and uses that for both the diff's current side and any new snapshot. Closes #884
/health is a route at the app root rather than under src/app/api/, which is the one exception to that rule, and the public list in src/proxy.ts now has two more entries. Both are facts the next person reading this file needs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
cevheri
self-requested a review
September 18, 2026 06:52
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
1 participant
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.
Eleven reported defects, one commit each. Every fix carries a test that was proven to fail without it, and the whole suite plus the 100% coverage gate is green locally.
The one that matters most
An inline cell edit chose its table from the tab title, falling back to a regex over the editor buffer. Both outlive the rows on screen: rename a tab, or type a new query without running it, and the UPDATE went to a different table than the one being edited. The write landed there.
A tab now records the statement that produced the result it is showing, and the edit resolves its table from that statement, read under the connection's own dialect by
src/lib/sql/update-target.ts. A query whose rows have no single base table is refused with a reason instead of guessed at.That reader is where most of the review effort should go. It is built on the folder's existing span scanner rather than a second hand-rolled one, and its refusals are deliberately wide: it turned out that nine separate constructs read as one table under one engine and another under the next, and each one of those was a measured wrong-table write before it was a refusal. The cases are pinned in
tests/unit/sql/update-target.test.tswith the engine each was measured on.Three further defects on that same path are fixed in the same commit, because they are only reachable through it:
row.index, the index into the filtered array, so a column filter built the UPDATE from one row's key and another row's valueexecuteQueryreturned void, so a refused UPDATE was reported as saved and its edit discarded with no way to retryClosing
Closes #881
Closes #882
Closes #883
Closes #856
Closes #880
Closes #884
Five more were closed as not-planned but are real and are fixed here, so they cannot be
Closeslines: #919 (icon-only buttons with no accessible name), #943 (the Koyeb button published a working JWT secret and admin password), #909 (/healthand/api/healthnow answer, so a probe pointed at the obvious path does not report a serving container unhealthy), #913 (checksums beside the snap packages and the SBOM), #908 (no JWT secret in production now refuses to boot instead of booting healthy and answering every login 503).What I deliberately did not fix
pendingChangesis not scoped to a tab. Edit a cell in tab A, switch to tab B, apply, and the edit is attributed to B's rows. That is reachable on main today with none of this branch's changes, it is a state-shape problem rather than a fix on this path, and a partial guard I tried made it worse, so it is named rather than patched. Happy to open it as its own issue.Notes on #880 and #884
Both had an outside contributor assigned. The 0.16.0 follow-up needed them sooner than expected, so I picked them up rather than leave them queued, and said so on each issue. That is a scheduling call, not a comment on their work.