Refactor: Playersearch standardization — active/inactive/banned tiers, full surface conversion, Advanced Search by baltinerdist · Pull Request #477 · amtgard/ORK3

baltinerdist · 2026-05-26T18:58:17Z

Opened early for visibility/review. Reviewed separately from #476 (units work + standalone award-giver fix).

What this does

Replaces ~40 divergent player-search implementations (6 incompatible backend endpoints, jQuery-UI + bespoke dropdowns) with one standardized player search, then layers on a consistent active → inactive → banned result model and a rich Advanced Search page.

One core, one endpoint, one component

One ranking core — SearchService::RankedPlayers(): concentric-ring ranking centered on the surface (Park → Kingdom → Everywhere), rank-don't-exclude by default, opt-in restrictTo / restrictKingdomIds / excludeKingdomId / excludeParkId / excludeIds. Kills the recurring "can't find people outside my kingdom" bug class.
One canonical endpoint — SearchAjax/players (+ session-less SOAP Search/Players for testing), returning an envelope {rows, hasMore, offset}. The 6 legacy endpoints (KingdomAjax / ParkAjax / SearchAjax-universal / AdminAjax / EventAjax playersearch + SearchService::Player) delegate to the core so ranking can't drift again.
One component — OrkPlayerSearch (custom dropdown, never jQuery UI; fixed-position for modals; dark-mode; ARIA combobox; excludeIds / reattach / preload / onClear), loaded app-wide.

Active / inactive / banned result model (new)

Every surface now returns three tiers in one consistent order:

Active first, then Inactive as a last resort (always shown, ranked after all active).
Banned (suspended OR penalty_box) shown only to viewers with CREATE/EDIT authority relevant to the surface, and scoped one level up: a park surface surfaces the kingdom's banned players; a kingdom / global / universal surface surfaces Amtgard-wide bans. Plain users never see banned players anywhere.
restrictTo:'kingdom' is now principality-family-aware (a "within kingdom" / kingdom-merge search still finds principality members).
Distinct Banned (red) / Suspended (amber) / Inactive (gray) badges; inactive rows dimmed, banned rows tinted; dark-mode aware.

Advanced Search page (new)

A full-page power-search at Search/advanced, opened in a new tab from the inline component's split Load more… | …or Advanced Search footer (carrying the typed term + current scope):

