Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions cases/base-share/a-shared-forms-picture.case.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { defineBugCase } from "../../framework/types";

// T6604: where a form's picture lives is stored as a short path, and the address
// a browser can fetch is worked out from it when the form is read. A shared form
// is read through two layers, and both worked it out - the second over the
// first's answer - so what came back was one address with another stuck on the
// front of it, which fetches nothing. The person who opens the link sees a form
// with a broken picture while the same form inside the product looks right,
// because inside it is read through one layer only.
export default defineBugCase({
id: "base-share/a-shared-forms-picture",
title: "A shared form's picture has one address, not two",
runner: "shared-form-cover-url",
timeoutMs: 180_000,
bug: {
issue: "T6604",
status: "fixed",
sourceCommits: ["573e0b70e"],
},
config: {
baseId: "seed-base",
tableNamePrefix: "e2e-lab-form-cover",
rowTitle: "a-row-behind-the-form",
storedPath: "form/e2e-lab-cover-image",
},
});
59 changes: 59 additions & 0 deletions cases/base-share/a-shared-forms-picture.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# base-share/a-shared-forms-picture

**T6604** — fixed. On the `shared-form-cover-url` runner.

## What the user sees

A shared form with a broken picture. The same form inside the product looks
right, so nothing is wrong with the picture or the form — only with what the
share link hands out.

The person seeing it is usually outside the company, filling the form in, and has
nothing to compare against.

## Why

Where a form's picture lives is stored as a short path. The address a browser can
fetch is worked out from that path when the form is read.

The shared form is read through two layers, and both worked it out — the second
over the first's answer. What came back was one address with another stuck on the
front of it. Inside the product the same view is read through one layer, which is
why it looks right there.

## What the checkpoint asserts

That the address was built **once**: the cover and the logo each carry exactly one
`http(s)://`, and each ends at the stored path.

Counting addresses rather than comparing against an expected string is
deliberate. What the storage prefix is depends on how the instance is deployed,
and pinning it would make this case about configuration instead of about the
doubling. Ending at the stored path is what says the address still points at the
right thing.

What is counted is the **scheme**, not `http://`. Joining one address onto
another leaves the inner one with a single slash — measured on the fix's parent,
the value is

```
http://127.0.0.1:PORT/api/attachments/read/public/http:/127.0.0.1:PORT/api/attachments/read/public/form/…
```

— so looking for the double slash finds one address in a string that plainly
holds two. The first version of this case did exactly that and passed on both
sides.

Both the cover and the logo are set and both are read, because the fix covers
both and either could regress alone.

## Why the fixture is shaped this way

The stored value must be a **short path**, and the runner refuses an address: an
address is exactly what the fix passes through untouched, so a fixture holding one
would be green on both sides.

Before the checkpoint, the form is read from **inside** the product and its
picture must carry the stored path. That is the control — it says the form and the
stored value are fine, so a doubled address afterwards is about the share path and
not about the fixture.
26 changes: 26 additions & 0 deletions cases/view/a-column-the-view-does-not-place.case.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { defineBugCase } from "../../framework/types";

// T6545: the notes a view keeps about a column say where it sits and how wide it
// is. Views made long enough ago have entries with a width and no position at all
// - a shape nothing writes any more. Read back, the missing position was passed
// through as missing, so whatever draws the view was handed a column with no
// place among the others.
export default defineBugCase({
id: "view/a-column-the-view-does-not-place",
title: "A column whose stored notes give it no place still gets one",
runner: "legacy-column-visibility-metadata",
timeoutMs: 180_000,
bug: {
issue: "T6545",
status: "fixed",
sourceCommits: ["fd32044e4"],
},
config: {
baseId: "seed-base",
tableNamePrefix: "e2e-lab-legacy-column-place",
rowTitle: "a-row-in-the-table",
legacy: "noPosition",
order: 1,
width: 241,
},
});
71 changes: 71 additions & 0 deletions cases/view/a-column-the-view-does-not-place.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
# view/a-column-the-view-does-not-place

