Refs #1671 · status: DONE · BFF: api/tcstepsreorder/index.php · screen: gui/templates/testcases/tcStepReorder.html

Re-ordering the steps of a single test case version. In 1.9.20 it was not a screen at all — it was the TableDnD drag-and-drop of the step list, and the capability existed only as its server side:

The template was never part of the shipped UI, so the endpoint had no URL of its own, no dialog, no right — and nobody ever reviewed it.

lib/ajax/stepReorder.php re-ordered steps with no authorization at all:

• no checkSessionValid(), so an anonymous request re-ordered steps
• no mgt_modify_tc (or any) right check, so any logged-in user — including <no rights> — re-ordered any version
• it read $_REQUEST, so a GET mutated data: classic CSRF, no token
• stepSeq was written without any membership check: step ids of a foreign version could be mixed in, silently corrupting both versions
• it ended with a hardcoded tLog(..., DEBUG_MODE ...) writing a debug row per call

It is now a non-mutating shim (issue #1673): session-guarded, 405 for every write verb, and a 302 of GET to the modern screen. The mutation is deliberately not replayed — a write verb is answered with a pointer to the BFF instead of silently succeeding.

Dashio layout: teal header + dark toolbar (Refresh / Close), a context card (Test project, Test suite, Test case, Version, Steps) and the order table. Reached without a version in the URL — the $actions->tcStepReorder hub entry, or a bookmark of the retired shim — it shows a version picker instead of an error card: filter on external id / name, live n / total counter, an empty state, and only the versions that actually have steps (there is nothing to re-order without one).

Rows show the position, an execution-type chip (Manual / Automated / Automated-proceed / Automated-audited plus the upload paperclip), the step actions and expected results as plain text — 2.0.1 keeps a RichEdit blob in tcsteps, so the markup is stripped server-side, keeping the line break — and the four move buttons. A version with fewer than two steps gets no move operation at all, and draggable is off.

api/tcstepsreorder/index.php

Guards: session auth, bffSameOriginGuard(), bffEnforceSession() (this endpoint writes, so the legacy inactivity timeout is enforced), mgt_view_tc for the two reads and mgt_modify_tc for the three writes — always checked on the owning project, which is proved by walking nodes_hierarchy.parent_id up to the project node rather than trusting a submitted id. Every submitted step id must be a testcase_step child of the addressed version, a reorder list must contain every step of that version exactly once (duplicates and partial lists are refused), and a version with fewer than two steps refuses a move.

A write answers no_change (not ok) when neither the stored order nor the stored numbering moved, so a boundary click and a pointless renumber are reported honestly instead of as a success.

Error contract: 400 bad_param / unknown_action, 401 session_expired, 403 forbidden (rights, or a version of another test project), 404 not_found (unknown version, or a node that is not a version), 405 for a write verb on GET.

• $actions->tcStepReorder in lib/functions/common.php — the hub entry
• a rights-gated Reorder steps button on every version card of gui/templates/testcases/tcView.html, passing that card's own tcversion_id (with several versions the popup must re-order the version the user clicked on) and disabled when the version has no steps
• lib/ajax/stepReorder.php — the retired shim

tcsr.* + footers.tcStepReorder — 62 keys per bundle in all 10 bundles (python3 -m json.tool validated), with real translations, inserted in alphabetical position so the existing key order is untouched. The renumber copy carries a {count} placeholder, interpolated with the screen's tpl() helper.

Suite 1671 in tmp/TLU_Test_Cases.md — 55 cases, all PASS, across the version picker (including the dead-picker regression), rendering/data contract (RichEdit stripping, type chips, boundary state), move, drag → Apply → Discard, renumber, 15 error and rights paths, the legacy shim, wiring and i18n. Fixture tmp/fixtures_1671.php (project 1 StepReorder Demo / TSR1671 with a 4-step version containing an automated step and RichEdit markup, a 2-step and a 1-step version, a second version of the same test case, plus a norights user). Event Viewer clean — zero new rows after the whole matrix.

dd8e8b5b4 BFF → ce0e7d932 screen → 1ee7c403c i18n ×10 + aside link + tcView entry + legacy shim → f08cd9520 the three test-found fixes (#1673, #1674).

Fix commit f08cd9520, landed on the default branch with the parent screen (#1671) and therefore never closed as its own issue. Independently re-verified on fdc4d19a6 — see the Regression — Issue #1674 suite in tmp/TLU_Test_Cases.md (11/11 PASS). This section records the approach.

Approach. The screen is one page with two cards that are only ever toggled, never re-created (#picker style="display:none", gui/templates/testcases/tcStepReorder.html:151), and each card is reachable from both entry points — the hub entry ?tproject_id=N shows the picker, the tcView.html button shows the step table, Change version (backToPicker(), line 190/345) goes back to the picker and clicking a picker row (openVersion(), line 484) returns to the step table.

jQuery delegated handlers are bound to an element at bind time, and the original code branched inside the single $(function(){…}):

if (!TCVER_ID) { $('#pkRows').on('click','tr',…); loadVersions(); }
else           { $('#rows').on('click','button.op',…); …; load(); }

so it permanently bound one card and left the other unbound — 8 handlers (#pkRows click, #rows button.op, #mask backdrop, the 5 drag events, #btnReset). Symptom: the DOM rendered perfectly while the network layer stayed completely silent — no exception, no console error, 0 POSTs.

Fix. Bind every handler unconditionally in a plain block and branch only the initial load (tcStepReorder.html:671-741):

if (TCVER_ID) { load(); } else { loadVersions(); }   // line 741

Alternatives rejected. (a) Re-binding on every card switch — duplicates the handlers and leaks on each navigation. (b) Splitting the two cards into two URLs — changes the UX and breaks the tcView.html entry contract. (c) $.on delegation on document — works, but is broader than needed and makes the two-card structure implicit. Unconditional binding is what the sibling modernized screens do.

Root cause. tsroWriteResult() compared the sorted id sequences before and after the write. The id set is invariant under every operation this screen performs — only step_number changes — so sort() on both sides yielded two identical arrays for every successful write and the endpoint answered status=no_change. The client then took the correct branch (tcStepReorder.html:583-584), so the move persisted in the DB while the UI showed no success notice: the user was told nothing happened. The defect was in the server's verdict, not in the UI.

The mirror defect: ?action=normalize rewrites step_number only (11,12,13,14 → 1,2,3,4) and leaves the id order untouched, so a real repair was also reported as a no-op.

Fix (api/tcstepsreorder/index.php:463-468) — compare the ordered id sequence and the ordered step_number sequence:

$changed = ($before['ids'] !== $afterIds) || ($before['numbers'] !== $afterNumbers);

Both halves are required and they cover opposite cases: move/reorder change the id order, normalize changes the numbers alone. Comparing only ids leaves the normalize defect; comparing only numbers misses a reorder that restores the same numbering.

Alternatives rejected. Returning a plain ok and letting the client diff — would need the whole prior order shipped to the browser and still could not tell a boundary refusal from a real write; the server already has both snapshots.

TLi18n.t('tcsr.normalizeConfirm') was called with no parameters, so the {count} placeholder in the bundle was never substituted and the confirm dialog read "Renumber the steps from 1 to {count} …". The interpolator already exists in gui/templates/i18n/i18n.js; the two call sites just omitted the argument. Fixed at tcStepReorder.html:636 and :651 with TLi18n.t('tcsr.normalizeConfirm', { count: STEPS.length }).

tsroWriteResult() / tsroIdsOf() / tsroNumbersOf() are local to api/tcstepsreorder/index.php — 1 definition, 3 call sites (move / reorder / normalize), all in that one file; no other endpoint calls them. The handler-binding block is the only $(function(){…}) in the screen, so the unconditional-bind change cannot affect another screen. The shared legacy write primitive testcase::set_step_number() is untouched, so 2.0.1 rows stay byte-compatible with 1.9.20 and with every modern consumer that does ORDER BY tcsteps.step_number. The sibling screen api/tcreorder (#1660) re-orders the specification tree, not steps, and is unaffected.

Fixture: project 9001 BugRepro → test case 9051 → version 9101 with steps 9201..9204 numbered 11,12,13,14. (2.0.1 folds the tree into nodes_hierarchy; the picker only walks project -> testcase -> testcase_version -> testcase_step.)

• picker → row → Move to top → POST …{"action":"move","step_id":9202,"position":"top"} → 200 {"status":"ok"}, DB 9202→1, 9201→2, 9203→3, 9204→4, notice "Step moved to the top of the version."
• gaps 11,12,13,14 → Renumber steps → confirm → {"status":"ok"}, DB 1,2,3,4
• the no-op guard (the fix must not over-correct into "everything is ok"): 2nd normalize → no_change; move up on the first step → no_change "Already at the boundary"; reorder to the stored order → no_change "Order already up to date"
• dialog copy → "Renumber the steps from 1 to 4 keeping the current order?"
• drag row 1 onto row 4 → pending Do A, Do C, Do D, Do B; Discard changes → restored Do B, Do A, Do C, Do D, Apply/Discard disabled again
• ?tcversion_id=9101 → Change version → picker row has data-ver="9101", clicking it re-renders the 4-row step table
• Event Viewer: events holds 1 row (log_level 16, activity LOGIN) — no new Error/Warning; console 0 errors, 0 warnings