Let an end time mean the card leaves the board

The shopfloor feed kept every ended notification up for a hardcoded 30 minutes,
flagged resolved. A card with an 8:00 end time was still on the board at 8:29,
which reads as an expiry that did not work - and in the carousel, grid and
banner sections it read that way with no visual sign at all, since only the
standard cards render the resolved state.

The tail is now notificationtypes.gracewindowminutes, set per type on the
Notification Types page and defaulting to 0, so an end time means what it says.
A type whose cards are worth acknowledging after they clear - an incident, say
- opts into a tail, and only that type's cards get one.

The feed widens its query to the largest configured tail and then holds each
row to its own type's window. That keeps one portable query rather than a
per-type interval expression in SQL, and with every type at 0 it collapses to
"still showing".

Also fixes resolved serializing as null rather than false for a card with no
end time, which the and-chain produced.

plugins/notifications/api/routes.py

@@ -140,6 +140,10 @@ def _apply_expiry_fields(t, data):
 
 _DISPLAY_STYLES = ('standard', 'carousel', 'grid', 'banner')
 
+# Ceiling on the per-type post-expiry tail. A day is already far longer than
+# "recently ended"; anything more is an end time that should have been later.
+_MAX_GRACE_MINUTES = 1440
+
 
 def _apply_display_fields(t, data):
     """Set shopfloor display-behavior columns on a NotificationType from request
@@ -153,6 +157,17 @@ def _apply_display_fields(t, data):
         if ds not in _DISPLAY_STYLES:
             return "displaystyle must be one of: %s" % ", ".join(_DISPLAY_STYLES)
         t.displaystyle = ds
+    if 'gracewindowminutes' in data:
+        raw = data.get('gracewindowminutes')
+        raw = 0 if raw in (None, '') else raw
+        try:
+            minutes = int(raw)
+        except (TypeError, ValueError):
+            return "gracewindowminutes must be a whole number of minutes"
+        if minutes < 0 or minutes > _MAX_GRACE_MINUTES:
+            return ("gracewindowminutes must be between 0 and %d"
+                    % _MAX_GRACE_MINUTES)
+        t.gracewindowminutes = minutes
     return None
 
 
@@ -163,10 +178,11 @@ def _config_version():
     reach pages that are already open."""
     types = NotificationType.query.order_by(NotificationType.notificationtypeid).all()
     parts = [
-        "%s|%s|%s|%d|%d|%s|%s|%s|%d" % (
+        "%s|%s|%s|%d|%d|%s|%s|%s|%d|%d" % (
             t.notificationtypeid, t.typecolor, t.displaystyle,
             int(bool(t.splitperemployee)), int(bool(t.showemployeephoto)),
             t.expirymode, t.expirydays, t.expiryhour, int(bool(t.isactive)),
+            int(t.gracewindowminutes or 0),
         )
         for t in types
     ]
@@ -698,8 +714,15 @@ def get_shopfloor_notifications():
         # All units: only show notifications with NULL businessunitid
         base_query = base_query.filter(Notification.businessunitid.is_(None))
 
-    # Current notifications (active now or ended within 30 minutes)
-    thirty_min_ago = now - timedelta(minutes=30)
+    # Current notifications: showing now, plus anything whose type asks to keep
+    # ended cards up for a while (gracewindowminutes, 0 by default - an end time
+    # means the card leaves the board then). The window is per type, so the SQL
+    # only widens to the largest configured tail and each row is then held to
+    # its own; that keeps this one portable query instead of a per-type interval
+    # expression, and with every type at 0 it collapses to "still showing".
+    widest_grace = db.session.query(
+        db.func.max(NotificationType.gracewindowminutes)).scalar() or 0
+    widest_grace_start = now - timedelta(minutes=int(widest_grace))
     current_query = base_query.filter(
         db.or_(
             # Active and currently showing
@@ -708,16 +731,23 @@ def get_shopfloor_notifications():
                 db.or_(Notification.starttime.is_(None), Notification.starttime <= now),
                 db.or_(Notification.endtime.is_(None), Notification.endtime >= now)
             ),
-            # Recently ended (within 30 min) - show as resolved
+            # Ended, but possibly inside its type's tail - narrowed below
             db.and_(
                 Notification.endtime.isnot(None),
-                Notification.endtime >= thirty_min_ago,
+                Notification.endtime >= widest_grace_start,
                 Notification.endtime < now
             )
         )
     ).order_by(Notification.notificationid.desc())
 
