[Bug]: Personnel config "Members by Type"/"Members by Status" show every member as Unassigned — joins legacy member.field3/field21, the app writes member_type_id/member_status_id

[rjonesbsink]

Which version are you running?

4.2.25

How is it installed?

Git clone on Windows (IIS 10 / PHP 8.4.22 / MySQL 8.0.46)

What page or screen were you on?

Personnel configuration — the "Members by Type" / "Members by Status" summary panel

What went wrong?

Every member shows as Unassigned in both summary panels, including members with a real, correctly-set type and status that display fine on the roster.

This is the same legacy-column pattern as #95 and #103, in a third file: api/personnel-config.php joins the legacy member.field3 / member.field21 columns, while the app writes member_type_id / member_status_id.

Cause

inc/member-write.php:53-54 writes the modern columns:

'member_type_id'   => !empty($input['member_type_id'])   ? (int) $input['member_type_id']   : null,
'member_status_id' => !empty($input['member_status_id']) ? (int) $input['member_status_id'] : null,

api/members.php reads those same columns, which is why the roster looks right (:278-279, :452-453, :485-486, :549-550).

api/personnel-config.php reads the legacy ones instead — six sites, no COALESCE:

:102, :118   (SELECT COUNT(*) FROM member m WHERE m.field3 = mt.id) AS member_count
:133         (SELECT COUNT(*) FROM member m WHERE m.field21 = ms.id) AS member_count
:203-204     LEFT JOIN member_types  mt ON m.field3  = mt.id   GROUP BY m.field3,  mt.name, mt.color
:215-216     LEFT JOIN member_status ms ON m.field21 = ms.id   GROUP BY m.field21, ms.status_val, ms.color
:393         SELECT COUNT(*) FROM member WHERE field3  = ?
:449         SELECT COUNT(*) FROM member WHERE field21 = ?

Nothing writes field3 / field21 any more, so the joins never match and every member falls into the null group — rendered as "Unassigned".

The two delete guards are the sharper edge

:393 and :449 are not display code. They are the "is this type/status still in use?" checks that guard deletion. Because they count on the legacy column, a member type with members genuinely assigned to it counts as zero, and the guard will not stop the delete.

I have not driven a delete to see what the caller does with that count, so I can't say whether it silently orphans members or is caught elsewhere — but it seemed worth separating from the cosmetic half rather than filing it all as a display bug.

Suggested direction

The codebase already contains the shape for this, one file over. api/teams.php:72 does exactly the right thing for the integer FK case:

ON mt.id = COALESCE(m.member_type_id, m.field3)

That is the plain integer COALESCE, not the NULLIF(named, '') string form #95 needed — the correct choice here, since these are FK ids rather than text. Applying the same to the six sites in personnel-config.php would line it up with a pattern already shipping.

Whether the fallback is wanted at all is your call — it only matters if some install still has unmigrated data in field3/field21. A straight switch to the modern column would be simpler if not.

Prior art checked

Searched openises/TicketsCAD, open and closed, for field3, field21, personnel-config, "Members by Type", "Members by Status", member_type_id unassigned, plus audit legacy columns, field1, phantom, dead_control_audit and remaining field usages. Summary of what is and is not already covered, so this doesn't get closed as a duplicate of work that didn't actually reach this file:

• [Bug]: Personnel reports show blank Last/First/Callsign/Type — they read legacy member.field1-8, the app writes the named columns #95 (closed) — same pattern, but the fix was scoped to api/reports.php and the shared member-name label. It never touched api/personnel-config.php. git log confirms that file's last change was v4.2.8 (97b15cb), well before the [Bug]: Personnel reports show blank Last/First/Callsign/Type — they read legacy member.field1-8, the app writes the named columns #95 fix, and its reads are bare joins with no COALESCE.
• [Bug]: Import/Export member CSV exports blank names — reads legacy field1/2/4/6 only; the import side writes them back, leaving imported members nameless in the roster #103 (closed) — same pattern again, scoped to inc/import-export.php. Does not mention this file.
• Question: what should happen to controls and columns that exist but nothing reads? #91 (closed) — the nearest thing to a tracking issue, but it covers the inverse class: columns that exist and nothing reads. It settled the "disable with a badge, don't silently remove" policy. field3/field21 are the opposite case — actively read, never written — so Question: what should happen to controls and columns that exist but nothing reads? #91's resolution does not cover them.
• No open tracking issue exists for auditing the remaining field* reads.

