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'