Five findings from the review of the WINC import, plus the rendering each fix needed to be usable. An undo that hit a conflict wrote status 'undone' anyway, and undo_batch returns immediately for an undone batch. So the one entry that needed a retry was the only one that could never get it: the human resolves the conflict, clicks Undo, and gets a no-op. A conflicted undo is now 'partially-undone', undoneat is stamped only when the batch is genuinely finished, and the batches table offers "Retry undo" for that status. The column was String(16) and the new value is exactly 16 characters, so migration 7d38 widens it to 32 rather than rely on an exact fit. _UNDO_REFERENCES listed only assets, so a business unit this batch created was hard-deleted even when something else still pointed at it. Two of the three missing referrers are real FKs (dashboarddefaults.businessunitid, businessunits.parentid): MySQL raises and the whole undo rolls back, so the batch cannot be undone at all. The third, notifications.businessunitid, has no FK, so the delete succeeded and left a notification pointing at an id that no longer exists - silently, on both engines. All four are listed and the guard is tested through the route. The reference guard also queried tables that need not exist. A lean site drops the tables of plugins it does not install (ADR-014), and notifications is plugin-owned, so on such a site the query raised 1146, rolled the undo back and left the batch permanently un-undoable. Absent tables are skipped; a table that does not exist cannot hold a reference. _PACKET_GROUPS reports on location, but apply_plan only ever creates business units - it sets locationid only when the location already resolved. A plan needing a new location asked the admin to acknowledge it, got the tick, wrote nothing, and reported nothing. build_packet now takes the groups the writer can actually create; anything else needed is 'uncreatable' and blocks at plan time, where the admin can act on it, rather than dying at commit on a message about acknowledging something the screen had no control for. The writer refuses the same case independently. Both the console packet and the import page name the blocking groups: setting blocking without printing the reason would leave the banner pointing at nothing. The web undo route caught ValueError only, so refusing a batch belonging to another importer surfaced as a 500. That refusal is new too: the journal is a general primitive and undo_batch does not filter on kind, so any other importer's batches were reversible through this endpoint, carrying WINC's references map - which lists none of that importer's tables, making every reference check pass and every delete unconditional. Nine tests. The two guards were verified by planting violations: dropping dashboarddefaults from the references map, and passing creatable=None, each fail the test written for them. Scope: product
191 lines
8.3 KiB
Python
191 lines
8.3 KiB
Python
"""Reusable approval / promotion gate for site-import writers.
|
|
|
|
IMPORT-AGNOSTIC. This turns a dry-run plan into an "approval packet": a frozen
|
|
summary of what an import WOULD do, plus a content hash of the exact plan it
|
|
describes. A writer refuses to run unless it is handed a packet whose hash still
|
|
matches the plan in front of it. That is the whole mechanism: a human approves a
|
|
specific, frozen plan, and the writer can prove nothing drifted since.
|
|
|
|
The same packet shape serves every importer, and it is deliberately the seam the
|
|
undo work will reuse: a writer records the packet's batch id alongside the
|
|
journal it writes, so approval, write, and undo all reference one identity.
|
|
|
|
A packet is a plain dict (JSON-serialisable) so it can be printed, saved next to
|
|
an export, diffed between runs, and shown in a UI without any of this code:
|
|
|
|
{
|
|
'kind': caller-supplied label, e.g. 'winc-machine-adoption'
|
|
'planhash': sha256 of the canonicalised plan rows
|
|
'rowcount': number of rows in the plan
|
|
'counts': caller-supplied summary dict (what the report shows)
|
|
'needscreate': { group -> { name -> count } } # requires acknowledgment
|
|
'unresolved': { group -> { value -> count } } # blocks approval
|
|
'blocking': True if any unresolved entries exist
|
|
}
|
|
|
|
The writer's contract, in order:
|
|
1. rebuild the plan from the same inputs,
|
|
2. build a packet from it,
|
|
3. compare planhash to the approved packet - abort on mismatch,
|
|
4. refuse if packet['blocking'],
|
|
5. require explicit acknowledgment for every needscreate group,
|
|
6. only then write, stamping packet['batchid'] onto the journal.
|
|
"""
|
|
|
|
import hashlib
|
|
import json
|
|
import uuid
|
|
from collections import OrderedDict
|
|
|
|
from scripts.site_imports.resolver import tally, NEEDS_CREATE, UNRESOLVED
|
|
|
|
|
|
def plan_hash(plan):
|
|
"""Stable sha256 over the plan's rows.
|
|
|
|
Canonicalised by serialising each row as sorted-key JSON so the hash depends
|
|
only on content, not on dict ordering or whitespace. Two runs that produce
|
|
the same decisions produce the same hash; any changed decision changes it,
|
|
which is exactly what the writer checks before it touches the database.
|
|
"""
|
|
hasher = hashlib.sha256()
|
|
for row in plan:
|
|
canonical = json.dumps(row, sort_keys=True, default=str,
|
|
separators=(',', ':'))
|
|
hasher.update(canonical.encode('utf-8'))
|
|
hasher.update(b'\n')
|
|
return hasher.hexdigest()
|
|
|
|
|
|
def build_packet(kind, plan, counts, groups, creatable=None):
|
|
"""Build an approval packet from a plan.
|
|
|
|
kind a short label identifying the importer, stored on the packet.
|
|
plan the list of per-row dicts the writer will act on.
|
|
counts the caller's summary dict (shown to the human as-is).
|
|
groups how to read each field's status/value columns out of a row:
|
|
[ (group_name, status_column, value_column), ... ]
|
|
e.g. ('businessunit', 'businessunit_status', 'raw_eq1'). The value
|
|
column is what a human would act on - the raw export value for
|
|
UNRESOLVED (so they know what to alias) and the canonical name for
|
|
NEEDS_CREATE (so they know what would be created).
|
|
creatable
|
|
the group names the caller's writer can actually create, or None
|
|
for "no opinion". A needs-create group outside this set is reported
|
|
under `uncreatable` and makes the packet blocking, because a plan
|
|
that promises rows nothing will create is as unrunnable as one with
|
|
unresolved values.
|
|
|
|
needscreate is keyed on the canonical name column when present; callers pass
|
|
the *_name column for that via the fourth tuple element when it differs from
|
|
the value column.
|
|
"""
|
|
needscreate = OrderedDict()
|
|
unresolved = OrderedDict()
|
|
for group in groups:
|
|
group_name, status_column, value_column = group[0], group[1], group[2]
|
|
name_column = group[3] if len(group) > 3 else value_column
|
|
unresolved_hits = tally(plan, status_column, value_column, UNRESOLVED)
|
|
needs_hits = tally(plan, status_column, name_column, NEEDS_CREATE)
|
|
if unresolved_hits:
|
|
unresolved[group_name] = unresolved_hits
|
|
if needs_hits:
|
|
needscreate[group_name] = needs_hits
|
|
|
|
# A needs-create group the writer cannot actually create BLOCKS the plan,
|
|
# the same as an unresolved value. It is the same failure either way: the
|
|
# import cannot do what the preview says it would.
|
|
#
|
|
# It has to be decided HERE rather than at write time. The writer's own
|
|
# refusal sits behind the acknowledgement check, and the UI only ever
|
|
# acknowledges the groups it renders a control for - so a plan needing an
|
|
# uncreatable group reached Commit looking clean, and died on a message
|
|
# about acknowledging something the screen had no checkbox for. Blocking at
|
|
# plan time puts it in the preview, where the admin can act on it.
|
|
#
|
|
# creatable=None means "no opinion" - the CLI callers that do not pass it
|
|
# keep the old behaviour.
|
|
uncreatable = OrderedDict()
|
|
if creatable is not None:
|
|
allowed = set(creatable)
|
|
for group_name, hits in needscreate.items():
|
|
if group_name not in allowed:
|
|
uncreatable[group_name] = hits
|
|
|
|
return OrderedDict([
|
|
('kind', kind),
|
|
('batchid', str(uuid.uuid4())),
|
|
('planhash', plan_hash(plan)),
|
|
('rowcount', len(plan)),
|
|
('counts', OrderedDict(counts)),
|
|
('needscreate', needscreate),
|
|
('uncreatable', uncreatable),
|
|
('unresolved', unresolved),
|
|
('blocking', bool(unresolved) or bool(uncreatable)),
|
|
])
|
|
|
|
|
|
def packet_matches_plan(packet, plan):
|
|
"""True if the approved packet still describes this exact plan."""
|
|
return packet.get('planhash') == plan_hash(plan)
|
|
|
|
|
|
def format_packet(packet):
|
|
"""Human-readable rendering of a packet for the console or a UI panel."""
|
|
lines = []
|
|
lines.append('APPROVAL PACKET: %s' % packet.get('kind', '?'))
|
|
lines.append(' batch id : %s' % packet.get('batchid', '?'))
|
|
lines.append(' plan hash: %s' % packet.get('planhash', '?'))
|
|
lines.append(' rows : %d' % packet.get('rowcount', 0))
|
|
|
|
needscreate = packet.get('needscreate') or {}
|
|
if needscreate:
|
|
for group, items in needscreate.items():
|
|
total = sum(items.values())
|
|
lines.append('')
|
|
lines.append(' WARNING: this import would CREATE %d %s(s). '
|
|
'Confirm these are correct; you can edit them later '
|
|
'in Settings:' % (len(items), group))
|
|
for name, count in items.items():
|
|
lines.append(' + %-28s (%d rows)' % (name, count))
|
|
lines.append(' total new %s rows: %d' % (group, total))
|
|
|
|
# An uncreatable group blocks, so it has to be ON SCREEN. Setting
|
|
# blocking without printing the reason left "BLOCKED (resolve the values
|
|
# above)" pointing at nothing.
|
|
uncreatable = packet.get('uncreatable') or {}
|
|
if uncreatable:
|
|
for group, items in uncreatable.items():
|
|
lines.append('')
|
|
lines.append(' BLOCKING: this import cannot create %s rows, but '
|
|
'%d are needed - create them first, or alias the '
|
|
'export values to rows that exist:'
|
|
% (group, len(items)))
|
|
for name, count in items.items():
|
|
lines.append(' x %-28s (%d rows)' % (name, count))
|
|
|
|
unresolved = packet.get('unresolved') or {}
|
|
if unresolved:
|
|
for group, items in unresolved.items():
|
|
lines.append('')
|
|
lines.append(' BLOCKING: %d unresolved %s value(s) - add an alias '
|
|
'for each before importing:' % (len(items), group))
|
|
for value, count in items.items():
|
|
lines.append(' ? %-28s (%d rows)' % (value, count))
|
|
|
|
lines.append('')
|
|
lines.append(' status : %s' % (
|
|
'BLOCKED (resolve the problems above)' if packet.get('blocking')
|
|
else 'READY for acknowledgment'))
|
|
return '\n'.join(lines)
|
|
|
|
|
|
def save_packet(packet, path):
|
|
with open(path, 'w', encoding='utf-8') as handle:
|
|
json.dump(packet, handle, indent=2)
|
|
|
|
|
|
def load_packet(path):
|
|
with open(path, encoding='utf-8') as handle:
|
|
return json.load(handle)
|