One correction I owe this thread. api/personnel-config.php is already named in #95's comments, in the dead_control_audit output:

[NEW] phantom:member.field3   — read by api/reports.php, api/teams.php,
                                 api/compliance.php, api/personnel-config.php
[NEW] phantom:member.field21  — read by api/personnel-config.php

I characterised those findings as "a direct consequence of the #95 fix itself" and "the behaviour is intended: the two just disagree by construction." That was correct for api/reports.php, whose COALESCE fallback genuinely does read the legacy column on purpose — and wrong for api/personnel-config.php, which has no fallback and predates the fix. The audit tool was right and I waved it off. That is why the signal was already on the board and nobody looked further, and it is the reason I checked the git history for this one rather than trusting the earlier reading.

The same dismissal covered api/teams.php and api/compliance.php. I have now checked both, and they split:

• api/teams.php — correct, no action needed. :72 uses COALESCE(m.member_type_id, m.field3) and :68/:116 use COALESCE(NULLIF(m.available, ''), m.field8). The dismissal genuinely did hold for this one.
• api/compliance.php — same defect as this issue. :49 selects m.field3 AS type_id and :52 joins LEFT JOIN member_types mt ON m.field3 = mt.id, both bare. I have not exercised that screen to see how it surfaces, so I am flagging it rather than reporting a symptom for it.

Also worth considering

Counting #95 and #103, that is four files with the identical defect and one (teams.php) that got it right. The pattern is consistent enough that a targeted sweep of the remaining field* reads against their modern counterparts would probably be cheaper than another round of one-off reports — #91 closed without covering this class, so nothing is currently tracking it.

Happy to do that pass and report what it finds if useful. I would rather ask than file these one at a time.

[rjonesbsink]

Ran the sweep I offered above, since it was cheap. Two more files have the same defect, and one of them carries a second, older bug that is independent of the legacy/named split.

