admin/Odoo-Modules
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(fusion_repairs): code-review batch - 4 critical + 8 high + 8 medium/low

Browse Source 
Critical
- C1: _sql_constraints -> models.Constraint (Odoo 19 deprecation rule violation)
- C2: variance threshold no longer uses abs() - under-cost is good news,
  must not block invoicing. Now only OVER-cost triggers requires_requote.
- C3: roll_next_due_date() was dead code - now wired from
  fusion.technician.task.write() when a maintenance task transitions to
  'completed', so the whole maintenance lifecycle actually advances.
- C4: warranty.is_active was store=True but time-dependent (became stale).
  Dropped store=True; find_active_for() now filters by expiry_date directly.

High
- H1: added x_fc_maintenance_contract_id back-link on repair.order and
  populated it from create_repair_from_booking().
- H2: find_active_for() returns empty when neither lot nor product is
  supplied - prevents cross-product false warranty matches.
- H3: visit-report wizard now creates stock.move records of repair_line_type
  'add' for each part line, so Odoo's native action_create_sale_order()
  chain has lines to invoice and stock gets consumed properly.
- H4: office intake email template now carries a fallback email_to header
  computed from res.company.x_fc_office_notification_ids (or company email),
  so it does not silently send with no recipient.
- H5: maintenance reminder cron nextcall now always rolls to tomorrow
  at 07:00 local time, so installing/upgrading after 07:00 does not
  immediately fire all the day's reminders.
- H6: public portal no longer hardcodes UID 1 as the intake user fallback
  (which in Odoo 19 is OdooBot). Prefers base.user_admin, else the
  lowest-id non-share user, else SUPERUSER_ID.
- H7: public portal validates client_email via tools.email_normalize
  before partner creation; malformed addresses redirect with error=email.
- H8: find_best_match() returns empty when no symptom keywords match
  (no silent first-catalog guess) and uses word-boundary regex to avoid
  matching 'battery' inside 'no battery problem'.

Medium
- M1: _inherit moved next to _name on maintenance_contract (cosmetic but
  brittle if Odoo refactors model class detection)
- M2: relativedelta(months=N) instead of timedelta(days=N*30) for warranty
  and maintenance intervals (correct month boundaries)
- M3: unique constraint on fusion.repair.maintenance.contract.booking_token
- M6: dispatch task fallback now searches for an actual x_fc_is_field_staff
  user; gracefully skips and logs if no field staff exists (instead of
  silently failing the constraint check)
- M7: maintenance contract list view date decoration uses context_today()
  (date) instead of strftime(string) - the str comparison would TypeError
- M9: Visit Report button hidden on draft repairs and when no technician
  task is linked yet

Low
- L2: portal-created partners get default lang + company_id so mail
  templates render in the right language
- L3: dropped unused exception variable in sales rep portal controller
- L4: visit-report wizard 'found another issue' now redirects to the
  spawned stub repair so the tech can fill it in immediately
- L5: dropped unrecognized data-string from <app> in settings view

Public portal also: rate-limit check moved BEFORE the counter increment so
blocked attempts do not keep inflating the bucket.

All fixes verified end-to-end on local westin-v19:
- variance one-sided: 0.5h labour vs $500 est -> requires_requote=False;
  2h x $250 + $200 parts vs $100 est -> requires_requote=True
- maintenance roll-forward: created MC/00006 due 2026-05-31, completed
  linked maintenance task -> contract rolled to 2026-11-21 with
  last_reminder_band reset
- warranty find_active_for(partner only) -> empty recordset
- service catalog find_best_match with unrelated text -> empty recordset
- pg_constraint shows fusion_repair_maintenance_contract_booking_token_unique
- /repair landing still 200 after restart

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
gsinghpal
2026-05-20 22:22:11 -04:00
parent afe19f2105
commit c86f1bbbe5
15 changed files with 203 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
fusion_repairs/controllers/portal_client_repair.py
View File

