ge-aerospace/shopdb-flask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

relationships: the cleanup tools stop acting on links that were deleted

Browse Source 
Deleting a relationship is soft, so the row survives with isactive False, and
three things read them without knowing that.

Re-adding a deleted link answered 409 "this relationship already exists" about a
link the page no longer shows, and there was no way forward from the UI at all -
the row cannot simply be inserted again, since the triple is unique.
Reactivating IS the create for an inactive row.

The inverse guard blocked on a deleted inverse, which made "remove the existing
one first" - the instruction in its own message - fail to unblock anything.

fix-controls-direction retired the reversed row whenever a correctly-directed
one existed, without checking whether that one was itself deleted. So it removed
the only live link and reported a successful clean-up. It now reactivates the
row pointing the right way before retiring the one pointing the wrong way.

These are the commands the docs tell an operator to run against production.
This commit is contained in:
cproudlock
2026-08-14 13:47:01 -04:00
parent c7dffce81e
commit 2df5028883
3 changed files with 113 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
shopdb/cli/__init__.py
View File

@@ -738,6 +738,13 @@ def fix_controls_direction():
relationshiptypeid=controls.relationshiptypeid,
).first()
if duplicate:
# The correctly-directed row exists - but it may itself be soft
# deleted, and deletion here IS soft. Retiring this row without
# looking left the pair with NO live link, while the command
# reported a successful clean-up. Reactivate the one pointing the
# right way before retiring the one pointing the wrong way.
if not duplicate.isactive:
duplicate.isactive = True
row.isactive = False # flipped row already exists, retire this one
deactivated += 1
else:

41
shopdb/core/api/assets.py
View File

@@ -768,14 +768,23 @@ def create_asset_relationship():
'An asset cannot be related to itself'
)
# Check for duplicate relationship
# Check for duplicate relationship.
#
# Deleting a relationship is SOFT (isactive = False, see
# delete_asset_relationship). The row therefore survives, and this lookup
# found it - so deleting a link and adding it back answered 409 "this
# relationship already exists" about a link the page no longer shows, with
# no way forward from the UI at all. The row cannot simply be inserted
# again either: assetrelationships is unique on (source, target, type).
#
# Reactivating IS the create, for an inactive row.
existing = AssetRelationship.query.filter_by(
sourceassetid=source_id,
targetassetid=target_id,
relationshiptypeid=type_id
).first()
if existing:
if existing and existing.isactive:
return error_response(
ErrorCodes.CONFLICT,
'This relationship already exists',
@@ -798,7 +807,11 @@ def create_asset_relationship():
targetassetid=source_id,
relationshiptypeid=type_id
).first()
if inverse is not None:
# isactive, for the same reason as the duplicate check above: a soft
# deleted inverse is a link somebody already removed, and blocking on it
# made "remove the existing one first" - the instruction in this very
# message - fail to unblock anything.
if inverse is not None and inverse.isactive:
return error_response(
ErrorCodes.CONFLICT,
f"The reverse of this relationship already exists "
@@ -808,14 +821,20 @@ def create_asset_relationship():
http_code=409
)
rel = AssetRelationship(
sourceassetid=source_id,
targetassetid=target_id,
relationshiptypeid=type_id,
notes=data.get('notes')
)
db.session.add(rel)
if existing is not None:
# The soft-deleted row from the duplicate check. Reuse it: the unique
# index on (source, target, type) means there is no second row to add.
rel = existing
rel.isactive = True
rel.notes = data.get('notes')
else:
rel = AssetRelationship(
sourceassetid=source_id,
targetassetid=target_id,
relationshiptypeid=type_id,
notes=data.get('notes')
)
db.session.add(rel)
apply_import_timestamps(rel, data)
db.session.flush()

76
tests/test_core/test_relationship_soft_delete.py Normal file
View File

@@ -0,0 +1,76 @@
"""Deleting a relationship is soft, and everything that reads them must know.
Rows are retired with isactive = False, never removed, and assetrelationships is
unique on (source, target, type). Anything that looks a relationship up without
filtering on isactive therefore sees links the UI does not show - and refuses to
create one it cannot create any other way, or retires the last live row believing
another one covers it.
"""
import pytest
from shopdb.core.models import Asset, AssetType
from shopdb.core.models.relationship import RelationshipType, AssetRelationship
@pytest.fixture
def rig(db):
if not AssetType.query.filter_by(assettype='computer').first():
db.session.add(AssetType(assettype='computer'))
if not AssetType.query.filter_by(assettype='machine').first():
db.session.add(AssetType(assettype='machine'))
controls = RelationshipType.query.filter_by(relationshiptype='controls').first()
if not controls:
controls = RelationshipType(relationshiptype='controls', isdirectional=True)
db.session.add(controls)
controls.isdirectional = True
db.session.commit()
computer = AssetType.query.filter_by(assettype='computer').first()
machine = AssetType.query.filter_by(assettype='machine').first()
pc = Asset(assetnumber='PCSOFT01', assettypeid=computer.assettypeid)
bay = Asset(assetnumber='BAYSOFT01', assettypeid=machine.assettypeid)
db.session.add_all([pc, bay])
db.session.commit()
return {'pc': pc, 'bay': bay, 'controls': controls}
def _create(client, headers, source, target, typeid):
return client.post('/api/assets/relationships',
json={'sourceassetid': source, 'targetassetid': target,
'relationshiptypeid': typeid},
headers=headers)
def test_a_deleted_relationship_can_be_created_again(client, db, rig, auth_headers):
"""Soft delete then re-add answered 409 about a link nothing displayed."""
created = _create(client, auth_headers, rig['pc'].assetid, rig['bay'].assetid,
rig['controls'].relationshiptypeid)
assert created.status_code == 201, created.get_json()
relid = created.get_json()['data']['relationshipid']
assert client.delete('/api/assets/relationships/%d' % relid,
headers=auth_headers).status_code == 200
again = _create(client, auth_headers, rig['pc'].assetid, rig['bay'].assetid,
rig['controls'].relationshiptypeid)
assert again.status_code in (200, 201), again.get_json()
row = db.session.get(AssetRelationship, relid)
assert row.isactive is True
assert AssetRelationship.query.filter_by(
sourceassetid=rig['pc'].assetid,
targetassetid=rig['bay'].assetid).count() == 1
def test_a_deleted_inverse_does_not_block_the_right_direction(
client, db, rig, auth_headers):
"""'Remove the existing one first' is the advice the guard itself gives."""
wrong = _create(client, auth_headers, rig['bay'].assetid, rig['pc'].assetid,
rig['controls'].relationshiptypeid)
assert wrong.status_code == 201
relid = wrong.get_json()['data']['relationshipid']
assert client.delete('/api/assets/relationships/%d' % relid,
headers=auth_headers).status_code == 200
right = _create(client, auth_headers, rig['pc'].assetid, rig['bay'].assetid,
rig['controls'].relationshiptypeid)
assert right.status_code in (200, 201), right.get_json()