Method: every member.fieldN read under api/ and inc/, classified by whether the same line carries a COALESCE fallback. Migrations, schema installers and import-export.php (already #103) excluded.

Two more files with this defect

[ BARE ] api/ics-positions.php:58-63   m.field1 AS last_name, m.field2 AS first_name, m.field26 AS callsign
[ BARE ] api/time-entries.php:251      m.field2 AS first_name, m.field1 AS last_name
[ BARE ] api/time-entries.php:266      m.field2 AS first_name, m.field1 AS last_name, m.field4 AS callsign
[ BARE ] api/time-entries.php:279      m.field2 AS first_name, m.field1 AS last_name

Confirmed on this install. Member 2 holds first_name = 'test', last_name = 'user', with field1/field2/field4/field26 all NULL. Both queries, run exactly as written:

api/ics-positions.php:58  →  last=NULL  first=NULL  callsign=NULL
api/time-entries.php:266  →  first=NULL last=NULL   callsign=NULL

So ICS position qualifications and time entries both render nameless members, the same way Personnel reports did before #95.

ics-positions.php also has the field26 bug

Separate from the above, and older. :59 reads m.field26 for callsign. Every other file in the tree uses field4. api/teams.php:21 documents that exact mistake being found and fixed there:

 *   member.field4    = callsign  (PRE-RELEASE-FIXES #17 — was incorrectly field26)

and tests/test_pre_release_fixes.php:119-122 pins teams.php to field4 so it cannot regress. ics-positions.php was never given the same correction, so PRE-RELEASE-FIXES #17 is still live in one file.

Worth noting this half is broken on legacy installs too, not just modern ones — a pre-migration install with a real callsign in field4 still gets NULL here. The reason it may never have been noticed is tools/install_fresh.php:676, which mirrors field4 → field26 for non-empty callsigns at install time specifically to feed the old teams.php SELECT. A fresh install therefore has data in field26 and looks fine; an upgraded one does not.

field26 is varchar(1024) on this install, which also does not look like a column intended to hold a callsign.

What the sweep found clean

For completeness, so this reads as a bounded result rather than a spot check:

• Correct fallback already: api/equipment.php:316-318, api/external/v1/teams.php:56-58, api/reports.php (:1659-1661, :1691-1693, :1774-1782, :1845-1846), api/teams.php:68-72.
• Bare but not a defect: api/reports.php:1779, :1958, :1966 — m.available = 'No' OR m.field8 = 'No' reads both columns, so it already covers either shape.
• Not reads at all: api/members.php:67 and inc/functions.php:397 are comments about GENERATION_EXPRESSION.

Where that leaves the count

Six files touched by this pattern: api/reports.php (#95), inc/import-export.php (#103), api/personnel-config.php (this issue), api/compliance.php, api/ics-positions.php, api/time-entries.php. Plus api/teams.php, which got it right and is the working reference.

I have no view on whether you would rather fix these here or split them — compliance.php and personnel-config.php are the type/status half, while ics-positions.php and time-entries.php are the name half and match #95/#103 more closely. Happy to split them out if separate issues are easier to land, and equally happy to leave them collected here. The field26 item is the one I would keep distinct regardless, since it is a different bug that happens to live in the same query.

[ejosterberg]

Fixed, and thank you for the follow-up sweep — that's exactly the kind of methodical check that makes these fast to land.

personnel-config.php (7 sites): switched to COALESCE(member_type_id, field3) / COALESCE(member_status_id, field21), matching the exact shape from teams.php you pointed at. Both delete guards now count the modern column too, so a type/status with real members assigned actually blocks the delete.

ics-positions.php: fixed the last_name/first_name legacy fallback, and separately fixed the field26-instead-of-field4 callsign bug — confirmed your read that this was a second, older, independent mistake in the same query. It's now on field4, matching teams.php's PRE-RELEASE-FIXES #17 correction.

time-entries.php: fixed all three sites you found (list, pending-queue, single-entry).

compliance.php: not in your report, but found while working the same area — same field3 disease in the overview action's type join. There was also a second, unrelated, more serious bug in the same two queries: they selected m.name, which isn't a real column on that table at all (first_name/last_name are). That's a hard SQL error, so the compliance overview and expiring-certs endpoints have actually been throwing a 500 on every install, independent of the field3 issue. Fixed to build the display name from first_name/last_name.

All fixed with the same COALESCE pattern already used in #95/#103/teams.php. New regression tests drive the real fixed queries against fixture data (not hand-seeded ideal rows) and reverting any of the changes fails the relevant assertions. Deployed and verified live.

[rjonesbsink]

Merged and verified here. personnel-config.php, ics-positions.php and time-entries.php all check out — the field26 → field4 correction is in, and both delete guards count the modern column now. 13 of the 14 assertions in the new test pass.

The one that fails looks like a schema-shape assumption in the test rather than a straightforward defect, so flagging it mainly because it will trip other testers the same way.

FAIL  api/compliance.php's fixed overview query did not resolve as expected — got array (
  'id' => 36,
  'name' => NULL,
  'callsign' => NULL,
  'type_id' => 14,
  'type_name' => 'GH112CompType',
  'type_color' => '#123456',
)
=== 13 passed, 1 failed ===

Reproducible, and not stale fixture data — no leftover GH112 rows between runs, and the id increments each time.

Why it fails here and presumably not on your installs

The assertion just above it reads:

PASS the fixed name-resolution query shape correctly resolves a real member on this install's generated-column schema (field1/field2/field4 -> last_name/first_name/callsign)

That description does not hold here. SHOW CREATE TABLE member on this install:

`first_name` varchar(64) DEFAULT NULL,
`last_name`  varchar(64) DEFAULT NULL,
`callsign`   varchar(32) DEFAULT NULL,
`phone_cell` varchar(20) GENERATED ALWAYS AS (`field7`) VIRTUAL,

Only phone_cell is generated. The three name columns are plain.

That matters for the compliance fixture at :240, which inserts field1/field2 only:

"INSERT INTO `{$prefix}member` (`field1`, `field2`, `member_type_id`) VALUES ('ComplianceGenLast', 'ComplianceGenFirst', ?)"

On a schema where the named columns are GENERATED from the legacy ones, that insert populates first_name/last_name as a side effect and CONCAT resolves. On a plain-column schema it does not, so CONCAT(m.first_name, ' ', m.last_name) returns NULL and the assertion fails. type_id resolves correctly either way (14, with the right type name and colour), so the field3 half of the fix is confirmed working — it is only the name half that is schema-dependent.

So the test passes or fails purely on which shape the install has, which is worth knowing regardless of what you decide about the query itself.

The narrower point about the query

compliance.php reads the modern columns bare, where the sibling files fixed in the same batch fall back:

api/compliance.php:59    CONCAT(m.first_name, ' ', m.last_name) AS name, m.callsign
api/compliance.php:194   CONCAT(m.first_name, ' ', m.last_name) AS member_name, m.callsign
api/ics-positions.php:71 COALESCE(NULLIF(m.last_name,  ''), m.field1) AS last_name
api/time-entries.php:257 COALESCE(NULLIF(m.last_name,  ''), m.field1) AS last_name

On a plain-column install, a member whose names live only in the legacy columns comes back with name NULL — and because CONCAT() propagates NULL from any argument, there is no partial result either. m.callsign has the same gap against m.field4.

I want to be straight that I have not shown this affecting a real user. On this install every member has the modern columns populated:

members total:                    1
with modern names populated:      1   -> CONCAT resolves fine
legacy-only names (modern NULL):  0   -> would return NULL

So the compliance overview works correctly here, and the important half of your fix — the endpoints returning rows at all instead of a 1054 — is confirmed. The gap is only reachable on an install that is both plain-column and still carrying legacy-only member rows. Whether that combination is worth covering is your call; if it is, CONCAT_WS(' ', ...) skips NULLs rather than propagating them, which reads closer to the sibling files than wrapping each argument.

Environment: v4.2.25 plus today's syncs, 212 migrations applied, 0 failed, idempotent re-run reports Pending: 0, schema OK at 190 tables / 1228 columns. Windows 11 / IIS 10 / PHP 8.4.22 / MySQL 8.0.46.

[ejosterberg]

Good catch, and you've independently confirmed something CI had already caught on my end minutes before your comment came in — small enough gap that a re-pull will already have it.

Your diagnosis is exactly right: the compliance.php fixture only ever wrote field1/field2, assuming first_name/last_name would mirror them (true on my dev database, which has those as generated columns — not true on a genuinely fresh install, where they're plain). CI's own fresh-install job hit precisely the failure you reproduced, which is how it got caught before it reached you. Fixed by probing information_schema for whether first_name is a generated column and writing whichever shape the install actually has — same technique an earlier test in this codebase already uses for this exact difference.

On the CONCAT-vs-COALESCE point: agreed compliance.php reads the modern columns bare rather than falling back to the legacy ones the way ics-positions.php/time-entries.php do. That was a deliberate, narrower choice for this fix specifically, since compliance.php isn't in the original report and the primary bug there was the hard SQL error (m.name) — I didn't want to widen scope into the full legacy-fallback treatment without it being asked for. You're right that it's a real gap on an install that's both plain-column and still has legacy-only rows. I'll leave it as-is unless you're hitting it on a real install — happy to add the COALESCE/CONCAT_WS treatment if so, and your CONCAT_WS suggestion is the right shape if it comes to that.

This CI-fix commit is on the public repo now (pushed right around when your comment landed) — a fresh git pull should get you to 14/14.

[rjonesbsink]

Verified — closing.

Verified on v4.2.26, merged and migrated on this install (212 migrations applied, 0 failed; idempotent re-run reports Pending: 0, schema OK at 190 tables / 1228 columns). Windows 11 / IIS 10 / PHP 8.4.22 / MySQL 8.0.46.

All four files confirmed, plus the test fix. tests/test_gh112_personnel_legacy_columns.php: 14 passed, 0 failed.

• personnel-config.php — 7 sites on COALESCE(member_type_id, field3) / COALESCE(member_status_id, field21), and both delete guards now count the modern column, so a type in real use actually blocks the delete.
• ics-positions.php — legacy name fallback in, and field26 corrected to field4 for callsign.
• time-entries.php — all three sites.
• compliance.php — the m.name 1054 is gone; overview and expiring-certs return rows instead of 500ing.

The test fixture fix deserves its own note: it now detects whether the install has GENERATED or plain name columns and writes whichever ones actually populate first_name/last_name. That is exactly the kind of assumption that only surfaces on someone else's schema, and it would have tripped every tester whose columns are plain rather than generated.

The compliance.php find is the one I would highlight — m.name is not a column on that table at all, so those two endpoints had been throwing on every install, independent of the legacy-column issue and invisible from outside. That came out of extending the sweep rather than fixing only what was reported.

Six files now carry this pattern correctly, with teams.php as the reference that had it right all along.