Aug 4 - Update code to follow up - MT11

2026-08-04 13:00:21 -04:00
parent ceb0b806af
commit 3c2489e289
10 changed files with 1468 additions and 49 deletions
@@ -35,7 +35,10 @@ from flask import (Blueprint, render_template, redirect, url_for, flash,
 from flask_login import login_required, current_user

 from app import db, csrf
-from app.models.inspection_schedule import InspectionSchedule
+from app.models.inspection_schedule import (InspectionSchedule,
+                                            FREQUENCY_CHOICES,
+                                            MONTH_MODE_DAY, MONTH_MODE_NTH,
+                                            DEFAULT_RUN_HOUR)
 from app.models.inspection import Inspection, InspectionTemplate
 from app.models.facility import Facility, Area
 from app.models.user import User
@@ -49,8 +52,9 @@ logger = logging.getLogger(__name__)

 bp = Blueprint('inspection_schedules', __name__, url_prefix='/inspection-schedules')

-_FREQUENCIES = ('daily', 'weekly', 'monthly', 'quarterly')
+_FREQUENCIES = FREQUENCY_CHOICES
 _MODES       = ('auto', 'plan')
+_MONTH_MODES = (MONTH_MODE_DAY, MONTH_MODE_NTH)


 # ── Helpers ───────────────────────────────────────────────────────────────────
@@ -58,20 +62,126 @@ _MODES       = ('auto', 'plan')
 def _compute_next_run(frequency: str, from_dt: datetime = None) -> datetime:
    """Return the next due datetime for a given cadence, at 06:00 local.

-    Monthly/quarterly advance by calendar months (targeting the same day-of-month
-    is avoided — we simply add 30/90 days, which is predictable and never raises
-    the datetime.replace(month=13) ValueError).
+    Plain interval maths with no day-of-week / day-of-month detail. Used only to
+    seed a schedule's FIRST occurrence when the manager does not pick a start
+    date; every subsequent roll-forward goes through
+    InspectionSchedule.fulfill(), which honours the phase46 recurrence columns.
+
+    Monthly and longer cadences advance by whole days rather than calendar
+    months here, which is predictable and never raises the
+    datetime.replace(month=13) ValueError. Kept as-is for the original four
+    frequencies so no existing schedule's first-run maths changes.
    """
    now = from_dt or now_eastern()
-    if frequency == 'daily':
+    if frequency == 'once':
+        base = now
+    elif frequency == 'daily':
        base = now + timedelta(days=1)
    elif frequency == 'weekly':
        base = now + timedelta(weeks=1)
    elif frequency == 'monthly':
        base = now + timedelta(days=30)
+    elif frequency == 'bi-annually':
+        base = now + timedelta(days=182)
+    elif frequency == 'annually':
+        base = now + timedelta(days=365)
    else:  # quarterly
        base = now + timedelta(days=90)
-    return base.replace(hour=6, minute=0, second=0, microsecond=0)
+    return base.replace(hour=DEFAULT_RUN_HOUR, minute=0, second=0, microsecond=0)
+
+
+def _parse_date(raw):
+    """Parse an ISO date from a form field. Returns None for blank/invalid."""
+    raw = (raw or '').strip()
+    if not raw:
+        return None
+    try:
+        return datetime.strptime(raw, '%Y-%m-%d').date()
+    except ValueError:
+        return None
+
+
+def _recurrence_errors(form, frequency):
+    """Validation messages for the recurrence block of *frequency* (phase46).
+
+    Only the block matching the chosen frequency is checked; the others are
+    ignored here and cleared on save by _apply_recurrence().
+    """
+    errors = []
+    if frequency == 'weekly':
+        if not form.getlist('weekdays'):
+            errors.append('Pick at least one day of the week.')
+    elif frequency in ('monthly', 'quarterly', 'bi-annually', 'annually'):
+        month_mode = form.get('month_mode') or MONTH_MODE_DAY
+        if month_mode not in _MONTH_MODES:
+            errors.append('Invalid monthly rule.')
+        elif month_mode == MONTH_MODE_NTH:
+            if not form.get('nth_week', type=int) or form.get('nth_weekday', type=int) is None:
+                errors.append('Choose which weekday of the month.')
+        else:
+            dom = form.get('day_of_month', type=int)
+            if not dom or not 1 <= dom <= 31:
+                errors.append('Enter a day of the month (1–31).')
+    return errors
+
+
+def _apply_recurrence(sched, form, frequency, due_date):
+    """Copy the recurrence block for *frequency* onto *sched*, clear the blocks
+    that no longer apply, set the end date, then snap the due date onto the rule.
+
+    Keeping the unused columns NULL means recurrence_label and the date maths
+    never read stale settings after a frequency change. Caller commits.
+    """
+    sched.frequency = frequency
+
+    if frequency == 'weekly':
+        sched.set_weekdays([int(v) for v in form.getlist('weekdays') if v.lstrip('-').isdigit()])
+    else:
+        sched.weekdays = None
+
+    if frequency in ('monthly', 'quarterly', 'bi-annually', 'annually'):
+        sched.month_mode = form.get('month_mode') or MONTH_MODE_DAY
+        if sched.month_mode == MONTH_MODE_NTH:
+            sched.day_of_month = None
+            sched.nth_week     = form.get('nth_week', type=int)
+            sched.nth_weekday  = form.get('nth_weekday', type=int)
+        else:
+            sched.day_of_month = form.get('day_of_month', type=int)
+            sched.nth_week     = None
+            sched.nth_weekday  = None
+    else:
+        sched.month_mode = sched.day_of_month = None
+        sched.nth_week = sched.nth_weekday = None
+
+    # End date (phase47) — a boundary, not a cadence setting. A one-time
+    # schedule has none: it ends by deactivating when it is completed.
+    sched.end_date = _parse_date(form.get('end_date')) if frequency != 'once' else None
+
+    # Snap the picked date forward onto the first matching occurrence.
+    sched.set_next_run_date(sched.align_due_date(due_date))
+
+
+def _end_date_errors(sched, form, frequency):
+    """End-date validation (phase47), run AFTER _apply_recurrence().
+
+    Two separate rejections:
+      * an end date on a one-time schedule — meaningless, and silently dropping
+        it would hide the mistake;
+      * an aligned first occurrence past the end date. align_due_date() can push
+        the picked date forward onto the rule (pick a Tuesday for a Mon/Wed/Fri
+        schedule and the first occurrence is Wednesday), so an end date that
+        looked valid against the picked date can still leave the schedule with
+        no occurrence it is ever allowed to run.
+    """
+    errors = []
+    raw_end = _parse_date(form.get('end_date'))
+    if raw_end and frequency == 'once':
+        errors.append('A one-time schedule has no end date — '
+                      'it closes when it is completed.')
+    elif raw_end and sched.due_date and not sched.is_within_end_date(sched.due_date):
+        errors.append(f'With this recurrence the first occurrence falls on '
+                      f'{sched.due_date:%b %d, %Y}, after the end date.')
+    return errors


 def _active_inspectors():
@@ -204,6 +314,7 @@ def create():
            errors.append('Invalid frequency.')
        if mode not in _MODES:
            errors.append('Invalid mode.')
+        errors.extend(_recurrence_errors(request.form, frequency))

        if errors:
            for e in errors:
@@ -211,6 +322,7 @@ def create():
            return render_template('inspection_schedules/form.html',
                                   templates=templates, facilities=facilities,
                                   inspectors=inspectors, frequencies=_FREQUENCIES,
+                           frequency_labels=InspectionSchedule.FREQUENCY_LABELS,
                                   title='New Inspection Schedule')

        schedule = InspectionSchedule(
@@ -225,13 +337,33 @@ def create():
            active       = True,
            created_by   = current_user.id,
            created_at   = now_eastern(),
+            # Seeded here so run_time has a time-of-day to preserve;
+            # _apply_recurrence() below rewrites the DATE part.
            next_run_at  = _compute_next_run(frequency),
        )
+        # Blank start date keeps the pre-phase46 behaviour exactly: the first
+        # occurrence lands one interval from now. A picked date wins.
+        start_date = _parse_date(request.form.get('next_due_date')) \
+            or schedule.next_run_at.date()
+        _apply_recurrence(schedule, request.form, frequency, start_date)
+
+        end_errors = _end_date_errors(schedule, request.form, frequency)
+        if end_errors:
+            # schedule was never added to the session — nothing to roll back.
+            for e in end_errors:
+                flash(e, 'warning')
+            return render_template('inspection_schedules/form.html',
+                                   templates=templates, facilities=facilities,
+                                   inspectors=inspectors, frequencies=_FREQUENCIES,
+                           frequency_labels=InspectionSchedule.FREQUENCY_LABELS,
+                                   title='New Inspection Schedule')
+
        db.session.add(schedule)
        db.session.commit()
        log_action(ACTION_CREATE, 'InspectionSchedule', schedule.id, schedule.name,
-                   f'frequency={frequency}; mode={mode}; template_id={template_id}; '
-                   f'facility_id={facility_id}')
+                   f'frequency={schedule.recurrence_label}; mode={mode}; '
+                   f'due={schedule.due_date}; end={schedule.end_date or "—"}; '
+                   f'template_id={template_id}; facility_id={facility_id}')
        # phase43: tell the inspector it's theirs (plan mode has no materialised
        # inspection to announce itself).
        _notify_assignee(schedule)