**T6545** — fixed. On the `legacy-column-visibility-metadata` runner,
`legacy: "noPosition"`.

## What the user sees

A table whose views will not load.

The notes a view keeps about each of its columns say where the column sits and
how wide it is. In views made long enough ago, some entries carry a width and
**no position at all** — a shape nothing writes any more, and not something
anyone did.

## Why

Nothing filled the gap in on the way out, and what a view says about a column is
checked there. An entry with no position does not pass that check, so the request
for the table's views fails — every view at once, not one column in one of them.

Measured on `66919acae`:

```
500 {"message":"Invalid View projection","domainCode":"view.invalid_projection",
"issues":[{"code":"invalid_union","errors":[[{"expected":"number", …}]]}]}
```

## What the checkpoint asserts

The views come back at all, then that the entry carries the column's place among
the columns, and the width the stored notes did carry.

The first of those is what catches this on a pre-fix commit — the request never
returns an entry to inspect. The other two are what says the gap was filled in
rather than papered over.

The width matters as much as the position here: filling the gap by replacing the
entry would satisfy "it has a position" while throwing away the only thing the
old notes actually said.

The position is compared against the column's own index among the table's fields
rather than a number written into the case, so the case does not encode a
particular default — only that a column gets the place it should have.

## Its sibling on this runner

`view/a-view-that-says-both-things-about-a-column` (T6597) is the other shape of
old notes: an entry carrying both the older visibility key and the current one.
Both shapes fail the same way — the view list refuses with `Invalid View
projection` — and they were fixed three days apart, this one first. What differs
is which part of the entry the check rejects: an unrecognised key there, a
missing number here.

Same fixture, same observation, two shapes of old data. That is why they share a
runner and differ only in the `legacy` config value.

## Why the fixture is written with SQL

Nothing writes either shape any more, which is also why a base carrying one
cannot get out of it from the interface. Before the checkpoint the fixture reads
the stored notes back and requires that they really are the shape this case is
about — for this one, that there is no `order` in them at all.

## The v1 column

v1 is red on every column of the acceptance matrix, `develop` included, and for
the same reason as its sibling's: v1 does not fail the request, it answers 200 and
hands the entry back exactly as stored — here, still without a position.