Filters: Active / Inactive toggles, officer-gated Banned (Amtgard-wide), kingdom→park cascade, optional last-attendance date range.
Results render as a DataTable (Reports styling): zebra striping, pagination, "Show N rows" / "Showing X–Y of N", sortable headers, responsive (no horizontal scrollbar on desktop, columns collapse on mobile).
Columns: persona, real name (officers only; a restricted player's real name is admin-only), home kingdom/park, last-attendance kingdom/park + date, status pill, link-out, and a copy-Player-ID button (instant CSS tooltip "Copy Player ID - Paste this into your search!").
Kingdom names are shortened in-cell ("The Kingdom of the Wetlands" → Wetlands).
Paste-an-ID flow: a pure-numeric query (in the inline component or Advanced Search) is a direct mundane_id lookup — copy an id here, paste it into any player search.

Surfaces converted (~40 call sites)

Award giver/recipient, attendance (kingdom / park / event + the Parknew sign-in modal with its scope toggle, already-attended exclusion, and inactive-reactivation confirm), recommendations, Move Player (Kingdom / Park / Admin — mode-driven exclude/restrict + cascade re-scope), Merge (cross-field exclude; kingdom merge now restrictTo:'kingdom'), Unit add member/manager (intentionally global), officer / permission grants (scoped + global), reconcile, Edit Officers (Kingdom + Park), and the universal header search.

Security

mysql_real_escape_string is a no-op polyfill in this codebase, so player/term LIKE interpolation was unescaped (SQL-injectable). Replaced every term interpolation in RankedPlayers, the legacy Player(), and the Event/Kingdom/Park name searches with a real likeEscape() (quote-doubling + % _ \ escaping). Banned-visibility and restricted-real-name gating are enforced server-side from the session token — client params can't widen them.

Surface bug fixes & cleanup

Award "Given At" handler no longer wipes the selected recipient id.
Reports_roster "Suspended By" passes the correct entity for park-scoped rosters.
Admin Move Player "current park" select re-scopes the search; Attendance re-scopes on kingdom/park change.
Removed dead #PlayerSearch jQuery-UI bindings and the superseded EventAjax/AdminAjax playersearch endpoints.

Testing

tests/playersearch/banned_matrix.sh — authenticated full matrix (active/inactive/banned × park/kingdom/global × anon/plain/officer/admin), 15/15 green; tests/playersearch/ranking.sh 12/12.
tests/playersearch/chrome-matrix-protocol.md — full in-Chrome test protocol.
Verified live in Chrome (dark mode): one-level-up banned scoping, attendance scope toggle, Move Player context, the Advanced Search DataTable, and the inline → Advanced Search → paste-ID round-trip.

Notes for review

Banned players are shown AND selectable on every surface (product decision) — rows are badged but not blocked; server-side operations still enforce their own rules.
Legacy default-template admin pages (Set Officers, Authorization, Event, Audit Log, the public Search page) are out of scope here (not part of the 3.5.x design language).
The newly-unused KingdomAjax/ParkAjax playersearch endpoints were left in place (harmless) now that move-player routes through SearchAjax/players.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Adds SearchService::RankedPlayers() with concentric-ring ranking (park->kingdom->global), abbreviation-prefix support via resolveAbbrevPrefix(), and SOAP registration for the session-less Search/Players action. 12-assertion curl ranking test. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

… SearchService::RankedPlayers Adds Controller_SearchAjax::players() as a same-origin, session-gated endpoint that delegates entirely to the existing SearchService::RankedPlayers() core — no SQL duplication. Returns [] for unauthenticated callers. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…k 3) New JS + CSS files implementing the one canonical player-search dropdown used by all search surfaces. Custom dropdown (no jQuery UI), position:fixed for modal safety, dark-mode via html[data-theme="dark"], &q= URL param, 220 ms debounce, keyboard navigation, and onSelect callback. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Add CSS and JS includes for the new OrkPlayerSearch component to the master theme file, making it available app-wide. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ilot) Replaces both jQuery UI .autocomplete() player-search blocks with OrkPlayerSearch.attach(); removes unused preloadedGivenByOfficers array. GivenBy is global (cross-kingdom, includeInactive+Suspended); GivenTo is kingdom-centered, active-only. Establishes the conversion recipe for the rollout. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…nent Replace jQuery UI autocomplete on #PlayerName in Attendance_kingdom, Attendance_park, and Attendance_event with OrkPlayerSearch.attach — single ranked server-side call (park→kingdom→global), no more 3-parallel-request hack in Attendance_park. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Replace three bespoke autocomplete implementations (edit-suspension "Suspended By", suspend-player "Player", suspend-player "Suspended By") with OrkPlayerSearch.attach. Park-scoped player search re-attaches on park dropdown change to preserve park filtering. Removes ~120 lines of duplicated fetch/render/keynav wiring and the three bespoke result divs. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ponent Convert #GivenBy (Admin_player), #PlayerName (Admin_suspendplayer, Admin_banplayer, Admin_moveplayer) and #PlayerName + #CreatePlayerName (Admin_manageevent) from jQuery UI .autocomplete() to OrkPlayerSearch.attach(). Uses ranked SearchAjax/players route; PenaltyBox radio logic updated to use p.Suspended from RankedPlayers response. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…h (superset of parallel + concurrent work) Converts 8 clean-path Xnew award/recommendation searches to OrkPlayerSearch.attach (pn-award-givenby, pn-edit-givenby, kn-award-player/givenby/rec, pk-award-player/givenby/rec), preserving officer-chip clearing and awardranks-on-select. Move/merge/event/reconcile deferred to the exception register. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…dditive rules) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Additive, backward-compatible: excludeIds (array or per-search fn); setOpts()/reattach() for live re-scoping (move-player cascades); preload list on empty-focus. Existing attach() unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…:RankedPlayers KingdomAjax/ParkAjax/EventAjax/AdminAjax playersearch + SearchAjax::universal player branch delegate to the shared ranking core (consistent park->kingdom->everywhere). Adds excludeKingdomId/excludeParkId for scope=exclude. Response shapes preserved; Search/Player SOAP untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Converts 5 bespoke exception surfaces in revised.js to the standard OrkPlayerSearch component, adding excludeKingdomId/excludeParkId support to SearchAjax/players and the OrkPlayerSearch component along the way. - kn-moveplayer: attach+reattach on mode change; 'in'→excludeKingdomId, 'within'/'out'→restrictTo:'kingdom' - pk-moveplayer: attach+reattach on mode change; 'in'→excludeParkId, 'out'→restrictTo:'park' - kn-merge (keep+remove): attach with cross-field excludeIds fn - pk-merge (keep+remove): attach with restrictTo:'park' + cross-field excludeIds fn - ev-PlayerName: attach with parkId+kingdomId ranking + dynamic excludeIds from attended rows; includeInactive+includeSuspended preserved Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…h component Replaces legacy cpAc()+cpSearchPlayersGlobal() calls in Admin_index.tpl with OrkPlayerSearch.attach() for the Move Player and Merge Players modals. Merge Players uses excludeIds lambdas so each field suppresses the player already chosen in the other field. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…omponent Replace both jQuery UI autocomplete calls in Admin_unit.tpl with OrkPlayerSearch.attach — global, unscoped (no parkId/kingdomId/restrictTo), includeInactive:true per the documented cross-kingdom/cross-park exception. Also removes the stale dead `park_id = $('#ParkId').val()` assignment that was never passed to the old AJAX call. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

