Saved views: a name cut after a space answers a server error, and re-saving the newest view says it needs a name #507

Closed
opened 2026-10-01 10:47:10 +00:00 by tiagoagueda · 0 comments
Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading table_views and running save_view with the snapshot's interpreter.

Checked again on 2026-10-01 against main at 0a5cad587: still present.

table_views decides whether a view was saved by comparing settings before and after, and finds the saved view again by comparing names normalised differently from the stored ones. Both go wrong on ordinary input.

  • Where: core/views_tables.py:63-76, table_views; core/tables.py:389-392, Table.save_view; core/tables.py:214, View.from_stored.

1. A long name can raise StopIteration

save_view stores " ".join(name.split())[:60], which ends in a space when the cut falls just after one. View.from_stored strips it on reading, but line 75 looks for v.name == " ".join(title.split())[:60], unstripped, with next() and no default. Checked: for a name of 59 letters, a space and three more letters, the stored name is the 59 letters and the comparison fails. next() raises StopIteration, the request answers a 500, and because requests are atomic (ATOMIC_REQUESTS) the save is rolled back: the person sees an error page and has no view.

2. Re-saving the newest view says it needs a name

saved == current (line 69) is the only test for "nothing was saved". save_view removes the view with the same slug and appends the new one at the end, so re-saving the most recently saved view with the same name, query and columns gives an identical dictionary. Checked: save_view(cur, "Waiting", "q=acme", ["name"]) == cur is True when Waiting is the last view. The person is told "A view needs a name." about a view that has one.

  • Why it matters: a server error on a valid name, and a false error on a harmless repeat.
  • Fix: normalise once in save_view, as name = " ".join((name or "").split())[:60].strip(), and have save_view return the slug it stored (or have the view compute the slug the same way), then find the view by slug with next(..., None). Decide "no name" from the cleaned name or slug being empty, not from the settings being unchanged; an unchanged re-save answers "View saved." and redirects to the view.

Related to #259 (closed), which added saved views.

Done when

  • tests/test_saved_views.py has tests that POSTing a 64-character name whose 60th character is a space to core:table_views redirects to the view rather than answering 500, and that saving the same view twice in a row answers "View saved." both times.
Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `table_views` and running `save_view` with the snapshot's interpreter. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. `table_views` decides whether a view was saved by comparing settings before and after, and finds the saved view again by comparing names normalised differently from the stored ones. Both go wrong on ordinary input. - **Where:** `core/views_tables.py:63-76`, `table_views`; `core/tables.py:389-392`, `Table.save_view`; `core/tables.py:214`, `View.from_stored`. ## 1. A long name can raise `StopIteration` `save_view` stores `" ".join(name.split())[:60]`, which ends in a space when the cut falls just after one. `View.from_stored` strips it on reading, but line 75 looks for `v.name == " ".join(title.split())[:60]`, unstripped, with `next()` and no default. Checked: for a name of 59 letters, a space and three more letters, the stored name is the 59 letters and the comparison fails. `next()` raises `StopIteration`, the request answers a 500, and because requests are atomic (`ATOMIC_REQUESTS`) the save is rolled back: the person sees an error page and has no view. ## 2. Re-saving the newest view says it needs a name `saved == current` (line 69) is the only test for "nothing was saved". `save_view` removes the view with the same slug and appends the new one at the end, so re-saving the most recently saved view with the same name, query and columns gives an identical dictionary. Checked: `save_view(cur, "Waiting", "q=acme", ["name"]) == cur` is `True` when *Waiting* is the last view. The person is told "A view needs a name." about a view that has one. - **Why it matters:** a server error on a valid name, and a false error on a harmless repeat. - **Fix:** normalise once in `save_view`, as `name = " ".join((name or "").split())[:60].strip()`, and have `save_view` return the slug it stored (or have the view compute the slug the same way), then find the view by slug with `next(..., None)`. Decide "no name" from the cleaned name or slug being empty, not from the settings being unchanged; an unchanged re-save answers "View saved." and redirects to the view. Related to #259 (closed), which added saved views. ## Done when - `tests/test_saved_views.py` has tests that POSTing a 64-character name whose 60th character is a space to `core:table_views` redirects to the view rather than answering 500, and that saving the same view twice in a row answers "View saved." both times.
tiagoagueda added this to the 0.6.0 milestone 2026-10-01 10:47:10 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Postulo/postulo#507
No description provided.