fix(billing): importer review fixes — surface failures, validate, dedupe
Resolves findings from the post-build review: - C1: a partial import was indistinguishable from success. action_run_import now logs failed rows at ERROR (survives nexa's log_level=warn) and the wizard shows red/amber banners with failed/skipped counts. - H3: an unrecognized billing_cycle silently fell back to monthly (wrong plan AND price). Now raised per-row -> failed[], never silently mis-billed. - M5: a NULL plan price silently became a $0 line. Prices now preserve NULL-vs-0.0; a missing price for the subscription's cycle is failed[]. - H2: post-connect query/schema errors now become a clean UserError, not a raw SQL traceback (matches the connection-error path). - M4: per-row failures now record the exception type and log a traceback. - MED#3: charge plan_id set explicitly False so re-runs re-assert the shadow-safe NULL even if it was changed between runs. - HIGH-edge: re-run only rewrites x_fc_* on existing subs; partner_id/plan_id/ line are set at creation only (never rewrite immutable fields). - account_link: partner email match is now case-insensitive (=ilike) to avoid duplicate partners against a differently-cased pre-existing partner. Shadow-safety invariant unchanged and re-confirmed. 52/52 green on odoo-trial.
This commit is contained in:
gsinghpal
@@ -33,12 +33,25 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
default=True,
|
||||
help="Read and report what would be imported, without writing anything.")
|
||||
result_summary = fields.Text(readonly=True)
|
||||
failed_count = fields.Integer(readonly=True)
|
||||
skipped_count = fields.Integer(readonly=True)
|
||||
|
||||
def action_run_import(self):
|
||||
self.ensure_one()
|
||||
data = self._read_nexacloud_rows()
|
||||
summary = self._import_rows(data, dry_run=self.dry_run)
|
||||
failed = summary.get("failed") or []
|
||||
skipped = summary.get("skipped") or []
|
||||
self.result_summary = json.dumps(summary, indent=2, default=str)
|
||||
self.failed_count = len(failed)
|
||||
self.skipped_count = len(skipped)
|
||||
# A partial billing import must be loud, not buried in the JSON. Log at ERROR
|
||||
# so it survives nexa's log_level=warn (INFO is suppressed there).
|
||||
if failed:
|
||||
_logger.error("NexaCloud import: %s row(s) FAILED%s: %s",
|
||||
len(failed), " (dry-run)" if self.dry_run else "", failed)
|
||||
if skipped:
|
||||
_logger.warning("NexaCloud import: %s row(s) skipped: %s", len(skipped), skipped)
|
||||
return {
|
||||
"type": "ir.actions.act_window",
|
||||
"res_model": self._name,
|
||||
@@ -83,6 +96,13 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
"current_period_start, current_period_end FROM subscriptions")
|
||||
data["subscriptions"] = [dict(r) for r in cur.fetchall()]
|
||||
return data
|
||||
except psycopg2.Error as e:
|
||||
# A query/schema error (e.g. a renamed/missing column) gets the same clean
|
||||
# operator message as a connection failure — not a raw SQL traceback. We
|
||||
# never return a partial `data` (the return is the last statement in `try`).
|
||||
raise UserError(
|
||||
"Failed reading from the NexaCloud database — the source schema may "
|
||||
"have changed. Underlying error:\n%s" % e)
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
@@ -127,8 +147,10 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
partner_by_user[str(u["id"])] = link.partner_id
|
||||
self._bump(summary, created, "partners")
|
||||
except Exception as e: # noqa: BLE001 - per-row isolation
|
||||
_logger.exception("NexaCloud import: user row %s failed", u.get("id"))
|
||||
summary["failed"].append(
|
||||
{"kind": "user", "id": str(u.get("id")), "error": str(e)})
|
||||
{"kind": "user", "id": str(u.get("id")),
|
||||
"error": "%s: %s" % (type(e).__name__, e)})
|
||||
|
||||
for p in data.get("plans", []):
|
||||
try:
|
||||
@@ -137,8 +159,10 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
plan_ctx_by_id[str(p["id"])] = ctx
|
||||
self._bump(summary, created, "plans")
|
||||
except Exception as e: # noqa: BLE001
|
||||
_logger.exception("NexaCloud import: plan row %s failed", p.get("id"))
|
||||
summary["failed"].append(
|
||||
{"kind": "plan", "id": str(p.get("id")), "error": str(e)})
|
||||
{"kind": "plan", "id": str(p.get("id")),
|
||||
"error": "%s: %s" % (type(e).__name__, e)})
|
||||
|
||||
for s in data.get("subscriptions", []):
|
||||
partner = partner_by_user.get(str(s.get("user_id") or ""))
|
||||
@@ -154,8 +178,10 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
service, partner, ctx, recurrence_plans, s)
|
||||
self._bump(summary, created, "subscriptions")
|
||||
except Exception as e: # noqa: BLE001
|
||||
_logger.exception("NexaCloud import: subscription row %s failed", s.get("id"))
|
||||
summary["failed"].append(
|
||||
{"kind": "subscription", "id": str(s.get("id")), "error": str(e)})
|
||||
{"kind": "subscription", "id": str(s.get("id")),
|
||||
"error": "%s: %s" % (type(e).__name__, e)})
|
||||
|
||||
_logger.info("NexaCloud import summary: %s", summary)
|
||||
return summary
|
||||
@@ -234,8 +260,12 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
Charge = self.env["fusion.billing.charge"]
|
||||
plan_code = str(prow["id"])
|
||||
name = prow.get("name") or plan_code
|
||||
price_monthly = float(prow.get("price_monthly") or 0.0)
|
||||
price_yearly = float(prow.get("price_yearly") or 0.0)
|
||||
# Preserve NULL vs 0.0: a missing price must NOT silently become a $0 line.
|
||||
# The subscription import raises on a missing price for its cycle (-> failed[]).
|
||||
raw_monthly = prow.get("price_monthly")
|
||||
raw_yearly = prow.get("price_yearly")
|
||||
price_monthly = float(raw_monthly) if raw_monthly is not None else None
|
||||
price_yearly = float(raw_yearly) if raw_yearly is not None else None
|
||||
created = False
|
||||
|
||||
sub_code = "NC-PLAN-%s" % plan_code
|
||||
@@ -244,7 +274,7 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
sub_product = Product.create({
|
||||
"name": "NexaCloud %s" % name, "default_code": sub_code,
|
||||
"type": "service", "recurring_invoice": True,
|
||||
"list_price": price_monthly})
|
||||
"list_price": price_monthly or 0.0})
|
||||
created = True
|
||||
|
||||
ov_code = "NC-CPU-OVG-%s" % plan_code
|
||||
@@ -261,7 +291,10 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
"price_per_unit": CPU_RATE_PER_CORE_HOUR,
|
||||
"unit_batch": CPU_SECONDS_PER_CORE_HOUR,
|
||||
"charge_model": "standard",
|
||||
# plan_id intentionally omitted (NULL) — shadow safety guarantee #3
|
||||
# Shadow safety guarantee #3: plan_id MUST stay NULL so the rating cron
|
||||
# never auto-mutates order lines. Set it explicitly (not just omitted) so a
|
||||
# re-run re-asserts NULL even if someone set it on the charge between runs.
|
||||
"plan_id": False,
|
||||
}
|
||||
charge = Charge.search(
|
||||
[("plan_code", "=", plan_code), ("metric_id", "=", metric.id)], limit=1)
|
||||
@@ -280,20 +313,29 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
SaleOrder = self.env["sale.order"]
|
||||
SaleOrderLine = self.env["sale.order.line"]
|
||||
sub_ext = str(srow["id"])
|
||||
cycle = (srow.get("billing_cycle") or "monthly").lower()
|
||||
cycle = (srow.get("billing_cycle") or "").strip().lower()
|
||||
if cycle not in ("monthly", "yearly"):
|
||||
raise UserError(
|
||||
"Subscription %s has an unrecognized billing_cycle %r — cannot pick a "
|
||||
"plan/price." % (sub_ext, srow.get("billing_cycle")))
|
||||
rec_plan = recurrence_plans["yearly"] if cycle == "yearly" else recurrence_plans["monthly"]
|
||||
price = plan_ctx["price_yearly"] if cycle == "yearly" else plan_ctx["price_monthly"]
|
||||
if price is None:
|
||||
raise UserError(
|
||||
"Subscription %s is billed %s but its plan has no %s price." % (
|
||||
sub_ext, cycle, cycle))
|
||||
product = plan_ctx["sub_product"]
|
||||
order_vals = {
|
||||
"partner_id": partner.id, "plan_id": rec_plan.id,
|
||||
"x_fc_nexacloud_subscription_id": sub_ext,
|
||||
# x_fc_* are always (re-)written; identity fields (partner_id/plan_id/order_line)
|
||||
# are set ONLY at creation, so a re-run never rewrites immutable fields on an
|
||||
# order that may since have been confirmed.
|
||||
shadow_vals = {
|
||||
"x_fc_nexacloud_deployment_id": str(srow.get("deployment_id") or ""),
|
||||
"x_fc_billing_service_id": service.id, "x_fc_shadow": True,
|
||||
}
|
||||
existing = SaleOrder.search(
|
||||
[("x_fc_nexacloud_subscription_id", "=", sub_ext)], limit=1)
|
||||
if existing:
|
||||
existing.write(order_vals)
|
||||
existing.write(shadow_vals)
|
||||
line = existing.order_line.filtered(lambda l: l.product_id == product)
|
||||
line_vals = {"product_uom_qty": 1, "price_unit": price}
|
||||
if line:
|
||||
@@ -304,9 +346,13 @@ class FusionBillingImportWizard(models.TransientModel):
|
||||
order = existing
|
||||
created = False
|
||||
else:
|
||||
order_vals["order_line"] = [(0, 0, {
|
||||
"product_id": product.id, "product_uom_qty": 1, "price_unit": price})]
|
||||
order = SaleOrder.create(order_vals)
|
||||
order = SaleOrder.create({
|
||||
"partner_id": partner.id, "plan_id": rec_plan.id,
|
||||
"x_fc_nexacloud_subscription_id": sub_ext,
|
||||
"order_line": [(0, 0, {
|
||||
"product_id": product.id, "product_uom_qty": 1, "price_unit": price})],
|
||||
**shadow_vals,
|
||||
})
|
||||
created = True
|
||||
# guarantee the explicit price stuck (a pricelist compute may have overwritten it)
|
||||
line = order.order_line.filtered(lambda l: l.product_id == product)
|
||||
|
||||
Reference in New Issue
Block a user