@@ -242,6 +374,7 @@ def create():
    return render_template('inspection_schedules/form.html',
                           templates=templates, facilities=facilities,
                           inspectors=inspectors, frequencies=_FREQUENCIES,
+                           frequency_labels=InspectionSchedule.FREQUENCY_LABELS,
                           title='New Inspection Schedule')


@@ -262,6 +395,20 @@ def edit(schedule_id):
        inspector_id          = request.form.get('inspector_id', type=int)
        frequency             = request.form.get('frequency', schedule.frequency)

+        if frequency not in _FREQUENCIES:
+            frequency = schedule.frequency
+        errors = _recurrence_errors(request.form, frequency)
+        if errors:
+            db.session.rollback()
+            for e in errors:
+                flash(e, 'warning')
+            return render_template('inspection_schedules/form.html',
+                                   schedule=schedule, templates=templates,
+                                   facilities=facilities, inspectors=inspectors,
+                                   frequencies=_FREQUENCIES,
+                                   frequency_labels=InspectionSchedule.FREQUENCY_LABELS,
+                                   title='Edit Inspection Schedule')
+
        if template_id and db.session.get(InspectionTemplate, template_id):
            schedule.template_id = template_id
        if facility_id and db.session.get(Facility, facility_id):
@@ -269,23 +416,53 @@ def edit(schedule_id):
        if inspector_id and db.session.get(User, inspector_id):
            schedule.inspector_id = inspector_id
        schedule.area_id   = request.form.get('area_id', type=int) or None
