From cc81e0cca32e35eb4e3eb0729857a089c5c4da28 Mon Sep 17 00:00:00 2001 From: NguyenND Date: Wed, 8 Apr 2026 10:38:50 -0400 Subject: [PATCH] 04/08 fixed survey email issue --- app/__init__.py | 1 + app/models.py | 25 +++- app/routes/admin.py | 15 +++ app/routes/auth.py | 1 + app/services/notification_service.py | 195 +++++++++++++++++---------- app/templates/admin/settings.html | 49 +++++++ 6 files changed, 210 insertions(+), 76 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index d4af6d1..3bd4fa1 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -315,6 +315,7 @@ def _seed_settings(): from app.models import SystemSetting defaults = [ ('registration_enabled', 'true', 'Allow new users to self-register via /auth/register'), + ('survey_enabled', 'true', 'Send a satisfaction survey email when a ticket is resolved'), ('app_timezone', 'America/New_York', 'Display timezone for all dates and times in the UI'), ('sla_critical_hours', '4', 'Hours before a CRITICAL ticket is considered overdue'), ('email_ingestion_enabled', '0', 'Enable automatic ticket creation from inbound email (1=on, 0=off)'), diff --git a/app/models.py b/app/models.py index 5f37862..da6788a 100644 --- a/app/models.py +++ b/app/models.py @@ -564,12 +564,27 @@ class TicketSatisfaction(db.Model): @classmethod def create_for_ticket(cls, ticket): - """Create a survey row for a newly-resolved ticket. - Returns None if a survey already exists. Caller must commit. + """Return a survey row ready for emailing. + + Behaviour: + - No existing row → create and return a new one. + - Existing row, not yet submitted (rating is NULL) → return it so + the email can be re-sent. This covers the case where the row was + created but the email thread crashed before sending. + - Existing row, already submitted → return None (survey is complete). + + Caller is responsible for committing the session. """ import secrets - if cls.query.filter_by(ticket_id=ticket.id).first(): - return None + existing = cls.query.filter_by(ticket_id=ticket.id).first() + if existing: + if existing.submitted: + # Employee already rated — do not re-send + return None + # Row exists but email was never sent (prior crash) — re-use it + # Regenerate the token so any stale links in old emails are invalidated + existing.survey_token = secrets.token_urlsafe(32) + return existing row = cls( ticket_id = ticket.id, user_id = ticket.created_by_id, @@ -583,4 +598,4 @@ class TicketSatisfaction(db.Model): return self.rating is not None def __repr__(self): - return f'' + return f'' \ No newline at end of file diff --git a/app/routes/admin.py b/app/routes/admin.py index 0b188e3..93e9192 100644 --- a/app/routes/admin.py +++ b/app/routes/admin.py @@ -1183,6 +1183,19 @@ def settings(): return redirect(url_for('admin.settings')) # ── Timezone setting ────────────────────────────────────────────────── + if form_type == 'survey': + new_value = '1' if request.form.get('survey_enabled') == '1' else '0' + old_value = SystemSetting.get('survey_enabled', '1') + SystemSetting.set('survey_enabled', new_value, + 'Send satisfaction survey email when a ticket is resolved') + db.session.commit() + state_label = 'enabled' if new_value == '1' else 'disabled' + log_action(current_user.id, 'setting_update', 'system_setting', None, + f'survey_enabled changed from {old_value} to {new_value}') + logger.info(f'[ADMIN SETTINGS] survey_enabled={new_value} by admin_id={current_user.id}') + flash(f'Satisfaction survey has been {state_label}.', 'success') + return redirect(url_for('admin.settings')) + if form_type == 'timezone': from zoneinfo import ZoneInfo, ZoneInfoNotFoundError new_tz = request.form.get('app_timezone', 'America/New_York').strip() @@ -1299,6 +1312,7 @@ def settings(): return redirect(url_for('admin.settings')) registration_enabled = SystemSetting.get_bool('registration_enabled', default=True) + survey_enabled = SystemSetting.get_bool('survey_enabled', default=True) email_settings = { 'enabled' : SystemSetting.get_bool('email_ingestion_enabled', default=False), 'host' : SystemSetting.get('email_ingestion_host', ''), @@ -1341,6 +1355,7 @@ def settings(): ] return render_template('admin/settings.html', registration_enabled=registration_enabled, + survey_enabled=survey_enabled, branding_settings=branding_settings, email_settings=email_settings, current_tz=current_tz, diff --git a/app/routes/auth.py b/app/routes/auth.py index 04b7471..ff6f689 100644 --- a/app/routes/auth.py +++ b/app/routes/auth.py @@ -327,3 +327,4 @@ def serve_logo(filename): abort(400) upload_dir = current_app.config['UPLOAD_FOLDER'] return send_from_directory(upload_dir, filename, as_attachment=False) + diff --git a/app/services/notification_service.py b/app/services/notification_service.py index 9c110e8..ee3ba19 100644 --- a/app/services/notification_service.py +++ b/app/services/notification_service.py @@ -319,102 +319,155 @@ def notify_assignment(ticket, assigned_by): def send_satisfaction_survey(ticket): """Send a satisfaction survey email when a ticket is resolved. - Creates a TicketSatisfaction row with a unique survey token, then - emails the ticket creator a link to rate their experience (1-5 stars). - The link is token-authenticated so the employee does not need to be - logged in to respond. - - URL construction - ---------------- - All other notification functions in this module use APP_BASE_URL from - config to build absolute URLs — NOT url_for(..., _external=True). - This function follows the same pattern. Using url_for inside an f-string - that is evaluated before the background thread starts causes a - RuntimeError ("Working outside of request context") which silently - swallows the entire function before the Thread is ever created. - - Called from update_ticket() after commit, when status → Resolved. + Every step is wrapped in explicit error handling and logged so failures + are visible in the application log rather than swallowed silently. """ from app.models import TicketSatisfaction from app import mail from threading import Thread from flask_mail import Message - creator = ticket.creator - if not creator or not creator.email_notif: + logger.info( + f'[SURVEY] send_satisfaction_survey called: ' + f'ticket_id={ticket.id} number={ticket.ticket_number}' + ) + + # Guard: survey feature must be enabled in system settings + from app.models import SystemSetting + if not SystemSetting.get_bool('survey_enabled', default=True): + logger.info('[SURVEY] Satisfaction survey is disabled in settings — skipping') + return + + # Guard: creator must exist and have email notifications enabled + creator = ticket.creator + if not creator: + logger.warning(f'[SURVEY] No creator found for ticket_id={ticket.id} — skipping') + return + if not creator.email_notif: + logger.info( + f'[SURVEY] Creator user_id={creator.id} has email_notif=False — skipping' + ) + return + + logger.info( + f'[SURVEY] Creator OK: user_id={creator.id} ' + f'email={creator.email} email_notif={creator.email_notif}' + ) + + # Create the TicketSatisfaction row + try: + survey = TicketSatisfaction.create_for_ticket(ticket) + except Exception as exc: + logger.error( + f'[SURVEY] create_for_ticket raised an exception for ' + f'ticket_id={ticket.id}: {exc}', exc_info=True + ) return - survey = TicketSatisfaction.create_for_ticket(ticket) if not survey: - return # already sent for this ticket + logger.info( + f'[SURVEY] Survey already exists for ticket_id={ticket.id} — skipping' + ) + return - db.session.commit() - logger.info(f'[SURVEY CREATED] ticket_id={ticket.id} token={survey.survey_token[:8]}…') + try: + db.session.commit() + except Exception as exc: + logger.error( + f'[SURVEY] db.session.commit() failed for ticket_id={ticket.id}: {exc}', + exc_info=True + ) + db.session.rollback() + return - # Build absolute URLs using APP_BASE_URL — identical to every other - # notification function in this file. url_for(_external=True) requires - # an active request context which is not guaranteed here. + logger.info( + f'[SURVEY CREATED] ticket_id={ticket.id} ' + f'survey_id={survey.id} token={survey.survey_token[:8]}...' + ) + + # Build absolute URLs using APP_BASE_URL — no url_for() which requires + # an active request context not guaranteed in all calling paths base_url = current_app.config.get('APP_BASE_URL', '').rstrip('/') + if not base_url: + logger.warning( + '[SURVEY] APP_BASE_URL is not set — survey links will be broken. ' + 'Set APP_BASE_URL in your .env (e.g. https://tickets.ltservicesinc.com)' + ) survey_url = f"{base_url}/survey/{survey.survey_token}" ticket_url = f"{base_url}/tickets/{ticket.id}" + logger.info(f'[SURVEY] survey_url={survey_url}') + stars_html = ''.join( - f'★' for i in range(1, 6) ) - html = f""" - -
-
-

⭐ How did we do?

-
-
-

Hi {creator.full_name},

-

- Your ticket {ticket.ticket_number} — - {ticket.title} — has been marked as resolved. - We'd love to hear how we did! -

-

- Click a star to rate your experience: -

-

- {stars_html} -

-

- - Or leave a detailed comment - -

-

- If you feel the issue is not fully resolved, you can - re-open your ticket - at any time. -

-
-
- TechDesk IT Helpdesk • This is an automated message. -
-
-""" + html = ( + '' + '
' + '
' + '

⭐ How did we do?

' + '
' + '
' + f'

Hi {creator.full_name},

' + f'

Your ticket {ticket.ticket_number}' + f' has been marked as resolved. We'd love to hear how we did!

' + '

' + 'How satisfied were you with the resolution?

' + f'

{stars_html}

' + f'

' + f'' + 'Or leave a written comment

' + f'

If the issue is not resolved, ' + f're-open your ticket.

' + '
' + '
' + 'TechDesk IT Helpdesk • This is an automated message.' + '
' + ) - msg = Message( - subject = f'[TechDesk] How did we do? — {ticket.ticket_number}', - recipients = [creator.email], - html = html, + # Capture locals needed inside the thread before spawning. + # Mirrors the pattern in send_email() exactly: + # - current_app._get_current_object() resolves the proxy to the real + # Flask app object so it is safe to reference inside a daemon thread + # that has no active context of its own yet. + # - The Message is built INSIDE the thread, within app.app_context(), + # not before — flask_mail reads app config at construction time. + app = current_app._get_current_object() + ticket_num = ticket.ticket_number + ticket_id = ticket.id + creator_name = creator.full_name + creator_email= creator.email + + logger.info( + f'[SURVEY] Spawning email thread: recipient={creator_email} ' + f'ticket_id={ticket_id}' ) def _send(): - with current_app.app_context(): + with app.app_context(): try: + msg = Message( + subject = f'[TechDesk] How did we do? — {ticket_num}', + recipients = [creator_email], + html = html, + ) mail.send(msg) - logger.info(f'[SURVEY EMAIL SENT] ticket_id={ticket.id} user_id={creator.id}') + logger.info( + f'[SURVEY EMAIL SENT] ticket_id={ticket_id} ' + f'recipient={creator_email}' + ) except Exception as exc: - logger.error(f'[SURVEY EMAIL FAILED] ticket_id={ticket.id} {exc}') + logger.error( + f'[SURVEY EMAIL FAILED] ticket_id={ticket_id} ' + f'recipient={creator_email}: {exc}', exc_info=True + ) - Thread(target=_send, daemon=True).start() \ No newline at end of file + t = Thread(target=_send, daemon=True) + t.start() + logger.info(f'[SURVEY] Email thread started for ticket_id={ticket_id}') diff --git a/app/templates/admin/settings.html b/app/templates/admin/settings.html index 211798d..e31f6f4 100644 --- a/app/templates/admin/settings.html +++ b/app/templates/admin/settings.html @@ -244,6 +244,55 @@ + +
+
+
+ Satisfaction Survey +
+
+
+

+ When enabled, employees automatically receive a one-click satisfaction survey email + whenever their ticket is marked as Resolved. Responses appear in the + Satisfaction Report. +

+
+ + +
+
+
+ Send Survey on Resolve +
+
+ Automatically email employees a 1–5 star rating prompt when their ticket is resolved. +
+
+
+ + {{ 'Enabled' if survey_enabled else 'Disabled' }} + + {% if survey_enabled %} + + {% else %} + + {% endif %} +
+
+
+
+
+