Bug 1565 cfieldsView Save Lock On Validation Error - sebiboga/testlink-upgraded GitHub Wiki

Bug fix β€” Issue #1565: cfieldsView saveCf() leaves saving=true locked when client-side validation fails β€” Save button dead until reload

Issue: #1565 Branch: fix/issue-1565 Status: FIXED & VERIFIED (2026-09-22)

Symptom

In the modern Custom Fields screen (gui/templates/cfields/cfieldsView.html), if the create/edit modal fails client-side validation (empty Label or Name), the Save button stops working for the rest of the session β€” every further click is silently swallowed β€” until the page is reloaded.

Repro steps (this run, fresh DB)

  1. http://localhost:8082/gui/templates/cfields/cfieldsView.html?tproject_id=1&tplan_id=0 (admin/admin).
  2. Click + Create Custom Field, leave Label/Name empty, click Save β†’ "Label and Name are required.".
  3. Fill Label = Repro CF, Name = repro_cf, click Save again.

Actual (pre-fix): no request fires (network panel unchanged), page-scope saving stays true. Expected: the second Save POSTs the create.

pre-fix locked state

Root cause chain

  1. saveCf() acquires a re-entrancy lock BEFORE validation β€” cfieldsView.html:344 if (saving) return; and :345 saving = true;.
  2. The validation branch β€” cfieldsView.html:359-362:
    if (!data.label || !data.name) {
      $('#modalError').text(TLi18n.t('cf.validation.required')).show();
      return;
    }
    
    returns WITHOUT releasing the lock.
  3. Every other exit path releases it: ajax success (:375) and error (:385) both do saving = false;. The validation path is the only gap.
  4. Consequence: the guard at :344 swallows every later Save click until a reload re-runs var saving = false; (:155).

Regression source: commit 06aaadab7 (Refs #953, "create-and-assign to current test project") introduced the saving guard and the resets only on the ajax handlers; the pre-existing validation return was not retrofitted.

Fix

Minimal one-line change β€” release the lock on the validation-error path:

if (!data.label || !data.name) {
  $('#modalError').text(TLi18n.t('cf.validation.required')).show();
  saving = false;
  return;
}

Rationale: the lock only exists to prevent double-submit while an ajax call is in flight; a client-side validation failure means nothing was sent, so the flag must be cleared for the user to correct the fields and retry (same contract as the server-error handler). Alternatives rejected: reordering validation before the lock (larger control-flow diff), try/finally (overkill for one return path).

Blast radius / scope: the saving lock is used only in cfieldsView.html and platformsAssign.html:341,347 (brace variant) β€” the latter acquires it only after its no-op checks and always resets via .always(), so it is unaffected. cfieldsAssignView.html:300 has the same validation branch but NO saving guard β†’ unaffected.

Verification

Browser regression matrix (admin, fresh DB) β€” 6/6 PASS (see tmp/TLU_Test_Cases.md suite 1565):

  1. Empty validate β†’ error shown AND saving === false (pre-fix true).
  2. Retry after validation failure β†’ POST /api/cfields/index.php [200], modal closes, row appears.
  3. Validate β†’ cancel β†’ reopen β†’ still unlocked.
  4. Edit path β†’ GET /{id} + PUT /{id} [200] + refresh.
  5. Server-error path (duplicate name) β†’ POST [400] Β«Custom field name already existsΒ», saving === false, retry possible.
  6. Event Viewer: only log_level=16 audit rows (login, create, update); zero Error/Warning; no new JS console errors.

post-fix error handling, lock released

Files changed

  • gui/templates/cfields/cfieldsView.html β€” 1 line added (saving = false;).
  • tmp/TLU_Test_Cases.md β€” Regression β€” Issue #1565 suite (6/6 PASS).
  • CHANGELOG β€” KEY BUGFIX entry.
  • Docs mirror + wiki + two screenshots.

Result

Issue #1565 closed as fixed: one-line release of the saving lock on the validation failure path; full regression matrix passes error-free.