Warranty and Lease Alerts Developer Guide - edmozley/freeitsm GitHub Wiki

Warranty and lease alerts โ€” developer guide

assets.lease_expiry is warranty_expiry's twin, and the surfacing machinery built for warranty generalised for free. The interesting parts are the two places where treating them as one thing would have broken the older one.

User-facing page: Warranty and lease alerts.


The shape

Warranty Lease
Column assets.warranty_expiry assets.lease_expiry
Surface setting asset_warranty_surface asset_lease_surface
Window setting asset_warranty_days (30) asset_lease_days (60)
Calendar marker source = 'asset_warranty' source = 'asset_lease'
Category Warranty Lease

Both settings are registered against Cap::ASSETS_WARRANTY in asset-management/settings/manifest.php. The tab's id stays warranty โ€” that is its stable address and what the capability is granted against, so renaming it would silently revoke the tab from anybody who already has it. Only its label changed, to Expiry alerts.


๐Ÿ”ด Trap 1: a new column must not take the old feature down

lease_expiry does not exist until Database Verification has run. Two consequences:

The asset list names it conditionally, the same guard the asset tag column uses:

$leaseReady = /* information_schema check */;
$leaseCol   = $leaseReady ? "a.lease_expiry," : "NULL AS lease_expiry,";

Both SELECTs and the GROUP BY go through it. Naming the column unconditionally turns the entire asset list into Unknown column on an install that has pulled the update and not yet run Verify.

The calendar sync checks for it separately and never folds it into the existing guard:

if (!awcColumnExists($conn, 'calendar_events', 'source')
    || !awcColumnExists($conn, 'assets', 'warranty_expiry')) {
    return ['success' => false, 'error' => 'Schema not ready'];
}
$leaseReady = awcColumnExists($conn, 'assets', 'lease_expiry');   // separate

Adding lease_expiry to that condition would have stopped warranty entries syncing too โ€” breaking a feature that was working fine to make room for one that is not there yet.

Prove this with the column genuinely absent, then again after adding it, with a negative control confirming the unguarded query really does fail. A guard only ever tested after the column exists has not been tested.

๐Ÿ”ด Trap 2: one marker for two kinds

awcSyncKind() clears and rebuilds one source at a time:

DELETE FROM calendar_events WHERE source = ?

Sharing a marker would mean switching leases off deleted the warranty entries beside them. Verified by switching each kind off in turn against real data and confirming the other was untouched, with hand-created events unmoved throughout.

Clearing happens even when the kind is switched off, and even when the column is missing โ€” that is what makes turning it off actually remove the entries rather than leaving the last set behind for ever.

๐Ÿ”ด Trap 3: the category is remembered by id, not by name

Renaming the category is the right thing for somebody to do โ€” these are stored rows, not interface text, so renaming is the only i18n they can have. The original lookup was by name, so a rename caused the next sync to miss it and create a second category, splitting the entries across both.

awcEnsureCategory() resolves most-specific-first:

  1. the id recorded in <source>_category_id, if that category still exists;
  2. otherwise whichever category the existing entries of this kind are already filed under โ€” which adopts a pre-existing rename with no migration;
  3. otherwise one matching the default name;
  4. otherwise create it.

Colour is only ever set at creation: a sync that reset it nightly would be quietly overruling somebody's preference.

โš ๏ธ Step 2 must run BEFORE the delete. awcSyncKind clears its entries and rebuilds them, so asking "which category are these in?" afterwards finds nothing and makes the duplicate anyway โ€” the exact bug, reintroduced by the order of two lines. awcAdoptExistingCategory() is called ahead of the DELETE for this reason.


The pill on the asset

expiryFlag(dateStr) in asset-management/index.php. Compares plain calendar days as YYYY-MM-DD strings โ€” both operands are that format, so a string comparison is a date comparison. Turning them into timestamps would make the answer depend on the reader's timezone, so a lease would read as expired in Auckland and current in London on the same day.

It refreshes after the save succeeds, not optimistically on change: a pill that updates and then fails to save leaves the screen stating a date the asset does not have, and looks freshly correct while doing it. Both in-memory copies (the list row and selectedAsset) are updated, or the next re-render puts the old value back.


Where the settings live, and why not in Watchtower

All three "surface" settings โ€” asset_warranty_surface, asset_lease_surface, software_renewal_surface โ€” live in the module that owns the data, not in Watchtower's settings.

Each controls the calendar as well as the dashboard. Moving the dashboard half into Watchtower would split one decision across two screens, and somebody switching it on in one place would fairly assume the other followed. Watchtower โ†’ Settings โ†’ Cards instead carries a link to the real setting: one place to look, still one place to set.


See also

โš ๏ธ **GitHub.com Fallback** โš ๏ธ