Morning Check Groups and Routing Developer Guide

Morning check groups and routing — Developer Guide

How optional grouping and routing are stored and resolved, why "not checked" is a deleted row, why attribution needed a new column rather than the obvious one, and the four bugs the build surfaced. The plain-language version is Morning check groups and routing.

Asked for in discussion #64. Shipped in 262d7af8.

---

1. 📁 The files involved

🔵 Schema

File	Role
database/freeitsm.sql	Table definitions and the foreign keys
includes/db_verify_schema.php	Columns + PK for auto-creation on verify
includes/db_verify_indexes.php	ix_mcg_team, ix_mcg_analyst, ix_mcc_group, ix_mcc_analyst, ix_mcrl_result
api/system/db_verify.php	$primaryKeys entries for the two non-id PKs

🟢 Service layer

File	Role
includes/services/morning_checks.php	listGroups(), saveGroup(), deleteGroup(), resolveAssignment(), clearResult(), assertExists(), nullableId(); plus the group/analyst handling in saveCheck() and ModifiedBy in recordResult()

🟠 API

File	Role
api/morning-checks/get_groups.php	Groups with their routing and check counts
api/morning-checks/save_group.php	Create/update — needs Cap::MORNING_CHECKS_GROUPS
api/morning-checks/delete_group.php	Delete; returns how many checks were ungrouped
api/morning-checks/clear_check_result.php	Put a check back to not checked
api/morning-checks/link_result.php	Record a raised ticket/task against a result
api/morning-checks/get_todays_checks.php	Rewritten SELECT — grouping, routing, attribution
api/morning-checks/get_all_checks.php	Now returns GroupID / AssignedAnalystID
api/morning-checks/add_check.php, update_check.php	Forward group/analyst conditionally

⚪ UI

File	Role
morning-checks/index.php	Group rows, Mine filter, the one-line row, raise + undo
morning-checks/settings/index.php	Groups tab; group + analyst pickers on the check modals
morning-checks/settings/manifest.php	The groups tab declaration
morning-checks/style.css	.mc-filter*, .mc-group-row, .mc-actions-col, .mc-icon-btn
morning-checks/help.php	Section 3
includes/capabilities.php	Cap::MORNING_CHECKS_GROUPS

🔴 Tables

Table / column	Role
morningChecks_Groups	GroupID PK, name, description, AssignedTeamID, AssignedAnalystID, IsActive, SortOrder
morningChecks_Checks.GroupID	Which group, nullable
morningChecks_Checks.AssignedAnalystID	Per-check override, nullable
morningChecks_ResultLinks	LinkID PK, ResultID, EntityType, EntityID, EntityRef
morningChecks_Results.ModifiedBy	Who set the status that is currently showing

---

2. 🧠 Routing is guidance, never permission

This is the design constraint everything else follows from, and it is worth stating in code review terms: there is no authorisation check anywhere that consults AssignedAnalystID. Completing a check is plain module access. resolveAssignment() produces a label for the UI and nothing else.

The trigger case from the discussion was somebody calling in sick. If routing gated completion, the round would stop on exactly the morning it matters most. Same family as Collision Detection — warn, do not block.

The manifest carries the warning at the point somebody would be tempted to change it:

// ⚠️ Granting this does NOT grant the right to complete a check, and
// withholding it does not withhold that either — completing a check
// is plain module access, so the round still gets done when the
// person it is routed to is away.
'id' => 'groups',
'cap' => Cap::MORNING_CHECKS_GROUPS,

Precedence

MorningChecksService::resolveAssignment() returns [analystId, label, source], most specific first:

1. the check's own AssignedAnalystID
2. its group's AssignedAnalystID
3. its group's AssignedTeamID
4. null — nobody in particular

source is 'check' | 'group' | 'team' | null and is what the dashboard uses to decide whether the label reads as a person or a team.

The Mine filter

Team membership is resolved server-side and emitted with the page, not fetched:

$tStmt = $conn->prepare("SELECT team_id FROM analyst_teams WHERE analyst_id = ?");

It never changes mid-round, and one fewer request on a page somebody opens first thing is worth having. The filter is hidden unless at least one check resolves to a non-null assignment, so an install that never makes a group sees no UI change at all.

---

3. 🧠 "Not checked" is a deleted row, not a null status

clearResult() DELETEs the morningChecks_Results row.

The alternative — keeping the row and nulling StatusID — would create a second way of expressing a state that already has one. Every reader (dashboard, 30-day chart, PDF export, v1 REST API) would have to learn that a row can exist and mean nothing, and each would be one forgotten IS NOT NULL away from counting a cleared check as a completed one.

"Not checked" is the absence of a result. It is what every check looks like before anyone touches it, and clearing should return it to exactly that.

The honest cost, stated in the confirm dialog: clearing discards the note and the attribution too. That is the right trade for I clicked green by accident; the alternative is a half-state that shows as unchecked while still crediting somebody.

Raised work survives. morningChecks_ResultLinks cascades on the result:

CONSTRAINT `fk_mcrl_result` FOREIGN KEY (`ResultID`)
    REFERENCES `morningChecks_Results` (`ResultID`) ON DELETE CASCADE

The link goes; the ticket or task does not. Correcting a mis-click must not quietly close real work somebody may already have started.

---

4. 🧠 Why attribution needed a new column