-        if frequency in _FREQUENCIES:
-            schedule.frequency = frequency
        mode = request.form.get('mode', schedule.mode)
        if mode in _MODES:
            schedule.mode = mode
        schedule.notes     = request.form.get('notes', '').strip() or None
        schedule.active    = bool(request.form.get('active'))
-        # Recompute the next run from now against the (possibly changed) cadence.
-        schedule.next_run_at = _compute_next_run(schedule.frequency)
+
+        # ── Due date (phase46 root-cause fix) ─────────────────────────────
+        # This route used to do `next_run_at = _compute_next_run(frequency)`
+        # unconditionally, recomputing the due date from *now* on every save.
+        # Renaming a schedule or editing its notes therefore silently threw
+        # away the manager's chosen due date and reset every reminder — which
+        # with an end date and a day-of-week rule would also skip occurrences.
+        # The due date is now only rewritten when the manager picks a new one,
+        # or when the recurrence rule no longer fits the existing one (in which
+        # case align_due_date() snaps it forward to the nearest valid date).
+        old_due = schedule.due_date
+        target  = (_parse_date(request.form.get('next_due_date'))
+                   or old_due
+                   or _compute_next_run(frequency).date())
+        _apply_recurrence(schedule, request.form, frequency, target)
+
+        end_errors = _end_date_errors(schedule, request.form, frequency)
+        if end_errors:
+            # schedule is persistent and already mutated — discard the pending
+            # changes before re-rendering so nothing leaks out on next flush.
+            db.session.rollback()
+            for e in end_errors:
+                flash(e, 'warning')
+            return render_template('inspection_schedules/form.html',
+                                   schedule=schedule, templates=templates,
+                                   facilities=facilities, inspectors=inspectors,
+                                   frequencies=_FREQUENCIES,
+                                   frequency_labels=InspectionSchedule.FREQUENCY_LABELS,
+                                   title='Edit Inspection Schedule')
+
        # New occurrence -> the previous occurrence's reminders no longer apply.
-        schedule.advance_notified = False
-        schedule.due_notified     = False
-        schedule.overdue_notified = False
+        # Unchanged due date -> keep the flags, or every unrelated edit would
+        # re-send the advance/due reminder the inspector already received.
+        if schedule.due_date != old_due:
+            schedule.advance_notified = False
+            schedule.due_notified     = False
+            schedule.overdue_notified = False

        db.session.commit()
        log_action(ACTION_UPDATE, 'InspectionSchedule', schedule.id, schedule.name,
-                   f'frequency={schedule.frequency}; mode={schedule.mode}; '
+                   f'frequency={schedule.recurrence_label}; mode={schedule.mode}; '
+                   f'due={schedule.due_date}; end={schedule.end_date or "—"}; '
                   f'active={schedule.active}')

        # phase43: notify on (re)assignment to a different inspector.