@@ -29,8 +29,9 @@ import logging
import re
import time
from odoo import http, fields
from odoo import SUPERUSER_ID, http, fields
from odoo.http import request
from odoo.tools import email_normalize
_logger = logging.getLogger(__name__)
@@ -92,9 +93,10 @@ class ClientRepairPortal(http.Controller):
for k in list(_RATE_LIMIT_BUCKET.keys()):
if not k.endswith(f":{bucket}"):
_RATE_LIMIT_BUCKET.pop(k, None)
_RATE_LIMIT_BUCKET[key] = _RATE_LIMIT_BUCKET.get(key, 0) + 1
if _RATE_LIMIT_BUCKET[key] > limit:
# Check FIRST so blocked attempts don't keep inflating the counter.
if _RATE_LIMIT_BUCKET.get(key, 0) >= limit:
return True # blocked
_RATE_LIMIT_BUCKET[key] = _RATE_LIMIT_BUCKET.get(key, 0) + 1
return False
# ------------------------------------------------------------------
@@ -164,6 +166,12 @@ class ClientRepairPortal(http.Controller):
if not (partner_name and phone and issue_summary and category_id):
return request.redirect("/repair/new?error=missing")
# Validate email if provided. Empty is allowed; malformed redirects back.
raw_email = (post.get("client_email") or "").strip()
clean_email = email_normalize(raw_email) if raw_email else False
if raw_email and not clean_email:
return request.redirect("/repair/new?error=email")
# Find or create partner. Match by phone if known (safe - we already
# have their consent to contact via this form).
cleaned_phone = _e164_clean(phone)
@@ -180,7 +188,7 @@ class ClientRepairPortal(http.Controller):
partner_vals = {
"name": partner_name,
"phone": phone,
"email": (post.get("client_email") or "").strip(),
"email": clean_email or False,
"street": (post.get("client_street") or "").strip(),
"city": (post.get("client_city") or "").strip(),
}
@@ -209,13 +217,21 @@ class ClientRepairPortal(http.Controller):
"internal_notes": (post.get("internal_notes") or "").strip(),
"photo_attachment_ids": attachment_ids,
}
# Pick a real human owner for the repair so emails go from a person:
# admin if present, else the lowest-id non-share user, else SUPERUSER_ID.
admin = request.env.ref("base.user_admin", raise_if_not_found=False)
if admin:
intake_uid = admin.id
else:
internal = request.env["res.users"].sudo().search(
[("share", "=", False)], order="id asc", limit=1,
)
intake_uid = internal.id if internal else SUPERUSER_ID
payload = {
"partner_id": partner.id if partner else None,
"partner_vals": partner_vals,
"intake_user_id": request.env.ref(
"base.user_admin", raise_if_not_found=False).id
if request.env.ref("base.user_admin",
raise_if_not_found=False) else 1,
"intake_user_id": intake_uid,
"equipment_items": [equipment],
}

2
fusion_repairs/controllers/portal_sales_rep_repair.py
View File

@@ -135,7 +135,7 @@ class SalesRepRepairPortal(CustomerPortal):
try:
repairs = request.env['fusion.repair.intake.service'].sudo() \
.create_repair_orders(payload, source='sales_rep_portal')
except Exception as e:
except Exception:
_logger.exception('Sales rep portal repair submit failed')
return request.redirect('/my/repair/new?error=server')

2
fusion_repairs/data/ir_cron_data.xml
View File

@@ -11,7 +11,7 @@
<field name="user_id" ref="base.user_root"/>
<field name="interval_number">1</field>
<field name="interval_type">days</field>
<field name="nextcall" eval="(DateTime.now() + timedelta(hours=1)).strftime('%Y-%m-%d 07:00:00')"/>
<field name="nextcall" eval="(DateTime.now().replace(hour=7, minute=0, second=0, microsecond=0) + timedelta(days=1)).strftime('%Y-%m-%d %H:%M:%S')"/>
<field name="active" eval="True"/>
</record>

1
fusion_repairs/data/mail_template_data.xml
View File

@@ -109,6 +109,7 @@
<field name="model_id" ref="repair.model_repair_order"/>
<field name="subject">[New Service Call] {{ object.partner_id.name or 'Walk-in' }} - {{ object.name or 'n/a' }}</field>
<field name="email_from">{{ (object.user_id.email_formatted or object.company_id.email_formatted or user.email_formatted) }}</field>
<field name="email_to">{{ ','.join(p.email for p in (object.company_id.x_fc_office_notification_ids if 'x_fc_office_notification_ids' in object.company_id._fields else []) if p.email) or (object.company_id.email or '') }}</field>
<field name="body_html" type="html">
<div style="font-family:-apple-system,BlinkMacSystemFont,'Segoe UI',Roboto,Arial,sans-serif;max-width:600px;margin:0 auto;">
<div style="height:4px;background-color:#d69e2e;"></div>