-    current_notifications = current_query.all()
+    def _within_grace(n):
+        """True unless the row ended outside its own type's tail."""
+        if n.endtime is None or n.endtime >= now:
+            return True
+        grace = (n.notificationtype.gracewindowminutes or 0) if n.notificationtype else 0
+        return n.endtime >= now - timedelta(minutes=int(grace))
+
+    current_notifications = [n for n in current_query.all() if _within_grace(n)]
 
     # Upcoming notifications (starts within next 5 days)
     five_days = now + timedelta(days=5)
@@ -731,7 +761,9 @@ def get_shopfloor_notifications():
 
     def notification_to_shopfloor(n, employee_override=None):
         """Convert notification to shopfloor format."""
-        is_resolved = n.endtime and n.endtime < now
+        # bool, not the datetime-or-None the and-chain yields: a card with no
+        # end time was serializing resolved as null.
+        is_resolved = bool(n.endtime and n.endtime < now)
         ntype = n.notificationtype
         show_photo = bool(ntype and ntype.showemployeephoto)
 

plugins/notifications/frontend/views/NotificationTypesList.vue

@@ -87,6 +87,18 @@
             </select>
           </label>
 
+          <label class="field">
+            <span>Keep showing after end (minutes)</span>
+            <input v-model.number="form.gracewindowminutes" type="number"
+                   min="0" max="1440" step="5" />
+            <small class="muted">
+              0 means a card leaves the shopfloor board the moment its end time
+              passes. A higher number keeps it up that long afterwards, marked
+              RESOLVED - useful for an incident worth acknowledging once it
+              clears.
+            </small>
+          </label>
+
           <label class="field checkbox">
             <input v-model="form.splitperemployee" type="checkbox" />
             <span>Split one card per employee</span>