So on the older engine this data never caused an outage and was never filled in
either. Reported, not enforced.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export default defineBugCase({
baseId: "seed-base",
tableNamePrefix: "e2e-lab-legacy-column-meta",
rowTitle: "a-row-in-the-table",
legacy: "bothVisibilityNotes",
order: 1,
width: 241,
},
Expand Down
2 changes: 2 additions & 0 deletions docs/triage-ledger.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ The shape is gone; the runner is not kept.
| `2d93fbef4` | T3303 | Written and run: a formula comparing a number column against blank already answers per row on the fix's parent, empty and zero included (run 32698802701). The half that was broken is the v1 generated-column conversion in `sql-conversion.visitor.ts`, which the lab does not exercise - the same file as the T5496 row above. |
| `7829d83c6` | T6925 | Written in **three** shapes now. The first two were green on both columns: an overdue column added over existing rows computes on the fix's parent, whether written as a bare yes/no comparison (run 32705428574) or as an IF() returning two words (run 32704974280). The third went at the actual cause named in the commit - an `IF()` whose branches are a **date** and a word, so the column is typed as text and every branch is trimmed - and it does not express the behaviour either: on `develop` that column is created without error and then computes **nothing at all**, for the row taking the date and for the row taking the word alike, so there is no correct answer for a pre-fix column to differ from. The path the commit's own reproduction uses is still untried: the computed backfill a **field conversion** runs (`table.update`), not the pass that fills a newly created column. |
| `6ee7f96c4` | T6500 | Written and run twice, green on the fix's parent both times. This is the **field-conversion** backfill the T6925 row names as untried, so that path has now been tried: a table with a number column, rows, and a formula column reading it, then the number column converted to text inside the checkpoint, and in the second shape converted back again - which is what the report describes people doing. Neither direction reproduces `operator does not exist: double precision = text`. Something narrower decides whether the stored column and the freshly computed value end up different kinds; a formula that simply echoes the column is not it. The production reports name six computed fields across two tables in one base, so the shape may need a chain rather than one formula. |
| `7b969558a` | T6890 | Sorting, and deliberately not a change to it: nullable sorts stop emitting a leading `(expr IS NULL)` key and use Postgres's own NULLS FIRST/LAST instead. The commit and the issue both say the point is that the order stays identical to v1's - what changes is the length of the sort list and whether an index can serve it. There is nothing for a case to tell apart. Performance lab, if anywhere. |
| `1ea5d6c40` | T6669 | A read that stopped doing work twice: producing `extra.searchHitIndex` re-ran the whole v1 pipeline - bootstrapping v1 metadata, rebuilding the search WHERE, rescanning for ids that were then thrown away - to get a per-page hit index the v2 read could compute from the ids it already had. The hit index itself is the same on both sides. |
| `d36e266aa` | T6912 | Written in two shapes and run twice, green on both columns each time. A payroll chain - rate rows rolling up into an employee's highest rate, a payroll line borrowing that rate and the employee's site, a view filtered on the borrowed site - built entirely through ordinary requests opens on the fix's parent (run 32708030924). The same chain with the borrowed total's rule stripped the way the T6911 case strips it also opens (run 32709591507). The commit's own reproduction is a stored column shape neither of those two produce; what distinguishes it is not established. The already-shipped T6911 case was also run against this parent on its own and stayed green (run 32705941080). |
| `6c0970d52` | T6509 | Written in two shapes and run twice, green on both columns each time: a link cell pointing at a row whose name is blank, saved a second time unchanged, comes back without an empty name and can be written straight back. First shape run 32825087075; second - the link naming the column it shows, and the unnamed row written as explicitly having no name - run 32825483798. The commit's own reproduction goes through the v2 contract's own record endpoints rather than the public ones, and what the two send differently is not established. |
| `d28589d10` | T6734 | Written in two shapes and run twice, green on both columns each time: a date borrowed across a one-to-one link arrives on the fix's parent, both when the borrowing column is added next to a link that already exists (run 32836154719) and when the host's own date column is converted into a borrowed one (run 32836945426). The commit's own reproduction drains the computed queue between each step; what the two do differently is not established. |
Expand Down
2 changes: 2 additions & 0 deletions framework/runner-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ import { runGroupOnAnUnreadableColumnCase } from "./runners/group-on-an-unreadab
import { runArchiveGrantedByTheMatrixCase } from "./runners/archive-granted-by-the-matrix.runner";
import { runCommentGrantedByTheMatrixCase } from "./runners/comment-granted-by-the-matrix.runner";
import { runLegacyColumnVisibilityMetadataCase } from "./runners/legacy-column-visibility-metadata.runner";
import { runSharedFormCoverUrlCase } from "./runners/shared-form-cover-url.runner";
import { runLookupOfRollupCreateCase } from "./runners/lookup-of-rollup-create.runner";
import type {
BugCase,
Expand Down Expand Up @@ -273,6 +274,7 @@ const runners: { [K in BugRunnerKind]: RunnerFn<K> } = {
"archive-granted-by-the-matrix": runArchiveGrantedByTheMatrixCase,
"comment-granted-by-the-matrix": runCommentGrantedByTheMatrixCase,
"legacy-column-visibility-metadata": runLegacyColumnVisibilityMetadataCase,
"shared-form-cover-url": runSharedFormCoverUrlCase,
};

export const executeRegisteredRunner = (
Expand Down
82 changes: 57 additions & 25 deletions framework/runners/legacy-column-visibility-metadata.runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ import { fixtureDb } from "../fixture-db";
import type { BugCaseFor, BugProbeResult, BugRunContext } from "../types";
import type { LegacyColumnVisibilityMetadataCaseConfig } from "../types";

// A view whose stored notes about a column say both "shown" and "not hidden" ->
// open the table -> checkpoint: the view comes back, and says one thing about
// that column.
// A view whose stored notes about a column are of a shape nothing writes any
// more -> open the table -> checkpoint: the view comes back, and the entry is
// settled.
//
// Two shapes, on one runner because the fixture and the observation are the
// same: write notes no request produces, then read the views.
//
// Which columns a view shows has been recorded two ways over the life of this
// product: an older note saying whether a column is SHOWN, and the current one
Expand Down Expand Up @@ -72,18 +75,24 @@ export const runLegacyColumnVisibilityMetadataCase = async (
feature: "getViews",
});

// What a view made long enough ago carries: the older note about whether
// the column is shown, beside the current one about whether it is hidden.
// Written with SQL because nothing writes that shape any more.
// What a view made long enough ago carries. Written with SQL because
// nothing writes either shape any more.
const db = fixtureDb(context.app);
const legacy = {
[columnId]: {
order: config.order,
visible: true,
hidden: false,
width: config.width,
},
};
const columnIndex = table.fields.findIndex(
(field: { id: string }) => field.id === columnId,
);
const legacy =
config.legacy === "bothVisibilityNotes"
? {
[columnId]: {
order: config.order,
visible: true,
hidden: false,
width: config.width,
},
}
: // No position at all - the other shape old views carry.
{ [columnId]: { width: config.width } };
await db.execute(
`UPDATE "view" SET "column_meta" = $1 WHERE "id" = $2`,
JSON.stringify(legacy),
Expand All @@ -97,14 +106,19 @@ export const runLegacyColumnVisibilityMetadataCase = async (
`SELECT "column_meta" AS "columnMeta" FROM "view" WHERE "id" = $1`,
viewId,
);
if (!String(stored[0]?.columnMeta ?? "").includes('"visible"')) {
const storedText = String(stored[0]?.columnMeta ?? "");
const missingMark =
config.legacy === "bothVisibilityNotes" ? '"visible"' : '"order"';
const present = storedText.includes(missingMark);
if (config.legacy === "bothVisibilityNotes" ? !present : present) {
throw new Error(
`the stored notes do not carry the older key: ${stored[0]?.columnMeta} - the fixture is not in place`,
`the stored notes are not the shape this case is about (${config.legacy}): ${storedText} - ` +
"the fixture is not in place",
);
}

const probe = await bugCheckpoint(
"a-view-carrying-both-notes-about-a-column-still-reads",
"a-view-with-old-notes-about-a-column-still-reads",
async () => {
const listed = await readViews();
const body =
Expand All @@ -129,16 +143,34 @@ export const runLegacyColumnVisibilityMetadataCase = async (
`the view came back with nothing about the column: ${body}`,
);
}
if ("visible" in entry) {
throw new Error(
`the view still says both things about the column: ${JSON.stringify(entry)} - ` +
"whatever reads this next is handed the contradiction",
);
if (config.legacy === "bothVisibilityNotes") {
if ("visible" in entry) {
throw new Error(
`the view still says both things about the column: ${JSON.stringify(entry)} - ` +
"whatever reads this next is handed the contradiction",
);
}
if (entry.hidden !== false) {
throw new Error(
`the view came back saying the column is ${JSON.stringify(entry.hidden)}, expected false: ` +
JSON.stringify(entry),
);
}
} else {
// The entry has to come back with a position. Where a column sits is
// not optional to whatever draws the view, and the stored notes do
// not say.
if (entry.order !== columnIndex) {
throw new Error(
`the view came back with the column at ${JSON.stringify(entry.order)}, expected ` +
`${columnIndex} - its place among the columns: ${JSON.stringify(entry)}`,
);
}
}
if (entry.hidden !== false) {
// Either way, what the notes did carry survives.
if (entry.width !== config.width) {
throw new Error(
`the view came back saying the column is ${JSON.stringify(entry.hidden)}, expected false: ` +
JSON.stringify(entry),
`the width was ${JSON.stringify(entry.width)}, expected ${config.width}: ${JSON.stringify(entry)}`,
);
}
return { entry };
Expand Down
Loading