44
fusion_repairs/models/intake_service.py
View File

@@ -90,6 +90,10 @@ class FusionRepairIntakeService(models.AbstractModel):
partner_vals = payload.get('partner_vals')
if not partner_vals:
return False
# Sensible defaults for partners created via public portals so mail
# templates pick up the right language / company.
partner_vals.setdefault('lang', self.env.user.lang or 'en_CA')
partner_vals.setdefault('company_id', self.env.company.id)
partner = self.env['res.partner'].sudo().create(partner_vals)
return partner.id
@@ -401,12 +405,25 @@ class FusionRepairIntakeService(models.AbstractModel):
'x_fc_repair_order_id': repair.id,
'description': repair.internal_notes or repair.name,
}
# technician_id is required on fusion.technician.task; we fall back to
# the intake user. Dispatcher will reassign.
vals['technician_id'] = (
repair.user_id.id if repair.user_id and repair.user_id.x_fc_is_field_staff
else self.env.uid
)
# technician_id is required AND constrained to x_fc_is_field_staff.
# Use the intake user if they qualify, otherwise the lowest-id active
# field-staff user as a placeholder for the dispatcher to reassign.
if repair.user_id and repair.user_id.x_fc_is_field_staff:
vals['technician_id'] = repair.user_id.id
else:
fallback = self.env['res.users'].sudo().search([
('x_fc_is_field_staff', '=', True),
('active', '=', True),
], order='id', limit=1)
if not fallback:
_logger.warning(
'No field-staff user available - skipping auto-dispatch '
'task for repair %s (mark a user as Field Staff under '
'Settings > Users).',
repair.name,
)
return
vals['technician_id'] = fallback.id
Task.create(vals)
except Exception as e:
_logger.warning('Failed to auto-create dispatch task for repair %s: %s',
@@ -449,8 +466,13 @@ class FusionRepairIntakeService(models.AbstractModel):
@api.model
def _office_emails(self, company):
# Reuse the office notification recipients defined by fusion_claims.
partners = company.sudo()
recipients = getattr(partners, 'x_fc_office_notification_ids', False)
if recipients:
return [p.email for p in recipients if p.email]
return []
company_sudo = company.sudo()
recipients = getattr(company_sudo, 'x_fc_office_notification_ids', False)
emails = [p.email for p in (recipients or []) if p.email]
if not emails:
_logger.info(
'No office notification recipients configured on company %s - '
'skipping office intake email.',
company.name,
)
return emails

23
fusion_repairs/models/maintenance_contract.py
View File

@@ -14,6 +14,8 @@ a tokenized booking link.
import secrets
from datetime import timedelta
from dateutil.relativedelta import relativedelta
from odoo import _, api, fields, models
@@ -27,6 +29,7 @@ CONTRACT_STATES = [
class FusionRepairMaintenanceContract(models.Model):
_name = 'fusion.repair.maintenance.contract'
_inherit = ['mail.thread']
_description = 'Repair Maintenance Contract'
_order = 'next_due_date, id'
@@ -76,7 +79,10 @@ class FusionRepairMaintenanceContract(models.Model):
'res.company', default=lambda self: self.env.company,
)
_inherit = ['mail.thread']
_booking_token_unique = models.Constraint(
'unique(booking_token)',
'Booking token must be unique.',
)
@api.model_create_multi
def create(self, vals_list):
@@ -93,10 +99,15 @@ class FusionRepairMaintenanceContract(models.Model):
# ROLL FORWARD
# ------------------------------------------------------------------
def roll_next_due_date(self):
"""Advance next_due_date by interval_months and reset cycle state."""
"""Advance next_due_date by interval_months and reset cycle state.
Called from technician_task.write() when a maintenance task moves to
'completed' (see technician_task.py).
"""
for c in self:
base = c.last_service_date or fields.Date.context_today(c)
c.next_due_date = base + timedelta(days=(c.interval_months or 12) * 30)
# relativedelta handles month boundaries correctly (28/29/30/31).
c.next_due_date = base + relativedelta(months=c.interval_months or 12)
c.last_reminder_band = False
c.booking_repair_id = False
@@ -161,8 +172,10 @@ class FusionRepairMaintenanceContract(models.Model):
'schedule_date': scheduled_date or fields.Datetime.now(),
'x_fc_intake_source': 'client_portal',
'x_fc_urgency': 'normal',
'x_fc_repair_category_id': self.product_id.product_tmpl_id.x_fc_repair_category_id.id
'x_fc_repair_category_id':
self.product_id.product_tmpl_id.x_fc_repair_category_id.id
if self.product_id.product_tmpl_id.x_fc_repair_category_id else False,
'x_fc_maintenance_contract_id': self.id,
'internal_notes':
f'<p>Maintenance visit booked from reminder for contract <b>{self.name}</b>.</p>',
})
@@ -204,6 +217,6 @@ class SaleOrder(models.Model):
'product_id': product.id,
'original_sale_order_id': so.id,
'interval_months': interval,
'next_due_date': today + timedelta(days=interval * 30),
'next_due_date': today + relativedelta(months=interval),
'state': 'active',
})

