From 4ac984b33dff9255da6dc654fd87859466ad8aa4 Mon Sep 17 00:00:00 2001 From: austin Date: Sun, 26 Jul 2026 16:51:31 -0500 Subject: [PATCH] One form per row in the admin, and buttons that don't trip its validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catalogue screen had a form per control: 467 forms, 464 CSRF tokens, 317 KB of HTML for a page that gets opened on a phone in a bakery. It is now one form per row with several submit buttons — the same `name="do"` pattern the catering table editor already used — which is 89 forms and 206 KB, and less markup to read. Only `save` looks at the text boxes, so moving a row cannot save a half-typed name. The buttons that ignore them carry `formnovalidate`, because the name box is `required` and a browser would otherwise refuse to submit "move down" while that box was empty — a validation error about something the button has nothing to do with. Verified on the running container: 89 forms, move and photo-remove still land, and the seeded catalogue comes back the same after a database reset. Co-Authored-By: Claude Opus 5 (1M context) --- .../com/itsthevine/web/AdminController.java | 120 +++++++++------- .../resources/templates/admin/catalogue.html | 135 ++++++++---------- .../com/itsthevine/web/AdminPagesTest.java | 5 +- .../com/itsthevine/web/AdminSecurityTest.java | 9 +- 4 files changed, 133 insertions(+), 136 deletions(-) diff --git a/src/main/java/com/itsthevine/web/AdminController.java b/src/main/java/com/itsthevine/web/AdminController.java index b0a2513..95494f6 100644 --- a/src/main/java/com/itsthevine/web/AdminController.java +++ b/src/main/java/com/itsthevine/web/AdminController.java @@ -58,14 +58,51 @@ public class AdminController { }); } + /** + * Everything you can do to one item, from one form. + * + *

One form per item rather than one per button: the page carries forty items, and a separate form + * for each control meant 467 forms and 464 CSRF tokens — 317 KB of HTML for a screen that is opened + * on a phone, in a bakery. {@code name="do"} says which button was pressed and its value carries the + * argument, exactly as the catering table editor does. + * + *

Only {@code save} looks at the name and category boxes. The other actions deliberately ignore + * them, so pressing "move down" halfway through retyping a name doesn't save the half-typed name. + * + * @param action {@code save}, {@code move:-1}, {@code move:1}, {@code delete}, or + * {@code photo::<-1|1|0>} — earlier, later, or remove + */ @PostMapping("/items/{id}") - public String describe(@PathVariable Long id, - @RequestParam String name, - @RequestParam String category, - RedirectAttributes flash) { + public String item(@PathVariable Long id, + @RequestParam(name = "do", defaultValue = "save") String action, + @RequestParam(required = false) String name, + @RequestParam(required = false) String category, + RedirectAttributes flash) { + String[] parts = action.split(":"); return run(flash, () -> { - catalogue.describeItem(id, name, category); - flash.addFlashAttribute("done", "Saved " + name.trim() + "."); + switch (parts[0]) { + case "save" -> { + catalogue.describeItem(id, name, category); + flash.addFlashAttribute("done", "Saved " + name.trim() + "."); + } + case "move" -> catalogue.moveItem(id, Integer.parseInt(parts[1])); + case "delete" -> { + catalogue.removeItem(id); + flash.addFlashAttribute("done", "Removed from the products page."); + } + case "photo" -> { + // The key is the middle field; it can contain slashes and dots but never a colon. + String key = parts[1]; + int move = Integer.parseInt(parts[2]); + if (move == 0) { + catalogue.removePhoto(id, key); + } else { + catalogue.movePhoto(id, key, move); + } + } + // A stale page, or a hand-edited form. Do nothing rather than guess. + default -> { } + } }); } @@ -79,38 +116,6 @@ public class AdminController { }); } - /** - * @param move {@code -1} or {@code 1} to shuffle the photo along, {@code 0} to remove it. One - * endpoint for the three buttons under a photo, because they are the same edit — which - * photos, in which order — and the server is what decides the resulting list. - */ - @PostMapping("/items/{id}/photos/arrange") - public String arrangePhoto(@PathVariable Long id, - @RequestParam String key, - @RequestParam int move, - RedirectAttributes flash) { - return run(flash, () -> { - if (move == 0) { - catalogue.removePhoto(id, key); - } else { - catalogue.movePhoto(id, key, move); - } - }); - } - - @PostMapping("/items/{id}/move") - public String move(@PathVariable Long id, @RequestParam int by, RedirectAttributes flash) { - return run(flash, () -> catalogue.moveItem(id, by)); - } - - @PostMapping("/items/{id}/delete") - public String remove(@PathVariable Long id, RedirectAttributes flash) { - return run(flash, () -> { - catalogue.removeItem(id); - flash.addFlashAttribute("done", "Removed from the products page."); - }); - } - // --- filters ------------------------------------------------------------- @PostMapping("/categories") @@ -121,24 +126,31 @@ public class AdminController { }); } + /** + * Everything you can do to one filter, from one form — same shape as an item. + * + * @param action {@code save}, {@code move:-1}, {@code move:1} or {@code delete} + */ @PostMapping("/categories/{id}") - public String renameFilter(@PathVariable Long id, @RequestParam String name, RedirectAttributes flash) { + public String filter(@PathVariable Long id, + @RequestParam(name = "do", defaultValue = "save") String action, + @RequestParam(required = false) String name, + RedirectAttributes flash) { + String[] parts = action.split(":"); return run(flash, () -> { - catalogue.renameFilter(id, name); - flash.addFlashAttribute("done", "Renamed to " + name.trim() + ", and everything filed under it moved too."); - }); - } - - @PostMapping("/categories/{id}/move") - public String moveFilter(@PathVariable Long id, @RequestParam int by, RedirectAttributes flash) { - return run(flash, () -> catalogue.moveFilter(id, by)); - } - - @PostMapping("/categories/{id}/delete") - public String removeFilter(@PathVariable Long id, RedirectAttributes flash) { - return run(flash, () -> { - catalogue.removeFilter(id); - flash.addFlashAttribute("done", "Category deleted."); + switch (parts[0]) { + case "save" -> { + catalogue.renameFilter(id, name); + flash.addFlashAttribute("done", + "Renamed to " + name.trim() + ", and everything filed under it moved too."); + } + case "move" -> catalogue.moveFilter(id, Integer.parseInt(parts[1])); + case "delete" -> { + catalogue.removeFilter(id); + flash.addFlashAttribute("done", "Category deleted."); + } + default -> { } + } }); } diff --git a/src/main/resources/templates/admin/catalogue.html b/src/main/resources/templates/admin/catalogue.html index dad18a9..84f4148 100644 --- a/src/main/resources/templates/admin/catalogue.html +++ b/src/main/resources/templates/admin/catalogue.html @@ -4,8 +4,18 @@