… search to OrkPlayerSearch Replaces the hand-rolled kn-ac fetch/render/keynav wiring in both permission templates with OrkPlayerSearch.attach, using appropriate scope: kingdom-scoped or park-scoped for the scoped page (determined at runtime from $type), and global (no restrictTo) for the global admin page. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…th cross-field excludeIds Replaces both #FromPlayerName and #ToPlayerName jQuery UI .autocomplete() calls with OrkPlayerSearch.attach(). Each field passes excludeIds: function() returning the other field's currently-selected MundaneId, preventing a player from being merged into themselves. Global scope (no center/restrictTo), includeInactive: true. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…nent Replace bespoke acSearch-based givenby autocomplete in wireRow() with OrkPlayerSearch.attach(), keyed to RcConfig.kingdomId for ranking with global scope (no restrictTo), includeInactive+includeSuspended for historical attribution. Location search left untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…yerSearch Last straggler jQuery-UI player autocomplete (#Suspendator), kingdom-scoped to the recording officer's kingdom. Completes the player-search standardization. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…Clear, a11y) + backend fixes Component: AbortController cancels stale in-flight searches; module-level shared click/scroll listeners (no per-attach leak); new onClear callback fired on edit so callers clear stale hidden ids; live uir read; ArrowUp guard; dark-mode badge contrast. Backend: AdminAjax/global passes session token (admins can find restricted-name players); Search/Players SOAP registration gains excludeKingdomId/excludeParkId; Reports_roster sp-player search uses reattach so park-change re-scopes. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ale hidden id on re-edit Converted templates dropped the old jQuery-UI change-handler that cleared the hidden MundaneId when a user edited the text after selecting. Wire onClear on every attach/reattach so the hidden id (and submit-enable state) is cleared the moment the user modifies the input, eliminating stale-id form submissions. Also replaces the redundant manual input-event listeners in Admin_permissions.tpl and Admin_permissions_global.tpl with the canonical onClear callback. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…arch.attach call sites Adds onClear callback to all 15 OrkPlayerSearch.attach calls in revised.js. Without it, editing the player name field after a selection left a stale hidden id, causing the wrong player to be submitted. Award givenby surfaces also deselect officer chips on clear, matching their onSelect behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