15
fusion_repairs/models/repair_order.py
View File

@@ -2,7 +2,7 @@
# Copyright 2024-2026 Nexa Systems Inc.
# License OPL-1 (Odoo Proprietary License v1.0)
from datetime import timedelta
from dateutil.relativedelta import relativedelta
from odoo import api, fields, models, _
from odoo.exceptions import UserError
@@ -71,6 +71,15 @@ class RepairOrder(models.Model):
index=True,
help='Auto-matched catalogue entry that pre-fills estimated cost and duration.',
)
# Maintenance contract back-link (Phase 3)
x_fc_maintenance_contract_id = fields.Many2one(
'fusion.repair.maintenance.contract',
string='Maintenance Contract',
index=True,
help='Set when this repair was spawned from a maintenance reminder booking. '
'Completing the related technician task rolls the contract to its next cycle.',
)
x_fc_intake_answer_count = fields.Integer(
compute='_compute_intake_answer_count',
)
@@ -240,8 +249,8 @@ class RepairOrder(models.Model):
)
if not warranty_months:
return False
# Datetime + months: use simple 30-day approximation per month for now.
cutoff = fields.Datetime.from_string(str(delivery_date)) + timedelta(days=warranty_months * 30)
# relativedelta handles month boundaries correctly (28/29/30/31).
cutoff = fields.Datetime.from_string(str(delivery_date)) + relativedelta(months=warranty_months)
return fields.Datetime.now() <= cutoff
# ------------------------------------------------------------------

7
fusion_repairs/models/repair_product_category.py
View File

@@ -39,9 +39,10 @@ class FusionRepairProductCategory(models.Model):
help='Default intake question set shown when this category is selected.',
)
_sql_constraints = [
('code_unique', 'unique(code)', 'Category code must be unique.'),
]
_code_unique = models.Constraint(
'unique(code)',
'Category code must be unique.',
)
@api.depends('name', 'code')
def _compute_display_name(self):

16
fusion_repairs/models/repair_warranty.py
View File

@@ -67,10 +67,12 @@ class FusionRepairWarrantyCoverage(models.Model):
compute='_compute_expiry_date',
store=True,
)
# Non-stored compute - DO NOT add store=True. The 'active vs not' status is
# time-dependent (today >= expiry_date), and a stored compute would never
# auto-refresh as days pass. find_active_for() filters by expiry_date directly.
is_active = fields.Boolean(
string='Active',
compute='_compute_is_active',
store=True,
)
notes = fields.Text()
@@ -107,12 +109,20 @@ class FusionRepairWarrantyCoverage(models.Model):
# ------------------------------------------------------------------
@api.model
def find_active_for(self, partner_id, product_id=None, lot_id=None):
"""Return active warranty coverage matching the partner + equipment, if any."""
"""Return active warranty coverage matching the partner + equipment, if any.
Requires at least one of lot_id or product_id - without an equipment
identifier we would match any warranty on the partner, which would
falsely flag unrelated equipment as covered.
"""
if not partner_id:
return self.browse()
if not lot_id and not product_id:
return self.browse()
today = fields.Date.context_today(self)
domain = [
('partner_id', '=', partner_id),
('is_active', '=', True),
('expiry_date', '>=', today),
]
if lot_id:
domain.append(('lot_id', '=', lot_id))