- + +

Categories

@@ -14,34 +24,26 @@

    -
  • -
    -
    - - -
    -
    - - -
    -
    - +
  • -
-

On the page

  • -
    +
    - - - - -
    - - -
    + +
    -
    +
    - - + +
    - +
    -
    - - - -
    -
    - - - -
    + +
    -
    - - - - -
    + +
    -
    - - -
    -
    - -
    +
    -
    + + + +
    + + +
diff --git a/src/test/java/com/itsthevine/web/AdminPagesTest.java b/src/test/java/com/itsthevine/web/AdminPagesTest.java index e29ea89..066072c 100644 --- a/src/test/java/com/itsthevine/web/AdminPagesTest.java +++ b/src/test/java/com/itsthevine/web/AdminPagesTest.java @@ -94,6 +94,7 @@ class AdminPagesTest { @Test void renamingAnItemLandsAndSaysSo() throws Exception { mvc.perform(post("/admin/items/1").with(user("morissa")).with(csrf()) + .param("do", "save") .param("name", "76th Birthday Cake (chocolate)") .param("category", "Cakes")) .andExpect(status().is3xxRedirection()) @@ -107,7 +108,7 @@ class AdminPagesTest { @Test void aRefusalComesBackAsASentenceTheEditorCanActOn() throws Exception { // Cookies has items filed under it, and deleting the button shouldn't decide what happens to them. - mvc.perform(post("/admin/categories/1/delete").with(user("morissa")).with(csrf())) + mvc.perform(post("/admin/categories/1").with(user("morissa")).with(csrf()).param("do", "delete")) .andExpect(redirectedUrl("/admin")) .andExpect(flash().attribute("problem", containsString("still filed under Cookies"))); } @@ -115,7 +116,7 @@ class AdminPagesTest { @Test void movingAnItemUpFromTheTopIsNotAnError() throws Exception { // The button is disabled in the page, but a stale page could still post this. - mvc.perform(post("/admin/items/1/move").with(user("morissa")).with(csrf()).param("by", "-1")) + mvc.perform(post("/admin/items/1").with(user("morissa")).with(csrf()).param("do", "move:-1")) .andExpect(redirectedUrl("/admin")) .andExpect(flash().attributeCount(0)); } diff --git a/src/test/java/com/itsthevine/web/AdminSecurityTest.java b/src/test/java/com/itsthevine/web/AdminSecurityTest.java index 843bb25..c99fcba 100644 --- a/src/test/java/com/itsthevine/web/AdminSecurityTest.java +++ b/src/test/java/com/itsthevine/web/AdminSecurityTest.java @@ -3,7 +3,6 @@ package com.itsthevine.web; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import org.junit.jupiter.api.BeforeEach; @@ -78,7 +77,8 @@ class AdminSecurityTest { mvc.perform(get("/admin/catering/tables/1")).andExpect(status().isUnauthorized()); mvc.perform(post("/admin/items").with(csrf()).param("name", "Free cake").param("category", "Cakes")) .andExpect(status().isUnauthorized()); - mvc.perform(post("/admin/items/1/delete").with(csrf())).andExpect(status().isUnauthorized()); + mvc.perform(post("/admin/items/1").with(csrf()).param("do", "delete")) + .andExpect(status().isUnauthorized()); mvc.perform(post("/admin/categories").with(csrf()).param("name", "x")) .andExpect(status().isUnauthorized()); // The prices are the one thing on this site a stranger would most enjoy editing. @@ -113,8 +113,9 @@ class AdminSecurityTest { @Test void theContactFormStillNeedsItsCsrfToken() throws Exception { - // Enabling the security starter enables CSRF for the PUBLIC contact form too. Without the token - // it 403s; the SPA reads the XSRF-TOKEN cookie and sends X-XSRF-TOKEN. + // Enabling the security starter enables CSRF for the PUBLIC contact form too. Without a token it + // 403s. The page's own form carries a hidden field (Spring Security fills it in for any th:action + // form); this JSON endpoint needs the X-XSRF-TOKEN header from the cookie. mvc.perform(post("/api/contact").contentType(MediaType.APPLICATION_JSON) .content("{\"name\":\"Ada\",\"email\":\"ada@example.com\",\"message\":\"hi\"}")) .andExpect(status().isForbidden());