From ef0c096e4855437a696de99497ed2d73b940aa8b Mon Sep 17 00:00:00 2001 From: gsinghpal Date: Thu, 21 May 2026 00:07:41 -0400 Subject: [PATCH] fix(fusion_repairs): Bundle 3 code-review fixes (H1-H5 + M1-M6 + L1) HIGH H1 X2 reminder flag was per-repair - multi-visit repairs missed reminders Moved x_fc_day_before_reminder_sent off repair.order onto fusion.technician.task so each scheduled visit is tracked separately. Cron now walks tasks directly with state-narrowed repair filter (confirmed/under_repair only, drops L1's draft inclusion). H2 X4 NPS cron used write_date - moved on every chatter/invoice write Added x_fc_done_at Datetime on repair.order, stamped on the first transition to state=done via write() override. Cron filters on ('x_fc_done_at', '<=', cutoff) instead of write_date. H3 X2 template's [:1] slice picked an arbitrary task, not tomorrow's Cron now passes the specific task via with_context(reminder_task_id=...). Template fetches that task by id; falls back to [:1] only for manual sends so chatter Send Email composer still works. H4 NPS Google-Search fallback URL not URL-encoded - breaks on &/spaces Template now uses url_encode({'q': company_name}) so "Westin & Sons" produces a working URL instead of truncating at the ampersand. H5 + L1 Loaner cron fired on drafts and used create_date instead of schedule_date Domain rewritten to: state in ('confirmed','under_repair'), exclude quote-only repairs, and EITHER schedule_date <= cutoff OR (schedule_date is False AND create_date <= cutoff). Added limit=200 ordered by create_date desc (M6). MEDIUM M1 Function-level datetime imports moved to module top date, datetime, timedelta imported once at the top of repair_order.py, removed from cron_send_day_before_reminders, cron_send_post_visit_nps, cron_offer_loaner_for_long_repairs. M2 _notifications_enabled duplicated - promoted to single source repair_order._notifications_enabled now delegates to fusion.repair.intake.service._notifications_enabled() (with a fallback ICP read if the service AbstractModel isn't available). M3 self.env.get('model') -> 'model' in self.env (Odoo standard idiom) Two call sites in repair_order.py converted. M4 + M5 Bare 'except: continue' + missing logger - operational blindness Added import logging + _logger to repair_order.py. All three crons now log exceptions with _logger.exception(). Activity-type ref check now warns + returns early if the xml id is missing (instead of passing activity_type_id=False which raises). For X2 and X4 the flag is set regardless of send-success so we don't retry indefinitely on permanently-misconfigured partners. M6 Loaner cron has limit=200 + order='create_date desc' Caps blast radius if 5000 stale draft repairs ever accumulate. L1 X2 state filter tightened: was ('not in', ('done','cancel')), now ('in', ('confirmed','under_repair')) so drafts and quote-only don't email "your tech is coming tomorrow". Verified - upgrade clean, no errors. Bumped to 19.0.1.3.1. Co-authored-by: Cursor --- fusion_repairs/__manifest__.py | 2 +- fusion_repairs/data/mail_template_data.xml | 9 +- fusion_repairs/models/repair_order.py | 145 +++++++++++++-------- fusion_repairs/models/technician_task.py | 7 + 4 files changed, 109 insertions(+), 54 deletions(-) diff --git a/fusion_repairs/__manifest__.py b/fusion_repairs/__manifest__.py index 4e0a7a13..ed2483bb 100644 --- a/fusion_repairs/__manifest__.py +++ b/fusion_repairs/__manifest__.py @@ -4,7 +4,7 @@ { 'name': 'Fusion Repairs', - 'version': '19.0.1.3.0', + 'version': '19.0.1.3.1', 'category': 'Inventory/Repairs', 'summary': 'Guided medical equipment repair intake, dispatch, maintenance, and self-service portal', 'description': """ diff --git a/fusion_repairs/data/mail_template_data.xml b/fusion_repairs/data/mail_template_data.xml index 0850ec6b..1ce415d0 100644 --- a/fusion_repairs/data/mail_template_data.xml +++ b/fusion_repairs/data/mail_template_data.xml @@ -77,8 +77,12 @@ reminder that your service call is scheduled for tomorrow.

+ + + - + @@ -121,7 +125,8 @@ We would love to hear how it went - your feedback helps other clients find us and helps us improve.

- + +
diff --git a/fusion_repairs/models/repair_order.py b/fusion_repairs/models/repair_order.py index df64d395..6c55cffe 100644 --- a/fusion_repairs/models/repair_order.py +++ b/fusion_repairs/models/repair_order.py @@ -2,11 +2,16 @@ # Copyright 2024-2026 Nexa Systems Inc. # License OPL-1 (Odoo Proprietary License v1.0) +import logging +from datetime import date, datetime, timedelta + from dateutil.relativedelta import relativedelta from odoo import api, fields, models, _ from odoo.exceptions import UserError +_logger = logging.getLogger(__name__) + INTAKE_SOURCES = [ ('backend_wizard', 'Backend Wizard (CS)'), @@ -136,91 +141,126 @@ class RepairOrder(models.Model): ) # ------------------------------------------------------------------ - # X2 / X4 - reminder + NPS sent-at flags (so the cron doesn't re-send) + # X4 + M3 - NPS sent flag + loaner offered flag + done-at stamp + # (X2 day-before flag now lives on fusion.technician.task per H1) # ------------------------------------------------------------------ - x_fc_day_before_reminder_sent = fields.Boolean( - string='Day-Before Reminder Sent', - copy=False, - ) x_fc_nps_email_sent = fields.Boolean( string='NPS Email Sent', copy=False, ) + x_fc_done_at = fields.Datetime( + string='Closed At', + copy=False, + readonly=True, + help='Stamped when the repair first transitions to state=done. ' + 'Drives the post-visit NPS cron (24h after close) without ' + 'getting pushed forward by every subsequent chatter message.', + ) + x_fc_loaner_offered = fields.Boolean( + string='Loaner Offered', + copy=False, + help='True once a loaner-offer activity has been posted for this ' + 'long-running repair (M3). Avoids re-posting daily.', + ) + + def write(self, vals): + # H2: stamp x_fc_done_at the first time state transitions to 'done' + # so the NPS cron has a stable timestamp (write_date moves on every + # chatter / invoice / attachment write). + if vals.get('state') == 'done': + for r in self: + if r.state != 'done' and not r.x_fc_done_at: + vals = dict(vals) + vals['x_fc_done_at'] = fields.Datetime.now() + break + return super().write(vals) # ------------------------------------------------------------------ - # X2 / X4 / M3 - reminder + NPS crons + loaner offer + # X2 / X4 / M3 crons # ------------------------------------------------------------------ @api.model def cron_send_day_before_reminders(self): - """X2: email the client the day before a scheduled tech visit. + """X2: email the client the day before each scheduled tech visit. - Walks repair.order records with a linked technician task scheduled - for tomorrow, sends one reminder per repair, marks - x_fc_day_before_reminder_sent=True so we never re-send. + Per-TASK flag (not per-repair) so multi-visit repairs get a + separate reminder for each individual visit. """ if not self._notifications_enabled(): return - from datetime import date, timedelta tomorrow = date.today() + timedelta(days=1) - # Find repairs with at least one task scheduled tomorrow that haven't - # been reminded yet. - repairs = self.search([ + Task = self.env['fusion.technician.task'].sudo() + tasks = Task.search([ + ('scheduled_date', '=', tomorrow), ('x_fc_day_before_reminder_sent', '=', False), - ('state', 'not in', ('done', 'cancel')), - ('x_fc_technician_task_ids.scheduled_date', '=', tomorrow), + ('x_fc_repair_order_id', '!=', False), + ('x_fc_repair_order_id.state', 'in', ('confirmed', 'under_repair')), ]) tpl = self.env.ref( 'fusion_repairs.email_template_visit_day_before', raise_if_not_found=False, ) if not tpl: + _logger.warning('X2 day-before cron: email template missing') return - for r in repairs: - if not r.partner_id or not r.partner_id.email: + for task in tasks: + repair = task.x_fc_repair_order_id + if not repair.partner_id or not repair.partner_id.email: + task.x_fc_day_before_reminder_sent = True # don't keep retrying continue try: - tpl.send_mail(r.id, force_send=False) - r.x_fc_day_before_reminder_sent = True + # Pass the specific task via context so the template renders + # the right scheduled date / technician (H3). + tpl.with_context(reminder_task_id=task.id) \ + .send_mail(repair.id, force_send=False) except Exception: - continue + _logger.exception('X2 day-before reminder failed for task %s', task.name) + # Still set the flag - the task's "tomorrow" is gone after midnight + # so retrying tomorrow would email about the wrong date. + task.x_fc_day_before_reminder_sent = True @api.model def cron_send_post_visit_nps(self): - """X4: send NPS / Google review email 24h after a repair is done.""" + """X4: send NPS / Google review email 24h after state=done. + + Uses x_fc_done_at (H2) so chatter writes don't push the timestamp + forward. + """ if not self._notifications_enabled(): return - from datetime import datetime, timedelta cutoff = datetime.now() - timedelta(hours=24) repairs = self.search([ ('state', '=', 'done'), ('x_fc_nps_email_sent', '=', False), - ('write_date', '<=', cutoff), + ('x_fc_done_at', '!=', False), + ('x_fc_done_at', '<=', cutoff), ]) tpl = self.env.ref( 'fusion_repairs.email_template_post_visit_nps', raise_if_not_found=False, ) if not tpl: + _logger.warning('X4 NPS cron: email template missing') return for r in repairs: if not r.partner_id or not r.partner_id.email: + r.x_fc_nps_email_sent = True # don't keep retrying continue try: tpl.send_mail(r.id, force_send=False) - r.x_fc_nps_email_sent = True except Exception: - continue + _logger.exception('X4 NPS email failed for repair %s', r.name) + r.x_fc_nps_email_sent = True @api.model def cron_offer_loaner_for_long_repairs(self): - """M3: post an Offer-Loaner activity when an open repair has been - sitting longer than fusion_repairs.loaner_offer_threshold_days - AND has no linked loaner checkout yet. + """M3: post an Offer-Loaner activity when a confirmed/in-repair + order has been waiting longer than threshold days. Soft-depends on fusion_loaners_management - silently no-ops when - the loaner model isn't installed. + the loaner model isn't installed. Uses schedule_date (or create_date + as fallback) so quote-only / draft repairs aren't bothered. """ - if not self.env.get('fusion.loaner.checkout'): + if 'fusion.loaner.checkout' not in self.env: return ICP = self.env['ir.config_parameter'].sudo() try: @@ -229,24 +269,29 @@ class RepairOrder(models.Model): )) except (ValueError, TypeError): threshold = 3 - from datetime import datetime, timedelta cutoff = datetime.now() - timedelta(days=threshold) - repairs = self.search([ - ('state', 'not in', ('done', 'cancel')), - ('create_date', '<=', cutoff), - ('x_fc_loaner_offered', '=', False), - ]) activity_type = self.env.ref( 'fusion_repairs.mail_activity_type_loaner_offer', raise_if_not_found=False, ) + if not activity_type: + _logger.warning('M3 loaner cron: activity type missing, skipping') + return + repairs = self.search([ + ('state', 'in', ('confirmed', 'under_repair')), + ('x_fc_is_quote_only', '=', False), + ('x_fc_loaner_offered', '=', False), + '|', + '&', ('schedule_date', '!=', False), ('schedule_date', '<=', cutoff), + '&', ('schedule_date', '=', False), ('create_date', '<=', cutoff), + ], limit=200, order='create_date desc') for r in repairs: try: r.activity_schedule( - activity_type_id=activity_type.id if activity_type else False, + activity_type_id=activity_type.id, summary='Offer a loaner unit', note=( - 'This repair has been open for more than %s days. ' + 'This repair has been waiting more than %s days. ' 'Consider offering the client a loaner unit while we ' 'complete the repair.' ) % threshold, @@ -254,19 +299,18 @@ class RepairOrder(models.Model): ) r.x_fc_loaner_offered = True except Exception: - continue - - x_fc_loaner_offered = fields.Boolean( - string='Loaner Offered', - copy=False, - help='True once a loaner-offer activity has been posted for this ' - 'long-running repair (M3). Avoids re-posting daily.', - ) + _logger.exception( + 'M3 loaner cron: activity_schedule failed for repair %s', + r.name, + ) @api.model def _notifications_enabled(self): - ICP = self.env['ir.config_parameter'].sudo() - return ICP.get_param( + # Delegate to the shared intake-service single source of truth (M2). + Service = self.env.get('fusion.repair.intake.service') + if Service: + return Service._notifications_enabled() + return self.env['ir.config_parameter'].sudo().get_param( 'fusion_repairs.enable_email_notifications', 'True' ) == 'True' @@ -275,8 +319,7 @@ class RepairOrder(models.Model): with this repair's partner. Soft-link - raises if the module is not installed.""" self.ensure_one() - Checkout = self.env.get('fusion.loaner.checkout') - if not Checkout: + if 'fusion.loaner.checkout' not in self.env: raise UserError(_( 'Loaner management is not installed. Install ' 'fusion_loaners_management to enable this feature.' diff --git a/fusion_repairs/models/technician_task.py b/fusion_repairs/models/technician_task.py index 59f97947..76162f83 100644 --- a/fusion_repairs/models/technician_task.py +++ b/fusion_repairs/models/technician_task.py @@ -35,6 +35,13 @@ class FusionTechnicianTaskRepairs(models.Model): index=True, ) + # X2: per-task day-before reminder flag. Per-task (not per-repair) so + # a repair with multiple visits gets a separate reminder for each one. + x_fc_day_before_reminder_sent = fields.Boolean( + string='Day-Before Reminder Sent', + copy=False, + ) + def write(self, vals): """When a maintenance task transitions to 'completed', roll the linked contract to its next cycle. Failure to roll never blocks
Scheduled
Technician