Skip to content

Modernize Move Reorder Test Suites suiteMove

github-actions[bot] edited this page Oct 14, 2026 · 5 revisions

Modernize: Move / Reorder Test Suites (suiteMove) — Refs #1740

Modern screen: gui/templates/testcases/suiteMove.html BFF: api/suitemove/index.php Entry point: Test Specification toolbar → Move / Reorder Test Suites (testSpec.html) Legacy predecessor: lib/ajax/dragdroptreenodes.php

Why

In 1.9.20 the move / re-parent / re-order of a test suite was performed by the ExtJS tree itself through lib/ajax/dragdroptreenodes.php — a GET request that called tree::move_node() and wrote nodes_hierarchy.parent_id. It had

  • no rights check (mgt_modify_tc was never consulted),
  • no proof that the moved node or the destination node belonged to the project,
  • no same-origin check, so any bookmark, crawler or cross-site <img> hit could re-parent a suite.

The endpoint is now a non-mutating session-guarded shim (browser → 302 to the modern screen, XHR → an escaped fragment); its legacy writes are never executed.

API

Route Purpose
GET ?action=init&tproject_id=N&container_id=M context (project name (prefix), container, rights can_modify) + the ordered sub-suites with test-case / sub-suite counts
GET ?action=suites&tproject_id=N[&exclude_id=K] the depth-indented list for the destination picker (excluding K and its whole subtree)
POST ?action=move {node_id, new_parent_id, position: top|bottom}
POST ?action=reorder {container_id, nodelist: "id,id,…"}

Contract: 401 anonymous, 403 no right / CSRF, 404 unknown or foreign node or project, 400 bad parameter, 405 wrong verb, 409 cycle, and a no-op is a success 200 {"status":"no_change"} (same as tcreorder, tcstepsreorder, reqtreereorder).

