Jul 2nd - Optimized code 6

Claude.md

@@ -510,6 +510,8 @@ Migrations live in `migrations/versions/`. The chain is:
                     └── 004_widen_system_setting_value — system_settings.value VARCHAR(500) → TEXT
                           └── 005_add_watchers_and_time_entries — creates ticket_watchers
                                                                    and time_entries tables
+                                └── 006_add_sla_breach_notified — adds tickets.sla_breach_notified;
+                                                                   retires sla_notified_tickets setting
 ```
 
 **Rules:**
@@ -845,6 +847,7 @@ When an error is reported:
 | `003_render_comments` | Backfill comment bodies to HTML; widen `alembic_version.version_num` to `VARCHAR(64)` |
 | `004_widen_system_setting_value` | Widen `system_settings.value` from `VARCHAR(500)` to `TEXT` |
 | `005_add_watchers_and_time_entries` | Create `ticket_watchers` (UniqueConstraint + index) and `time_entries` (index) tables |
+| `006_add_sla_breach_notified` | Add `tickets.sla_breach_notified` boolean; backfill from and retire the `sla_notified_tickets` SystemSetting |
 
 ## 21. Browser Tab Notification Counter (added 2026-04-17)
 

app/models.py

@@ -113,6 +113,12 @@ class Ticket(db.Model):
     closed_at      = db.Column(db.DateTime)
     due_date       = db.Column(db.DateTime)
 
+    # Set once an SLA breach notification has been sent for this ticket, so
+    # the 30-minute scheduler job doesn't re-notify every run. Cleared when
+    # the ticket resolves/closes/re-opens (see sla_service.clear_sla_notification)
+    # so a re-opened ticket gets a fresh alert if it breaches again.
+    sla_breach_notified = db.Column(db.Boolean, nullable=False, default=False, server_default='0')
+
     ai_generated   = db.Column(db.Boolean, default=False)
     internal_notes = db.Column(db.Text)
     resolution_notes = db.Column(db.Text)

app/routes/admin.py

@@ -617,15 +617,26 @@ def kb_delete_attachment(article_id, att_id):
     att = KBAttachment.query.filter_by(id=att_id, article_id=article_id).first_or_404()
     upload_dir = current_app.config['UPLOAD_FOLDER']
     filepath = os.path.join(upload_dir, att.stored_name)
-    if os.path.exists(filepath):
+    att_id_val, filename = att.id, att.filename
-        os.remove(filepath)
+
-    log_action(current_user.id, 'kb_attachment_delete', 'kb_attachment', att.id,
+    # DB row is the source of truth — delete and commit it first, then remove
-               f'article_id={article_id} filename={att.filename}')
+    # the physical file. If the process dies mid-operation this leaves at
-    logger.info(f'[KB ATTACHMENT DELETE] att_id={att.id} article_id={article_id} by user_id={current_user.id}')
+    # worst an orphan file on disk (harmless, cleanable later) rather than a
+    # DB row pointing at a file that's already gone (a broken download link).
+    log_action(current_user.id, 'kb_attachment_delete', 'kb_attachment', att_id_val,
+               f'article_id={article_id} filename={filename}')
     db.session.delete(att)
     db.session.commit()
+    logger.info(f'[KB ATTACHMENT DELETE] att_id={att_id_val} article_id={article_id} by user_id={current_user.id}')
+
+    if os.path.exists(filepath):
+        try:
+            os.remove(filepath)
+        except OSError as exc:
+            logger.warning(f'[KB ATTACHMENT DELETE] Could not remove file {filepath}: {exc}')
+
     # Return JSON so the edit page can remove the row without a full reload
-    return jsonify({'ok': True, 'att_id': att.id})
+    return jsonify({'ok': True, 'att_id': att_id_val})
 
 
 

app/services/sla_service.py

@@ -16,11 +16,11 @@ Responsibilities
 
 Notification suppression
 ------------------------
-A dedicated SystemSetting key  sla_notified_tickets  stores a
+Ticket.sla_breach_notified is a per-ticket boolean flag, set once a breach
-comma-separated list of ticket IDs that have already received a breach
+notification has been sent so the 30-minute scheduler run doesn't re-notify
-notification.  When a ticket is resolved or closed the ID is removed
+every time.  When a ticket is resolved or closed the flag is cleared so
-from the list so the suppression does not persist across re-opens
+the suppression does not persist across re-opens (edge case: ticket
-(edge case: ticket re-opened after resolution — unlikely but handled).
+re-opened after resolution — unlikely but handled).
 
 Design notes
 ------------
@@ -137,10 +137,10 @@ def check_sla_breaches(app):
     """Scheduled job: find overdue tickets and notify responsible parties.
 
     Safe to call repeatedly — already-notified tickets are suppressed via
-    the  sla_notified_tickets  SystemSetting key.  The suppression list is
+    Ticket.sla_breach_notified.  The flag is cleared for a ticket when it
-    cleared for a ticket when it transitions to resolved/closed (handled by
+    transitions to resolved/closed (handled by the update_ticket route) or
-    the update_ticket route clearing it on status change) or when the ticket
+    when the ticket is re-opened, ensuring fresh notifications if the issue
-    is re-opened, ensuring fresh notifications if the issue resurfaces.
+    resurfaces.
     """
     with app.app_context():
         from app.services.license_service import feature_enabled
@@ -155,24 +155,18 @@ def check_sla_breaches(app):
 
 def _run_sla_check(app):
     from app import db
-    from app.models import (
+    from app.models import Ticket, TicketStatus, User, UserRole, NotificationType
-        Ticket, TicketStatus, User, UserRole,
-        NotificationType, SystemSetting,
-    )
     from app.services.notification_service import create_notification, send_email
     from flask import render_template_string
 
     now = datetime.utcnow()
 
-    # ── Load suppression list ──────────────────────────────────────────────────
+    # ── Query overdue, not-yet-notified open tickets ───────────────────────────
-    raw = SystemSetting.get('sla_notified_tickets', '')
-    already_notified = set(int(x) for x in raw.split(',') if x.strip().isdigit())
-
-    # ── Query overdue open tickets ─────────────────────────────────────────────
     overdue = Ticket.query.filter(
         Ticket.status.in_([TicketStatus.OPEN, TicketStatus.IN_PROGRESS]),
         Ticket.due_date.isnot(None),
         Ticket.due_date < now,
+        Ticket.sla_breach_notified.is_(False),
     ).all()
 
     if not overdue:
@@ -185,9 +179,6 @@ def _run_sla_check(app):
     newly_notified = []
 
     for ticket in overdue:
-        if ticket.id in already_notified:
-            continue   # already sent — skip
-
         ticket_url    = f'{base_url}/tickets/{ticket.id}'
         overdue_mins  = int((now - ticket.due_date).total_seconds() / 60)
         overdue_label = (
@@ -263,18 +254,11 @@ def _run_sla_check(app):
         if it_dept_email:
             send_email(subject, [it_dept_email], html)
 
+        ticket.sla_breach_notified = True
         newly_notified.append(ticket.id)
         db.session.commit()
 
-    # ── Update suppression list ────────────────────────────────────────────────
     if newly_notified:
-        updated = already_notified | set(newly_notified)
-        SystemSetting.set(
-            'sla_notified_tickets',
-            ','.join(str(i) for i in sorted(updated)),
-            'Comma-separated ticket IDs that have received SLA breach notifications',
-        )
-        db.session.commit()
         logger.info(
             f'[SLA] Notified {len(newly_notified)} breach(es): '
             f'{[str(i) for i in newly_notified]}'
@@ -282,17 +266,14 @@ def _run_sla_check(app):
 
 
 def clear_sla_notification(ticket_id: int):
-    """Remove a ticket from the SLA suppression list.
+    """Clear the SLA breach-notified flag for a ticket.
 
     Call this when a ticket is resolved, closed, or re-opened so that
     subsequent breaches (if the ticket re-opens) trigger fresh alerts.
     Callers are responsible for committing after calling this function.
     """
-    from app.models import SystemSetting
+    from app import db
-    raw     = SystemSetting.get('sla_notified_tickets', '')
+    from app.models import Ticket
-    current = set(int(x) for x in raw.split(',') if x.strip().isdigit())
+    ticket = db.session.get(Ticket, ticket_id)
-    current.discard(ticket_id)
+    if ticket:
-    SystemSetting.set(
+        ticket.sla_breach_notified = False
-        'sla_notified_tickets',
-        ','.join(str(i) for i in sorted(current)),
-    )

migrations/versions/006_add_sla_breach_notified.py

@@ -0,0 +1,62 @@
+"""Add tickets.sla_breach_notified column; retire sla_notified_tickets setting
+
+Revision ID: 006_add_sla_breach_notified
+Revises: 005_add_watchers_and_time_entries
+Create Date: 2026-07-02
+
+Rationale
+---------
+SLA breach-notification suppression was previously tracked as a
+comma-separated list of ticket IDs in a single SystemSetting row
+(sla_notified_tickets). That works but doesn't scale cleanly and requires
+parsing a string on every 30-minute scheduler run. This migration replaces
+it with a plain boolean column on the ticket itself — the natural place for
+a per-ticket flag — and backfills it from the existing SystemSetting value
+so already-notified tickets don't get re-notified after the upgrade.
+
+Apply
+-----
+    flask db upgrade
+
+Rollback
+--------
+    flask db downgrade
+    (Note: the backfilled sla_breach_notified flags are not restored back
+    into a SystemSetting row on downgrade — if you roll back and then
+    upgrade again, already-breached tickets will re-notify once.)
+"""
+
+from alembic import op
+import sqlalchemy as sa
+
+revision      = '006_add_sla_breach_notified'
+down_revision = '005_add_watchers_and_time_entries'
+branch_labels = None
+depends_on    = None
+
+
+def upgrade():
+    op.add_column(
+        'tickets',
+        sa.Column('sla_breach_notified', sa.Boolean, nullable=False, server_default='0'),
+    )
+
+    # Backfill from the old comma-separated SystemSetting, if present.
+    conn = op.get_bind()
+    row = conn.execute(
+        sa.text("SELECT value FROM system_settings WHERE `key` = 'sla_notified_tickets'")
+    ).fetchone()
+    if row and row[0]:
+        ticket_ids = [int(x) for x in row[0].split(',') if x.strip().isdigit()]
+        if ticket_ids:
+            conn.execute(
+                sa.text(
+                    "UPDATE tickets SET sla_breach_notified = 1 WHERE id IN :ids"
+                ).bindparams(sa.bindparam('ids', expanding=True)),
+                {'ids': ticket_ids},
+            )
+    conn.execute(sa.text("DELETE FROM system_settings WHERE `key` = 'sla_notified_tickets'"))
+
+
+def downgrade():
+    op.drop_column('tickets', 'sla_breach_notified')