After selecting a player and editing the text, the hidden id stayed stale. Adds onClear to cp-mp-player-name (Move Player), cp-mgp-keep-name and cp-mgp-remove-name (Merge Players) in Admin_index.tpl, and to the wireRow givenby attach in Playernew_reconcile.tpl, each mirroring its onSelect minus the id-set so the submit guard re-evaluates correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ehind modals) Modal overlays use z-index up to 999999; the .ops-ac-results dropdown was 10000, so inside any modal (Xnew award/rec/move/merge, Admin modals) the ranked results rendered BEHIND the modal and appeared as zero results. Raised to 2147483000. Non-modal pages were unaffected. Found via in-browser visual testing (headless DOM reads saw the items but not the occlusion). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…(was static) Conversion baked a static $kid/$pid; the old code read $('#KingdomId').val() live so the player search followed the kingdom/park selector. Restored: init scope from the live hidden fields and reattach() on #KingdomName/#ParkName select/change so ranking tracks the chosen scope. (Attendance_event keeps the event's fixed location.) Found via in-browser testing. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…rs autopopulate; convert Edit Officers search to playersearch Officer data uses lowercase roles (monarch/regent/prime_minister/gmr) but code matched title-case ('Monarch'/'Regent'): - controller.Kingdom/Park: $preloadOfficers built nothing → award 'Given By' quick-buttons (kingdom/park monarch+regent) never rendered. Now case-insensitive. - revised.js Edit Officers: officerMap keyed/looked-up by title-case → no autopopulate, Vacate hidden. Now normalized (normRole) so current holders prefill + Vacate shows. - Edit Officers per-role search was a MISSED conversion (jQuery UI Search/Player) → now OrkPlayerSearch (kingdom-scoped), modal style unchanged. Pre-existing role-case bug; found via in-browser testing. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…nversion as Kingdom Mirror of the Kingdom Edit Officers fix for the park officer editor (normRole autopopulate so current holders prefill + Vacate shows; per-role search converted from jQuery UI to OrkPlayerSearch, kept kingdom-wide per the existing comment). Verified: 5 park holders autopopulate with Vacate. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ero monarch/regent display Same lowercase-role mismatch as Kingdom/Park: $preloadOfficers (award Given By quick-chips) on Playernew/Admin/legacy-Award pages never populated, and Kingdomnew/Parknew hero monarch/regent extraction never matched. All officer-role comparisons now case-insensitive; chip Role labels prettified (e.g. 'Kingdom Monarch'). Verified: Playernew add-award shows 4 chips (kingdom + park monarch/regent). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…SQLi fix Rework SearchService::RankedPlayers into a three-tier model (active → inactive last-resort → banned) behind every OrkPlayerSearch surface: - Inactive players always included, ranked after all active (last resort). - Banned (suspended OR penalty_box) shown ONLY to viewers with CREATE/EDIT authority relevant to the surface, scoped ONE LEVEL UP: a park surface surfaces the kingdom's banned players, a kingdom/global surface surfaces Amtgard-wide. Plain users never see banned. - restrictTo:'kingdom' is now principality-family-aware (centralized). - Added restrictKingdomIds (fixes KingdomAjax scope=own silent-drop bug), excludeIds, offset, and a stable mundane_id paging tiebreak. - SECURITY: mysql_real_escape_string is a no-op polyfill here — replaced every player/term LIKE interpolation in RankedPlayers + legacy Player() + the Event/Kingdom/Park name searches with a real likeEscape(). - Pure-numeric query is a direct mundane_id lookup (paste a copied id). Endpoint: SearchAjax/players now returns {rows,hasMore,offset} (limit+1 probe) and accepts the full surface context; the 5 other RankedPlayers callers convert to the options array. Component: Load more pagination, server-side excludeIds, context opts, ARIA combobox + live region, distinct Banned/Suspended/Inactive badges, robustness (abort-on-close, error/loading rows, resize reposition, blur-close, scrollIntoView, flip-up), and refresh() for scope-toggle re-query. Adds tests/playersearch/banned_matrix.sh (authenticated full matrix, 15/15). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

…+ fixes Convert every remaining hand-rolled 3.5.x player search to the shared component: - Parknew "Enter Attendance" modal (scope toggle park/kingdom/global via reattach+refresh, already-entered excludeIds, inactive-reactivation confirm and last-class prefill preserved). - Kingdom/Park "Move Player" (mode "in" → excludeKingdomId/excludeParkId, "within"/"out" → restrictTo) and Admin control-panel Move Player (cascade = hard kingdom/park filter). - Unit add member/manager (stays GLOBAL per the documented exception; excludeIds = current members). Surface bug fixes: - Award GivenAt handler no longer wipes the recipient MundaneId. - Reports_roster Edit-Suspension "Suspended By" passes the correct entity (park id on a park-scoped roster, not as kingdomId). - Admin Move Player SrcPark select re-scopes the player search. - Attendance_event re-scopes when the kingdom/park selector changes. - Kingdom merge gains restrictTo:'kingdom' (parity with park merge). Dead code removed: #PlayerSearch jQuery-UI bindings in default.js + orkui.js, and the superseded EventAjax auth/playersearch + AdminAjax global/playersearch action branches. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