The result upsert set CreatedBy on INSERT only. So whoever set a status first was credited forever — meaning an analyst covering for an absent colleague did the work and the board thanked the colleague. Precisely the scenario the feature exists to support.

The obvious fix is to overwrite CreatedBy on update. That is wrong: the v1 REST API publishes created_by under the meaning who first recorded a result today. Redefining it would silently change a published contract for every existing integration.

So ModifiedBy was added, stamped on both branches, and the read resolves:

'CompletedBy' => $r['ModifiedBy'] ?: $r['CreatedBy'],

CreatedBy keeps its published meaning; the fallback covers rows written before the column existed.

---

5. ⚠️ Adapter semantics — present-but-null means "clear"

saveCheck() uses array_key_exists, not isset, throughout the update branch. Clearing an assignment means sending null, and isset() reads that as not mentioned, leave it alone — which would make an assignment impossible to remove once set.

That obliges the adapters to forward the keys only when the caller sent them:

if (array_key_exists('groupId', $input))   { $payload['group_id']            = $input['groupId']; }
if (array_key_exists('analystId', $input)) { $payload['assigned_analyst_id'] = $input['analystId']; }

Forwarding unconditionally would wipe grouping on any partial update — a reorder save, for instance, which sends neither.

---

6. 🐛 Bugs the build surfaced

A grouped check could vanish from the round, silently

db_verify creates no foreign keys — they live only in freeitsm.sql, so an upgraded install has no database-level guard. And the dashboard only renders a grouped check when its group is active:

WHERE c.IsActive = 1
  AND (c.GroupID IS NULL OR g.IsActive = 1)

A GroupID pointing at a row that does not exist therefore removes the check from the round with nothing on screen to explain it. assertExists() now rejects a non-existent group or analyst id at write time.

deleteGroup() was never the risk — it nulls the members first and returns the count so the confirm can say how many checks are about to become ungrouped:

$conn->prepare("UPDATE morningChecks_Checks SET GroupID = NULL WHERE GroupID = ?")->execute([$id]);
$conn->prepare("DELETE FROM morningChecks_Groups WHERE GroupID = ?")->execute([$id]);

The edit modal would have un-grouped every check it touched

get_all_checks.php did not return GroupID / AssignedAnalystID. The settings edit modal populates its pickers from that payload, so a grouped check would have opened showing No group — and saved that back. Adding a picker without adding the field to the read is the whole bug.

Cast only when set, so null stays null rather than selecting whichever option has id 0:

$check['GroupID'] = $check['GroupID'] !== null ? (int)$check['GroupID'] : null;

Changing status silently deleted the note

The overwrite branch writes Notes = ? unconditionally, and the dashboard sends '' for any status whose RequiresNotes is false. So moving a check to such a status destroyed the note with no warning — and since RequiresNotes is per-status configuration, on a default install Amber is one of them.

Now confirmed, naming the target status. Only the no-notes branch needs it: the notes modal pre-fills with the existing text, so that path never discards anything.

openRaiseModal was called but never defined

Defined 0 times, called once. Worth recording only because of how it was found — not by a test, but by someone using the button.

---

7. ⚠️ Traps

\uXXXX escapes do not work in single-quoted PHP. Several new lang strings were written as 'Delete “{name}”?' and reached the browser literally. The same mistake had reached five CHANGELOG.local.md rows. House style is straight "{name}" in confirms and literal — elsewhere.

Two things make this harder to clean up than it looks:

• Editors that normalise unicode will match — against a real em dash, so a search-and-replace can report success while leaving the file unchanged. Verify by require-ing the lang file and printing the value, not by re-reading the source.
• sed 's/\\"/"/g' breaks legitimate escapes in double-quoted strings elsewhere in the file. Do byte replacement in a script that lints and requires the result before committing it.

Use the global showConfirm() from assets/js/confirm.js (auto-loaded by the waffle menu), never native confirm(). Both new destructive actions initially shipped with the native dialog.

Non-id primary keys need a $primaryKeys entry in api/system/db_verify.php or verification fails with "Key column 'id' doesn't exist":

'morningChecks_Groups'      => 'GroupID',
'morningChecks_ResultLinks' => 'LinkID',

---

8. Sort order

Ungrouped checks sort last:

ORDER BY (c.GroupID IS NULL), g.SortOrder, g.GroupName, c.SortOrder, c.CheckName

A round with some grouped and some not reads better as the named sections, then the rest than as a nameless block at the top. The dashboard labels the remainder Other checks.

---

9. Behaviour change worth noting

Raise is now offered on every check, not only on statuses that require notes. Requiring-notes is a fair proxy for something is wrong but a poor one for I want to follow this up — a green check can still warrant a task. Undo, by contrast, appears only when ResultID !== null, because an unset on a check nobody has touched is a button that cannot do anything.

The raise modal branches on kind: create_ticket.php for a ticket, ../api/tasks/save.php for a task, then posts to link_result.php either way.

---

10. 🔴 Outstanding

• 23 locales. Every new key is English-only and falls back silently. Relevant to the in-progress German catch-up — see Adding a Language — Developer Guide.
• The v1 REST API does not yet expose groups, routing or result links.

---

Related pages

• Morning check groups and routing — the plain-language version
• Morning Checks — the module overview
• REST API — Morning Checks