@@ -170,9 +182,12 @@ function swatchColor(c) {
 }
 
 function expiryLabel(t) {
-  if (t.expirymode === 'duration' && t.expirydays) return `${t.expirydays} day(s)`
-  if (t.expirymode === 'dailytime') return `daily @ ${String(t.expiryhour ?? 8).padStart(2, '0')}:00 ET`
-  return 'none'
+  const grace = t.gracewindowminutes ?? 0
+  // The tail only means something next to the rule that ends the card.
+  const tail = grace > 0 ? ` (+${grace}m resolved)` : ''
+  if (t.expirymode === 'duration' && t.expirydays) return `${t.expirydays} day(s)${tail}`
+  if (t.expirymode === 'dailytime') return `daily @ ${String(t.expiryhour ?? 8).padStart(2, '0')}:00 ET${tail}`
+  return grace > 0 ? `none${tail}` : 'none'
 }
 
 async function load() {
@@ -199,6 +214,7 @@ function openNew() {
     expirymode: 'none',
     expirydays: null,
     expiryhour: null,
+    gracewindowminutes: 0,
     isactive: true
   }
   editing.value = true
@@ -217,6 +233,7 @@ function openEdit(t) {
     expirymode: t.expirymode || 'none',
     expirydays: t.expirydays ?? null,
     expiryhour: t.expiryhour ?? null,
+    gracewindowminutes: t.gracewindowminutes ?? 0,
     isactive: t.isactive !== false
   }
   editing.value = true

plugins/notifications/migrations/versions/0003_gracewindowminutes.py

@@ -0,0 +1,47 @@
+"""Add notificationtypes.gracewindowminutes (post-expiry tail on the board).
+
+The shopfloor feed kept every ended notification on the board for a hardcoded
+30 minutes, flagged resolved. A card with an 8:00 end time therefore sat there
+until 8:30, and in the carousel/grid/banner sections it did so with no visual
+sign it had ended at all. The tail is now per type and defaults to 0, so an end
+time means what it says; a type that wants an acknowledged tail opts in.
+
+Idempotent; downgrade drops the column.
+
+Revision ID: notifications0003grace
+Revises: notifications0002buidx
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+revision = 'notifications0003grace'
+down_revision = 'notifications0002buidx'
+branch_labels = None
+depends_on = None
+
+_TABLE = 'notificationtypes'
+_COLUMN = 'gracewindowminutes'
+
+
+def _column_names(insp, table):
+    return {c['name'] for c in insp.get_columns(table)}
+
+
+def upgrade():
+    bind = op.get_bind()
+    insp = sa.inspect(bind)
+    if _TABLE not in insp.get_table_names():
+        return
+    if _COLUMN not in _column_names(insp, _TABLE):
+        op.add_column(_TABLE, sa.Column(_COLUMN, sa.Integer(), nullable=False,
+                                        server_default='0'))
+
+
+def downgrade():
+    bind = op.get_bind()
+    insp = sa.inspect(bind)
+    if _TABLE not in insp.get_table_names():
+        return
+    if _COLUMN in _column_names(insp, _TABLE):
+        op.drop_column(_TABLE, _COLUMN)

plugins/notifications/models/notification.py

@@ -75,6 +75,14 @@ class NotificationType(db.Model):
     showemployeephoto = db.Column(db.Boolean, default=False)
     displaystyle = db.Column(db.String(20), default='standard')
 
+    # Minutes a card stays on the shopfloor board AFTER its end time, shown as
+    # RESOLVED. 0 (the default) means it leaves the board the moment it ends,
+    # which is what a reader expects from an end time. A type whose cards are
+    # worth acknowledging after the fact - an incident that just cleared - can
+    # opt into a tail.
+    gracewindowminutes = db.Column(db.Integer, nullable=False,
+                                   server_default='0', default=0)
+
     def __repr__(self):
         return f"<NotificationType {self.typename}>"
 
@@ -91,7 +99,8 @@ class NotificationType(db.Model):
             'expiryminute': self.expiryminute if self.expiryminute is not None else 0,
             'splitperemployee': bool(self.splitperemployee),
             'showemployeephoto': bool(self.showemployeephoto),
-            'displaystyle': self.displaystyle or 'standard'
+            'displaystyle': self.displaystyle or 'standard',
+            'gracewindowminutes': int(self.gracewindowminutes or 0)
         }
 
 

tests/test_plugin_migrations.py

@@ -64,8 +64,8 @@ EXPECTED_HEAD_REVISION['network'] = 'network0002model'
 # printedparts is post-cutover: its 0001 really creates its tables; 0004 adds
 # the per-transaction revision column.
 EXPECTED_HEAD_REVISION['printedparts'] = 'printedparts0004txnrev'
-# notifications indexes businessunitid on top of its anchor.
-EXPECTED_HEAD_REVISION['notifications'] = 'notifications0002buidx'
+# notifications indexes businessunitid, then adds the per-type grace window.
+EXPECTED_HEAD_REVISION['notifications'] = 'notifications0003grace'
 
 # Plugins built after the cutover: their 0001 baseline really creates tables the
 # core chain never owned.

tests/test_plugins/test_shopfloor_grace_window.py

@@ -0,0 +1,141 @@
+"""Tests for the per-type post-expiry window on the shopfloor feed.
+
+The board used to keep every ended notification for a hardcoded 30 minutes,
+flagged resolved. A card with an 8:00 end time was therefore still up at 8:29,
+and in the carousel/grid/banner sections nothing marked it as ended at all. The
+tail is now notificationtypes.gracewindowminutes, default 0: an end time means
+the card leaves the board then, and a type opts in if its cards are worth
+acknowledging afterwards.
+"""
+
+from datetime import datetime, timedelta, timezone
+
+from plugins.notifications.models import Notification, NotificationType
+
+
+def _utcnow():
+    return datetime.now(timezone.utc).replace(tzinfo=None)
+
+
+def _make_type(db, typename, grace=0, displaystyle='standard'):
+    t = NotificationType(typename=typename, typecolor='#17a2b8', isactive=True,
+                         displaystyle=displaystyle, gracewindowminutes=grace)
+    db.session.add(t)
+    db.session.commit()
+    return t
+
+
+def _make_note(db, ntype, ended_minutes_ago=None, text='msg'):
+    """Shopfloor note that ended N minutes ago (None = still running)."""
+    now = _utcnow()
+    n = Notification(
+        notificationtypeid=ntype.notificationtypeid,
+        notification=text,
+        businessunitid=None,
+        isactive=True,
+        isshopfloor=True,
+        starttime=now - timedelta(hours=2),
+        endtime=None if ended_minutes_ago is None
+        else now - timedelta(minutes=ended_minutes_ago),
+    )
+    db.session.add(n)
+    db.session.commit()
+    return n
+
+
+def _current(client):
+    resp = client.get('/api/notifications/shopfloor')
+    assert resp.status_code == 200, resp.get_json()
+    return resp.get_json()['data']['current']
+
+
+def test_default_type_drops_the_card_at_its_end_time(client, db):
+    """The reported bug: an 8:00 end time meant gone at 8:30."""
+    ntype = _make_type(db, 'General')
+    _make_note(db, ntype, ended_minutes_ago=1)
+
+    assert _current(client) == []
+
+
+def test_new_type_defaults_to_no_tail(client, db):
+    """A type created without naming the field gets 0, not a surprise tail."""
+    ntype = NotificationType(typename='Fresh', typecolor='#17a2b8', isactive=True)
+    db.session.add(ntype)
+    db.session.commit()
+
+    assert (ntype.gracewindowminutes or 0) == 0
+
+
+def test_running_card_is_unaffected(client, db):
+    ntype = _make_type(db, 'General')
+    _make_note(db, ntype)
+
+    current = _current(client)
+    assert len(current) == 1
+    assert current[0]['resolved'] is False
+
+
+def test_type_with_a_tail_keeps_the_card_marked_resolved(client, db):
+    ntype = _make_type(db, 'Incident', grace=30)
+    _make_note(db, ntype, ended_minutes_ago=10)
+
+    current = _current(client)
+    assert len(current) == 1
+    assert current[0]['resolved'] is True
+
+
+def test_card_leaves_once_past_its_own_tail(client, db):
+    ntype = _make_type(db, 'Incident', grace=30)
+    _make_note(db, ntype, ended_minutes_ago=31)
+
+    assert _current(client) == []
+
+
+def test_each_type_is_held_to_its_own_window(client, db):
+    """The widest tail must not leak onto a type that asked for none.
+
+    The query widens to the largest configured window, so without the per-row
+    check the no-tail type would ride along on the incident type's 60 minutes.
+    """
+    lingering = _make_type(db, 'Incident', grace=60)
+    immediate = _make_type(db, 'General', grace=0)
+    _make_note(db, lingering, ended_minutes_ago=20, text='incident')
+    _make_note(db, immediate, ended_minutes_ago=20, text='general')
+
+    texts = {c['notification'] for c in _current(client)}
+    assert texts == {'incident'}
+
+
+def test_carousel_type_obeys_the_window_too(client, db):
+    """Recognition is a carousel card, where nothing renders `resolved`."""
+    ntype = _make_type(db, 'Recognition', grace=0, displaystyle='carousel')
+    _make_note(db, ntype, ended_minutes_ago=5)
+
+    assert _current(client) == []
+
+
+def test_api_rejects_a_negative_window(client, db, auth_headers):
+    resp = client.post('/api/notifications/types',
+                       json={'typename': 'Bad', 'gracewindowminutes': -5},
+                       headers=auth_headers)
+
+    assert resp.status_code == 400
+    assert 'gracewindowminutes' in resp.get_data(as_text=True)
+
+
+def test_api_round_trips_the_window(client, db, auth_headers):
+    create = client.post('/api/notifications/types',
+                         json={'typename': 'Incident', 'gracewindowminutes': 45},
+                         headers=auth_headers)
+    assert create.status_code in (200, 201), create.get_json()
+    typeid = create.get_json()['data']['notificationtypeid']
+
+    listed = client.get('/api/notifications/types', headers=auth_headers)
+    row = next(t for t in listed.get_json()['data']
+               if t['notificationtypeid'] == typeid)
+    assert row['gracewindowminutes'] == 45
+
+    update = client.put(f'/api/notifications/types/{typeid}',
+                        json={'gracewindowminutes': 0}, headers=auth_headers)
+    assert update.status_code == 200, update.get_json()
+    assert update.get_json()['data']['gracewindowminutes'] == 0