New full-page rich player search (route Search/advanced, opens in a new tab from the inline component's split "…or Advanced Search" footer, carrying the typed term + surface scope). Officer/admin tool styled with the Reports look. - Backend SearchService::AdvancedPlayers(): same tier/auth model as RankedPlayers plus home + last-attendance (kingdom/park/date) columns and gated real names (officers see them; a "restricted" player's real name shows only to global admins). Filters: Active/Inactive toggles, officer-gated Banned (Amtgard-wide), kingdom→park cascade, optional last-attendance date range. needFilter guard prevents a full-table scan. - Page: jQuery DataTable (responsive — no horizontal scrollbar on desktop, collapses on mobile) with zebra striping, pagination, "Show N rows" / "Showing X–Y of N", sortable headers; shortened kingdom names ("The Kingdom of the Wetlands" → "Wetlands"); per-row link-out + copy-ID button (instant CSS tooltips, never native title); dark-mode throughout. Fixes a theme-wide duplicate-scrollbar (html,body height:100% + overflow-x forcing body into its own scroll container), page-scoped. Adds tests/playersearch/chrome-matrix-protocol.md (full in-Chrome test matrix). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

…overflow-y:auto) The mobile-nav width clamp set `html, body { overflow-x: hidden }`, which per the CSS spec forces `overflow-y` to compute as `auto`. Combined with `height: 100%`, that turned <body> into its own vertical scroll container on top of the window — a duplicate scrollbar on every tall page. Switch to `overflow-x: clip`: it still clamps horizontal overflow (preserving the Android fixed-bar fix) but does NOT establish a scroll container or couple overflow-y, so the body no longer scrolls and only the viewport's single scrollbar remains. Verified across Home, Kingdom, and Advanced Search (light + dark). Removes the now-redundant page-scoped html/body override added to Search_advanced.tpl. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

baltinerdist and others added 30 commits June 9, 2026 17:14

Docs: playersearch standardization design spec

b763a1e

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Docs: playersearch standardization implementation plan

31ce32f

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Enhancement: load OrkPlayerSearch component globally in theme

5f81023

Add CSS and JS includes for the new OrkPlayerSearch component to the master theme file, making it available app-wide. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Docs: playersearch exception register (deferred surfaces + proposed a…

f8aeca6

…dditive rules) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

baltinerdist force-pushed the refactor/playersearch branch from 83bb92b to fcb5bdc Compare June 9, 2026 21:25

baltinerdist and others added 3 commits June 10, 2026 11:12

baltinerdist changed the title ~~WIP: Playersearch component standardization~~ WIP: Playersearch standardization — active/inactive/banned tiers, full surface conversion, Advanced Search Jun 10, 2026

baltinerdist changed the title ~~WIP: Playersearch standardization — active/inactive/banned tiers, full surface conversion, Advanced Search~~ Refactor: Playersearch standardization — active/inactive/banned tiers, full surface conversion, Advanced Search Jun 10, 2026

baltinerdist marked this pull request as ready for review June 10, 2026 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor: Playersearch standardization — active/inactive/banned tiers, full surface conversion, Advanced Search#477

Refactor: Playersearch standardization — active/inactive/banned tiers, full surface conversion, Advanced Search#477
baltinerdist wants to merge 34 commits into
amtgard:masterfrom
baltinerdist:refactor/playersearch

baltinerdist commented May 26, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

baltinerdist commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this does

One core, one endpoint, one component

Active / inactive / banned result model (new)

Advanced Search page (new)

Surfaces converted (~40 call sites)

Security

Surface bug fixes & cleanup

Testing

Notes for review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

baltinerdist commented May 26, 2026 •

edited

Loading