diff --git a/app/api/inspections.py b/app/api/inspections.py index fa07ae2..6f1a2c8 100644 --- a/app/api/inspections.py +++ b/app/api/inspections.py @@ -146,11 +146,13 @@ def _fulfill_schedule(inspection): """Roll the originating schedule forward. Caller commits. Mirrors routes/inspections.py exactly, including passing `_compute_next_run` - as `next_run_fn`. That argument is NOT optional in practice: MT's - `InspectionSchedule.fulfill()` leaves `next_run_at` untouched when it is - omitted, so the schedule would stay permanently due and keep firing overdue - reminders. The deferred import mirrors the web route and avoids a module-load - cycle between the api and routes packages. + as `next_run_fn`. As of phase46 that argument is accepted and ignored: the + cadence maths moved onto `InspectionSchedule.advance_due_date()`, which owns + the recurrence columns and the end-date boundary. Before phase46 omitting it + silently left `next_run_at` untouched and the schedule stayed permanently + due; the call is kept as-is so this file needs no behavioural change. The + deferred import mirrors the web route and avoids a module-load cycle between + the api and routes packages. """ if not inspection.inspection_schedule_id: return diff --git a/app/api/scheduled.py b/app/api/scheduled.py index 2ac4be6..a3f7310 100644 --- a/app/api/scheduled.py +++ b/app/api/scheduled.py @@ -56,7 +56,18 @@ def _scheduled_payload(s): 'frequency': s.frequency, 'frequency_label': s.frequency_label, 'mode': s.mode, + # phase46 recurrence detail. `recurrence_label` is the display string + # ("Weekly · Mon, Wed, Fri"); the raw fields let the iPad render its own. + 'recurrence_label': s.recurrence_label, + 'weekdays': s.weekday_list, + 'month_mode': s.month_mode, + 'day_of_month': s.day_of_month, + 'nth_week': s.nth_week, + 'nth_weekday': s.nth_weekday, 'next_due_date': s.next_run_at.date().isoformat() if s.next_run_at else None, + # phase47. Additive: the iPad decodes explicit CodingKeys, so a build + # that predates this key ignores it rather than failing to decode. + 'end_date': s.end_date.isoformat() if s.end_date else None, 'is_overdue': s.is_overdue(), 'notes': s.notes or None, } diff --git a/app/models/inspection_schedule.py b/app/models/inspection_schedule.py index 64f26f4..654ec76 100644 --- a/app/models/inspection_schedule.py +++ b/app/models/inspection_schedule.py @@ -27,12 +27,93 @@ phase43 adds the single-tenant "plan" semantics alongside that: deactivates a one-time schedule or rolls a recurring one forward. `next_run_at` is the due datetime for both modes. + +phase45/46/47 port the single-tenant recurrence + end-date model +(ST phase43 + phase44) onto this table without renaming anything: + + phase45 frequency ENUM gains 'once', 'bi-annually' and 'annually' + phase46 weekdays / month_mode / day_of_month / nth_week / nth_weekday + phase47 end_date + +`next_run_at` keeps its name, its DATETIME type and its index — it remains the +due datetime, and is ST's `next_due_date` by another name. All recurrence maths +happens on its DATE part; the TIME part is preserved across roll-forwards +(defaulting to 06:00, the hour `_compute_next_run()` has always used) so the +auto-mode cron keeps firing at the same time of day. + +Two dates, deliberately distinct: + next_run_at — mutable state. The next occurrence. Rewritten by fulfill() + after every completed inspection. + end_date — fixed boundary. The last date an occurrence may fall on, set by + the manager and never rewritten. NULL = repeat indefinitely. """ +import calendar +from datetime import date, datetime, time, timedelta + from app import db from app.utils.time_utils import now_eastern +# Every frequency this table accepts (phase45). 'bi-annually' is every 6 months. +FREQUENCY_CHOICES = ('once', 'daily', 'weekly', 'monthly', + 'quarterly', 'bi-annually', 'annually') + +# Recurring frequencies that advance by whole calendar months. +_MONTH_STEPS = { + 'monthly': 1, + 'quarterly': 3, + 'bi-annually': 6, + 'annually': 12, +} + +# Monthly recurrence styles (phase46). Stored as VARCHAR, not ENUM, so adding a +# style later needs no 3-step MySQL ENUM migration. +MONTH_MODE_DAY = 'day_of_month' # "the 15th of every month" +MONTH_MODE_NTH = 'nth_weekday' # "the 2nd Tuesday of every month" + +# Python weekday numbering: Monday=0 … Sunday=6 (matches date.weekday()). +WEEKDAY_NAMES = ('Monday', 'Tuesday', 'Wednesday', 'Thursday', + 'Friday', 'Saturday', 'Sunday') +WEEKDAY_ABBREV = ('Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat', 'Sun') + +# nth_week: 1–4 are literal, 5 means "5th (or last if the month is short)", +# -1 means "last" explicitly. +NTH_WEEK_LABELS = {1: '1st', 2: '2nd', 3: '3rd', 4: '4th', 5: '5th', -1: 'Last'} + +# The hour auto-mode schedules have always been due at (see _compute_next_run() +# in app/routes/inspection_schedules.py). Used when a schedule has no +# next_run_at yet and therefore no time-of-day to preserve. +DEFAULT_RUN_HOUR = 6 + + +def _last_day_of(year, month): + return calendar.monthrange(year, month)[1] + + +def _shift_month(year, month, n=1): + """Return (year, month) shifted by *n* months.""" + idx = year * 12 + (month - 1) + n + return idx // 12, idx % 12 + 1 + + +def _nth_weekday_of(year, month, weekday, nth): + """Date of the *nth* *weekday* in a month. + + ``nth == -1`` means the last one. A requested 5th occurrence that does not + exist falls back to the 4th, so every month yields a valid date. + """ + last = _last_day_of(year, month) + if nth == -1: + d = date(year, month, last) + return d - timedelta(days=(d.weekday() - weekday) % 7) + first = date(year, month, 1) + day = 1 + ((weekday - first.weekday()) % 7) + (nth - 1) * 7 + while day > last: + day -= 7 + return date(year, month, day) + + class InspectionSchedule(db.Model): __tablename__ = 'inspection_schedules' @@ -56,9 +137,11 @@ class InspectionSchedule(db.Model): nullable=False ) - # daily | weekly | monthly | quarterly — mirrors InspectionTemplate.frequency + # once | daily | weekly | monthly | quarterly | bi-annually | annually + # (phase45 — 'once', 'bi-annually' and 'annually' added; the original four + # values are unchanged, so no existing row is affected.) frequency = db.Column( - db.Enum('daily', 'weekly', 'monthly', 'quarterly'), + db.Enum(*FREQUENCY_CHOICES), nullable=False, default='weekly' ) active = db.Column(db.Boolean, nullable=False, default=True) @@ -76,6 +159,30 @@ class InspectionSchedule(db.Model): last_run_at = db.Column(db.DateTime, nullable=True) # last successful materialisation next_run_at = db.Column(db.DateTime, nullable=True) # when the next inspection is due + # ── Recurrence detail (phase46) ────────────────────────────────────────── + # weekly : CSV of Python weekday ints, e.g. '0,2,4' = Mon/Wed/Fri. + # NULL/empty falls back to the legacy "every 7 days" behaviour. + # monthly and the other month-stepping frequencies: + # month_mode picks which pair of columns applies — + # MONTH_MODE_DAY → day_of_month; MONTH_MODE_NTH → nth_week + nth_weekday. + # NULL falls back to the legacy "same day next period" behaviour. + # Every pre-phase46 row keeps NULLs here and therefore keeps its exact + # current cadence. + weekdays = db.Column(db.String(20), nullable=True) + month_mode = db.Column(db.String(20), nullable=True) + day_of_month = db.Column(db.SmallInteger, nullable=True) + nth_week = db.Column(db.SmallInteger, nullable=True) + nth_weekday = db.Column(db.SmallInteger, nullable=True) + + # ── End-date boundary (phase47) ────────────────────────────────────────── + # Fixed boundary set by the manager, never rewritten by the app — unlike + # next_run_at, which fulfill() advances after every completed inspection. + # NULL = repeat indefinitely, which is what every pre-phase47 row is. Only + # meaningful for recurring schedules; the create/edit routes force it to + # NULL when frequency == 'once'. Applies to BOTH modes: without it an auto + # schedule would keep materialising inspections past its boundary forever. + end_date = db.Column(db.Date, nullable=True) + # phase43 — plan mode bookkeeping last_completed_at = db.Column(db.DateTime, nullable=True) # Per-occurrence reminder de-dup flags; reset when a recurring schedule rolls forward. @@ -91,10 +198,13 @@ class InspectionSchedule(db.Model): creator = db.relationship('User', foreign_keys=[created_by]) FREQUENCY_LABELS = { - 'daily': 'Daily', - 'weekly': 'Weekly', - 'monthly': 'Monthly', - 'quarterly': 'Quarterly', + 'once': 'One-time', + 'daily': 'Daily', + 'weekly': 'Weekly', + 'monthly': 'Monthly', + 'quarterly': 'Quarterly', + 'bi-annually': 'Every 6 months', + 'annually': 'Annually', } @property @@ -113,18 +223,223 @@ class InspectionSchedule(db.Model): today = today or now_eastern().date() return self.next_run_at.date() < today + # ── Recurrence accessors (phase46) ─────────────────────────────────────── + + @property + def weekday_list(self): + """Selected weekdays as a sorted list of ints (Mon=0). [] if unset.""" + if not self.weekdays: + return [] + out = set() + for part in str(self.weekdays).split(','): + part = part.strip() + if part.lstrip('-').isdigit() and 0 <= int(part) <= 6: + out.add(int(part)) + return sorted(out) + + def set_weekdays(self, values): + """Store an iterable of weekday ints as the CSV column (None if empty).""" + clean = sorted({int(v) for v in (values or []) if 0 <= int(v) <= 6}) + self.weekdays = ','.join(str(v) for v in clean) or None + + @property + def recurrence_label(self): + """Human summary of the recurrence rule, e.g. 'Weekly · Mon, Wed, Fri'.""" + base = self.frequency_label + if self.frequency == 'weekly': + days = self.weekday_list + if days: + return f"{base} · {', '.join(WEEKDAY_ABBREV[d] for d in days)}" + elif self.frequency in _MONTH_STEPS: + if self.month_mode == MONTH_MODE_NTH and self.nth_week and self.nth_weekday is not None: + nth = NTH_WEEK_LABELS.get(self.nth_week, str(self.nth_week)) + return f'{base} · {nth} {WEEKDAY_NAMES[self.nth_weekday]}' + if self.day_of_month: + return f'{base} · day {self.day_of_month}' + return base + + # ── Date arithmetic (phase46) ──────────────────────────────────────────── + + @property + def run_time(self): + """Time-of-day this schedule is due at. + + Preserved across roll-forwards so an auto schedule keeps materialising + at the hour it always has. Falls back to DEFAULT_RUN_HOUR for a schedule + that has no next_run_at yet. + """ + if self.next_run_at is not None: + return self.next_run_at.time() + return time(hour=DEFAULT_RUN_HOUR) + + def set_next_run_date(self, d): + """Set next_run_at to date *d* keeping the current time-of-day.""" + if d is None: + self.next_run_at = None + else: + self.next_run_at = datetime.combine(d, self.run_time) + + @staticmethod + def _add_interval(d, frequency): + """Return date *d* advanced by one plain interval of *frequency*. + + Fallback used when no day-of-week / day-of-month detail is configured + (every pre-phase46 row). Prefer :meth:`next_occurrence_after`. + """ + if frequency == 'daily': + return d + timedelta(days=1) + if frequency == 'weekly': + return d + timedelta(weeks=1) + step = _MONTH_STEPS.get(frequency) + if step: + year, month = _shift_month(d.year, d.month, step) + return date(year, month, min(d.day, _last_day_of(year, month))) + return d # 'once' has no next interval + + def next_occurrence_after(self, d): + """First occurrence strictly after date *d*, honouring the day rules.""" + if self.frequency == 'weekly': + days = self.weekday_list + if days: + for step in range(1, 8): + cand = d + timedelta(days=step) + if cand.weekday() in days: + return cand + elif self.frequency in _MONTH_STEPS: + year, month = _shift_month(d.year, d.month, _MONTH_STEPS[self.frequency]) + if self.month_mode == MONTH_MODE_NTH and self.nth_week and self.nth_weekday is not None: + return _nth_weekday_of(year, month, self.nth_weekday, self.nth_week) + if self.day_of_month: + return date(year, month, min(self.day_of_month, _last_day_of(year, month))) + return self._add_interval(d, self.frequency) + + def align_due_date(self, d): + """Snap *d* forward to the first date on/after it that fits the rule. + + Lets a manager pick any start date and still get, say, Mon/Wed/Fri: + picking a Tuesday for a Mon/Wed/Fri schedule yields that Wednesday. + """ + if self.frequency == 'weekly': + days = self.weekday_list + if days: + for step in range(0, 7): + cand = d + timedelta(days=step) + if cand.weekday() in days: + return cand + elif self.frequency in _MONTH_STEPS: + if self.month_mode == MONTH_MODE_NTH and self.nth_week and self.nth_weekday is not None: + cand = _nth_weekday_of(d.year, d.month, self.nth_weekday, self.nth_week) + elif self.day_of_month: + cand = date(d.year, d.month, + min(self.day_of_month, _last_day_of(d.year, d.month))) + else: + return d + if cand < d: + return self.next_occurrence_after(cand) + return cand + return d + + # ── End-date boundary (phase47) ────────────────────────────────────────── + + def is_within_end_date(self, d): + """True if date *d* is on or before the end date (inclusive). + + No end date means the schedule repeats indefinitely, so every date + qualifies. + """ + return self.end_date is None or d <= self.end_date + + @property + def is_expired(self): + """True once the end date has passed. + + Independent of `active`: a schedule can be inactive because it expired + or because a manager paused it, and the list view distinguishes the two. + Compares against the *end date* rather than next_run_at, which may have + been advanced past the boundary by fulfill(). + """ + if self.end_date is None: + return False + return self.end_date < now_eastern().date() + + def expire_if_past_end_date(self, today=None): + """Deactivate a schedule whose end date has passed. Caller commits. + + Returns True if this call changed anything. Needed because a schedule + can reach its end date *without ever being completed* — fulfill() never + runs, so the boundary would otherwise be checked nowhere and the cron + would keep materialising (auto) or re-alerting as overdue (plan) + forever. Called from the /run cron endpoint. + """ + today = today or now_eastern().date() + if self.active and self.end_date is not None and self.end_date < today: + self.active = False + return True + return False + + # ── Roll-forward ───────────────────────────────────────────────────────── + + def advance_due_date(self, now=None): + """Move next_run_at to the first occurrence after today. Caller commits. + + Shared by fulfill() (plan mode, the inspector submitted the inspection) + and the cron materialiser (auto mode, an occurrence was produced), so + both modes obey the same recurrence rules and the same end-date + boundary. A 'once' schedule deactivates and its due date is left where + it is. Returns the new due date, or None for 'once'. + + Advances from the current due date rather than from now, so a Mon/Wed/Fri + schedule dealt with late stays on Mon/Wed/Fri. + """ + now = now or now_eastern() + if self.frequency == 'once': + self.active = False + return None + + today = now.date() + base = self.due_date or today + nxt = self.next_occurrence_after(base) + guard = 0 + # Guard stops a misconfigured row spinning; 400 covers a daily schedule + # left untouched for over a year. + while nxt <= today and guard < 400: + nxt = self.next_occurrence_after(nxt) + guard += 1 + self.set_next_run_date(nxt) + + # Past the manager's boundary: this was the last occurrence. next_run_at + # is left at the computed value rather than clamped, so the row still + # shows which occurrence it stopped before. + if not self.is_within_end_date(nxt): + self.active = False + return nxt + def fulfill(self, next_run_fn=None): """Mark this occurrence complete. Caller commits. - Recurring schedules roll their due date forward past today and reset the - reminder flags; MT has no 'once' frequency, so a schedule stays active. - `next_run_fn(frequency, from_dt)` computes the next due datetime — the - route passes `_compute_next_run` so the cadence math lives in one place. + One-time schedules deactivate. Recurring ones roll their due date + forward past today, honouring the phase46 day rules, and reset the + reminder flags. A recurring schedule whose next occurrence would fall + past its end date deactivates instead (phase47). + + `next_run_fn` is accepted and IGNORED, retained only so the existing + `fulfill(next_run_fn=_compute_next_run)` call sites in + routes/inspections.py and api/inspections.py keep working unchanged. + Before phase46 the cadence maths lived in the route and omitting this + argument silently left next_run_at untouched — leaving the schedule + perpetually due. The maths now lives here, on the object that owns the + recurrence columns, so that failure mode is unreachable. """ now = now_eastern() self.last_completed_at = now - if next_run_fn is not None: - self.next_run_at = next_run_fn(self.frequency, now) + + self.advance_due_date(now) + if not self.active: + # 'once', or the roll-forward crossed the end date. Either way this + # was the last occurrence — leave the reminder flags set so nothing + # re-fires against a closed schedule. + return + self.advance_notified = False self.due_notified = False self.overdue_notified = False diff --git a/app/routes/inspection_schedules.py b/app/routes/inspection_schedules.py index f3ea7ba..2fe0fa5 100644 --- a/app/routes/inspection_schedules.py +++ b/app/routes/inspection_schedules.py @@ -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): diff --git a/app/templates/inspection_schedules/form.html b/app/templates/inspection_schedules/form.html index ac56370..f4407c6 100644 --- a/app/templates/inspection_schedules/form.html +++ b/app/templates/inspection_schedules/form.html @@ -29,16 +29,109 @@
- {% for f in frequencies %} + {{ frequency_labels.get(f, f|title) }} {% endfor %}
+ {# ── Recurrence detail (phase46) ──────────────────────────────────── + Only the block matching the chosen frequency is shown; the server + validates the same block and clears the others on save. #} + + + + +
+
+ + +
+ {% if schedule %} + When the next occurrence is due. Leave unchanged and saving will + not move it. + {% else %} + Leave blank to start one full period from now. + {% endif %} +
+
+
+ + +
+ Last date this schedule may produce an occurrence. Blank = repeats + indefinitely. +
+
+
+
@@ -103,12 +196,15 @@

- Saving recomputes the next {{ 'due date' if schedule.mode == 'plan' else 'run' }} - from now, and resets this occurrence's reminders. Currently: + Current {{ 'due date' if schedule.mode == 'plan' else 'run' }}: {{ schedule.next_run_at.strftime('%Y-%m-%d %H:%M') if schedule.next_run_at else '—' }} {% if schedule.last_completed_at %} · last completed {{ schedule.last_completed_at.strftime('%Y-%m-%d %H:%M') }} {% endif %} + {% if schedule.end_date %}· ends {{ schedule.end_date.strftime('%Y-%m-%d') }}{% endif %} +
+ Reminders for this occurrence are only reset if the due date actually + moves, so renaming or re-noting a schedule will not re-send them.

{% endif %}
@@ -124,6 +220,46 @@