@@ -298,6 +475,7 @@ def edit(schedule_id):
    return render_template('inspection_schedules/form.html', schedule=schedule,
                           templates=templates, facilities=facilities,
                           inspectors=inspectors, frequencies=_FREQUENCIES,
+                           frequency_labels=InspectionSchedule.FREQUENCY_LABELS,
                           title='Edit Inspection Schedule')


@@ -328,10 +506,13 @@ def run_now(schedule_id):
    now = now_eastern()
    inspection = _materialise(schedule, now)
    schedule.last_run_at = now
-    schedule.next_run_at = _compute_next_run(schedule.frequency, now)
+    # phase46/47: honours the day rules, the end-date boundary and 'once'
+    # (which deactivates), instead of the old flat interval.
+    schedule.advance_due_date(now)
    db.session.commit()
    log_action(ACTION_UPDATE, 'InspectionSchedule', schedule.id, schedule.name,
-               f'manual run_now by {current_user.username}; inspection_id={inspection.id}')
+               f'manual run_now by {current_user.username}; inspection_id={inspection.id}; '
+               f'next_due={schedule.due_date}; active={schedule.active}')
    flash(f'Inspection created from "{schedule.name}". It is now in the inspector\'s queue.',
          'success')
    return redirect(url_for('inspection_schedules.index'))
@@ -402,8 +583,28 @@ def run():
        logger.warning('INSPECTION SCHEDULES RUN REJECTED | bad/missing token')
        return jsonify({'ok': False, 'error': 'unauthorized'}), 403

-    now = now_eastern()
+    now   = now_eastern()
+    today = now.date()
    schedules = InspectionSchedule.query.filter_by(active=True).all()
+
+    # ── Expiry sweep (phase47), BEFORE any materialisation or reminder work ──
+    # advance_due_date() closes out a schedule that reaches its boundary by
+    # producing an occurrence; this covers the one that reaches it without ever
+    # doing so — otherwise an auto schedule would keep materialising and a plan
+    # schedule would keep re-alerting as overdue, indefinitely.
+    expired = 0
+    live    = []
+    for s in schedules:
+        if s.expire_if_past_end_date(today):
+            expired += 1
+            logger.info('INSPECTION SCHEDULE EXPIRED | schedule_id=%s | end_date=%s',
+                        s.id, s.end_date)
+        else:
+            live.append(s)
+    if expired:
+        db.session.commit()
+    schedules = live
+
    # Only 'auto' schedules materialise. 'plan' schedules wait for the inspector
    # to click Start; they get reminders instead (below).
    auto = [s for s in schedules if s.mode != 'plan']
@@ -414,23 +615,24 @@ def run():
        try:
            inspection = _materialise(schedule, now)
            schedule.last_run_at = now
-            schedule.next_run_at = _compute_next_run(schedule.frequency, now)
+            schedule.advance_due_date(now)
            created += 1
            logger.info('INSPECTION SCHEDULE MATERIALISED | schedule_id=%s | inspection_id=%s',
                        schedule.id, inspection.id)
        except Exception as exc:
-            # Advance next_run_at anyway so one broken schedule can't wedge the
+            # Advance the due date anyway so one broken schedule can't wedge the
            # whole cron run on every subsequent tick.
-            schedule.next_run_at = _compute_next_run(schedule.frequency, now)
+            schedule.advance_due_date(now)
            logger.error('INSPECTION SCHEDULE FAILED | schedule_id=%s | error=%s',
                         schedule.id, exc)
    db.session.commit()

    sent = _dispatch_reminders([s for s in schedules if s.mode == 'plan'], now)

-    logger.info('INSPECTION SCHEDULES CRON | due=%s | created=%s | reminders=%s',
-                len(due), created, sent)
-    return jsonify({'ok': True, 'due': len(due), 'created': created, 'reminders': sent})
+    logger.info('INSPECTION SCHEDULES CRON | expired=%s | due=%s | created=%s | reminders=%s',
+                expired, len(due), created, sent)
+    return jsonify({'ok': True, 'expired': expired, 'due': len(due),
+                    'created': created, 'reminders': sent})


 def _dispatch_reminders(plans, now):