From 11d5bb4e3b0f0bedb6935d8dbb3c8981a7cbd040 Mon Sep 17 00:00:00 2001 From: Vahid Bazzaz Date: Sat, 25 Jul 2026 15:12:08 +0400 Subject: [PATCH 1/4] Validate warehouse location name length instead of silently truncating it Creating/editing a warehouse location let users type a name longer than the DB column, which MySQL then silently truncated on save with no feedback. Widens locations.label to varchar(50) and adds matching client + server validation so an over-limit name is rejected with a clear inline error (plus a tooltip explaining the limit) instead of being cut off. Also fixes two latent bugs this surfaced: the data-max-count character counter's jQuery plugin was never actually loaded on any page, and its counter text was hardcoded in Dutch with an unused English translate key sitting right next to it. --- assets/js/magic.js | 13 +++++ .../6_1_Warehouse_Locations.js | 56 +++++++++++++++++++ ...725095602_widen_locations_label_column.php | 23 ++++++++ include/locations_edit.php | 7 ++- templates/cms_footer.tpl | 3 +- templates/cms_form_location.tpl | 2 +- templates/cms_form_number.tpl | 2 +- templates/cms_form_text.tpl | 2 +- templates/cms_form_url.tpl | 2 +- 9 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 cypress/e2e/1_feature_tests/6_1_Warehouse_Locations.js create mode 100644 db/migrations/20260725095602_widen_locations_label_column.php diff --git a/assets/js/magic.js b/assets/js/magic.js index 122a34001..7292d1901 100644 --- a/assets/js/magic.js +++ b/assets/js/magic.js @@ -566,6 +566,19 @@ $(function() { }, ignoreTitle: true }); + + // Block submission when a data-max-count field is over its limit, + // instead of letting the value silently get truncated on save. + el.find("[data-max-count]").each(function() { + var field = $(this); + var max = field.data("max-count"); + field.rules("add", { + maxlength: max, + messages: { + maxlength: "Please use " + max + " characters or fewer." + } + }); + }); }); // Dirty forms check, voor meer info: https://github.com/codedance/jquery.AreYouSure diff --git a/cypress/e2e/1_feature_tests/6_1_Warehouse_Locations.js b/cypress/e2e/1_feature_tests/6_1_Warehouse_Locations.js new file mode 100644 index 000000000..07cae9243 --- /dev/null +++ b/cypress/e2e/1_feature_tests/6_1_Warehouse_Locations.js @@ -0,0 +1,56 @@ +context("6_1_Warehouse_Locations_Test", () => { + // exact-length strings, built programmatically so the boundary is never off-by-one + const Test_label_prefix = "aaa_LocLenTest_"; + const Test_label_at_limit = Test_label_prefix.padEnd(50, "X"); // exactly 50 chars + const Test_label_over_limit = Test_label_prefix.padEnd(51, "X"); // exactly 51 chars + + function NavigateToNewLocationForm() { + cy.visit('/?action=locations_edit&origin=locations'); + } + + function FillLocationForm(label) { + cy.get("input[id='field_label']").clear().type(label); + cy.selectOptionByText("box_state_id", "Instock"); + } + + function ArchiveTestedLocation(label) { + cy.visit('/?action=locations'); + cy.get('body').then(($body) => { + if ($body.text().includes(label)) { + cy.checkGridCheckboxByText(label); + cy.get("button[data-testid='reactivate-cms-user']").click(); + cy.getConfirmActionButton().click(); + } + }); + } + + beforeEach(function () { + cy.loginAsCoordinator(); + }); + + afterEach(function () { + ArchiveTestedLocation(Test_label_at_limit); + }); + + it("6_1_1 Rejects a location name over the character limit with an inline error, and does not save it", () => { + NavigateToNewLocationForm(); + FillLocationForm(Test_label_over_limit); + cy.getButtonWithText("Save and close").click(); + + cy.checkQtipWithText("qtip-content", "Please use 50 characters or fewer."); + // still on the form - the too-long name was never submitted + cy.url().should('include', 'action=locations_edit'); + + cy.visit('/?action=locations'); + cy.get('body').should('not.contain', Test_label_prefix); + }); + + it("6_1_2 Saves a location name that is exactly at the character limit", () => { + NavigateToNewLocationForm(); + FillLocationForm(Test_label_at_limit); + cy.getButtonWithText("Save and close").click(); + + cy.url().should('include', 'action=locations'); + cy.getRowWithText(Test_label_at_limit).should('exist'); + }); +}); diff --git a/db/migrations/20260725095602_widen_locations_label_column.php b/db/migrations/20260725095602_widen_locations_label_column.php new file mode 100644 index 000000000..1745bde36 --- /dev/null +++ b/db/migrations/20260725095602_widen_locations_label_column.php @@ -0,0 +1,23 @@ +table('locations') + ->changeColumn('label', 'string', [ + 'null' => false, + 'limit' => 50, + ]) + ->update() + ; + } +} diff --git a/include/locations_edit.php b/include/locations_edit.php index 5cbee9ebe..b3b93f66a 100644 --- a/include/locations_edit.php +++ b/include/locations_edit.php @@ -3,11 +3,16 @@ $table = 'locations'; $action = 'locations_edit'; $is_admin = $_SESSION['user']['is_admin']; +$labelMaxLength = 50; if ($_POST) { // check if you have access to the location you want to update verify_campaccess_location($_POST['id']); + if (mb_strlen((string) $_POST['label']) > $labelMaxLength) { + throw new Exception("Location name is too long. Please use {$labelMaxLength} characters or fewer."); + } + if (in_array($_POST['box_state_id'][0], ['3', '4', '7', '8'])) { throw new Exception('You cannot create Locations with this box state!'); } @@ -42,7 +47,7 @@ $cmsmain->assign('title', 'Warehouse Location'); addfield('hidden', '', 'id'); -addfield('text', 'Label', 'label', ['required' => true]); +addfield('text', 'Label', 'label', ['required' => true, 'maxlength' => $labelMaxLength, 'tooltip' => 'Keeping location names short (max '.$labelMaxLength.' characters) keeps navigation fast for warehouse and distribution teams on mobile.']); addfield('select', 'Default Status of Boxes', 'box_state_id', ['required' => true, 'tooltip' => 'If a box is moved to this location it will be assigned this status by default.', 'query' => 'SELECT id AS value, label FROM box_state WHERE id in (1,5,6) ORDER BY id']); addfield('html', 'About Locations', '

Locations are physical areas that hold stock. Giving locations a default status for Boxes will help you track of where your stock is going.