From ab301df9acfcf6718764627d24ed33bd5b0f2580 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Fri, 14 Aug 2026 13:45:51 -0400 Subject: [PATCH] network: generate a device's asset number instead of asking twice Every network device on this fleet already follows one convention, applied by hand: AP-, SW-, SVR-, IDF-. 45 records, no exceptions. The create form demanded the asset number anyway, so the same value was typed twice and the convention held only as long as everyone remembered it. The prefix now lives on the device type, and a blank asset number is generated as -. Left explicit, an asset number always wins: a device carrying a real identifier of its own - a vendor tag, a controller name, a serial - keeps it. That is the platform rule, adopt where an identifier exists and derive only where none does. The prefix is NOT derived from the type name. "Access Point" and "Access Panel" both initialise to AP, and assetnumber is unique, so the second type would collide with the first on every device it created. It is nullable, so a type that wants no prefix generates the bare name rather than needing one invented. Names are sanitised before they reach a business key - the existing data already shows why, with IDF-Telco-Demarc-#1 carrying a '#' into an identifier. An existing prefix is never stacked: IDF-03 under type IDF stays IDF-03. --- plugins/network/api/routes.py | 18 ++- .../frontend/views/NetworkDeviceForm.vue | 34 +++++- .../frontend/views/NetworkTypesList.vue | 25 +++- .../versions/0003_networkdevicetype_prefix.py | 69 +++++++++++ plugins/network/models/network_device.py | 10 ++ plugins/network/services/__init__.py | 1 + plugins/network/services/assetnumbers.py | 70 ++++++++++++ tests/test_plugin_migrations.py | 4 +- .../test_network_assetnumber_generation.py | 108 ++++++++++++++++++ 9 files changed, 328 insertions(+), 11 deletions(-) create mode 100644 plugins/network/migrations/versions/0003_networkdevicetype_prefix.py create mode 100644 plugins/network/services/__init__.py create mode 100644 plugins/network/services/assetnumbers.py create mode 100644 tests/test_plugins/test_network_assetnumber_generation.py diff --git a/plugins/network/api/routes.py b/plugins/network/api/routes.py index a011fa8..d7a7eae 100644 --- a/plugins/network/api/routes.py +++ b/plugins/network/api/routes.py @@ -5,6 +5,7 @@ from flask_jwt_extended import jwt_required from shopdb.api import db, Asset, AssetType, Vendor, Communication, CommunicationType, AuditLog, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query +from ..services.assetnumbers import generate_for_type from ..models import NetworkDevice, NetworkDeviceType, Subnet, VLAN from shopdb.api import require_permission, apply_import_timestamps @@ -111,7 +112,8 @@ def create_network_device_type(): t = NetworkDeviceType( networkdevicetype=data['networkdevicetype'], description=data.get('description'), - icon=data.get('icon'), color=data.get('color') + icon=data.get('icon'), color=data.get('color'), + prefix=data.get('prefix') ) db.session.add(t) @@ -146,7 +148,8 @@ def update_network_device_type(type_id: int): http_code=409 ) - for key in ['networkdevicetype', 'description', 'icon', 'color', 'isactive']: + for key in ['networkdevicetype', 'description', 'icon', 'color', 'prefix', + 'isactive']: if key in data: setattr(t, key, data[key]) @@ -359,8 +362,17 @@ def create_network_device(): if not data: return error_response(ErrorCodes.VALIDATION_ERROR, 'No data provided') + # Generate from the type prefix + name when left blank. Only when blank: a + # device carrying a real identifier of its own - vendor tag, controller + # name, serial - keeps it, which is the platform rule. if not data.get('assetnumber'): - return error_response(ErrorCodes.VALIDATION_ERROR, 'assetnumber is required') + data['assetnumber'] = generate_for_type( + data.get('networkdevicetypeid'), data.get('name')) + if not data.get('assetnumber'): + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'assetnumber is required (or give a name, and a device type with a ' + 'prefix, to have one generated)') # Check for duplicate assetnumber if Asset.query.filter_by(assetnumber=data['assetnumber']).first(): diff --git a/plugins/network/frontend/views/NetworkDeviceForm.vue b/plugins/network/frontend/views/NetworkDeviceForm.vue index dbaa4b3..e474954 100644 --- a/plugins/network/frontend/views/NetworkDeviceForm.vue +++ b/plugins/network/frontend/views/NetworkDeviceForm.vue @@ -12,15 +12,24 @@
- + + + + Will be created as {{ generatedAssetNumber }} +
@@ -343,6 +352,27 @@ const form = ref({ }) const deviceTypes = ref([]) + +// Preview of what the backend will generate. Mirrors +// plugins/network/services/assetnumbers.py - the server is authoritative and +// re-derives it; this only shows the operator what they are about to get. +const generatedAssetNumber = computed(() => { + const cleaned = (value) => (value || '') + .trim() + .replace(/[\s_/\\]+/g, '-') + .replace(/[^A-Za-z0-9.-]/g, '') + .replace(/-{2,}/g, '-') + .replace(/^-+|-+$/g, '') + const name = cleaned(form.value.name) + if (!name) return '' + const found = deviceTypes.value.find( + t => String(t.networkdevicetypeid) === String(form.value.networkdevicetypeid)) + const prefix = cleaned(found?.prefix).toUpperCase() + if (!prefix) return name + // never stack a prefix the name already carries (IDF-03 under type IDF) + if (name.toUpperCase().startsWith(prefix + '-')) return name + return `${prefix}-${name}` +}) const vendors = ref([]) const models = ref([]) const locations = ref([]) diff --git a/plugins/network/frontend/views/NetworkTypesList.vue b/plugins/network/frontend/views/NetworkTypesList.vue index 344a3f7..cb430ba 100644 --- a/plugins/network/frontend/views/NetworkTypesList.vue +++ b/plugins/network/frontend/views/NetworkTypesList.vue @@ -14,6 +14,7 @@ Network Device Type + Prefix Description Color Actions @@ -22,6 +23,7 @@ {{ t.networkdevicetype }} + {{ t.prefix }}-- {{ t.description || '-' }} {{ t.color || 'auto' }} @@ -31,7 +33,7 @@ - No network device types found + No network device types found @@ -48,6 +50,21 @@
+
+ + + + + New devices will be numbered {{ form.prefix.toUpperCase() }}-<name> + +
@@ -88,7 +105,7 @@ const showModal = ref(false) const editing = ref(null) const saving = ref(false) const error = ref('') -const form = ref({ networkdevicetype: '', description: '', color: '', isactive: true }) +const form = ref({ networkdevicetype: '', prefix: '', description: '', color: '', isactive: true }) onMounted(loadData) @@ -107,8 +124,8 @@ async function loadData() { function openModal(item = null) { editing.value = item form.value = item - ? { networkdevicetype: item.networkdevicetype || '', description: item.description || '', color: item.color || '', isactive: item.isactive !== false } - : { networkdevicetype: '', description: '', color: '', isactive: true } + ? { networkdevicetype: item.networkdevicetype || '', prefix: item.prefix || '', description: item.description || '', color: item.color || '', isactive: item.isactive !== false } + : { networkdevicetype: '', prefix: '', description: '', color: '', isactive: true } error.value = '' showModal.value = true } diff --git a/plugins/network/migrations/versions/0003_networkdevicetype_prefix.py b/plugins/network/migrations/versions/0003_networkdevicetype_prefix.py new file mode 100644 index 0000000..4ff2193 --- /dev/null +++ b/plugins/network/migrations/versions/0003_networkdevicetype_prefix.py @@ -0,0 +1,69 @@ +"""Asset-number prefix per network device type. + +Every network device already follows one convention, applied by hand: +AP-, SW-, SVR-, IDF-. 45 records, no exceptions. This +stores the prefix on the type so the create path can generate the asset number +instead of asking someone to retype the same value twice. + +Nullable: a type with no prefix generates the bare name. Nobody has to invent +one for a type that does not want it. + +It deliberately is NOT derived from the type name - 'Access Point' and 'Access +Panel' both initialise to AP, and assetnumber is unique, so the second type +would collide with the first on every device it created. + +Backfilled for the four types already in use so existing conventions carry +forward. A site whose types are named differently gets NULL and generates bare +names, which is correct rather than wrong. +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'network0003prefix' +down_revision = 'network0002model' +branch_labels = None +depends_on = None + + +# type name -> prefix, from what the existing records already use +SEEDPREFIXES = { + 'Access Point': 'AP', + 'Switch': 'SW', + 'Server': 'SVR', + 'IDF': 'IDF', +} + + +def upgrade(): + # Guarded like network0002model: on a FRESH database the table is built from + # the SQLAlchemy models, which already declare this column, so an + # unconditional add fails with "duplicate column name". + bind = op.get_bind() + insp = sa.inspect(bind) + if 'networkdevicetypes' not in insp.get_table_names(): + return + cols = {c['name'] for c in insp.get_columns('networkdevicetypes')} + if 'prefix' not in cols: + op.add_column('networkdevicetypes', + sa.Column('prefix', sa.String(length=12), nullable=True)) + + # Backfill only where the type exists and has no prefix yet, so a re-run and + # a site that already set its own are both left alone. + for typename, prefix in SEEDPREFIXES.items(): + bind.execute( + sa.text('UPDATE networkdevicetypes SET prefix = :prefix ' + 'WHERE networkdevicetype = :typename ' + 'AND (prefix IS NULL OR prefix = :empty)'), + {'prefix': prefix, 'typename': typename, 'empty': ''}) + + +def downgrade(): + bind = op.get_bind() + insp = sa.inspect(bind) + if 'networkdevicetypes' not in insp.get_table_names(): + return + cols = {c['name'] for c in insp.get_columns('networkdevicetypes')} + if 'prefix' in cols: + op.drop_column('networkdevicetypes', 'prefix') diff --git a/plugins/network/models/network_device.py b/plugins/network/models/network_device.py index d8ce412..1f267f1 100644 --- a/plugins/network/models/network_device.py +++ b/plugins/network/models/network_device.py @@ -16,6 +16,16 @@ class NetworkDeviceType(BaseModel): description = db.Column(db.Text) icon = db.Column(db.String(50), comment='Icon name for UI') color = db.Column(db.String(20), comment='CSS color for UI/map markers') + # Asset-number prefix for devices of this type: AP-, SW-. + # NULLABLE on purpose - a type with no prefix generates the bare name, so + # nobody has to invent one for a type that does not want it. + # + # It cannot be derived from the type NAME: 'Access Point' and 'Access Panel' + # both initialise to AP, and assetnumber is unique, so the second type would + # collide with the first on every device. + prefix = db.Column( + db.String(12), + comment='Asset-number prefix for this type (AP, SW, SVR). Blank = none') def __repr__(self): return f"" diff --git a/plugins/network/services/__init__.py b/plugins/network/services/__init__.py new file mode 100644 index 0000000..4cc7a75 --- /dev/null +++ b/plugins/network/services/__init__.py @@ -0,0 +1 @@ +"""Network plugin services.""" diff --git a/plugins/network/services/assetnumbers.py b/plugins/network/services/assetnumbers.py new file mode 100644 index 0000000..f6582e5 --- /dev/null +++ b/plugins/network/services/assetnumbers.py @@ -0,0 +1,70 @@ +"""Generate a network device's asset number from its type prefix and name. + +Every network device on this fleet already follows one convention, applied by +hand: AP-, SW-, SVR-, IDF-. This turns that into +something the create path does, so the same value is not typed twice. + +Generation only fills a BLANK asset number. A device that carries a real +identifier of its own - a vendor tag, a controller name, a serial - keeps it. +That is the platform rule: adopt an external identifier where one exists, derive +one only where none does. +""" + +import re + + +# Characters that must not reach a unique business key. assetnumber ends up in +# URLs and in joins other subsystems make, and the existing data already shows +# why this matters: IDF-Telco-Demarc-#1 came from the name 'Telco Demarc #1', +# carrying a '#' into an identifier. +_SEPARATORS = re.compile(r'[\s_/\\]+') +_ILLEGAL = re.compile(r'[^A-Za-z0-9.-]') +_RUNS = re.compile(r'-{2,}') + + +def sanitize(value): + """A name reduced to something safe to use as an identifier. + + Whitespace and slashes become single dashes; anything outside + letters/digits/dot/dash is dropped rather than transliterated, because a + guessed transliteration in a business key is worse than a shorter one. + """ + text = (value or '').strip() + if not text: + return '' + text = _SEPARATORS.sub('-', text) + text = _ILLEGAL.sub('', text) + text = _RUNS.sub('-', text) + return text.strip('-') + + +def generate(prefix, name): + """'-', or the bare name when the type has no prefix. + + Returns '' when there is nothing to build from, so the caller can fall back + to demanding an explicit value rather than inventing one. + + Does NOT stack an existing prefix: a name already starting with the prefix + (IDF-03 under type IDF) is returned as-is, because IDF-IDF-03 is nobody's + intent. Matched case-insensitively on the prefix plus its dash. + """ + cleanname = sanitize(name) + if not cleanname: + return '' + cleanprefix = sanitize(prefix).upper() + if not cleanprefix: + return cleanname + if cleanname.upper().startswith(cleanprefix + '-'): + return cleanname + return '{}-{}'.format(cleanprefix, cleanname) + + +def generate_for_type(networkdevicetypeid, name): + """Same, looking the prefix up from the device type. '' if no type given.""" + if not networkdevicetypeid: + return sanitize(name) + from shopdb.api import db + from ..models import NetworkDeviceType + + devicetype = db.session.get(NetworkDeviceType, networkdevicetypeid) + return generate(devicetype.prefix if devicetype else None, name) diff --git a/tests/test_plugin_migrations.py b/tests/test_plugin_migrations.py index 15bc406..198add1 100644 --- a/tests/test_plugin_migrations.py +++ b/tests/test_plugin_migrations.py @@ -52,7 +52,7 @@ EXPECTED_HEAD_REVISION['measuringtools'] = 'measuringtools0001baseline' EXPECTED_HEAD_REVISION['backups'] = 'backups0003clearlastseen' # geenforce adds the content-addressed blob store (manifestblobs) on top of its # baseline. -EXPECTED_HEAD_REVISION['geenforce'] = 'geenforce0003subtype' +EXPECTED_HEAD_REVISION['geenforce'] = 'geenforce0004minlib' # printers adds the printersupplyalerts crossing-state table on top of its anchor. EXPECTED_HEAD_REVISION['printers'] = 'printers0002supplyalerts' # machines (renamed from equipment) keeps its original anchor id and adds the @@ -64,7 +64,7 @@ EXPECTED_HEAD_REVISION['employees'] = 'employees0002photo' EXPECTED_HEAD_REVISION['usb'] = 'usb0002dropmachineid' # network links a device to a catalog model, which is where its photo comes # from - machines, PCs and printers already had that link. -EXPECTED_HEAD_REVISION['network'] = 'network0002model' +EXPECTED_HEAD_REVISION['network'] = 'network0003prefix' # printedparts is post-cutover: its 0001 really creates its tables; 0004 adds # the per-transaction revision column. EXPECTED_HEAD_REVISION['printedparts'] = 'printedparts0004txnrev' diff --git a/tests/test_plugins/test_network_assetnumber_generation.py b/tests/test_plugins/test_network_assetnumber_generation.py new file mode 100644 index 0000000..c77b0f9 --- /dev/null +++ b/tests/test_plugins/test_network_assetnumber_generation.py @@ -0,0 +1,108 @@ +"""Asset numbers generated from the device type's prefix. + +Every network device on this fleet already followed one convention applied by +hand - AP-, SW-, SVR-, IDF-, 45 records, no exceptions. +Generation codifies it so the same value is not typed twice, and must reproduce +those records exactly rather than inventing a new scheme. +""" + +import pytest + +from shopdb.extensions import db +from shopdb.core.models import AssetType +from plugins.network.models import NetworkDeviceType + + +def _devicetype(db, name, prefix): + """A device type, with the core asset type the create path needs present.""" + if not AssetType.query.filter_by(assettype='network_device').first(): + db.session.add(AssetType(assettype='network_device')) + devicetype = NetworkDeviceType(networkdevicetype=name, prefix=prefix) + db.session.add(devicetype) + db.session.commit() + return devicetype +from plugins.network.services.assetnumbers import generate, sanitize + + +@pytest.mark.parametrize('prefix,name,expected', [ + ('AP', 'AIRwdMUSwestj02', 'AP-AIRwdMUSwestj02'), + ('SW', 'AIRsdMUSwestj01', 'SW-AIRsdMUSwestj01'), + ('SVR', 'AVEWP4546V01', 'SVR-AVEWP4546V01'), +]) +def test_reproduces_the_existing_convention(prefix, name, expected): + assert generate(prefix, name) == expected + + +def test_a_type_with_no_prefix_gives_the_bare_name(): + """Nullable on purpose - nobody invents a prefix for a type that wants none.""" + assert generate(None, 'Bare Name') == 'Bare-Name' + assert generate('', 'Bare Name') == 'Bare-Name' + + +def test_an_already_prefixed_name_is_not_stacked(): + """IDF-03 under type IDF is IDF-03, never IDF-IDF-03. Real row on this data.""" + assert generate('IDF', 'IDF-03') == 'IDF-03' + assert generate('idf', 'idf-03') == 'idf-03' + + +def test_characters_that_break_a_business_key_are_stripped(): + """assetnumber lands in URLs and in joins. The existing IDF-Telco-Demarc-#1 + row shows a '#' reaching a unique key, which is what this prevents.""" + assert generate('IDF', 'Telco Demarc #1') == 'IDF-Telco-Demarc-1' + assert generate('APNL', 'Door 4 / West') == 'APNL-Door-4-West' + assert sanitize(' spaced out ') == 'spaced-out' + + +def test_nothing_to_build_from_returns_empty(): + """So the caller demands an explicit value instead of inventing one.""" + assert generate('AP', '') == '' + assert generate('AP', None) == '' + + +def test_create_generates_when_assetnumber_is_blank(client, db, auth_headers): + devicetype = _devicetype(db, 'Access Panel', 'APNL') + + resp = client.post('/api/network', json={ + 'name': 'Dock Door 4', + 'networkdevicetypeid': devicetype.networkdevicetypeid, + }, headers=auth_headers) + assert resp.status_code == 201, resp.get_json() + assert resp.get_json()['data']['assetnumber'] == 'APNL-Dock-Door-4' + + +def test_an_explicit_assetnumber_always_wins(client, db, auth_headers): + """Adopt an external identifier where one exists - a vendor tag beats a + generated value, which is why generation only fills a BLANK field.""" + devicetype = _devicetype(db, 'Access Panel 2', 'APNL') + + resp = client.post('/api/network', json={ + 'assetnumber': 'VENDORTAG-99', + 'name': 'Dock Door 5', + 'networkdevicetypeid': devicetype.networkdevicetypeid, + }, headers=auth_headers) + assert resp.status_code == 201, resp.get_json() + assert resp.get_json()['data']['assetnumber'] == 'VENDORTAG-99' + + +def test_no_name_and_no_assetnumber_is_refused(client, db, auth_headers): + devicetype = _devicetype(db, 'Access Panel 3', 'APNL') + + resp = client.post('/api/network', json={ + 'networkdevicetypeid': devicetype.networkdevicetypeid, + }, headers=auth_headers) + assert resp.status_code == 400 + assert 'assetnumber is required' in resp.get_json()['data']['error']['message'] + + +def test_prefix_round_trips_through_the_type_api(client, db, auth_headers): + created = client.post('/api/network/types', json={ + 'networkdevicetype': 'Door Controller', 'prefix': 'DOOR', + }, headers=auth_headers) + assert created.status_code == 201, created.get_json() + assert created.get_json()['data']['prefix'] == 'DOOR' + + typeid = created.get_json()['data']['networkdevicetypeid'] + updated = client.put(f'/api/network/types/{typeid}', json={'prefix': 'DR'}, + headers=auth_headers) + assert updated.status_code == 200, updated.get_json() + assert updated.get_json()['data']['prefix'] == 'DR'