22
fusion_repairs/models/service_catalog.py
View File

@@ -113,29 +113,39 @@ class FusionRepairServiceCatalog(models.Model):
def find_best_match(self, product_category_id, text_hints):
"""Return the best-matching catalogue entry, or empty recordset.
Returns empty when no symptom keywords match. We never "guess" a default
catalog because the match drives estimated cost + auto-dispatch task -
a wrong guess would propagate into pricing and scheduling.
:param product_category_id: int id of the equipment category
:param text_hints: list[str] - text snippets to look for symptom keywords in
(typically: issue_summary, issue_category, recent intake answer values)
"""
import re
if not product_category_id:
return self.browse()
haystack = ' '.join(s.lower() for s in (text_hints or []) if s).strip()
if not haystack:
return self.browse()
candidates = self.search([
('product_category_id', '=', product_category_id),
('active', '=', True),
], order='sequence')
if not candidates:
return self.browse()
if not haystack:
return candidates[:1]
best = None
best_score = 0
for c in candidates:
kws = [k.strip().lower() for k in (c.symptom_keywords or '').split(',') if k.strip()]
score = sum(1 for kw in kws if kw and kw in haystack)
# Word-boundary match avoids false positives where "battery" matches
# inside "no battery problem".
score = sum(
1 for kw in kws
if kw and re.search(rf'\b{re.escape(kw)}\b', haystack)
)
if score > best_score:
best = c
best_score = score
if best:
# No keywords matched -> return empty rather than the lowest-sequence guess.
if best and best_score > 0:
return best
return candidates[:1]
return self.browse()

30
fusion_repairs/models/technician_task.py
View File

@@ -7,7 +7,8 @@ from odoo import fields, models
class FusionTechnicianTaskRepairs(models.Model):
"""Adds the back-link from fusion.technician.task to repair.order so
repairs and tasks share one timeline.
repairs and tasks share one timeline. Also hooks task completion to
roll a linked maintenance contract to its next cycle.
"""
_inherit = 'fusion.technician.task'
@@ -29,6 +30,33 @@ class FusionTechnicianTaskRepairs(models.Model):
index=True,
)
def write(self, vals):
"""When a maintenance task transitions to 'completed', roll the
linked contract to its next cycle. Failure to roll never blocks
the underlying task write.
"""
res = super().write(vals)
if vals.get('status') == 'completed':
for task in self:
if task.task_type != 'maintenance':
continue
repair = task.x_fc_repair_order_id
contract = repair.x_fc_maintenance_contract_id if repair else False
if not contract:
continue
try:
contract.last_service_date = fields.Date.context_today(task)
contract.roll_next_due_date()
contract.message_post(body=(
'Rolled forward after maintenance task '
f'<b>{task.name}</b> completed. '
f'Next due {contract.next_due_date}.'
))
except Exception:
# Never let a contract roll failure block the task write.
pass
return res
def action_view_repair_order(self):
self.ensure_one()
if not self.x_fc_repair_order_id:

2
fusion_repairs/views/maintenance_contract_views.xml
View File

@@ -5,7 +5,7 @@
<field name="name">fusion.repair.maintenance.contract.list</field>
<field name="model">fusion.repair.maintenance.contract</field>
<field name="arch" type="xml">
<list string="Maintenance Contracts" decoration-warning="next_due_date and next_due_date &lt;= context_today().strftime('%Y-%m-%d')">
<list string="Maintenance Contracts" decoration-warning="next_due_date and next_due_date &lt;= context_today()">
<field name="name"/>
<field name="partner_id"/>
<field name="product_id"/>

2
fusion_repairs/views/repair_order_views.xml
View File