Guarantees

  • mgt_modify_tc on the owning project.
  • Every id proven to be a node_type_id = 2 node of that project (the ownership walk also proves a suite really hangs off a test project).
  • A container can never be moved into its own subtree (cycle guard).
  • The destination picker walks down from the project node, so the walk itself proves ownership — no per-suite query (#1757).

Bugs found while testing

Issue Defect
#1742 every suite listed twice in the picker
#1743 "move to first/last" refused for every legitimate move (node_order is not dense in 2.0.1)
#1744 empty project name in the context bar
#1745 cycle guard inverted — moving a container into its descendant detached the subtree
#1746 a real reorder answered no_change
#1747 can_modify ignored, write actions always rendered
#1748 one E_WARNING: Undefined array key "node_type_id" per row
#1750 toolbar tooltip reused a row-button title
#1751 another agent's commit deleted all smv.* keys from the 10 bundles

Mandatory code-review fixes

Issue Defect
#1752 the container picker filtered the current container out of its own selector
#1753 a no-op answered 400 status:error; the reorder no-op was decided after the write
#1754 fill() re-escaped TLi18n.t()'s output — R&D Suite → R$$&D Suite
#1755 BUSY latched forever on an unhandled path, disabling every action
#1756 a write after the session timeout printed a raw server string instead of the login page
#1757 the picker was O(all suites × depth) across the whole installation
#1758 a malformed new_parent_id (float / scientific / JSON body) was silently degraded into an in-container reorder — see Bugfix-Issue-1758
#1759 403 on a foreign suite leaked that node's existence (now 404) — see Bugfix-Issue-1759

Bugfix #1751 — smv.* i18n keys verified restored in all 10 bundles

Concurrency clobber: commit 9ca0fa7bb ("feat(tcview): full platforms panel", Refs #1037) rewrote all ten gui/templates/i18n/*.json from a stale base, so every smv.* key another agent had added in 0674bbdd5 was dropped (git show 9ca0fa7bb:gui/templates/i18n/en.json counts smv.* = 0). TLi18n then fell back to the raw key / hardcoded English (gui/templates/i18n/i18n.js:175), so the whole screen rendered untranslated outside English.

The repair landed in 7163125d6 (sibling fix for #1750), which re-added the keys in all ten bundles; that commit is an ancestor of the default branch, so the symptom is not reproducible. This ticket was never closed after the fix. Verification only — no new code change.

Evidence:

  • working-tree en.json → 48 smv.* keys; every bundle de/es/fr/it/ja/pt/ro/ru/zh → 48 each.
  • every smv.* + footers.suiteMove key referenced by suiteMove.html / testSpec.html exists in en.json → missing: [].
  • bash ai/verify_i18n_coverage.sh → 9 bundle(s) passed, 0 failed, exit 0 (7369 keys each).
  • runtime ?locale=ro → title "Mută / reordonează suite de test", table header "Suite de test", footer "TestLink 2.0.1 - Mută / reordonează suite de test", move modal "Mută suita de test" with the {suite} placeholder interpolated; ?locale=de → "Testsuite verschieben / neu sortieren".
  • events table after the test: 1 audit LOGIN row (log_level=16), no Error/Warning; console clean.

Regression suite Issue #1751 in tmp/TLU_Test_Cases.md: 10/10 PASS.

Bugfix #1757 — the destination picker no longer scans the whole installation

suitMoveAllSuites() (api/suitemove/index.php:509) backs the destination <select> of the screen. Before 13f2ca183 its body (13f2ca183^) issued one unfiltered SELECT … FROM nodes_hierarchy WHERE node_type_id = <testsuite> — every suite of every test project — and then called suitMoveOwningProject() for each row (one SELECT per ancestor, up to 64). Cost per request was therefore O(all suites × depth), independent of the project being addressed, on both entry points (init :684, suites :704); measured on a 164-suite installation it took 1391 SQL statements just to list a 2-suite project. The 13f2ca183 rewrite walks down from the project node (WHERE parent_id = <visited id> per level, cached, depth-first, $visited set): the walk itself proves ownership, so no per-row ancestor query is needed (same shape as tcreoSuitesOf(), api/tcreorder/index.php:392). That commit is an ancestor of the default branch, so the symptom is not reproducible. This ticket was filed (2026-09-30T13:31:13Z) 43 s after the fix commit was authored (13:30:30Z) — it documented code that no longer existed by the time it landed. Verification only — no new code change.

Evidence (same fixture, same DB — pre-fix body reconstructed verbatim from 13f2ca183^ as a temporary sibling endpoint, removed afterwards):

  • action=init SQL statements (SHOW GLOBAL STATUS LIKE 'Questions', warmed): OLD 1391 / NEW 29 for SM1757SMALL (2 suites); OLD 2765 / NEW 211 for SM1757BIG (162 suites). The new cost tracks the addressed project, not the installation.
  • ?action=init&tproject_id=1 → all_suites = exactly suites 2,3 (none of BIG's 162).
  • ?action=suites&tproject_id=4&exclude_id=5 → 144 entries; excluded suite 5 and its whole subtree absent.
  • git merge-base --is-ancestor 13f2ca183 origin/sebiboga → true.
  • events table after the run: only log_level=16 LOGIN / testproject-CREATE audit rows; 0 Error/Warning (log_level IN (1,2)).

Regression suite Issue #1757 in tmp/TLU_Test_Cases.md: 9/9 PASS. No screenshot: this is a backend query-count defect with no UI change.

Verification

Suite 1740 in tmp/TLU_Test_Cases.md: 57/57 PASS (incl. 11 review-fix cases) + 30 API contract checks. Event Viewer clean (0 new Error/Warning rows). Screenshots: docs/screenshots/issue-1740-suitemove-0{1,2,3}-*.png.

Screenshots

Bugfix #1751 — screen rendered in Romanian (keys no longer fall back to English)

Move / Reorder Test Suites in Romanian

The screen for Top A (2 sub-suites)

Move / Reorder Test Suites

The Move-to dialog (destination excludes the suite and its whole subtree)

Move dialog

Access denied (role 3, <no rights>)

Access denied

Clone this wiki locally