From 5108ba8aaa300770014d7bba4e18fa04ed487387 Mon Sep 17 00:00:00 2001
From: cproudlock
Date: Mon, 10 Aug 2026 14:48:51 -0400
Subject: [PATCH] computers: a second PC claiming a machine is a claim, not a
handover
Treating "another PC is linked to this machine" as proof of replacement was
wrong. A PC imaged for machine 3010 carries that number from the bench, before
it has replaced anything, and several PCs sharing one machine number is a
normal state at this site: the part markers do it. Both PCs then reported on
their own schedules, each report moved the link and raised an alert, and the
pair traded the machine back and forth for as long as both were alive.
The PC holding a machine now keeps it while it is still alive. Alive means it
has reported within MACHINE_CLAIM_QUIET_HOURS and its asset is still In Use. A
challenger is recorded as a dormant link instead, which doubles as the marker
saying the claim has already been announced, so a PC sitting on a bench does
not alert on every collector cycle.
The handover still happens on its own once the old PC has been quiet for a day,
which is what a PC pulled off a machine does. Moving the old PC off In Use -
Retired, Inventory, In Repair - hands the machine over on the next report,
which gives IT a one-step way to force a swap the moment it happens rather than
waiting out the window. A day is long enough that a PC switched off overnight,
or one behind a network outage, never loses its bay to a spare.
Alerts for both cases are gated on a new computers_machinelink_alerts setting
and ship OFF. Several part markers legitimately share a machine number here, so
the alerts would fire on correct data. Links, warnings in the collector
response, and archived history are unaffected; only the sending is gated.
Also: the alert goes through send_alert rather than resolving recipients by
hand, which had missed the SMTP_ALERT_RECIPIENTS environment fallback, so a
site configuring SMTP by environment would have got the webhook and no email.
---
CHANGELOG.md | 21 +-
plugins/computers/plugin.py | 228 ++++++++++++++++++---
tests/test_core/test_collector_contract.py | 193 +++++++++++++++--
3 files changed, 400 insertions(+), 42 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 832c005..4ee7097 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -55,10 +55,23 @@ ADR-007 and ADR-002.
that had been pulled out. The link is a `controls` relationship carrying a
`collector:machine` origin label, so a link made by hand is never touched.
Superseding a PC archives the old link rather than deleting it, keeping the
- history of which PC ran a machine and when, and raises an alert by email and
- webhook. The retired PC's status is deliberately left alone: the collector
- cannot tell whether it was shelved, sent for repair or re-imaged for another
- bay, so a person decides.
+ history of which PC ran a machine and when. The retired PC's status is
+ deliberately left alone: the collector cannot tell whether it was shelved,
+ sent for repair or re-imaged for another bay, so a person decides.
+
+ A second PC reporting a machine another PC already holds is treated as a
+ claim, not a handover. A PC imaged for a machine carries that number before
+ it ever reaches the floor, so the PC actually running the machine keeps the
+ link while it is still reporting and still In Use, and the challenger is
+ recorded as a dormant link. The handover completes on its own once the old PC
+ has been quiet for a day, or immediately when someone moves it off In Use.
+ Without this the two PCs traded the link back and forth on every collector
+ cycle.
+
+ Alerts for both cases go out by email and webhook, gated on a new
+ `computers_machinelink_alerts` setting that ships OFF: a site may legitimately
+ run several PCs on one machine number, and there the alerts fire on correct
+ data. The collector response warns either way.
- Plugin contract 0.16.0: `get_settings_defaults()` lets a plugin declare the
settings it owns (key, default, type, category, description, and whether an
unauthenticated caller may read it). See `docs/PLUGIN-HOOKS.md`.
diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py
index 19e9a01..69cc4e6 100644
--- a/plugins/computers/plugin.py
+++ b/plugins/computers/plugin.py
@@ -34,6 +34,12 @@ MEASURINGTOOL_LINK_ORIGIN = 'collector:measuringtool'
# touched.
MACHINE_LINK_ORIGIN = 'collector:machine'
+# How long a PC holding a machine may go without reporting before a second PC
+# claiming that machine is treated as its replacement. A swap resolves itself
+# within a day; a PC off overnight or behind a network outage keeps its bay.
+# Shorter than this and a spare imaged on the bench could steal a live machine.
+MACHINE_CLAIM_QUIET_HOURS = 24
+
class ComputersPlugin(BasePlugin):
"""
@@ -145,6 +151,46 @@ class ComputersPlugin(BasePlugin):
},
}
+ def get_settings_defaults(self) -> List[dict]:
+ """Settings this plugin owns.
+
+ The framework seeds these at install, at enable, and on every
+ `flask plugin upgrade-all`, so a key added in a later version reaches a
+ site that installed an earlier one.
+ """
+ return [
+ {
+ 'key': 'computers_machinelink_alerts',
+ 'value': 'false',
+ 'valuetype': 'boolean',
+ 'category': 'computers',
+ 'description': 'Email and webhook alerts when a PC takes over '
+ 'a machine or claims one another PC still runs. '
+ 'Off while machine numbers are shared between '
+ 'PCs; the collector response still warns.',
+ },
+ ]
+
+ def _machinelink_alerts_enabled(self):
+ """Whether the PC-to-machine alerts may be sent.
+
+ Off by default, deliberately. A site may legitimately run several PCs
+ on one machine number - part markers do at West Jefferson - and there
+ both the handover and the contested case fire on normal, correct data,
+ which is noise rather than news. Turn it on at a site where a machine
+ number means exactly one PC.
+
+ The links, the warnings in the collector response, and the archived
+ history all continue regardless. Only the sending is gated.
+ """
+ from shopdb.api import Setting
+
+ setting = Setting.query.filter_by(
+ key='computers_machinelink_alerts').first()
+ if not setting:
+ return False
+ return (setting.value or '').strip().lower() in ('true', '1', 'yes')
+
def apply_collector_payload(self, payload: Dict) -> Dict:
"""Idempotent upsert of a PC from a collector payload (by hostname)."""
from datetime import datetime, timezone
@@ -527,6 +573,14 @@ class ComputersPlugin(BasePlugin):
deliberately NOT changed: the collector cannot tell whether it was
shelved, broken or re-imaged for another bay, and guessing would
overwrite whatever a person deliberately set.
+
+ A second PC reporting a machine another PC already holds is a CLAIM, not
+ proof of replacement. A PC imaged on the bench for machine 3010 carries
+ that number before it ever reaches the floor, and treating the claim as
+ a handover made the two PCs trade the link back and forth at collector
+ cadence, alerting on every pass. The incumbent therefore keeps the
+ machine while it is still alive, and the challenger is recorded as a
+ dormant link. See _incumbent_has_yielded for what alive means.
"""
from shopdb.api import AssetRelationship, RelationshipType, Asset
@@ -555,8 +609,8 @@ class ComputersPlugin(BasePlugin):
if machine.assetid == pcasset.assetid:
return []
- # This PC's own collector links: keep the one for the reported machine,
- # archive any other (the PC moved bays).
+ # This PC's own collector links: the one for the reported machine is
+ # settled below, any other is archived (the PC moved bays).
mine = AssetRelationship.query.filter(
AssetRelationship.sourceassetid == pcasset.assetid,
AssetRelationship.relationshiptypeid == controls.relationshiptypeid,
@@ -566,10 +620,48 @@ class ComputersPlugin(BasePlugin):
link = None
for rel in mine:
if rel.targetassetid == machine.assetid:
- rel.isactive = True
link = rel
else:
rel.isactive = False
+
+ # Whoever actively holds this machine now, if it is not this PC.
+ held = AssetRelationship.query.filter(
+ AssetRelationship.targetassetid == machine.assetid,
+ AssetRelationship.relationshiptypeid == controls.relationshiptypeid,
+ AssetRelationship.label == MACHINE_LINK_ORIGIN,
+ AssetRelationship.sourceassetid != pcasset.assetid,
+ AssetRelationship.isactive.is_(True),
+ ).all()
+ blocking = [rel for rel in held
+ if not self._incumbent_has_yielded(rel.sourceassetid)]
+
+ if blocking:
+ # Contested. The incumbent is still reporting and still In Use, so
+ # this is a claim on a bay it has not taken over yet. Record the
+ # claim dormant and leave the live link where it is; the dormant row
+ # is also the marker that says this was already announced, which is
+ # what stops an alert on every report.
+ names = ', '.join(self._assetname(rel.sourceassetid)
+ for rel in blocking)
+ warnings.append(
+ 'machine {} is still held by {}; {} claim recorded but not '
+ 'linked'.format(machine.assetnumber, names, comp.hostname))
+ if link is None:
+ db.session.add(AssetRelationship(
+ sourceassetid=pcasset.assetid,
+ targetassetid=machine.assetid,
+ relationshiptypeid=controls.relationshiptypeid,
+ label=MACHINE_LINK_ORIGIN,
+ isactive=False))
+ self._alert_machine_contested(names, comp, machine)
+ else:
+ link.isactive = False
+ return [{'assetid': machine.assetid,
+ 'machinenumber': machine.assetnumber,
+ 'contestedby': names,
+ 'superseded': 0}]
+
+ # Uncontested: take the machine.
if link is None:
link = AssetRelationship(
sourceassetid=pcasset.assetid,
@@ -578,19 +670,14 @@ class ComputersPlugin(BasePlugin):
label=MACHINE_LINK_ORIGIN,
isactive=True)
db.session.add(link)
+ else:
+ link.isactive = True
- # Any OTHER pc still collector-linked to this machine has been replaced.
- superseded = AssetRelationship.query.filter(
- AssetRelationship.targetassetid == machine.assetid,
- AssetRelationship.relationshiptypeid == controls.relationshiptypeid,
- AssetRelationship.label == MACHINE_LINK_ORIGIN,
- AssetRelationship.sourceassetid != pcasset.assetid,
- AssetRelationship.isactive.is_(True),
- ).all()
- for rel in superseded:
+ # Anything still holding it has yielded: gone quiet, or taken off In
+ # Use by a person. That is a replacement, so archive and say so.
+ for rel in held:
rel.isactive = False
- old = db.session.get(Asset, rel.sourceassetid)
- oldname = old.assetnumber if old else str(rel.sourceassetid)
+ oldname = self._assetname(rel.sourceassetid)
warnings.append(
'machine {} was taken over from {}; check that PC'.format(
machine.assetnumber, oldname))
@@ -598,24 +685,119 @@ class ComputersPlugin(BasePlugin):
return [{'assetid': machine.assetid,
'machinenumber': machine.assetnumber,
- 'superseded': len(superseded)}]
+ 'superseded': len(held)}]
+
+ def _assetname(self, assetid):
+ """Readable name for an asset id, for warnings and alerts."""
+ from shopdb.api import Asset
+
+ asset = db.session.get(Asset, assetid)
+ return asset.assetnumber if asset else str(assetid)
+
+ def _incumbent_has_yielded(self, assetid):
+ """True when the PC currently holding a machine has given it up.
+
+ Two ways to yield, and both are evidence rather than a guess:
+
+ It went quiet. A PC pulled off a machine stops reporting, so silence
+ past MACHINE_CLAIM_QUIET_HOURS is the handover signal. The window is
+ long enough that a PC switched off overnight, or one behind a network
+ outage, never loses its bay to a spare sitting on the bench.
+
+ Or a person moved it off In Use. Setting the old PC to Retired,
+ Inventory or In Repair is a deliberate statement that it no longer runs
+ the machine, and it is the one-step way to force a handover the moment
+ the swap happens instead of waiting out the window.
+
+ An asset with no computer extension row cannot report at all, so it
+ cannot be alive; it yields.
+ """
+ from datetime import datetime, timedelta, timezone
+
+ from shopdb.api import Asset
+
+ computer = Computer.query.filter_by(assetid=assetid).first()
+ if not computer:
+ return True
+
+ asset = db.session.get(Asset, assetid)
+ status = asset.status.status if asset and asset.status else None
+ if status and status != 'In Use':
+ return True
+
+ reported = computer.lastreporteddate
+ if not reported:
+ return True
+ quiet = datetime.now(timezone.utc).replace(tzinfo=None) - reported
+ return quiet > timedelta(hours=MACHINE_CLAIM_QUIET_HOURS)
+
+ def _alert_machine_contested(self, holdername, newcomp, machine):
+ """Say that a PC claims a machine another PC is still running.
+
+ Fires ONCE, on the report that first records the dormant claim. The
+ dormant relationship row is the marker: while it exists the claim is
+ already known, so a PC sitting on the bench for a fortnight does not
+ alert every collector cycle.
+
+ Nothing is changed in the data by this. It exists because the two
+ legitimate readings - a swap in progress, or a machine number typed
+ onto the wrong PC at imaging - look identical to the collector, and
+ only a person can tell them apart.
+ """
+ import logging as _logging
+ if not self._machinelink_alerts_enabled():
+ return
+ try:
+ from shopdb.api import send_alert, Setting
+
+ newname = newcomp.hostname if newcomp else 'a new PC'
+ machinename = machine.assetnumber or machine.name
+ subject = 'Machine {} claimed by {}, still run by {}'.format(
+ machinename, newname, holdername)
+ base = (Setting.get('site_base_url') or '').rstrip('/')
+ link = '{}/pcs/{}'.format(base, newcomp.computerid) if base else ''
+ linkhtml = ('View {1}
'.format(link, newname)
+ if link else '')
+ html = (
+ '{0} reports that it runs machine '
+ '{1}, but {2} is still '
+ 'reporting on that machine and is still In Use, so the link '
+ 'has been left where it is.
'
+ 'If this is a swap in progress, nothing needs doing: {0} '
+ 'takes the machine once {2} stops reporting for a day, or '
+ 'straight away if you set {2} to Retired, Inventory or In '
+ 'Repair. If instead {0} was imaged with the wrong machine '
+ 'number, correct it on {0}.
{3}'.format(
+ newname, machinename, holdername, linkhtml))
+ send_alert(subject, html)
+ except Exception:
+ _logging.getLogger(__name__).exception(
+ 'machine-contested alert failed for machine %s',
+ getattr(machine, 'assetnumber', '?'))
def _alert_pc_superseded(self, oldname, newcomp, machine):
"""Tell a human a PC was replaced on a machine. Best effort, never raises.
Deliberately an ALERT and not a status change: the collector cannot tell
whether the old PC was shelved, sent for repair or re-imaged for another
- bay, so it says what happened and lets a person decide. Uses the same
- email + webhook path as the toner alerts, falling back to the site-wide
- alert_recipients when nothing more specific is set.
+ bay, so it says what happened and lets a person decide.
+
+ Goes through send_alert, which is the site's configured alert fan-out:
+ email to the SMTP settings' alert recipients plus the alert webhook.
+ Resolving those recipients by hand would have missed the
+ SMTP_ALERT_RECIPIENTS environment fallback, so a site that configures
+ SMTP by environment rather than in the UI would have got the webhook
+ and no email.
NOT a shopfloor notification: that board is for operators (General,
Recertification, Recognition), and an IT asset message does not belong
in front of the floor.
"""
import logging as _logging
+ if not self._machinelink_alerts_enabled():
+ return
try:
- from shopdb.api import send_email, send_webhook, Setting
+ from shopdb.api import send_alert, Setting
newname = newcomp.hostname if newcomp else 'a new PC'
machinename = machine.assetnumber or machine.name
@@ -635,13 +817,7 @@ class ComputersPlugin(BasePlugin):
'changed - set it to Inventory, In Repair or Retired as '
'appropriate.
{3}'.format(newname, machinename, oldname,
linkhtml))
- send_webhook(subject, subject)
-
- raw = Setting.get('alert_recipients') or ''
- recipients = [a.strip() for a in raw.replace(';', ',').split(',')
- if a.strip()]
- if recipients:
- send_email(recipients, subject, html)
+ send_alert(subject, html)
except Exception:
_logging.getLogger(__name__).exception(
'PC-superseded alert failed for machine %s',
diff --git a/tests/test_core/test_collector_contract.py b/tests/test_core/test_collector_contract.py
index 011f49a..9824d97 100644
--- a/tests/test_core/test_collector_contract.py
+++ b/tests/test_core/test_collector_contract.py
@@ -565,6 +565,52 @@ def _links(machineassetid, activeonly=True):
return q.all()
+def _go_quiet(db, hostname, hours=48):
+ """Backdate a PC's last report so it reads as gone from the floor.
+
+ A PC pulled off a machine simply stops reporting, and there is no other
+ signal. Tests that want a completed handover have to age the incumbent
+ past MACHINE_CLAIM_QUIET_HOURS; without this it is a live PC and the
+ challenger is correctly held off.
+ """
+ from datetime import datetime, timedelta, timezone
+ from plugins.computers.models import Computer
+
+ comp = Computer.query.filter(Computer.hostname.ilike(hostname)).first()
+ comp.lastreporteddate = (datetime.now(timezone.utc).replace(tzinfo=None)
+ - timedelta(hours=hours))
+ db.session.commit()
+ return comp
+
+
+def _enable_machinelink_alerts(db):
+ """Turn the PC-to-machine alerts on.
+
+ They ship OFF: several part markers legitimately share one machine number
+ (0613, 0615, WJPRT), so on this site the alerts would fire on correct data.
+ A test that wants to see one has to say so.
+ """
+ from shopdb.core.models import Setting
+ db.session.add(Setting(key='computers_machinelink_alerts', value='true'))
+ db.session.commit()
+
+
+def _set_status(db, hostname, statusname):
+ """Put a PC's asset on a named status, the way a person would in the UI."""
+ from shopdb.core.models import AssetStatus
+ from plugins.computers.models import Computer
+
+ status = AssetStatus.query.filter_by(status=statusname).first()
+ if not status:
+ status = AssetStatus(status=statusname, isactive=True)
+ db.session.add(status)
+ db.session.flush()
+ comp = Computer.query.filter(Computer.hostname.ilike(hostname)).first()
+ comp.asset.statusid = status.statusid
+ db.session.commit()
+ return comp
+
+
def test_pc_reporting_an_existing_machine_number_does_not_500(
client, db, collector_key, computer_assettype, machine_3015):
"""The machine owns assetnumber 3015. Naming the PC 3015 too hit the unique
@@ -601,6 +647,7 @@ def test_replacing_a_pc_hands_the_machine_over(
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
+ _go_quiet(db, 'OLDPC')
r = client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
@@ -614,12 +661,102 @@ def test_replacing_a_pc_hands_the_machine_over(
assert any('taken over' in w for w in r.get_json()['data']['warnings'])
+def test_a_live_pc_keeps_its_machine_against_a_new_claim(
+ client, db, collector_key, computer_assettype, machine_3015):
+ """A PC imaged for machine 3015 carries that number before it ever reaches
+ the floor. While the PC actually running the machine is still reporting,
+ the claim is not a handover and must not move the link."""
+ from shopdb.core.models import Asset
+ client.post('/api/collector/computers',
+ json={'hostname': 'OLDPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+ r = client.post('/api/collector/computers',
+ json={'hostname': 'NEWPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+
+ active = _links(machine_3015.assetid)
+ assert len(active) == 1
+ assert db.session.get(Asset, active[0].sourceassetid).assetnumber == 'OLDPC'
+ assert any('still held by' in w for w in r.get_json()['data']['warnings'])
+
+ # The claim is recorded, dormant, so nothing about it is lost.
+ assert len(_links(machine_3015.assetid, activeonly=False)) == 2
+
+
+def test_a_contested_claim_does_not_flap_or_realert(
+ client, db, collector_key, computer_assettype, machine_3015,
+ monkeypatch):
+ """Both PCs keep reporting on their own schedules. The old code let them
+ trade the link on every pass and alerted each time."""
+ from shopdb.core.models import Asset
+ import shopdb.api as shopdbapi
+
+ alerts = []
+ monkeypatch.setattr(shopdbapi, 'send_alert',
+ lambda subj, html, **k: alerts.append(subj))
+ _enable_machinelink_alerts(db)
+
+ for _ in range(3):
+ client.post('/api/collector/computers',
+ json={'hostname': 'OLDPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+ client.post('/api/collector/computers',
+ json={'hostname': 'NEWPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+
+ active = _links(machine_3015.assetid)
+ assert len(active) == 1
+ assert db.session.get(Asset, active[0].sourceassetid).assetnumber == 'OLDPC'
+ assert len(alerts) == 1, 'contested claim must alert once, not every cycle'
+
+
+def test_retiring_the_old_pc_hands_the_machine_over_at_once(
+ client, db, collector_key, computer_assettype, machine_3015):
+ """The one-step way to force a swap without waiting out the quiet window:
+ a person says the old PC is no longer in use."""
+ from shopdb.core.models import Asset
+ client.post('/api/collector/computers',
+ json={'hostname': 'OLDPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+ client.post('/api/collector/computers',
+ json={'hostname': 'NEWPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+ _set_status(db, 'OLDPC', 'Retired')
+
+ client.post('/api/collector/computers',
+ json={'hostname': 'NEWPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+
+ active = _links(machine_3015.assetid)
+ assert len(active) == 1
+ assert db.session.get(Asset, active[0].sourceassetid).assetnumber == 'NEWPC'
+
+
+def test_a_pc_off_overnight_does_not_lose_its_machine(
+ client, db, collector_key, computer_assettype, machine_3015):
+ """Shorter than the quiet window is not gone. A machine must not change
+ hands because a PC was switched off for the night or the network dropped."""
+ from shopdb.core.models import Asset
+ client.post('/api/collector/computers',
+ json={'hostname': 'OLDPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+ _go_quiet(db, 'OLDPC', hours=14)
+ client.post('/api/collector/computers',
+ json={'hostname': 'NEWPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+
+ active = _links(machine_3015.assetid)
+ assert len(active) == 1
+ assert db.session.get(Asset, active[0].sourceassetid).assetnumber == 'OLDPC'
+
+
def test_the_old_link_is_archived_not_deleted(
client, db, collector_key, computer_assettype, machine_3015):
"""History has to survive: 'which PC ran 3015 in June' stays answerable."""
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
+ _go_quiet(db, 'OLDPC')
client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
@@ -636,6 +773,7 @@ def test_the_old_pc_status_is_left_alone(
headers={'X-API-Key': collector_key})
old = Computer.query.filter(Computer.hostname.ilike('OLDPC')).first()
before = old.asset.statusid
+ _go_quiet(db, 'OLDPC')
client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
@@ -674,33 +812,37 @@ def test_supersede_alerts_email_and_webhook(client, db, collector_key,
# Patch on shopdb.api, which is where the plugin imports them from - the
# names are bound there at import time, so patching shopdb.utils.mailer
# would silently have no effect and the real (disabled) mailer would run.
+ # send_alert is the site's configured fan-out: alert_recipients by email
+ # plus the alert webhook. Patch the fan-out, not its two legs - resolving
+ # recipients here by hand is exactly the bug this replaced.
sent = {}
- monkeypatch.setattr(
- shopdbapi, 'send_email',
- lambda to, subj, html, **k: sent.update(email=(to, subj, html)))
- monkeypatch.setattr(shopdbapi, 'send_webhook',
- lambda subj, text, **k: sent.update(webhook=subj))
+ monkeypatch.setattr(shopdbapi, 'send_alert',
+ lambda subj, html, **k: sent.update(alert=(subj, html)))
db.session.add(Setting(key='alert_recipients', value='it@example.com'))
db.session.add(Setting(key='site_base_url', value='https://shopdb/shopdb'))
db.session.commit()
+ _enable_machinelink_alerts(db)
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
+ _go_quiet(db, 'OLDPC')
client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
- assert 'webhook' in sent, 'webhook not fired on supersede'
- assert 'email' in sent, 'no email sent on supersede'
- assert sent['email'][0] == ['it@example.com']
- assert '3015' in sent['email'][1] and 'OLDPC' in sent['email'][1]
+ assert 'alert' in sent, 'no alert raised on supersede'
+ subject, html = sent['alert']
+ assert '3015' in subject and 'OLDPC' in subject and 'NEWPC' in subject
+ # Must be the handover alert, not the contested-claim one. Both name the
+ # same three things, so the subjects alone do not tell them apart.
+ assert 'replaced' in subject, subject
# The link must be keyed on computerid: /pcs/:id resolves through
# GET /api/computers/, so an assetid opens the wrong PC.
from plugins.computers.models import Computer
newpc = Computer.query.filter_by(hostname='NEWPC').first()
- assert '/pcs/{}"'.format(newpc.computerid) in sent['email'][2]
+ assert '/pcs/{}"'.format(newpc.computerid) in html
def test_alert_failure_never_breaks_the_collector(client, db, collector_key,
@@ -711,14 +853,41 @@ def test_alert_failure_never_breaks_the_collector(client, db, collector_key,
def boom(*a, **k):
raise RuntimeError('smtp down')
- monkeypatch.setattr(shopdbapi, 'send_email', boom)
- monkeypatch.setattr(shopdbapi, 'send_webhook', boom)
+ monkeypatch.setattr(shopdbapi, 'send_alert', boom)
+ _enable_machinelink_alerts(db)
client.post('/api/collector/computers',
json={'hostname': 'OLDPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
+ _go_quiet(db, 'OLDPC')
r = client.post('/api/collector/computers',
json={'hostname': 'NEWPC', 'machinenumber': '3015'},
headers={'X-API-Key': collector_key})
assert r.status_code == 200
assert len(_links(machine_3015.assetid)) == 1
+
+
+def test_machinelink_alerts_are_off_by_default(client, db, collector_key,
+ computer_assettype,
+ machine_3015, monkeypatch):
+ """Several part markers legitimately share a machine number (0613, 0615,
+ WJPRT), so on this site a handover alert would fire on correct data. The
+ link, the archive and the collector warning all still happen - only the
+ sending is gated, and it ships off."""
+ import shopdb.api as shopdbapi
+
+ alerts = []
+ monkeypatch.setattr(shopdbapi, 'send_alert',
+ lambda subj, html, **k: alerts.append(subj))
+
+ client.post('/api/collector/computers',
+ json={'hostname': 'OLDPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+ _go_quiet(db, 'OLDPC')
+ r = client.post('/api/collector/computers',
+ json={'hostname': 'NEWPC', 'machinenumber': '3015'},
+ headers={'X-API-Key': collector_key})
+
+ assert alerts == []
+ assert len(_links(machine_3015.assetid)) == 1
+ assert any('taken over' in w for w in r.get_json()['data']['warnings'])