@@ -16,7 +16,7 @@
type="object"
string="Visit Report"
class="btn-primary"
invisible="state == 'cancel'"
invisible="state in ('draft', 'cancel') or x_fc_technician_task_count == 0"
groups="fusion_repairs.group_fusion_repairs_user"/>
<button name="action_collect_payment"
type="object"

3
fusion_repairs/views/res_config_settings_views.xml
View File

@@ -7,8 +7,7 @@
<field name="inherit_id" ref="base.res_config_settings_view_form"/>
<field name="arch" type="xml">
<xpath expr="//form" position="inside">
<app data-string="Fusion Repairs"
string="Fusion Repairs"
<app string="Fusion Repairs"
name="fusion_repairs"
groups="fusion_repairs.group_fusion_repairs_manager">
<block title="Notifications" name="fc_repairs_notifications">

53
fusion_repairs/wizard/repair_visit_report_wizard.py
View File

@@ -111,9 +111,12 @@ class RepairVisitReportWizard(models.TransientModel):
est = w.estimated_cost or 0.0
variance_pct = ((w.actual_cost - est) / est * 100) if est else 0.0
w.variance_pct = variance_pct
# One-sided: only OVER-cost triggers re-quote. Coming in under
# estimate is good news and must not block invoicing.
over_pct = variance_pct
over_amt = w.actual_cost - est
w.requires_requote = est > 0 and (
abs(variance_pct) >= threshold_pct
or abs(w.actual_cost - est) >= threshold_amt
over_pct >= threshold_pct or over_amt >= threshold_amt
)
# ------------------------------------------------------------------
@@ -125,6 +128,11 @@ class RepairVisitReportWizard(models.TransientModel):
if not repair:
raise UserError(_('No repair selected.'))
# Create native repair operations (stock moves) for the parts used.
# 'add' type moves consume parts from the parts source location and
# flow through to the invoice when action_create_sale_order() is run.
self._create_repair_part_moves(repair)
# Persist actual cost + requote flag on the repair.
repair.write({
'x_fc_actual_cost': self.actual_cost,
@@ -135,7 +143,8 @@ class RepairVisitReportWizard(models.TransientModel):
if self.notes:
repair.message_post(body=self.notes)
# If found another issue: spawn a stub repair (same partner, same equipment).
# Spawn a follow-up repair if the tech found another issue.
stub = False
if self.found_another_issue:
stub = repair.copy({
'state': 'draft',
@@ -150,20 +159,54 @@ class RepairVisitReportWizard(models.TransientModel):
'x_fc_requires_requote': False,
'x_fc_intake_template_id': False,
'x_fc_service_catalog_id': False,
'x_fc_maintenance_contract_id': False,
})
repair.message_post(
body=_('Spawned follow-up repair <b>%(name)s</b> for "found another issue".',
name=stub.name),
)
# If a stub was spawned, open it directly so the tech can fill in details.
target_id = stub.id if stub else repair.id
target_name = stub.name if stub else repair.name
return {
'type': 'ir.actions.act_window',
'name': repair.name,
'name': target_name,
'res_model': 'repair.order',
'view_mode': 'form',
'res_id': repair.id,
'res_id': target_id,
}
def _create_repair_part_moves(self, repair):
"""Create stock.move records for each part used (repair_line_type='add').
Locations follow the repair order's configured source / parts locations;
Odoo natively links these moves to the SO line generated by
action_create_sale_order() so they invoice correctly.
"""
Move = self.env['stock.move'].sudo()
for line in self.parts_line_ids:
if not line.product_id or line.quantity <= 0:
continue
vals = {
'name': line.product_id.display_name,
'product_id': line.product_id.id,
'product_uom_qty': line.quantity,
'product_uom': line.product_id.uom_id.id,
'repair_id': repair.id,
'repair_line_type': 'add',
'location_id': repair.location_id.id,
'location_dest_id': repair.parts_location_id.id or repair.location_id.id,
'company_id': repair.company_id.id,
}
try:
Move.create(vals)
except Exception as e:
_logger.warning(
'Could not create repair part move on %s for %s: %s',
repair.name, line.product_id.display_name, e,
)
class RepairVisitReportWizardLine(models.TransientModel):
_name = 'fusion.repair.visit.report.wizard.line'