From fbd99041b0d2b0998c0b83ab7f74b1ca0e717801 Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Mon, 17 Aug 2026 13:03:54 -0400 Subject: [PATCH] Buildings and levels for the floor map, and make every identifier searchable --- CHANGELOG.md | 61 ++ README.md | 23 + frontend/src/api/index.js | 57 ++ .../src/components/EmbeddedLocationMap.vue | 289 +++--- .../src/components/LocationMapTooltip.vue | 55 +- frontend/src/components/ShopFloorMap.vue | 29 +- frontend/src/composables/mapConfig.js | 164 ++- frontend/src/composables/mapConfig.spec.js | 144 +++ frontend/src/composables/systemSettings.js | 4 +- frontend/src/views/MapEditor.vue | 293 +++++- frontend/src/views/MapView.vue | 11 +- frontend/src/views/SearchResults.vue | 964 +++++++++--------- .../src/views/settings/FloorMapSettings.vue | 473 +++++++-- .../versions/7d33_buildings_and_levels.py | 213 ++++ plugins/computers/api/routes.py | 3 +- plugins/computers/frontend/views/PCForm.vue | 35 +- plugins/computers/manifest.json | 46 +- plugins/geenforce/api/routes.py | 1 + plugins/knowledgebase/api/routes.py | 41 +- .../frontend/views/KnowledgeBaseForm.vue | 4 +- .../frontend/views/KnowledgeBaseList.vue | 4 +- plugins/machines/api/routes.py | 3 +- .../machines/frontend/views/MachineForm.vue | 39 +- plugins/machines/manifest.json | 2 +- plugins/measuringtools/api/routes.py | 3 +- plugins/measuringtools/manifest.json | 2 +- plugins/network/api/routes.py | 3 +- .../frontend/views/NetworkDeviceForm.vue | 36 +- plugins/network/manifest.json | 44 +- plugins/printers/api/asset_routes.py | 7 +- .../printers/frontend/views/PrinterForm.vue | 35 +- .../frontend/views/PrinterInstallerMap.vue | 16 +- plugins/printers/manifest.json | 2 +- plugins/warranty/api/routes.py | 1 + scripts/check-naming-and-style.sh | 26 + shopdb/__init__.py | 4 +- shopdb/api/__init__.py | 26 +- shopdb/core/api/__init__.py | 4 + shopdb/core/api/assets.py | 4 +- shopdb/core/api/maplevels.py | 350 +++++++ shopdb/core/api/mappositions.py | 403 ++++++++ shopdb/core/api/search.py | 139 ++- shopdb/core/api/settings.py | 2 + shopdb/core/models/__init__.py | 6 + shopdb/core/models/asset.py | 30 +- shopdb/core/models/location.py | 7 + shopdb/core/models/maplevel.py | 109 ++ shopdb/core/models/mapsnapshot.py | 76 ++ shopdb/utils/imagesize.py | 134 +++ tests/test_api_namespace.py | 68 +- tests/test_core/test_map_transform.py | 236 +++++ tests/test_core/test_search_coverage.py | 201 ++++ .../test_knowledgebase_visibility.py | 106 ++ .../test_plugins/test_printer_install_list.py | 15 +- 54 files changed, 4173 insertions(+), 880 deletions(-) create mode 100644 frontend/src/composables/mapConfig.spec.js create mode 100644 migrations/versions/7d33_buildings_and_levels.py create mode 100644 shopdb/core/api/maplevels.py create mode 100644 shopdb/core/api/mappositions.py create mode 100644 shopdb/core/models/maplevel.py create mode 100644 shopdb/core/models/mapsnapshot.py create mode 100644 shopdb/utils/imagesize.py create mode 100644 tests/test_core/test_map_transform.py create mode 100644 tests/test_core/test_search_coverage.py create mode 100644 tests/test_plugins/test_knowledgebase_visibility.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 1dc03eae..15d1ffc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,67 @@ ADR-007 and ADR-002. ## [Unreleased] +The floor map became a set of drawings instead of one picture. A site can hold +more than one building, a building more than one level, and every map position +now records which drawing its coordinates belong to. Driven by a real move: a +second floor was added, a new blueprint changed size, and machines relocated. + +### Added + +- Buildings and levels (ADR-017). Each level owns its own blueprint per theme + and its own native pixel size; `assets.mapx`/`mapy` are pixels of the level + named by the new `assets.levelid`, not of the site. +- A bulk repositioning tool on the map editor: filter by unplaced, by + needs-review, or by level, search, then place markers and confirm them. New + `POST /api/mappositions/positions` and `/verify`. +- Landmark recalibration (`POST /api/mappositions/transform`). Name two or more + points that appear on both the old and new drawing and every marker on the + level moves onto the new one. Defaults to a dry run that reports each old and + new position and anything that would land off the drawing. The transform is + solved PER AXIS from the landmarks, never derived from image dimensions: a + taller drawing that gained a level below did not rescale, and a + dimension-derived scale would stretch Y by 1.57 and be wrong everywhere. +- Position snapshots with restore, taken before any bulk change including a + restore, so an undo is itself undoable. +- A buildings and levels admin under Settings: name and order levels, see each + level's id and marker count, upload a blueprint per theme, choose the default. +- `assets.mapverifiedat`, the record of when a position was last confirmed + against the current drawing. A bulk transform clears it, because a transform + is a starting guess and nothing in the coordinates says which markers moved. + +### Changed + +- Plugin contract to 0.20.0 (additive; see CONTRACT-STABILITY.md). A plugin that + writes a map position must now write its level. +- A position whose level is unknown renders as "level unknown" and is NOT drawn + on the default level. Drawing it there would look entirely correct while + pointing at the wrong part of the building. +- `GET /api/printers/install-list?format=text` gained `levelid` as a NINTH + field, appended. Fields 0-7 are unchanged because the shipped Pascal installer + reads them by index. +- Knowledge base: an article whose topic is a retired (`isactive = 0`) + application no longer appears in listings, searches or the counts, and the + topic picker offers every active application rather than only installable + ones. + +### Migration + +- `7d33_buildings_and_levels` converts an existing single-map site: it creates + one building and one default level carrying the blueprint paths and dimensions + from the old `map_*` settings, then assigns every already-placed asset and + location to it. Nothing moves on screen. The old settings rows are left in + place so a rollback still finds them. Verified end to end on MySQL 5.6 from a + production-shaped database. + +### Known gaps + +- The Pascal printer-installer map still compiles in one drawing's dimensions and + bundles one blueprint, so it is accurate for the default level only. See + section 6 of PRINTER-INSTALLER.md. +- The setup wizard still asks for a single site-wide blueprint and writes + settings that no longer drive the map. +- Map PDF export covers the current level only. + ## [0.10.0] - 2026-08-17 A security release. The Windows installer left the directories it creates diff --git a/README.md b/README.md index 38c26ef7..88d2d512 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,29 @@ example of the whole sequence. For every environment variable and Setting key see the CONFIG page in the project wiki. +### What the Windows installer requires + +The installer checks all of this before it changes anything and refuses rather +than half-installing. + +| | Minimum | Notes | +|---|---|---| +| Windows Server | **2019** (build 10.0.17763) | Ships **IIS 10.0**, which is therefore the earliest IIS supported. Server 2016 is also IIS 10.0 but falls below the build floor and is refused. | +| Windows client (test boxes) | **10 22H2** (build 10.0.19045) or 11 | **Pro or higher** - Home has no IIS at all. | +| Architecture | 64-bit | The payload is cp314 win_amd64. | +| IIS Web Server role | installed beforehand | `Install-WindowsFeature -Name Web-Server -IncludeManagementTools`. Needs no internet. | +| Free disk | 5 GB | Refused below this. 40 GB is comfortable once uploads and config backups accumulate. | +| Database | bundled MySQL 8.4 LTS, or your own | Bring credentials if you use an existing server. | + +The binding constraint is the operating system rather than IIS: the payload is +Python 3.14 plus HttpPlatformHandler and URL Rewrite, and the handler itself runs +on older IIS. An older host is untested rather than known-broken. + +If you must deploy onto something older, the INSTALL-WINDOWS-IIS page in the project wiki +is the manual procedure and has no OS gate - it even documents MySQL 5.6, which is +that era of machine. The trade is stated there: it produces a server the +installer will not subsequently upgrade. + ## Configuration Environment variables (`.env`): diff --git a/frontend/src/api/index.js b/frontend/src/api/index.js index d95d150c..831dcb59 100644 --- a/frontend/src/api/index.js +++ b/frontend/src/api/index.js @@ -863,6 +863,63 @@ export const setupApi = { } } +// Buildings and the levels within them (ADR-017). Reads are public - the +// printer installer map draws a blueprint before anyone logs in. +export const mapLevelsApi = { + list() { + return api.get('/maplevels') + }, + get(levelid) { + return api.get(`/maplevels/${levelid}`) + }, + createBuilding(payload) { + return api.post('/maplevels/buildings', payload) + }, + updateBuilding(buildingid, payload) { + return api.patch(`/maplevels/buildings/${buildingid}`, payload) + }, + create(payload) { + return api.post('/maplevels', payload) + }, + update(levelid, payload) { + return api.patch(`/maplevels/${levelid}`, payload) + }, + remove(levelid) { + return api.delete(`/maplevels/${levelid}`) + }, + // Returns the image's real pixel size alongside the stored dimensions. On an + // empty level the server adopts them; on a populated one it refuses and says + // so, because changing the coordinate space moves every marker on it. + uploadBlueprint(levelid, theme, file) { + const form = new FormData() + form.append('file', file) + form.append('theme', theme) + return api.post(`/maplevels/${levelid}/blueprint`, form, { + headers: { 'Content-Type': 'multipart/form-data' }, + }) + }, +} + +// Bulk marker positions: the landmark transform, bulk placement, review state +// and undo. Every write snapshots first. +export const mapPositionsApi = { + setPositions(positions, verified = true) { + return api.post('/mappositions/positions', { positions, verified }) + }, + transform(payload) { + return api.post('/mappositions/transform', payload) + }, + verify(assetids, unverify = false) { + return api.post('/mappositions/verify', { assetids, unverify }) + }, + snapshots() { + return api.get('/mappositions/snapshots') + }, + restore(snapshotid) { + return api.post(`/mappositions/snapshots/${snapshotid}/restore`) + }, +} + export const settingsApi = { list(params = {}) { return api.get('/settings', { params }) diff --git a/frontend/src/components/EmbeddedLocationMap.vue b/frontend/src/components/EmbeddedLocationMap.vue index 22180a69..59aef58e 100644 --- a/frontend/src/components/EmbeddedLocationMap.vue +++ b/frontend/src/components/EmbeddedLocationMap.vue @@ -1,141 +1,148 @@ - - - - - + + + + + diff --git a/frontend/src/components/LocationMapTooltip.vue b/frontend/src/components/LocationMapTooltip.vue index 4b527d15..d58fff81 100644 --- a/frontend/src/components/LocationMapTooltip.vue +++ b/frontend/src/components/LocationMapTooltip.vue @@ -12,7 +12,15 @@ @wheel.prevent="onWheel" >
-
+
+ Level unknown + + This asset has a position ({{ props.left }}, {{ props.top }}) but no + level, so there is no drawing to show it on. Set its level on the + asset, or place it in the map editor. + +
+
import { ref, computed, nextTick, watch } from 'vue' import { currentTheme } from '../stores/theme' -import { loadMapConfig, blueprintUrlFor, state as mapConfig } from '../composables/mapConfig' +import { loadMapConfig, blueprintUrlFor, dimensionsFor, hasLevel, levelName, state as mapConfig } from '../composables/mapConfig' // Fetch this facility's blueprint + dimensions once; computeds below react // when it loads. @@ -52,7 +60,11 @@ loadMapConfig() const props = defineProps({ left: { type: Number, default: null }, top: { type: Number, default: null }, - machineName: { type: String, default: '' } + machineName: { type: String, default: '' }, + // Which drawing left/top are pixels of (ADR-017). A position without one + // cannot be rendered: the same coordinates land somewhere different on every + // level, so this shows what is missing instead of guessing the default. + levelid: { type: Number, default: null }, }) const visible = ref(false) @@ -67,18 +79,29 @@ const hasPosition = computed(() => { return props.left !== null && props.top !== null }) -const blueprintUrl = computed(() => { - // Reading currentTheme keeps this reactive to theme changes. - return blueprintUrlFor(currentTheme.value) +// A position we cannot place: coordinates but no level, or a level this instance +// does not know. Rendering the default blueprint here would look correct and be +// wrong, so the tooltip says so instead. +const levelUnknown = computed(() => { + return hasPosition.value && !hasLevel(props.levelid) }) -// Calculate marker position as percentage of the facility blueprint size +const levelLabel = computed(() => levelName(props.levelid)) + +const blueprintUrl = computed(() => { + // Reading currentTheme keeps this reactive to theme changes. + return blueprintUrlFor(currentTheme.value, props.levelid) +}) + +// Marker position as a percentage of THIS LEVEL's native size. Percentages of +// the wrong level's dimensions is precisely how a marker ends up plausibly +// placed and wrong. const markerX = computed(() => { - return (props.left / mapConfig.width) * 100 + return (props.left / dimensionsFor(props.levelid).width) * 100 }) const markerY = computed(() => { - return (props.top / mapConfig.height) * 100 + return (props.top / dimensionsFor(props.levelid).height) * 100 }) // Marker style with counter-scale to maintain constant size @@ -197,6 +220,20 @@ watch(currentTheme, () => { .location-tooltip-wrapper:hover { color: var(--primary, #1976d2); } + +.map-level-unknown { + display: flex; + flex-direction: column; + gap: 0.35rem; + padding: 0.75rem; + max-width: 18rem; + font-size: 0.8rem; + color: var(--text-light); +} + +.map-level-unknown strong { + color: var(--warning); +} diff --git a/frontend/src/views/MapView.vue b/frontend/src/views/MapView.vue index 31059eef..f6b001d6 100644 --- a/frontend/src/views/MapView.vue +++ b/frontend/src/views/MapView.vue @@ -82,7 +82,7 @@ import ShopFloorMap from '../components/ShopFloorMap.vue' import { assetsApi } from '../api' import { currentTheme } from '../stores/theme' import { useAuthStore } from '../stores/auth' -import { loadMapConfig, blueprintUrlFor, state as mapConfig } from '../composables/mapConfig' +import { loadMapConfig, blueprintUrlFor, dimensionsFor, levelName, state as mapConfig } from '@/composables/mapConfig' import { exportMapPdf } from '../utils/mapPdf' import { assetTypeLabel, assetDetailRoute } from '../utils/assetTypes' import { getSubtypeId } from '../utils/mapColors' @@ -245,9 +245,12 @@ async function exportPdf() { // blueprintUrlFor applies withBase - the raw setting value is a // root-relative /api path, which 404s under a subpath mount like /ops. // The PDF always uses the light blueprint: it prints on white paper. - blueprintUrl: blueprintUrlFor('light'), - mapWidth: mapConfig.width, - mapHeight: mapConfig.height, + blueprintUrl: blueprintUrlFor('light', mapConfig.currentlevelid), + mapWidth: dimensionsFor(mapConfig.currentlevelid).width, + mapHeight: dimensionsFor(mapConfig.currentlevelid).height, + // Named on the sheet, because a floor plan with no level on it is not + // identifiable once it is printed and carried to the floor. + levelname: levelName(mapConfig.currentlevelid), selectedType: selectedType.value, subtypeColors: subtypeColorMap.value, subtypeNames: subtypeNameMap.value, diff --git a/frontend/src/views/SearchResults.vue b/frontend/src/views/SearchResults.vue index 3e7c5240..7842f029 100644 --- a/frontend/src/views/SearchResults.vue +++ b/frontend/src/views/SearchResults.vue @@ -1,479 +1,485 @@ - - - - - + + + + + diff --git a/frontend/src/views/settings/FloorMapSettings.vue b/frontend/src/views/settings/FloorMapSettings.vue index f7cd9ef6..a99e6077 100644 --- a/frontend/src/views/settings/FloorMapSettings.vue +++ b/frontend/src/views/settings/FloorMapSettings.vue @@ -1,101 +1,416 @@ + + diff --git a/migrations/versions/7d33_buildings_and_levels.py b/migrations/versions/7d33_buildings_and_levels.py new file mode 100644 index 00000000..f05a9648 --- /dev/null +++ b/migrations/versions/7d33_buildings_and_levels.py @@ -0,0 +1,213 @@ +"""Buildings and levels: a map is a drawing per level, not one image per site. + +See ADR-017. `map_blueprint_light`, `map_blueprint_dark`, `map_width` and +`map_height` described one image for the whole site, and `assets.mapx`/`mapy` +were pixels in it. A second level and a likely second building make that a +table. + +This migration is written so nothing renders differently the day it lands: the +four settings become one building and one level, marked default, and every asset +that has a position points at it. The settings rows are left in place here and +retired separately, so a rollback does not lose the blueprint paths. + +`levelid` is nullable because an asset with no position needs no level. A +position WITHOUT a level is the case the UI refuses to guess about, and after +this migration no such row exists. + +Revision ID: 7d33_buildings_and_levels +Revises: 7d32_displayrole_kiosk_vocabulary +""" +from alembic import op +import sqlalchemy as sa + + +revision = '7d33_buildings_and_levels' +down_revision = '7d32_displayrole_kiosk_vocabulary' +branch_labels = None +depends_on = None + +# What the settings said before this table existed. Read at upgrade time; these +# are only the fallbacks for a site that never set them. +DEFAULT_WIDTH = 3300 +DEFAULT_HEIGHT = 2550 +PLACEHOLDER = '/static/images/floorplan-placeholder.svg' + + +def _existing(insp, name): + return name in insp.get_table_names() + + +def upgrade(): + bind = op.get_bind() + insp = sa.inspect(bind) + + # Guarded like every other table-creating migration in this project: on a + # FRESH database the tables are built from the SQLAlchemy models, which + # already declare them, so an unconditional create fails. + if not _existing(insp, 'buildings'): + op.create_table( + 'buildings', + sa.Column('buildingid', sa.Integer, primary_key=True), + sa.Column('buildingname', sa.String(100), nullable=False, + unique=True), + sa.Column('sortorder', sa.Integer, nullable=False, + server_default='0'), + sa.Column('createddate', sa.DateTime, nullable=False), + sa.Column('modifieddate', sa.DateTime, nullable=False), + sa.Column('isactive', sa.Boolean, nullable=False, + server_default=sa.true()), + ) + + if not _existing(insp, 'maplevels'): + op.create_table( + 'maplevels', + sa.Column('levelid', sa.Integer, primary_key=True), + sa.Column('buildingid', sa.Integer, + sa.ForeignKey('buildings.buildingid'), nullable=False, + index=True), + sa.Column('levelname', sa.String(100), nullable=False), + sa.Column('sortorder', sa.Integer, nullable=False, + server_default='0'), + sa.Column('blueprintlight', sa.String(255), nullable=True), + sa.Column('blueprintdark', sa.String(255), nullable=True), + sa.Column('mapwidth', sa.Integer, nullable=False, + server_default=str(DEFAULT_WIDTH)), + sa.Column('mapheight', sa.Integer, nullable=False, + server_default=str(DEFAULT_HEIGHT)), + sa.Column('isdefault', sa.Boolean, nullable=False, + server_default=sa.false()), + sa.Column('createddate', sa.DateTime, nullable=False), + sa.Column('modifieddate', sa.DateTime, nullable=False), + sa.Column('isactive', sa.Boolean, nullable=False, + server_default=sa.true()), + sa.UniqueConstraint('buildingid', 'levelname', + name='uq_maplevel_building_name'), + ) + + # Positions have no history, and a bulk transform rewrites hundreds of them + # at once. A snapshot table is what makes that reversible - without it, the + # honest advice would be "back up the database first", which nobody does + # before a UI action. + if not _existing(insp, 'mappositionsnapshots'): + op.create_table( + 'mappositionsnapshots', + sa.Column('snapshotid', sa.Integer, primary_key=True), + sa.Column('levelid', sa.Integer, nullable=True), + sa.Column('reason', sa.String(255), nullable=True), + sa.Column('assetcount', sa.Integer, nullable=False, + server_default='0'), + # The positions themselves, as JSON: assetid, mapx, mapy, levelid, + # mapverifiedat per row. Deliberately not a child table - a snapshot + # is read back whole or not at all, and one row per snapshot keeps + # restore a single statement. + sa.Column('positionsjson', sa.Text, nullable=False), + sa.Column('restoredat', sa.DateTime, nullable=True), + sa.Column('createdby', sa.String(100), nullable=True), + sa.Column('createddate', sa.DateTime, nullable=False), + sa.Column('modifieddate', sa.DateTime, nullable=False), + sa.Column('isactive', sa.Boolean, nullable=False, + server_default=sa.true()), + ) + + assetcolumns = {c['name'] for c in insp.get_columns('assets')} + if 'levelid' not in assetcolumns: + op.add_column('assets', sa.Column('levelid', sa.Integer, nullable=True)) + op.create_index('idx_assets_levelid', 'assets', ['levelid']) + # The FK is added separately from the column so a site whose assets + # table is large is not rewritten twice. + op.create_foreign_key('fk_assets_levelid', 'assets', 'maplevels', + ['levelid'], ['levelid']) + if 'mapverifiedat' not in assetcolumns: + op.add_column('assets', + sa.Column('mapverifiedat', sa.DateTime, nullable=True)) + + # Locations carry map coordinates too - they are the default position for + # assets at that location - so they need a level for exactly the same + # reason. Missed on the first pass and caught by the payload gate. + locationcolumns = {c['name'] for c in insp.get_columns('locations')} + if 'levelid' not in locationcolumns: + op.add_column('locations', sa.Column('levelid', sa.Integer, nullable=True)) + op.create_index('idx_locations_levelid', 'locations', ['levelid']) + op.create_foreign_key('fk_locations_levelid', 'locations', 'maplevels', + ['levelid'], ['levelid']) + + # --- carry the settings forward ------------------------------------- + # Only when there is nothing here yet: re-running must not create a second + # default level, and a site that has already set its levels up must not have + # them joined by a stale one built from retired settings. + existinglevels = bind.execute( + sa.text('SELECT COUNT(*) FROM maplevels')).scalar() or 0 + if existinglevels: + return + + settings = dict(bind.execute(sa.text( + "SELECT `key`, value FROM settings WHERE `key` IN " + "('map_blueprint_light','map_blueprint_dark','map_width','map_height')" + )).fetchall()) + + def _int(value, fallback): + try: + number = int(str(value).strip()) + return number if number > 0 else fallback + except (TypeError, ValueError): + return fallback + + bind.execute(sa.text( + 'INSERT INTO buildings (buildingname, sortorder, createddate, ' + 'modifieddate, isactive) VALUES (:name, 0, :now, :now, :active)'), + {'name': 'Main', 'now': sa.func.now(), 'active': True}) + buildingid = bind.execute(sa.text( + 'SELECT buildingid FROM buildings WHERE buildingname = :name'), + {'name': 'Main'}).scalar() + + bind.execute(sa.text( + 'INSERT INTO maplevels (buildingid, levelname, sortorder, ' + 'blueprintlight, blueprintdark, mapwidth, mapheight, isdefault, ' + 'createddate, modifieddate, isactive) VALUES (:building, :name, 0, ' + ':light, :dark, :width, :height, :isdefault, :now, :now, :active)'), + {'building': buildingid, + 'name': 'Ground floor', + 'light': settings.get('map_blueprint_light') or PLACEHOLDER, + 'dark': settings.get('map_blueprint_dark') or PLACEHOLDER, + 'width': _int(settings.get('map_width'), DEFAULT_WIDTH), + 'height': _int(settings.get('map_height'), DEFAULT_HEIGHT), + 'isdefault': True, 'now': sa.func.now(), 'active': True}) + + levelid = bind.execute(sa.text( + 'SELECT levelid FROM maplevels WHERE isdefault = :flag'), + {'flag': True}).scalar() + + # Every asset that already has a position had it in this one drawing's + # coordinate space, so it belongs to this level. An asset with no position + # is left null: it needs no level until somebody places it. + bind.execute(sa.text( + 'UPDATE assets SET levelid = :levelid ' + 'WHERE mapx IS NOT NULL AND mapy IS NOT NULL AND levelid IS NULL'), + {'levelid': levelid}) + bind.execute(sa.text( + 'UPDATE locations SET levelid = :levelid ' + 'WHERE mapx IS NOT NULL AND mapy IS NOT NULL AND levelid IS NULL'), + {'levelid': levelid}) + + +def downgrade(): + bind = op.get_bind() + insp = sa.inspect(bind) + assetcolumns = {c['name'] for c in insp.get_columns('assets')} + if 'levelid' in assetcolumns: + op.drop_constraint('fk_assets_levelid', 'assets', type_='foreignkey') + op.drop_index('idx_assets_levelid', table_name='assets') + op.drop_column('assets', 'levelid') + if 'mapverifiedat' in assetcolumns: + op.drop_column('assets', 'mapverifiedat') + locationcolumns = {c['name'] for c in insp.get_columns('locations')} + if 'levelid' in locationcolumns: + op.drop_constraint('fk_locations_levelid', 'locations', type_='foreignkey') + op.drop_index('idx_locations_levelid', table_name='locations') + op.drop_column('locations', 'levelid') + if _existing(insp, 'mappositionsnapshots'): + op.drop_table('mappositionsnapshots') + if _existing(insp, 'maplevels'): + op.drop_table('maplevels') + if _existing(insp, 'buildings'): + op.drop_table('buildings') diff --git a/plugins/computers/api/routes.py b/plugins/computers/api/routes.py index 1351d764..90315174 100644 --- a/plugins/computers/api/routes.py +++ b/plugins/computers/api/routes.py @@ -560,6 +560,7 @@ def create_computer(): locationid=data.get('locationid'), businessunitid=data.get('businessunitid'), mapx=data.get('mapx'), + levelid=data.get('levelid'), mapy=data.get('mapy'), notes=data.get('notes') ) @@ -658,7 +659,7 @@ def update_computer(computer_id: int): # Update asset fields asset_fields = ['assetnumber', 'name', 'serialnumber', 'gaugelabreference', 'maintenancereference', 'statusid', - 'locationid', 'businessunitid', 'mapx', 'mapy', 'notes', 'isactive'] + 'locationid', 'businessunitid', 'mapx', 'mapy', 'levelid', 'notes', 'isactive'] for key in asset_fields: if key in data: old_val = getattr(asset, key) diff --git a/plugins/computers/frontend/views/PCForm.vue b/plugins/computers/frontend/views/PCForm.vue index b4cf50dc..408dfbdd 100644 --- a/plugins/computers/frontend/views/PCForm.vue +++ b/plugins/computers/frontend/views/PCForm.vue @@ -261,9 +261,11 @@
Position: {{ form.mapx }}, {{ form.mapy }} + on {{ levelName(form.levelid) }} + level not set
-
@@ -272,8 +274,19 @@
+
+ + + + The position is pixels on this drawing, so pick the level first. + +
{ notes: pc.notes || '', mapx: pc.mapx ?? null, mapy: pc.mapy ?? null, + levelid: pc.levelid ?? null, ipaddress: primaryComm?.ipaddress || '' } } @@ -474,10 +495,21 @@ function handlePositionPicked(position) { tempMapPosition.value = position } +function openMapPicker() { + loadMapConfig().then(() => { + pickerLevelId.value = form.value.levelid || mapConfig.defaultlevelid + showMapPicker.value = true + }) +} + function confirmMapPosition() { if (tempMapPosition.value) { form.value.mapx = tempMapPosition.value.left form.value.mapy = tempMapPosition.value.top + // Never one without the other: coordinates saved with no level render as + // "level unknown", and coordinates saved against the wrong level render + // convincingly in the wrong place. + form.value.levelid = pickerLevelId.value } showMapPicker.value = false } @@ -485,6 +517,7 @@ function confirmMapPosition() { function clearMapPosition() { form.value.mapx = null form.value.mapy = null + form.value.levelid = null tempMapPosition.value = null } diff --git a/plugins/computers/manifest.json b/plugins/computers/manifest.json index c0ce3513..7d4038de 100644 --- a/plugins/computers/manifest.json +++ b/plugins/computers/manifest.json @@ -1,23 +1,23 @@ -{ - "name": "computers", - "version": "1.0.0", - "description": "Computer management plugin for PCs, servers, and workstations with software tracking", - "author": "ShopDB Team", - "dependencies": [], - "core_version": ">=0.1.0,<1.0.0", - "api_prefix": "/api/computers", - "provides": { - "asset_type": "computer", - "features": [ - "computer_tracking", - "software_inventory", - "remote_access", - "os_management" - ] - }, - "settings": { - "enable_winrm": true, - "enable_vnc": true, - "auto_report_interval_hours": 24 - } -} +{ + "name": "computers", + "version": "1.0.0", + "description": "Computer management plugin for PCs, servers, and workstations with software tracking", + "author": "ShopDB Team", + "dependencies": [], + "core_version": ">=0.20.0,<1.0.0", + "api_prefix": "/api/computers", + "provides": { + "asset_type": "computer", + "features": [ + "computer_tracking", + "software_inventory", + "remote_access", + "os_management" + ] + }, + "settings": { + "enable_winrm": true, + "enable_vnc": true, + "auto_report_interval_hours": 24 + } +} diff --git a/plugins/geenforce/api/routes.py b/plugins/geenforce/api/routes.py index 2d2bcd7a..cfae51de 100644 --- a/plugins/geenforce/api/routes.py +++ b/plugins/geenforce/api/routes.py @@ -960,6 +960,7 @@ def _asset_facts(hostnames): 'assetnumber': asset.assetnumber, 'location': (asset.location.locationname if asset.location else None), 'mapx': asset.mapx, + 'levelid': asset.levelid, 'mapy': asset.mapy, 'machinenumber': None, 'machineassetid': None, 'machinepluginid': None, diff --git a/plugins/knowledgebase/api/routes.py b/plugins/knowledgebase/api/routes.py index 16c95d1f..bae722bb 100644 --- a/plugins/knowledgebase/api/routes.py +++ b/plugins/knowledgebase/api/routes.py @@ -21,13 +21,35 @@ from shopdb.api import require_permission, apply_import_timestamps knowledgebase_bp = Blueprint('knowledgebase', __name__) +def _visible_articles(): + """Active articles whose topic is not a retired application. + + An article about a decommissioned application is not something anyone should + find by browsing or searching: it describes a thing that is no longer in + service, and presenting it alongside live documentation reads as though it + were current. + + An article with NO topic still shows. Not every article is about an + application, and a null topic is not a retired one. + + Expressed as a subquery rather than a join because the topic sort below joins + Application itself, and two joins onto the same table in one query collide. + """ + retired = db.session.query(Application.appid).filter( + Application.isactive.is_(False)) + return KnowledgeBase.query.filter( + KnowledgeBase.isactive.is_(True), + db.or_(KnowledgeBase.appid.is_(None), + KnowledgeBase.appid.notin_(retired))) + + @knowledgebase_bp.route('', methods=['GET']) @jwt_required(optional=True) def list_articles(): """List all knowledge base articles.""" page, per_page = get_pagination_params(request) - query = KnowledgeBase.query.filter_by(isactive=True) + query = _visible_articles() # Search: title, keywords, and the topic (its Application's name). The topic # is matched via an appid subquery instead of a join so it does not collide @@ -35,8 +57,13 @@ def list_articles(): # clause and still match on title/keywords. if search := request.args.get('search'): like = f'%{search}%' + # Active applications only. A retired application is not a topic anyone + # should be offered: matching its name surfaced its articles and printed + # the retired app as their subject, which reads as though it were still + # in service. topic_appids = db.session.query(Application.appid).filter( - Application.appname.ilike(like)) + Application.appname.ilike(like), + Application.isactive.is_(True)) query = query.filter( db.or_( KnowledgeBase.shortdescription.ilike(like), @@ -100,11 +127,11 @@ def list_articles(): @jwt_required(optional=True) def get_stats(): """Get knowledge base statistics.""" - total_clicks = db.session.query( - db.func.coalesce(db.func.sum(KnowledgeBase.clicks), 0) - ).filter(KnowledgeBase.isactive == True).scalar() - - total_articles = KnowledgeBase.query.filter_by(isactive=True).count() + # Counted over the same set the list shows. A total that includes articles + # nobody can see is a total nobody can reconcile. + visible = _visible_articles() + total_clicks = sum(article.clicks or 0 for article in visible) + total_articles = visible.count() return success_response({ 'totalclicks': int(total_clicks), diff --git a/plugins/knowledgebase/frontend/views/KnowledgeBaseForm.vue b/plugins/knowledgebase/frontend/views/KnowledgeBaseForm.vue index 50f01e6a..8e155864 100644 --- a/plugins/knowledgebase/frontend/views/KnowledgeBaseForm.vue +++ b/plugins/knowledgebase/frontend/views/KnowledgeBaseForm.vue @@ -106,7 +106,9 @@ const applications = ref([]) onMounted(async () => { try { // Load applications for topic dropdown - const appsRes = await applicationsApi.list({ perpage: 1000 }) + const appsRes = await applicationsApi.list({ perpage: 1000, showhidden: true }) // isactive is the only filter that applies to a topic: + // ishidden governs whether an application shows on the tiles page, which + // says nothing about whether it can be the subject of an article. applications.value = appsRes.data.data || [] // Load article if editing diff --git a/plugins/knowledgebase/frontend/views/KnowledgeBaseList.vue b/plugins/knowledgebase/frontend/views/KnowledgeBaseList.vue index b37ad652..700576af 100644 --- a/plugins/knowledgebase/frontend/views/KnowledgeBaseList.vue +++ b/plugins/knowledgebase/frontend/views/KnowledgeBaseList.vue @@ -171,7 +171,9 @@ async function loadArticles() { async function loadTopics() { try { - const response = await applicationsApi.list({ perpage: 1000 }) + const response = await applicationsApi.list({ perpage: 1000, showhidden: true }) // isactive is the only filter that applies to a topic: + // ishidden governs whether an application shows on the tiles page, which + // says nothing about whether it can be the subject of an article. topics.value = response.data.data || [] } catch (error) { console.error('Error loading topics:', error) diff --git a/plugins/machines/api/routes.py b/plugins/machines/api/routes.py index c6040dd1..a3a1dda8 100644 --- a/plugins/machines/api/routes.py +++ b/plugins/machines/api/routes.py @@ -371,6 +371,7 @@ def create_machine(): locationid=data.get('locationid'), businessunitid=data.get('businessunitid'), mapx=data.get('mapx'), + levelid=data.get('levelid'), mapy=data.get('mapy'), notes=data.get('notes') ) @@ -446,7 +447,7 @@ def update_machine(machine_id: int): # Update asset fields asset_fields = ['assetnumber', 'name', 'gaugelabreference', 'maintenancereference', 'serialnumber', 'statusid', - 'locationid', 'businessunitid', 'mapx', 'mapy', + 'locationid', 'businessunitid', 'mapx', 'mapy', 'levelid', 'notes', 'isactive'] for key in asset_fields: if key in data: diff --git a/plugins/machines/frontend/views/MachineForm.vue b/plugins/machines/frontend/views/MachineForm.vue index dde53cdd..3d499ed0 100644 --- a/plugins/machines/frontend/views/MachineForm.vue +++ b/plugins/machines/frontend/views/MachineForm.vue @@ -239,9 +239,11 @@
Position: {{ form.mapx }}, {{ form.mapy }} + on {{ levelName(form.levelid) }} + level not set
-
@@ -250,8 +252,19 @@
+
+ + + + The position is pixels on this drawing, so pick the level first. + +
{ islocationonly: data.machine?.islocationonly || false, notes: data.notes || '', mapx: data.mapx ?? null, - mapy: data.mapy ?? null + mapy: data.mapy ?? null, + levelid: data.levelid ?? null, } // Load existing relationships to find controlling PC @@ -535,10 +556,21 @@ function handlePositionPicked(position) { tempMapPosition.value = position } +function openMapPicker() { + loadMapConfig().then(() => { + pickerLevelId.value = form.value.levelid || mapConfig.defaultlevelid + showMapPicker.value = true + }) +} + function confirmMapPosition() { if (tempMapPosition.value) { form.value.mapx = tempMapPosition.value.left form.value.mapy = tempMapPosition.value.top + // Never one without the other: coordinates saved with no level render as + // "level unknown", and coordinates saved against the wrong level render + // convincingly in the wrong place. + form.value.levelid = pickerLevelId.value } showMapPicker.value = false } @@ -546,6 +578,7 @@ function confirmMapPosition() { function clearMapPosition() { form.value.mapx = null form.value.mapy = null + form.value.levelid = null tempMapPosition.value = null } diff --git a/plugins/machines/manifest.json b/plugins/machines/manifest.json index 58542b34..9252e458 100644 --- a/plugins/machines/manifest.json +++ b/plugins/machines/manifest.json @@ -4,7 +4,7 @@ "description": "Machine management plugin for CNCs, CMMs, lathes, grinders, and other manufacturing machines", "author": "ShopDB Team", "dependencies": [], - "core_version": ">=0.1.0,<1.0.0", + "core_version": ">=0.20.0,<1.0.0", "api_prefix": "/api/machines", "provides": { "asset_type": "machine", diff --git a/plugins/measuringtools/api/routes.py b/plugins/measuringtools/api/routes.py index ab69583e..77395769 100644 --- a/plugins/measuringtools/api/routes.py +++ b/plugins/measuringtools/api/routes.py @@ -263,6 +263,7 @@ def create_tool(): locationid=data.get('locationid'), businessunitid=data.get('businessunitid'), mapx=data.get('mapx'), + levelid=data.get('levelid'), mapy=data.get('mapy'), notes=data.get('notes'), ) @@ -291,7 +292,7 @@ def create_tool(): # Asset core fields writable through this plugin's write path. _ASSET_FIELDS = ('assetnumber', 'name', 'gaugelabreference', 'maintenancereference', 'serialnumber', - 'statusid', 'locationid', 'businessunitid', 'mapx', 'mapy', + 'statusid', 'locationid', 'businessunitid', 'mapx', 'mapy', 'levelid', 'notes', 'isactive') # Extension fields with plain assignment (dates handled separately). _TOOL_FIELDS = ('measuringtooltypeid', 'calibrationintervaldays', diff --git a/plugins/measuringtools/manifest.json b/plugins/measuringtools/manifest.json index f61c03e3..f2f584af 100644 --- a/plugins/measuringtools/manifest.json +++ b/plugins/measuringtools/manifest.json @@ -5,7 +5,7 @@ "description": "Metrology and inspection instruments (gauges, calipers, thread gages, bore gages) with a calibration lifecycle and derived calibration status.", "author": "ShopDB Team", "dependencies": [], - "core_version": ">=0.6.0,<1.0.0", + "core_version": ">=0.20.0,<1.0.0", "api_prefix": "/api/measuringtools", "default_enabled": false, "provides": { diff --git a/plugins/network/api/routes.py b/plugins/network/api/routes.py index d7a7eae5..9e1b80d6 100644 --- a/plugins/network/api/routes.py +++ b/plugins/network/api/routes.py @@ -412,6 +412,7 @@ def create_network_device(): locationid=data.get('locationid'), businessunitid=data.get('businessunitid'), mapx=data.get('mapx'), + levelid=data.get('levelid'), mapy=data.get('mapy'), notes=data.get('notes') ) @@ -502,7 +503,7 @@ def update_network_device(device_id: int): # Update asset fields asset_fields = ['assetnumber', 'name', 'serialnumber', 'gaugelabreference', 'maintenancereference', 'statusid', - 'locationid', 'businessunitid', 'mapx', 'mapy', 'notes', 'isactive'] + 'locationid', 'businessunitid', 'mapx', 'mapy', 'levelid', 'notes', 'isactive'] for key in asset_fields: if key in data: old_val = getattr(asset, key) diff --git a/plugins/network/frontend/views/NetworkDeviceForm.vue b/plugins/network/frontend/views/NetworkDeviceForm.vue index e4749547..d9bccd1c 100644 --- a/plugins/network/frontend/views/NetworkDeviceForm.vue +++ b/plugins/network/frontend/views/NetworkDeviceForm.vue @@ -234,9 +234,11 @@
Position: {{ form.mapx }}, {{ form.mapy }} + on {{ levelName(form.levelid) }} + level not set
-
@@ -246,8 +248,19 @@
+
+ + + + The position is pixels on this drawing, so pick the level first. + +
{ + pickerLevelId.value = form.value.levelid || mapConfig.defaultlevelid + showMapPicker.value = true + }) +} + function confirmMapPosition() { if (tempMapPosition.value) { form.value.mapx = tempMapPosition.value.left form.value.mapy = tempMapPosition.value.top + // Never one without the other: coordinates saved with no level render as + // "level unknown", and coordinates saved against the wrong level render + // convincingly in the wrong place. + form.value.levelid = pickerLevelId.value } showMapPicker.value = false } @@ -561,6 +594,7 @@ function confirmMapPosition() { function clearMapPosition() { form.value.mapx = null form.value.mapy = null + form.value.levelid = null tempMapPosition.value = null } diff --git a/plugins/network/manifest.json b/plugins/network/manifest.json index fa56c2b9..bb477dc5 100644 --- a/plugins/network/manifest.json +++ b/plugins/network/manifest.json @@ -1,22 +1,22 @@ -{ - "name": "network", - "version": "1.0.0", - "description": "Network device management plugin for switches, APs, cameras, and IDFs", - "author": "ShopDB Team", - "dependencies": [], - "core_version": ">=0.1.0,<1.0.0", - "api_prefix": "/api/network", - "provides": { - "asset_type": "network_device", - "features": [ - "network_device_tracking", - "port_management", - "firmware_tracking", - "poe_monitoring" - ] - }, - "settings": { - "enable_snmp_polling": false, - "snmp_community": "public" - } -} +{ + "name": "network", + "version": "1.0.0", + "description": "Network device management plugin for switches, APs, cameras, and IDFs", + "author": "ShopDB Team", + "dependencies": [], + "core_version": ">=0.20.0,<1.0.0", + "api_prefix": "/api/network", + "provides": { + "asset_type": "network_device", + "features": [ + "network_device_tracking", + "port_management", + "firmware_tracking", + "poe_monitoring" + ] + }, + "settings": { + "enable_snmp_polling": false, + "snmp_community": "public" + } +} diff --git a/plugins/printers/api/asset_routes.py b/plugins/printers/api/asset_routes.py index d8046585..12275b8b 100644 --- a/plugins/printers/api/asset_routes.py +++ b/plugins/printers/api/asset_routes.py @@ -358,6 +358,7 @@ def printer_install_list(): 'iscsf': printer.iscsf, 'locationname': asset.location.locationname if asset.location else None, 'mapx': asset.mapx, + 'levelid': asset.levelid, 'mapy': asset.mapy, }) @@ -366,7 +367,7 @@ def printer_install_list(): # hand-rolled JSON parser. The web map uses the default JSON. if request.args.get('format') == 'text': fields = ('printerid', 'windowsname', 'vendorname', 'modelnumber', - 'hostname', 'ipaddress', 'mapx', 'mapy') + 'hostname', 'ipaddress', 'mapx', 'mapy', 'levelid') lines = [_text_line(row, fields) for row in rows] return Response('\n'.join(lines), mimetype='text/plain') @@ -738,6 +739,7 @@ def create_printer(): locationid=data.get('locationid'), businessunitid=data.get('businessunitid'), mapx=data.get('mapx'), + levelid=data.get('levelid'), mapy=data.get('mapy'), notes=data.get('notes') ) @@ -819,7 +821,7 @@ def update_printer(printer_id: int): # Update asset fields (optional identifiers gated per-type in Settings) asset_fields = ['assetnumber', 'name', 'serialnumber', 'gaugelabreference', 'maintenancereference', 'statusid', - 'locationid', 'businessunitid', 'mapx', 'mapy', + 'locationid', 'businessunitid', 'mapx', 'mapy', 'levelid', 'notes', 'isactive'] for key in asset_fields: if key in data: @@ -1035,6 +1037,7 @@ def _get_low_supplies_data(): 'model': model_number, 'location': location_name, 'mapx': asset.mapx, + 'levelid': asset.levelid, 'mapy': asset.mapy, 'supplies': annotated }) diff --git a/plugins/printers/frontend/views/PrinterForm.vue b/plugins/printers/frontend/views/PrinterForm.vue index 75a1edc6..40eca7a7 100644 --- a/plugins/printers/frontend/views/PrinterForm.vue +++ b/plugins/printers/frontend/views/PrinterForm.vue @@ -250,9 +250,11 @@
Position: {{ form.mapx }}, {{ form.mapy }} + on {{ levelName(form.levelid) }} + level not set
-
@@ -261,8 +263,19 @@
+
+ + + + The position is pixels on this drawing, so pick the level first. + +
{ notes: printer.notes || '', mapx: printer.mapx ?? null, mapy: printer.mapy ?? null, + levelid: printer.levelid ?? null, // Printer-specific ipaddress: primaryComm?.ipaddress || '', csfname: ext.sharename || '', @@ -534,10 +555,21 @@ function handlePositionPicked(position) { tempMapPosition.value = position } +function openMapPicker() { + loadMapConfig().then(() => { + pickerLevelId.value = form.value.levelid || mapConfig.defaultlevelid + showMapPicker.value = true + }) +} + function confirmMapPosition() { if (tempMapPosition.value) { form.value.mapx = tempMapPosition.value.left form.value.mapy = tempMapPosition.value.top + // Never one without the other: coordinates saved with no level render as + // "level unknown", and coordinates saved against the wrong level render + // convincingly in the wrong place. + form.value.levelid = pickerLevelId.value } showMapPicker.value = false } @@ -545,6 +577,7 @@ function confirmMapPosition() { function clearMapPosition() { form.value.mapx = null form.value.mapy = null + form.value.levelid = null tempMapPosition.value = null } diff --git a/plugins/printers/frontend/views/PrinterInstallerMap.vue b/plugins/printers/frontend/views/PrinterInstallerMap.vue index 775bda2e..eff870a3 100644 --- a/plugins/printers/frontend/views/PrinterInstallerMap.vue +++ b/plugins/printers/frontend/views/PrinterInstallerMap.vue @@ -43,7 +43,7 @@ import { ref, computed, onMounted, onUnmounted, watch } from 'vue' import L from 'leaflet' import 'leaflet/dist/leaflet.css' -import { loadMapConfig, blueprintUrlFor, state as mapConfig } from '@/composables/mapConfig' +import { loadMapConfig, blueprintUrlFor, dimensionsFor, state as mapConfig } from '@/composables/mapConfig' import { printersApi } from '@/api' import { withBase } from '@/utils/basePath' import { currentTheme } from '@/stores/theme' @@ -55,8 +55,10 @@ const selected = ref({}) // printerid -> true let map = null let imageOverlay = null let markers = {} // printerid -> circleMarker -let MAP_WIDTH = mapConfig.width -let MAP_HEIGHT = mapConfig.height +// The level being shown. The installer map runs before anyone logs in, which is +// why /api/maplevels is public - without it there is no blueprint to draw. +let MAP_WIDTH = 0 +let MAP_HEIGHT = 0 const SELECTED_COLOR = '#e53935' const NORMAL_COLOR = '#4CAF50' @@ -120,13 +122,13 @@ function renderMarkers() { } watch(currentTheme, (theme) => { - if (imageOverlay) imageOverlay.setUrl(blueprintUrlFor(theme)) + if (imageOverlay) imageOverlay.setUrl(blueprintUrlFor(theme, mapConfig.currentlevelid)) }) onMounted(async () => { await loadMapConfig() - MAP_WIDTH = mapConfig.width - MAP_HEIGHT = mapConfig.height + MAP_WIDTH = dimensionsFor(mapConfig.currentlevelid).width + MAP_HEIGHT = dimensionsFor(mapConfig.currentlevelid).height try { const response = await printersApi.installList() @@ -142,7 +144,7 @@ onMounted(async () => { attributionControl: false, }) const bounds = [[0, 0], [MAP_HEIGHT, MAP_WIDTH]] - imageOverlay = L.imageOverlay(blueprintUrlFor(currentTheme.value), bounds).addTo(map) + imageOverlay = L.imageOverlay(blueprintUrlFor(currentTheme.value, mapConfig.currentlevelid), bounds).addTo(map) map.setView([MAP_HEIGHT / 2, MAP_WIDTH / 2], -2) map.setMaxBounds(bounds) renderMarkers() diff --git a/plugins/printers/manifest.json b/plugins/printers/manifest.json index 9d45007b..75ae7812 100644 --- a/plugins/printers/manifest.json +++ b/plugins/printers/manifest.json @@ -4,7 +4,7 @@ "description": "Printer management plugin with Zabbix integration, supply tracking, and QR codes", "author": "ShopDB Team", "dependencies": [], - "core_version": ">=0.16.0,<1.0.0", + "core_version": ">=0.20.0,<1.0.0", "api_prefix": "/api/printers", "provides": { "machine_category": "Printer", diff --git a/plugins/warranty/api/routes.py b/plugins/warranty/api/routes.py index dd8ef34c..ec71eadc 100644 --- a/plugins/warranty/api/routes.py +++ b/plugins/warranty/api/routes.py @@ -88,6 +88,7 @@ def _related_machine(asset): # worth showing, so the caller decides what to do with a missing # position rather than the row being dropped. 'mapx': candidate.mapx, + 'levelid': candidate.levelid, 'mapy': candidate.mapy, 'locationid': candidate.locationid, 'locationname': (candidate.location.locationname diff --git a/scripts/check-naming-and-style.sh b/scripts/check-naming-and-style.sh index 123e28ff..3b8804d2 100755 --- a/scripts/check-naming-and-style.sh +++ b/scripts/check-naming-and-style.sh @@ -158,6 +158,32 @@ if [ -n "$CODE_LIB_IMPORTS" ]; then VIOLATIONS=$((VIOLATIONS + 1)) fi +# ADR-017: a marker position is pixels in ONE LEVEL's coordinate space, so a +# payload that emits mapx/mapy without levelid gives the consumer coordinates and +# no drawing to put them on. The consumer then either renders nothing or - worse, +# and what a reasonable implementation does - falls back to the default level, +# drawing one building's ground floor behind a marker positioned for another +# building's mezzanine. That renders perfectly and points at the wrong place. +# +# Eight files emit positions across twenty sites. This is the check that says +# whether all of them travel with their level, because reading them by eye is +# how the twentieth gets missed. +echo "==> Checking that emitted map positions carry their level (ADR-017)..." +POSITION_FILES=$(grep -rln "'mapx':" --include='*.py' shopdb/ plugins/ 2>/dev/null \ + | grep -v '/tests\?/' || true) +MISSING_LEVEL="" +for candidate in $POSITION_FILES; do + if ! grep -q "'levelid'" "$candidate"; then + MISSING_LEVEL="$MISSING_LEVEL$candidate"$'\n' + fi +done +if [ -n "$MISSING_LEVEL" ]; then + echo "FAIL: these emit 'mapx' but never 'levelid' - a position with no level" + echo " cannot be rendered on the right drawing:" + echo "$MISSING_LEVEL" | sed 's/^/ /' + VIOLATIONS=$((VIOLATIONS + 1)) +fi + # ENFORCING. It was report-only while the backlog was worked off, and the hit # count then did not move for weeks - a rule that only prints is read as no rule. # Set SITE_LITERALS_ENFORCE=0 to drop back to reporting for a local run. diff --git a/shopdb/__init__.py b/shopdb/__init__.py index 10636fe2..28a39adc 100644 --- a/shopdb/__init__.py +++ b/shopdb/__init__.py @@ -57,7 +57,7 @@ from .plugins import plugin_manager # permission / empty / position. BREAKING for any plugin still using the old # shape, which is why it is recorded here: the change itself shipped earlier # without a bump, and a contract that changes silently is not a contract. -__contract_version__ = '0.19.0' +__contract_version__ = '0.20.0' # Product release version (see ADR-007). The product version and the # plugin-contract version above are distinct series with independent @@ -190,6 +190,8 @@ CORE_BLUEPRINT_NAMES = ( 'models', 'businessunits', 'locations', + 'maplevels', + 'mappositions', 'operatingsystems', 'dashboard', 'dashboarddefaults', diff --git a/shopdb/api/__init__.py b/shopdb/api/__init__.py index a196f286..5a11f324 100644 --- a/shopdb/api/__init__.py +++ b/shopdb/api/__init__.py @@ -196,7 +196,10 @@ def _walk_related_for_position(asset, visited, depth): n_mapx = getattr(neighbor, 'mapx', None) n_mapy = getattr(neighbor, 'mapy', None) if n_mapx is not None and n_mapy is not None: - return (n_mapx, n_mapy) + # The neighbour's LEVEL travels with its coordinates. Returning the + # pair alone would leave the caller drawing a machine's position on + # whatever level the PC that borrowed it happens to claim. + return (n_mapx, n_mapy, getattr(neighbor, 'levelid', None)) recursed = _walk_related_for_position(neighbor, visited, depth + 1) if recursed is not None: @@ -214,18 +217,28 @@ def resolve_asset_position(asset) -> Optional[Dict[str, Any]]: 3. Asset's location coords (asset.location.mapx, .mapy) 4. None (asset is unplaced, rendered in a tray) - Returns a dict {'mapx', 'mapy', 'positionsource'} where positionsource - is one of 'self', 'related', 'location'. Returns None when no priority - yields coordinates. + Returns a dict {'mapx', 'mapy', 'levelid', 'positionsource'} where + positionsource is one of 'self', 'related', 'location'. Returns None when no + priority yields coordinates. + + LEVELID COMES FROM WHICHEVER SOURCE SUPPLIED THE COORDINATES, not from the + asset (ADR-017). A PC with no position of its own that inherits from the + machine it controls is at the MACHINE's coordinates on the MACHINE's level; + using the PC's own level - which may be null, or may be a different building + entirely - would draw those coordinates on the wrong drawing, and the result + looks entirely reasonable. """ mapx = getattr(asset, 'mapx', None) mapy = getattr(asset, 'mapy', None) if mapx is not None and mapy is not None: - return {'mapx': mapx, 'mapy': mapy, 'positionsource': 'self'} + return {'mapx': mapx, 'mapy': mapy, + 'levelid': getattr(asset, 'levelid', None), + 'positionsource': 'self'} related = _walk_related_for_position(asset, set(), 0) if related is not None: - return {'mapx': related[0], 'mapy': related[1], 'positionsource': 'related'} + return {'mapx': related[0], 'mapy': related[1], 'levelid': related[2], + 'positionsource': 'related'} location = getattr(asset, 'location', None) if location is not None: @@ -235,6 +248,7 @@ def resolve_asset_position(asset) -> Optional[Dict[str, Any]]: return { 'mapx': loc_mapx, 'mapy': loc_mapy, + 'levelid': getattr(location, 'levelid', None), 'positionsource': 'location', } diff --git a/shopdb/core/api/__init__.py b/shopdb/core/api/__init__.py index 8fc8057f..633aef9d 100644 --- a/shopdb/core/api/__init__.py +++ b/shopdb/core/api/__init__.py @@ -8,6 +8,8 @@ from .vendors import vendors_bp from .models import models_bp from .businessunits import businessunits_bp from .locations import locations_bp +from .maplevels import maplevels_bp +from .mappositions import mappositions_bp from .operatingsystems import operatingsystems_bp from .dashboard import dashboard_bp from .dashboarddefaults import dashboarddefaults_bp @@ -34,6 +36,8 @@ __all__ = [ 'models_bp', 'businessunits_bp', 'locations_bp', + 'maplevels_bp', + 'mappositions_bp', 'operatingsystems_bp', 'dashboard_bp', 'dashboarddefaults_bp', diff --git a/shopdb/core/api/assets.py b/shopdb/core/api/assets.py index c958e9e5..1a0b0f9f 100644 --- a/shopdb/core/api/assets.py +++ b/shopdb/core/api/assets.py @@ -476,6 +476,7 @@ def create_asset(): locationid=data.get('locationid'), businessunitid=data.get('businessunitid'), mapx=data.get('mapx'), + levelid=data.get('levelid'), mapy=data.get('mapy'), notes=data.get('notes') ) @@ -517,7 +518,7 @@ def update_asset(asset_id: int): # Update allowed fields allowed_fields = [ 'assetnumber', 'name', 'serialnumber', 'assettypeid', 'statusid', - 'locationid', 'businessunitid', 'mapx', 'mapy', 'notes', 'isactive' + 'locationid', 'businessunitid', 'mapx', 'mapy', 'levelid', 'notes', 'isactive' ] for key in allowed_fields: @@ -1114,6 +1115,7 @@ def get_assets_map(): 'displayname': asset.display_name, 'serialnumber': asset.serialnumber, 'mapx': asset.mapx, + 'levelid': asset.levelid, 'mapy': asset.mapy, 'assettype': asset.assettype.assettype if asset.assettype else None, 'assettypeid': asset.assettypeid, diff --git a/shopdb/core/api/maplevels.py b/shopdb/core/api/maplevels.py new file mode 100644 index 00000000..061732ee --- /dev/null +++ b/shopdb/core/api/maplevels.py @@ -0,0 +1,350 @@ +"""Buildings and levels: the drawings a marker can be placed on (ADR-017). + +Reads are UNAUTHENTICATED. The printer installer map runs before anyone logs in +and needs a blueprint to draw, exactly as the printer install-list and the slide +feed already do. A level name, a blueprint path and a pixel size are not +secrets - the marker positions drawn on them already render on kiosk pages. + +Writes require admin: adding a level changes where every marker on it appears. +""" + +import os + +from flask import Blueprint, request, current_app +from flask_jwt_extended import jwt_required +from werkzeug.utils import secure_filename + +from shopdb.extensions import db +from shopdb.core.models import Building, MapLevel, Asset, AuditLog +from shopdb.utils.responses import success_response, error_response, ErrorCodes +from shopdb.utils.authz import require_role +from shopdb.api import send_upload +from shopdb.utils.imagesize import image_size + +maplevels_bp = Blueprint('maplevels', __name__) + +# Same set the map blueprint upload already accepts. SVG stays allowed because a +# floor plan is vector by nature; it is served through send_upload, which sends +# the sandbox headers that stop one executing as script. +BLUEPRINT_EXTENSIONS = {'.png', '.jpg', '.jpeg', '.gif', '.webp', '.svg'} + + +def _blueprint_dir(): + return os.path.join(current_app.instance_path, 'maps') + + +# ============================================================================= +# Read - public +# ============================================================================= + +@maplevels_bp.route('', methods=['GET']) +def list_levels(): + """Every active level, grouped by building, in display order. + + One call, because every consumer needs the whole list: the map switches + between levels, and a hover preview has to resolve an arbitrary asset's + level to a blueprint without knowing in advance which one it is. + """ + buildings = (Building.query.filter_by(isactive=True) + .order_by(Building.sortorder, Building.buildingid).all()) + + # How many markers sit on each level, in one grouped query rather than one + # per level. The admin page needs it to say what a rename or a resize is + # about to affect, and it is what makes the delete refusal predictable + # instead of a surprise. + counts = dict(db.session.query(Asset.levelid, db.func.count(Asset.assetid)) + .filter(Asset.levelid.isnot(None), Asset.isactive.is_(True)) + .group_by(Asset.levelid).all()) + + payload = [] + for building in buildings: + entry = building.to_dict() + for level in entry.get('levels', []): + level['assetcount'] = counts.get(level['levelid'], 0) + payload.append(entry) + + default = MapLevel.default_level() + return success_response({ + 'buildings': payload, + 'defaultlevelid': default.levelid if default else None, + 'totalplaced': sum(counts.values()), + }) + + +@maplevels_bp.route('/', methods=['GET']) +def get_level(levelid): + level = db.session.get(MapLevel, levelid) + if not level or not level.isactive: + return error_response(ErrorCodes.NOT_FOUND, 'No such level', + http_code=404) + return success_response(level.to_dict()) + + +@maplevels_bp.route('//blueprint/', methods=['GET']) +def serve_blueprint(filename, levelid=None): + """Serve a level's blueprint image. Public, for the same reason as above.""" + return send_upload(_blueprint_dir(), filename) + + +# ============================================================================= +# Write - admin +# ============================================================================= + +@maplevels_bp.route('/buildings', methods=['POST']) +@jwt_required() +@require_role('admin') +def create_building(): + data = request.get_json() or {} + name = (data.get('buildingname') or '').strip() + if not name: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'buildingname is required') + if Building.query.filter(db.func.lower(Building.buildingname) + == name.lower()).first(): + return error_response(ErrorCodes.CONFLICT, + f"Building '{name}' already exists", + http_code=409) + building = Building(buildingname=name, + sortorder=int(data.get('sortorder') or 0)) + db.session.add(building) + db.session.commit() + AuditLog.log('created', 'Building', entityid=building.buildingid, + entityname=name) + db.session.commit() + return success_response(building.to_dict(), message='Building created', + http_code=201) + + +@maplevels_bp.route('/buildings/', methods=['PUT', 'PATCH']) +@jwt_required() +@require_role('admin') +def update_building(buildingid): + """Rename or reorder a building. Levels move with it; nothing repositions.""" + building = db.session.get(Building, buildingid) + if not building: + return error_response(ErrorCodes.NOT_FOUND, 'No such building', + http_code=404) + data = request.get_json() or {} + if 'buildingname' in data: + name = (data['buildingname'] or '').strip() + if not name: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'buildingname cannot be blank') + clash = Building.query.filter( + db.func.lower(Building.buildingname) == name.lower(), + Building.buildingid != buildingid).first() + if clash: + return error_response(ErrorCodes.CONFLICT, + f"Building '{name}' already exists", + http_code=409) + building.buildingname = name + if data.get('sortorder') is not None: + building.sortorder = int(data['sortorder']) + if 'isactive' in data: + building.isactive = bool(data['isactive']) + db.session.commit() + AuditLog.log('updated', 'Building', entityid=buildingid, + entityname=building.buildingname) + db.session.commit() + return success_response(building.to_dict(), message='Building updated') + + +@maplevels_bp.route('', methods=['POST']) +@jwt_required() +@require_role('admin') +def create_level(): + data = request.get_json() or {} + name = (data.get('levelname') or '').strip() + buildingid = data.get('buildingid') + if not name or not buildingid: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'buildingname and buildingid are required') + if not db.session.get(Building, buildingid): + return error_response(ErrorCodes.NOT_FOUND, 'No such building', + http_code=404) + + level = MapLevel( + buildingid=buildingid, + levelname=name, + sortorder=int(data.get('sortorder') or 0), + blueprintlight=data.get('blueprintlight'), + blueprintdark=data.get('blueprintdark'), + mapwidth=int(data.get('mapwidth') or 3300), + mapheight=int(data.get('mapheight') or 2550), + ) + db.session.add(level) + db.session.flush() + _apply_default(level, data.get('isdefault')) + db.session.commit() + AuditLog.log('created', 'MapLevel', entityid=level.levelid, + entityname=name) + db.session.commit() + return success_response(level.to_dict(), message='Level created', + http_code=201) + + +@maplevels_bp.route('/', methods=['PUT', 'PATCH']) +@jwt_required() +@require_role('admin') +def update_level(levelid): + level = db.session.get(MapLevel, levelid) + if not level: + return error_response(ErrorCodes.NOT_FOUND, 'No such level', + http_code=404) + data = request.get_json() or {} + + # The dimensions are the coordinate space every marker on this level is + # expressed in, so changing them moves every marker relative to the image. + # Report it rather than doing it silently; the caller decides whether to run + # a transform (POST /api/assets/map/transform). + warnings = [] + for field in ('mapwidth', 'mapheight'): + if field in data and int(data[field] or 0) != getattr(level, field): + warnings.append( + '%s changed from %s to %s; existing positions on this level are ' + 'still in the old coordinate space' % (field, + getattr(level, field), + data[field])) + + for field in ('levelname', 'blueprintlight', 'blueprintdark'): + if field in data: + setattr(level, field, data[field]) + for field in ('sortorder', 'mapwidth', 'mapheight'): + if field in data and data[field] is not None: + setattr(level, field, int(data[field])) + if 'isactive' in data: + level.isactive = bool(data['isactive']) + _apply_default(level, data.get('isdefault')) + + db.session.commit() + AuditLog.log('updated', 'MapLevel', entityid=level.levelid, + entityname=level.levelname) + db.session.commit() + payload = level.to_dict() + if warnings: + payload['warnings'] = warnings + return success_response(payload, message='Level updated') + + +@maplevels_bp.route('/', methods=['DELETE']) +@jwt_required() +@require_role('admin') +def delete_level(levelid): + """Deactivate a level, refusing while assets are still placed on it. + + Deleting the drawing out from under a marker would leave a position in a + coordinate space that no longer exists - unrenderable, and indistinguishable + from a marker that was never placed. + """ + level = db.session.get(MapLevel, levelid) + if not level: + return error_response(ErrorCodes.NOT_FOUND, 'No such level', + http_code=404) + placed = Asset.query.filter_by(levelid=levelid, isactive=True).count() + if placed: + return error_response( + ErrorCodes.CONFLICT, + f'{placed} asset(s) are placed on this level. Move them to another ' + f'level first.', http_code=409) + if level.isdefault: + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'This is the default level. Make another level the default first.') + level.isactive = False + db.session.commit() + AuditLog.log('deleted', 'MapLevel', entityid=levelid, + entityname=level.levelname) + db.session.commit() + return success_response(message='Level deactivated') + + +@maplevels_bp.route('//blueprint', methods=['POST']) +@jwt_required() +@require_role('admin') +def upload_blueprint(levelid): + """Upload this level's blueprint for one theme. + + multipart/form-data: file=, theme=light|dark. The native pixel size + is NOT inferred from the image - it is stated on the level, because that is + what existing coordinates mean and guessing it would move every marker. + """ + level = db.session.get(MapLevel, levelid) + if not level: + return error_response(ErrorCodes.NOT_FOUND, 'No such level', + http_code=404) + theme = (request.form.get('theme') or '').strip().lower() + if theme not in ('light', 'dark'): + return error_response(ErrorCodes.VALIDATION_ERROR, + 'theme must be light or dark') + upload = request.files.get('file') + if not upload or not upload.filename: + return error_response(ErrorCodes.VALIDATION_ERROR, 'No file provided') + ext = os.path.splitext(upload.filename)[1].lower() + if ext not in BLUEPRINT_EXTENSIONS: + return error_response(ErrorCodes.VALIDATION_ERROR, + f'Unsupported image type {ext}') + + os.makedirs(_blueprint_dir(), exist_ok=True) + filename = secure_filename(f'level-{levelid}-{theme}{ext}') + raw = upload.read() + upload.seek(0) + upload.save(os.path.join(_blueprint_dir(), filename)) + url = f'/api/maplevels/{levelid}/blueprint/{filename}' + setattr(level, f'blueprint{theme}', url) + + # The image's real pixel size, read from its header. What happens next + # depends entirely on whether anything is already placed on this level. + detectedwidth, detectedheight = image_size(raw) + placed = Asset.query.filter_by(levelid=levelid, isactive=True).count() + sizenote = None + if detectedwidth and detectedheight: + matches = (detectedwidth == level.mapwidth + and detectedheight == level.mapheight) + if matches: + sizenote = None + elif not placed: + # Nothing is placed here yet, so no coordinate can be invalidated: + # adopt the image's own size, which is almost certainly what the + # operator wanted and saves them typing it. + level.mapwidth = detectedwidth + level.mapheight = detectedheight + sizenote = ('dimensions set from the image: %d x %d' + % (detectedwidth, detectedheight)) + else: + # Markers exist in the OLD coordinate space. Silently adopting the + # new size would move every one of them relative to the drawing + # while looking like a successful upload, so this reports and + # changes nothing. Resizing is a transform, not an upload. + sizenote = ( + 'this image is %d x %d but the level is set to %d x %d, and %d ' + 'marker(s) are placed in the current space. The dimensions were ' + 'NOT changed: set them with a landmark transform ' + '(POST /api/mappositions/transform) so the markers move with ' + 'them.' % (detectedwidth, detectedheight, level.mapwidth, + level.mapheight, placed)) + + db.session.commit() + AuditLog.log('updated', 'MapLevel', entityid=levelid, + entityname=level.levelname, + changes={f'blueprint{theme}': {'new': url}}) + db.session.commit() + payload = {'levelid': levelid, f'blueprint{theme}': url, + 'mapwidth': level.mapwidth, 'mapheight': level.mapheight, + 'detectedwidth': detectedwidth, 'detectedheight': detectedheight, + 'placedassets': placed} + if sizenote: + payload['sizenote'] = sizenote + return success_response(payload, message='Blueprint uploaded') + + +def _apply_default(level, requested): + """Make this level the default, clearing the flag elsewhere. + + Exactly-one-default is a rule across rows, which no column constraint can + express, so it is enforced here - the one place that sets the flag. + """ + if not requested: + return + MapLevel.query.filter(MapLevel.levelid != level.levelid).update( + {'isdefault': False}) + level.isdefault = True diff --git a/shopdb/core/api/mappositions.py b/shopdb/core/api/mappositions.py new file mode 100644 index 00000000..777edd90 --- /dev/null +++ b/shopdb/core/api/mappositions.py @@ -0,0 +1,403 @@ +"""Bulk marker positions: transform, place, verify, undo (ADR-017). + +A new blueprint invalidates every marker on a level at once, so the operations +here are deliberately bulk. Three rules shape all of them: + +**A transform is derived from landmarks, never from image sizes.** The real case +that motivated this is 3300x2550 to 3308x4000, where the second level was added +below the first: the correct transform is identity scale with a Y offset, and a +scale derived from the dimension ratio would stretch every Y by 1.57 and be wrong +everywhere. Dimensions describe the canvas; landmarks describe the drawing. + +**A transform is a guess, so it clears the review state.** The levels were +redrawn and machines moved. Nothing in a coordinate says whether the machine it +points at is still there, so every transformed marker is unreviewed until a human +confirms it. + +**Nothing bulk happens without a snapshot.** Positions have no history, and an +operation that rewrites hundreds of rows needs a way back that is not a database +restore. +""" + +from datetime import datetime, timezone + +from flask import Blueprint, request +from flask_jwt_extended import jwt_required, get_jwt_identity + +from shopdb.extensions import db +from shopdb.core.models import (Asset, AuditLog, MapLevel, + MapPositionSnapshot, User) +from shopdb.utils.responses import success_response, error_response, ErrorCodes +from shopdb.utils.authz import require_permission + +mappositions_bp = Blueprint('mappositions', __name__) + + +def _now(): + return datetime.now(timezone.utc).replace(tzinfo=None) + + +def _actor(): + try: + user = db.session.get(User, int(get_jwt_identity())) + return user.username if user else None + except (TypeError, ValueError): + return None + + +# ============================================================================= +# The transform +# ============================================================================= + +def solve_axis(pairs): + """Least-squares scale and offset for one axis: new = scale * old + offset. + + Two pairs solve it exactly; more are averaged, which is worth having because + a landmark picked by eye on a scanned drawing carries a few pixels of error + and three points let that cancel instead of accumulate. + + Returns None when the landmarks cannot determine the axis - every old value + identical, so the denominator is zero. That is a real user error (two + landmarks on the same column) and it must be reported rather than papered + over with a scale of 1, which would look like it worked. + """ + count = len(pairs) + if count < 2: + return None + sumold = sum(old for old, _ in pairs) + sumnew = sum(new for _, new in pairs) + sumoldsq = sum(old * old for old, _ in pairs) + sumcross = sum(old * new for old, new in pairs) + denominator = count * sumoldsq - sumold * sumold + if abs(denominator) < 1e-9: + return None + scale = (count * sumcross - sumold * sumnew) / denominator + offset = (sumnew - scale * sumold) / count + return scale, offset + + +def derive_transform(landmarks): + """Per-axis transform from [{'fromx','fromy','tox','toy'}, ...]. + + Per-axis rather than uniform on purpose. A level added below another changes + the canvas height without rescaling anything, so Y gets an offset and X gets + neither; forcing one scale onto both axes cannot express that. + """ + try: + xpairs = [(float(mark['fromx']), float(mark['tox'])) for mark in landmarks] + ypairs = [(float(mark['fromy']), float(mark['toy'])) for mark in landmarks] + except (KeyError, TypeError, ValueError): + return None, 'each landmark needs numeric fromx, fromy, tox and toy' + + if len(landmarks) < 2: + return None, 'at least two landmarks are required' + + xsolution = solve_axis(xpairs) + ysolution = solve_axis(ypairs) + if xsolution is None: + return None, ('the landmarks do not vary in X, so no horizontal scale ' + 'can be derived - pick points that differ across the drawing') + if ysolution is None: + return None, ('the landmarks do not vary in Y, so no vertical scale ' + 'can be derived - pick points that differ down the drawing') + + return { + 'scalex': xsolution[0], 'offsetx': xsolution[1], + 'scaley': ysolution[0], 'offsety': ysolution[1], + }, None + + +def apply_transform(asset, transform): + return ( + int(round(asset.mapx * transform['scalex'] + transform['offsetx'])), + int(round(asset.mapy * transform['scaley'] + transform['offsety'])), + ) + + +@mappositions_bp.route('/transform', methods=['POST']) +@jwt_required() +@require_permission('assets.edit') +def transform_positions(): + """Move every placed marker on a level by a transform read off landmarks. + + Body: + levelid the level whose markers are being moved (required) + landmarks [{fromx, fromy, tox, toy}, ...] - two or more points, each the + same physical feature on the old drawing and the new one + tolevelid optional: write the results to a different level, for splitting + one drawing into two + assetids optional: restrict to these assets + dryrun default TRUE. Nothing is written unless this is explicitly false. + + Dry run returns every marker's old and new position and whether it lands + outside the target level, which is the only way to see a bad landmark pair + before it has moved 300 markers. + """ + data = request.get_json() or {} + levelid = data.get('levelid') + level = db.session.get(MapLevel, levelid) if levelid else None + if not level: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'levelid is required and must exist') + + target = level + if data.get('tolevelid'): + target = db.session.get(MapLevel, data['tolevelid']) + if not target: + return error_response(ErrorCodes.NOT_FOUND, 'No such tolevelid', + http_code=404) + + transform, problem = derive_transform(data.get('landmarks') or []) + if problem: + return error_response(ErrorCodes.VALIDATION_ERROR, problem) + + query = Asset.query.filter( + Asset.levelid == level.levelid, + Asset.mapx.isnot(None), Asset.mapy.isnot(None), + Asset.isactive.is_(True)) + if data.get('assetids'): + query = query.filter(Asset.assetid.in_(data['assetids'])) + assets = query.order_by(Asset.assetid).all() + + moves = [] + outofbounds = 0 + for asset in assets: + newx, newy = apply_transform(asset, transform) + outside = not (0 <= newx <= target.mapwidth and + 0 <= newy <= target.mapheight) + if outside: + outofbounds += 1 + moves.append({ + 'assetid': asset.assetid, + 'assetnumber': asset.assetnumber, + 'name': asset.name, + 'fromx': asset.mapx, 'fromy': asset.mapy, + 'tox': newx, 'toy': newy, + 'outofbounds': outside, + }) + + # The derived transform is reported back whichever mode this is, because it + # is the number a human can sanity-check: a Y scale of 1.57 on a level that + # only grew taller is the mistake this endpoint exists to avoid, and it is + # obvious in the response and invisible in the result. + payload = { + 'transform': transform, + 'levelid': level.levelid, + 'tolevelid': target.levelid, + 'assetcount': len(moves), + 'outofboundscount': outofbounds, + 'moves': moves, + 'dryrun': True, + } + + if data.get('dryrun', True): + return success_response(payload) + + snapshot = MapPositionSnapshot.capture( + assets, + reason='transform of %s (%d markers)' % (level.levelname, len(moves)), + levelid=level.levelid, createdby=_actor()) + + for asset, move in zip(assets, moves): + asset.mapx = move['tox'] + asset.mapy = move['toy'] + asset.levelid = target.levelid + # Cleared, not preserved: a transformed position is a guess, and the + # whole point of the review pass is to tell guesses from confirmations. + asset.mapverifiedat = None + db.session.commit() + + AuditLog.log('updated', 'Asset', entityname='%d marker(s) transformed' + % len(moves), + changes={'transform': transform, + 'snapshotid': snapshot.snapshotid}) + db.session.commit() + + payload['dryrun'] = False + payload['snapshotid'] = snapshot.snapshotid + return success_response(payload, message='%d marker(s) moved; snapshot %d ' + 'can restore them' % (len(moves), + snapshot.snapshotid)) + + +# ============================================================================= +# Bulk place and verify +# ============================================================================= + +@mappositions_bp.route('/positions', methods=['POST']) +@jwt_required() +@require_permission('assets.edit') +def set_positions(): + """Set positions for many assets at once. + + Body: {positions: [{assetid, mapx, mapy, levelid}], verified: bool} + + `levelid` is required per position rather than taken from a single body-level + value, because a bulk save from the editor can legitimately span levels, and + inferring it would be the guess this whole feature exists to remove. + """ + data = request.get_json() or {} + rows = data.get('positions') or [] + if not rows: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'positions is required and must not be empty') + + wanted = {} + for row in rows: + assetid = row.get('assetid') + levelid = row.get('levelid') + if not assetid or not levelid: + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'every position needs an assetid and a levelid; a position ' + 'without a level cannot be rendered') + if row.get('mapx') is None or row.get('mapy') is None: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'every position needs mapx and mapy') + wanted[int(assetid)] = row + + levelids = {int(row['levelid']) for row in wanted.values()} + known = {level.levelid for level in + MapLevel.query.filter(MapLevel.levelid.in_(levelids)).all()} + missing = levelids - known + if missing: + return error_response(ErrorCodes.NOT_FOUND, + 'unknown levelid(s): %s' + % ', '.join(str(one) for one in sorted(missing)), + http_code=404) + + assets = Asset.query.filter(Asset.assetid.in_(wanted)).all() + found = {asset.assetid for asset in assets} + unknown = set(wanted) - found + if unknown: + return error_response(ErrorCodes.NOT_FOUND, + 'unknown assetid(s): %s' + % ', '.join(str(one) for one in sorted(unknown)), + http_code=404) + + snapshot = MapPositionSnapshot.capture( + assets, reason='bulk position set (%d markers)' % len(assets), + createdby=_actor()) + + verified = _now() if data.get('verified') else None + for asset in assets: + row = wanted[asset.assetid] + asset.mapx = int(row['mapx']) + asset.mapy = int(row['mapy']) + asset.levelid = int(row['levelid']) + # Placing a marker by hand IS the confirmation, so this stamps it. A + # caller that is only nudging things about can pass verified=false. + if verified or data.get('verified') is not False: + asset.mapverifiedat = verified or _now() + db.session.commit() + + AuditLog.log('updated', 'Asset', + entityname='%d marker position(s) set' % len(assets), + changes={'snapshotid': snapshot.snapshotid}) + db.session.commit() + return success_response({'updated': len(assets), + 'snapshotid': snapshot.snapshotid}, + message='%d position(s) saved' % len(assets)) + + +@mappositions_bp.route('/verify', methods=['POST']) +@jwt_required() +@require_permission('assets.edit') +def verify_positions(): + """Mark markers as reviewed against the current drawing without moving them. + + This is the common case in a review pass: the transform put it in the right + place and a human agrees. No snapshot, because nothing about the position + changes - only the statement that somebody looked. + """ + data = request.get_json() or {} + assetids = data.get('assetids') or [] + if not assetids: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'assetids is required') + stamp = None if data.get('unverify') else _now() + updated = (Asset.query.filter(Asset.assetid.in_(assetids)) + .update({'mapverifiedat': stamp}, synchronize_session=False)) + db.session.commit() + return success_response({'updated': updated}, + message='%d marker(s) %s' % + (updated, 'unverified' if stamp is None + else 'marked reviewed')) + + +# ============================================================================= +# Undo +# ============================================================================= + +@mappositions_bp.route('/snapshots', methods=['GET']) +@jwt_required() +@require_permission('assets.view') +def list_snapshots(): + """Snapshots, newest first. Metadata only - the positions are large.""" + snapshots = (MapPositionSnapshot.query + .order_by(MapPositionSnapshot.snapshotid.desc()) + .limit(50).all()) + return success_response([one.to_dict() for one in snapshots]) + + +@mappositions_bp.route('/snapshots//restore', methods=['POST']) +@jwt_required() +@require_permission('assets.edit') +def restore_snapshot(snapshotid): + """Put the positions in this snapshot back. + + Takes its own snapshot first, so an undo is itself undoable - which matters + because the most likely reason to restore is a transform that looked right in + the preview and wrong on the drawing, and the second attempt is rarely the + last one either. + """ + snapshot = db.session.get(MapPositionSnapshot, snapshotid) + if not snapshot: + return error_response(ErrorCodes.NOT_FOUND, 'No such snapshot', + http_code=404) + + rows = snapshot.positions + if not rows: + return error_response(ErrorCodes.VALIDATION_ERROR, + 'this snapshot holds no positions') + + assetids = [row['assetid'] for row in rows] + assets = {asset.assetid: asset for asset in + Asset.query.filter(Asset.assetid.in_(assetids)).all()} + + MapPositionSnapshot.capture( + list(assets.values()), + reason='before restoring snapshot %d' % snapshotid, + levelid=snapshot.levelid, createdby=_actor()) + + restored = 0 + skipped = [] + for row in rows: + asset = assets.get(row['assetid']) + if asset is None: + # The asset was deleted since the snapshot. Say so rather than + # failing the whole restore: the other 299 markers still want + # putting back. + skipped.append(row['assetid']) + continue + asset.mapx = row.get('mapx') + asset.mapy = row.get('mapy') + asset.levelid = row.get('levelid') + stamp = row.get('mapverifiedat') + asset.mapverifiedat = datetime.fromisoformat(stamp) if stamp else None + restored += 1 + + snapshot.restoredat = _now() + db.session.commit() + + AuditLog.log('updated', 'Asset', + entityname='%d marker(s) restored from snapshot %d' + % (restored, snapshotid), + changes={'snapshotid': snapshotid, 'skipped': skipped}) + db.session.commit() + + return success_response( + {'restored': restored, 'skippedassetids': skipped}, + message='%d marker(s) restored%s' % + (restored, '; %d asset(s) no longer exist' % len(skipped) + if skipped else '')) diff --git a/shopdb/core/api/search.py b/shopdb/core/api/search.py index 9dfe4977..f39c7a4d 100644 --- a/shopdb/core/api/search.py +++ b/shopdb/core/api/search.py @@ -236,13 +236,26 @@ def _search_applications(query, search_term): def _search_knowledgebase(query, search_term): - """Search Knowledge Base by description and keywords.""" + """Search Knowledge Base by description and keywords. + + An article whose topic is a RETIRED application is excluded, matching + GET /api/knowledgebase. Filtering it out of the plugin's own listing while + global search still returned it is not a rule at all: the article was two + keystrokes away, and the result printed the retired application's name as its + subject, which reads as though it were still in service. + + A null topic still matches. Not every article is about an application. + """ results = [] try: _require_enabled('knowledgebase') from plugins.knowledgebase.models import KnowledgeBase + retired = db.session.query(Application.appid).filter( + Application.isactive.is_(False)) kb_articles = KnowledgeBase.query.filter( KnowledgeBase.isactive == True, + db.or_(KnowledgeBase.appid.is_(None), + KnowledgeBase.appid.notin_(retired)), _word_match(query, KnowledgeBase.shortdescription, KnowledgeBase.keywords) ).limit(20).all() @@ -339,7 +352,22 @@ def _search_employees(query, search_term): def _search_assets(query, search_term): - """Search unified Assets table by number, name, serial, notes.""" + """Search unified Assets table by number, name, serial, notes and the two + optional identifiers. + + gaugelabreference and maintenancereference are searched for EVERY asset type + (ADR-001). Settings lets a site enable either identifier on machines, PCs, + printers and network devices, but only the measuring-tools searcher looked at + gaugelabreference and nothing looked at maintenancereference at all - so a + tag an operator was told to record was one nobody could search by. An + identifier that can be entered has to be findable, or it is a write-only + field. + + The per-type `identifier___enabled` toggles are NOT applied + here. They govern whether the field is SHOWN on that type; a value already in + the row is still the tag written on the physical machine, and matching it is + strictly better than returning nothing to someone reading it off a label. + """ results = [] try: assets = Asset.query.join(AssetType).options( @@ -348,7 +376,8 @@ def _search_assets(query, search_term): ).filter( Asset.isactive == True, _word_match(query, Asset.assetnumber, Asset.name, - Asset.serialnumber, Asset.notes) + Asset.serialnumber, Asset.notes, + Asset.gaugelabreference, Asset.maintenancereference) ).limit(15).all() for asset in assets: @@ -357,6 +386,10 @@ def _search_assets(query, search_term): relevance = 100 elif asset.name and query.lower() == asset.name.lower(): relevance = 90 + elif asset.gaugelabreference and query.lower() == asset.gaugelabreference.lower(): + relevance = 88 + elif asset.maintenancereference and query.lower() == asset.maintenancereference.lower(): + relevance = 86 elif asset.serialnumber and query.lower() == asset.serialnumber.lower(): relevance = 85 elif asset.name and query.lower() in asset.name.lower(): @@ -408,6 +441,104 @@ def _search_measuringtools(query, search_term): return results +def _search_usbdevices(query, search_term): + """Search USB devices by serial, asset tag and product name. + + A USB device is NOT an asset - it lives in the usb plugin's own table - so + the generic asset search cannot see it and these records were unreachable + from search entirely. Serial number is the field people actually have in + hand: it is what is printed on the stick they are holding. + + currentusername is deliberately NOT searched. It records who holds the + device, and making search a way to list what a named person has checked out + is a different feature from finding a device. + """ + results = [] + try: + _require_enabled('usb') + from plugins.usb.models import USBDevice + devices = USBDevice.query.filter( + USBDevice.isactive == True, + _word_match(query, USBDevice.serialnumber, USBDevice.assetnumber, + USBDevice.label, USBDevice.productname) + ).limit(10).all() + + for device in devices: + relevance = 20 + if query.lower() == (device.serialnumber or '').lower(): + relevance = 100 + elif query.lower() == (device.assetnumber or '').lower(): + relevance = 90 + elif query.lower() == (device.label or '').lower(): + relevance = 85 + elif query.lower() in (device.label or '').lower(): + relevance = 50 + elif query.lower() in (device.productname or '').lower(): + relevance = 40 + + results.append({ + 'type': 'usb_device', + 'id': device.usbdeviceid, + 'title': device.label or device.productname or device.serialnumber, + 'subtitle': device.assetnumber or device.serialnumber, + 'url': f'/usb/{device.usbdeviceid}', + 'relevance': relevance, + }) + except ImportError: + pass # usb plugin absent or disabled + except Exception as e: + logger.error(f"USB device search failed: {e}") + return results + + +def _search_printeditems(query, search_term): + """Search printed items by bin code, gage-lab tag, name and description. + + Printed items are their own records, not assets, so the generic asset search + never covered them. itemcode (the bin label, e.g. 3DP-0042) and gagelabtag + are both unique and both printed on physical labels, which makes them the + likeliest thing anyone types into search. + """ + results = [] + try: + _require_enabled('printedparts') + from plugins.printedparts.models import PrintedItem + items = PrintedItem.query.filter( + PrintedItem.isactive == True, + _word_match(query, PrintedItem.itemcode, PrintedItem.gagelabtag, + PrintedItem.itemname, PrintedItem.itemdescription) + ).limit(10).all() + + for item in items: + relevance = 20 + if query.lower() == (item.itemcode or '').lower(): + relevance = 100 + elif query.lower() == (item.gagelabtag or '').lower(): + relevance = 95 + elif query.lower() == (item.itemname or '').lower(): + relevance = 90 + elif query.lower() in (item.itemname or '').lower(): + relevance = 50 + + subtitle = item.itemcode or item.gagelabtag + if item.binlocation: + subtitle = f'{subtitle} - {item.binlocation}' if subtitle else item.binlocation + + results.append({ + 'type': 'printed_item', + 'id': item.printeditemid, + 'title': item.itemname, + 'subtitle': subtitle, + 'url': f'/printedparts/{item.printeditemid}', + 'relevance': relevance, + }) + except ImportError: + pass # printedparts plugin absent or disabled + except Exception as e: + logger.error(f"Printed item search failed: {e}") + return results + + def _search_customfields(query, search_term): """Search custom-field VALUES for fields flagged searchable. @@ -886,6 +1017,8 @@ def global_search(): results.extend(_search_employees(query, search_term)) results.extend(_search_assets(query, search_term)) results.extend(_search_measuringtools(query, search_term)) + results.extend(_search_usbdevices(query, search_term)) + results.extend(_search_printeditems(query, search_term)) results.extend(_search_customfields(query, search_term)) results.extend(_search_notifications(query, search_term)) results.extend(_search_hostnames(query, search_term)) diff --git a/shopdb/core/api/settings.py b/shopdb/core/api/settings.py index b98c5ecd..60c919e5 100644 --- a/shopdb/core/api/settings.py +++ b/shopdb/core/api/settings.py @@ -109,6 +109,8 @@ SEARCH_DOMAINS = { 'measuring_tool': 'Measuring Tools', 'notification': 'Notifications', 'subnet': 'Subnets', + 'usb_device': 'USB Devices', + 'printed_item': 'Printed Items', } def _declared_default(key: str) -> dict: diff --git a/shopdb/core/models/__init__.py b/shopdb/core/models/__init__.py index af6bddba..4a48735f 100644 --- a/shopdb/core/models/__init__.py +++ b/shopdb/core/models/__init__.py @@ -8,6 +8,8 @@ from .model import Model from .businessunit import BusinessUnit from .dashboarddefault import DashboardDefault from .location import Location, LocationType, derive_locationcode +from .maplevel import Building, MapLevel +from .mapsnapshot import MapPositionSnapshot from .operatingsystem import OperatingSystem from .relationship import AssetRelationship, RelationshipType, RelationshipTypePropagation from .communication import Communication, CommunicationType @@ -37,6 +39,10 @@ __all__ = [ 'DashboardDefault', 'Location', 'LocationType', + # Map model (ADR-017): which drawing renders an asset, at what size + 'Building', + 'MapLevel', + 'MapPositionSnapshot', 'derive_locationcode', 'OperatingSystem', # Relationships diff --git a/shopdb/core/models/asset.py b/shopdb/core/models/asset.py index 5f0db55c..a65d7fd9 100644 --- a/shopdb/core/models/asset.py +++ b/shopdb/core/models/asset.py @@ -117,9 +117,24 @@ class Asset(BaseModel, SoftDeleteMixin, AuditMixin): nullable=True ) - # Floor map position (ADR-001: asset-specific override; nullable) - mapx = db.Column(db.Integer, comment='X coordinate on floor map (ADR-001)') - mapy = db.Column(db.Integer, comment='Y coordinate on floor map (ADR-001)') + # Floor map position (ADR-001: asset-specific override; nullable). + # + # Absolute pixels in the NATIVE COORDINATE SPACE OF ITS LEVEL, not of the + # site (ADR-017). levelid says which drawing they are pixels of, and without + # it a position cannot be rendered - a marker drawn on the wrong level's + # blueprint looks perfectly correct and points at the wrong place, so the UI + # shows "level unknown" rather than assuming the default. + mapx = db.Column(db.Integer, comment='X coordinate on this level (ADR-017)') + mapy = db.Column(db.Integer, comment='Y coordinate on this level (ADR-017)') + levelid = db.Column( + db.Integer, db.ForeignKey('maplevels.levelid'), nullable=True, + index=True, + comment='Which drawing mapx/mapy are pixels of (ADR-017)') + # When the position was last CONFIRMED against the current drawing. A bulk + # transform clears it, because a transform is a starting guess: the levels + # were redrawn and machines moved, and nothing in the coordinates says which + # markers are now stale. Null means "not yet reviewed on this drawing". + mapverifiedat = db.Column(db.DateTime, nullable=True) # Notes notes = db.Column(db.Text, nullable=True) @@ -198,6 +213,10 @@ class Asset(BaseModel, SoftDeleteMixin, AuditMixin): 'locationname': related.location.locationname if related.location else None, 'mapx': related.mapx, 'mapy': related.mapy, + # The level belongs to whichever asset supplied the + # coordinates (ADR-017). Inheriting a position without its + # level draws it on the borrower's drawing instead. + 'levelid': related.levelid, 'inheritedfrom': related.assetnumber } @@ -250,6 +269,11 @@ class Asset(BaseModel, SoftDeleteMixin, AuditMixin): result['mapx'] = inherited['mapx'] if result.get('mapy') is None: result['mapy'] = inherited['mapy'] + # Coordinates and their level move together, always. Copying the + # position while leaving levelid as this asset's own is how an + # inherited marker lands on the wrong drawing. + if result.get('levelid') is None: + result['levelid'] = inherited.get('levelid') # Operation/short code of the resolved location (own or inherited). # Derived from the location name's leading token; labels can encode a diff --git a/shopdb/core/models/location.py b/shopdb/core/models/location.py index 1453b589..802f5cad 100644 --- a/shopdb/core/models/location.py +++ b/shopdb/core/models/location.py @@ -70,6 +70,13 @@ class Location(BaseModel): # chain priority 3. mapx = db.Column(db.Integer, comment='Default X coordinate for assets at this location') mapy = db.Column(db.Integer, comment='Default Y coordinate for assets at this location') + # Which drawing those coordinates are pixels of (ADR-017). A location is on a + # level as much as an asset is, and an asset with no position of its own + # inherits BOTH the coordinates and the level from here - inheriting the + # coordinates alone would draw them on whatever level the asset claims. + levelid = db.Column( + db.Integer, db.ForeignKey('maplevels.levelid'), nullable=True, index=True, + comment='Which level mapx/mapy are pixels of (ADR-017)') # Relationships locationtype = db.relationship('LocationType') diff --git a/shopdb/core/models/maplevel.py b/shopdb/core/models/maplevel.py new file mode 100644 index 00000000..16a0342e --- /dev/null +++ b/shopdb/core/models/maplevel.py @@ -0,0 +1,109 @@ +"""Building + MapLevel models: which drawing renders an asset, at what size. + +See ADR-017. A site used to have one floor map, described by four settings, and +`assets.mapx`/`mapy` were pixels in that one image. A second level and a likely +second building make that a table rather than a setting. + +The alternative - stacking levels on one tall canvas - was rejected because it +turns "which level is this on" into `mapy > 2550`: an inference over a magic +number that changes whenever the drawing is re-exported. +""" + +from shopdb.extensions import db +from .base import BaseModel + + +class Building(BaseModel): + """A building at this site. Groups levels; holds nothing a level needs. + + Separate from Location deliberately (ADR-017): a Location answers which + operation owns an asset, a building groups the drawings it appears on. + """ + __tablename__ = 'buildings' + + buildingid = db.Column(db.Integer, primary_key=True) + buildingname = db.Column(db.String(100), nullable=False, unique=True) + # Display order. Buildings have no natural ordering and their names are not + # reliably ordinal ('Main', 'Annex', 'Building 2'), so the order is stated. + sortorder = db.Column(db.Integer, nullable=False, default=0) + + levels = db.relationship( + 'MapLevel', back_populates='building', + order_by='MapLevel.sortorder', cascade='all, delete-orphan') + + def __repr__(self): + return f"" + + def to_dict(self): + data = super().to_dict() + data['levels'] = [level.to_dict() for level in self.levels + if level.isactive] + return data + + +class MapLevel(BaseModel): + """One drawing: a level of a building, with its own blueprint and size. + + WHY THE DIMENSIONS LIVE HERE. They were site-wide settings, which cannot + express a mezzanine drawn at a different scale from the floor below it, and + certainly not a second building. `mapx`/`mapy` are absolute pixels in THIS + level's coordinate space, so a level without its own dimensions cannot place + a marker correctly. + + Name and order are separate columns on purpose: levels are not reliably + numbered (basement, ground, mezzanine, roof), and `sortorder` gives + adjacency and up/down navigation without pretending the names are ordinal. + It also lets a mezzanine be inserted later without renumbering anything. + """ + __tablename__ = 'maplevels' + + levelid = db.Column(db.Integer, primary_key=True) + buildingid = db.Column( + db.Integer, db.ForeignKey('buildings.buildingid'), nullable=False, + index=True) + levelname = db.Column(db.String(100), nullable=False) + sortorder = db.Column(db.Integer, nullable=False, default=0) + + # Both themes, because the map renders in whichever the viewer is using and + # a light-on-white blueprint is unreadable in dark mode. Either may be + # blank; the renderer falls back to the other rather than to nothing. + blueprintlight = db.Column(db.String(255), nullable=True) + blueprintdark = db.Column(db.String(255), nullable=True) + + # Native pixel size of the blueprint. Positions are absolute pixels in this + # space (ADR-017), so these are what a marker's coordinates mean. + mapwidth = db.Column(db.Integer, nullable=False, default=3300) + mapheight = db.Column(db.Integer, nullable=False, default=2550) + + # Exactly one level carries this. It is where an asset with no level lands, + # and what the map opens on. Enforced in the API rather than by a constraint, + # because "exactly one" across rows is not a column-level rule. + isdefault = db.Column(db.Boolean, nullable=False, default=False) + + building = db.relationship('Building', back_populates='levels') + + __table_args__ = ( + db.UniqueConstraint('buildingid', 'levelname', + name='uq_maplevel_building_name'), + ) + + def __repr__(self): + return f"" + + def to_dict(self): + data = super().to_dict() + data['buildingname'] = self.building.buildingname if self.building else None + return data + + @classmethod + def default_level(cls): + """The default level, or the lowest-sorted one if none is marked. + + Never returns None on a seeded database: the migration that created this + table also created one level from the settings it replaced. + """ + level = cls.query.filter_by(isdefault=True, isactive=True).first() + if level is not None: + return level + return (cls.query.filter_by(isactive=True) + .order_by(cls.sortorder, cls.levelid).first()) diff --git a/shopdb/core/models/mapsnapshot.py b/shopdb/core/models/mapsnapshot.py new file mode 100644 index 00000000..cb1ee682 --- /dev/null +++ b/shopdb/core/models/mapsnapshot.py @@ -0,0 +1,76 @@ +"""A saved set of marker positions, so a bulk change can be undone. + +Positions had no history. A landmark transform rewrites every marker on a level +in one statement, and without a way back the honest instruction would be "take a +database backup first" - which nobody does before clicking a button in a UI, so +in practice the feature would either not be used or be used once, badly. + +One row holds the whole set as JSON rather than a row per asset. A snapshot is +read back whole or not at all, so restore stays a single statement, and there is +no orphan-child case to reason about. +""" + +import json + +from shopdb.extensions import db +from .base import BaseModel + + +class MapPositionSnapshot(BaseModel): + __tablename__ = 'mappositionsnapshots' + + snapshotid = db.Column(db.Integer, primary_key=True) + # Which level the operation targeted. Nullable because a snapshot may span + # levels (moving assets between them), and then no single level owns it. + levelid = db.Column(db.Integer, nullable=True) + reason = db.Column(db.String(255), nullable=True) + assetcount = db.Column(db.Integer, nullable=False, default=0) + positionsjson = db.Column(db.Text, nullable=False) + # Set when this snapshot has been restored, so the history reads as what + # happened rather than as a list of identical-looking saves. + restoredat = db.Column(db.DateTime, nullable=True) + createdby = db.Column(db.String(100), nullable=True) + + def __repr__(self): + return f"" + + @property + def positions(self): + try: + return json.loads(self.positionsjson or '[]') + except ValueError: + return [] + + def to_dict(self): + """Metadata only. The positions themselves are large and nobody browsing + a list of snapshots wants them.""" + data = super().to_dict() + data.pop('positionsjson', None) + return data + + @classmethod + def capture(cls, assets, reason, levelid=None, createdby=None): + """Record the CURRENT positions of these assets, before they change. + + Includes levelid and mapverifiedat, not just the coordinates: a restore + has to put a marker back on the level it was on and with the review state + it had, or undo would silently mark reviewed work as unreviewed. + """ + rows = [{ + 'assetid': asset.assetid, + 'mapx': asset.mapx, + 'mapy': asset.mapy, + 'levelid': asset.levelid, + 'mapverifiedat': asset.mapverifiedat.isoformat() + if asset.mapverifiedat else None, + } for asset in assets] + snapshot = cls( + levelid=levelid, + reason=reason, + assetcount=len(rows), + positionsjson=json.dumps(rows, separators=(',', ':')), + createdby=createdby, + ) + db.session.add(snapshot) + db.session.flush() + return snapshot diff --git a/shopdb/utils/imagesize.py b/shopdb/utils/imagesize.py new file mode 100644 index 00000000..f0fa90db --- /dev/null +++ b/shopdb/utils/imagesize.py @@ -0,0 +1,134 @@ +"""Read an image's pixel dimensions from its header, with no image library. + +A level's `mapwidth`/`mapheight` are the coordinate space every marker on it is +expressed in, so getting them wrong moves every marker relative to the drawing. +Reading them off the uploaded file removes the most likely way to get them +wrong, which is somebody typing what they remember. + +Pillow would do this in one line and is not a dependency. Adding it would mean a +new cp314 win_amd64 wheel in the offline installer's hash-pinned wheelhouse - +built on Windows, verified against bundle-lock.json, shipped in a 240 MB +installer - to read two integers out of a header. This is the cheaper trade. + +Returns (width, height), or (None, None) when the format is not one of these or +the header is truncated. A caller must treat None as "ask the operator" and +never as a default. +""" + +import re +import struct + + +def png_size(data): + # An IHDR chunk always follows the 8-byte signature, and its first two + # fields are width and height as big-endian 32-bit integers. + if len(data) < 24 or data[:8] != b'\x89PNG\r\n\x1a\n': + return None, None + if data[12:16] != b'IHDR': + return None, None + width, height = struct.unpack('>II', data[16:24]) + return width, height + + +def gif_size(data): + if len(data) < 10 or data[:6] not in (b'GIF87a', b'GIF89a'): + return None, None + width, height = struct.unpack('> 14) & 0x3FFF) + 1 + if kind == b'VP8X': + width = int.from_bytes(data[24:27], 'little') + 1 + height = int.from_bytes(data[27:30], 'little') + 1 + return width, height + return None, None + + +def jpeg_size(data): + """Walk the marker segments to the start-of-frame, which carries the size. + + JPEG has no fixed header offset - the dimensions live in whichever SOF + marker the encoder used, after any number of application and comment + segments of varying length. So this walks rather than indexes. + """ + if len(data) < 4 or data[:2] != b'\xff\xd8': + return None, None + index = 2 + end = len(data) + while index < end - 9: + if data[index] != 0xFF: + index += 1 + continue + marker = data[index + 1] + # SOF0 through SOF15, excluding the DHT/JPG/DAC markers interleaved + # in that range, all carry height then width at the same offset. + if marker in (0xC0, 0xC1, 0xC2, 0xC3, 0xC5, 0xC6, 0xC7, + 0xC9, 0xCA, 0xCB, 0xCD, 0xCE, 0xCF): + height, width = struct.unpack('>HH', data[index + 5:index + 9]) + return width, height + if marker in (0xD8, 0x01) or 0xD0 <= marker <= 0xD9: + index += 2 + continue + segment = struct.unpack('>H', data[index + 2:index + 4])[0] + index += 2 + segment + return None, None + + +def svg_size(data): + """SVG states a size in attributes, or implies one through viewBox. + + Width and height may carry units (mm, in, pt) or be percentages, and a + percentage says nothing about pixels - so a unit that is not px falls back to + the viewBox, which is unitless user space and is what a renderer scales to. + """ + try: + head = data[:4096].decode('utf-8', errors='replace') + except Exception: + return None, None + if ' middle -> root, where only root has coords.""" - root = _make_asset(assetid=3, mapx=1, mapy=2) + root = _make_asset(assetid=3, mapx=1, mapy=2, levelid=5) middle = _make_asset(assetid=2, outgoing=[_make_rel('partof', root)]) child = _make_asset(assetid=1, outgoing=[_make_rel('partof', middle)]) result = resolve_asset_position(child) - assert result == {'mapx': 1, 'mapy': 2, 'positionsource': 'related'} + # Carried back up two hops from the node that actually has coordinates. + assert result == {'mapx': 1, 'mapy': 2, 'levelid': 5, + 'positionsource': 'related'} def test_resolve_asset_position_breaks_cycles(): @@ -195,12 +219,13 @@ def test_resolve_asset_position_depth_cap_is_three(): def test_resolve_asset_position_self_beats_related(): """Priority 1 beats priority 2: asset's own coords win even when a related neighbor would also resolve.""" - neighbor = _make_asset(assetid=2, mapx=99, mapy=99) + neighbor = _make_asset(assetid=2, mapx=99, mapy=99, levelid=9) rel = _make_rel('partof', neighbor) - asset = _make_asset(assetid=1, mapx=1, mapy=2, outgoing=[rel]) + asset = _make_asset(assetid=1, mapx=1, mapy=2, levelid=1, outgoing=[rel]) result = resolve_asset_position(asset) - assert result == {'mapx': 1, 'mapy': 2, 'positionsource': 'self'} + assert result == {'mapx': 1, 'mapy': 2, 'levelid': 1, + 'positionsource': 'self'} def test_resolve_asset_position_related_beats_location(): @@ -209,13 +234,18 @@ def test_resolve_asset_position_related_beats_location(): class FakeLocation: mapx = 500 mapy = 600 + levelid = 9 - neighbor = _make_asset(assetid=2, mapx=10, mapy=20) + neighbor = _make_asset(assetid=2, mapx=10, mapy=20, levelid=2) rel = _make_rel('partof', neighbor) - asset = _make_asset(assetid=1, outgoing=[rel], location=FakeLocation()) + asset = _make_asset(assetid=1, levelid=7, outgoing=[rel], + location=FakeLocation()) result = resolve_asset_position(asset) - assert result == {'mapx': 10, 'mapy': 20, 'positionsource': 'related'} + # Three levels are in play - the asset's own stale 7, the location's 9, and + # the neighbour's 2. Only the one that owns the coordinates is correct. + assert result == {'mapx': 10, 'mapy': 20, 'levelid': 2, + 'positionsource': 'related'} def test_resolve_asset_position_inactive_edge_skipped(): diff --git a/tests/test_core/test_map_transform.py b/tests/test_core/test_map_transform.py new file mode 100644 index 00000000..7a1365fe --- /dev/null +++ b/tests/test_core/test_map_transform.py @@ -0,0 +1,236 @@ +"""The landmark transform, and the wrong answer it must not give. + +The case this exists for: a site's blueprint goes from 3300x2550 to 3308x4000 +because a second level was added below the first. The existing floor is drawn at +the same scale - so the correct transform is identity, or identity plus an +offset. A transform derived from the ratio of image dimensions would scale Y by +4000/2550 = 1.5686 and be wrong for every marker on the level, while looking +like arithmetic somebody had thought about. + +So the first test here is not that the transform works. It is that it does not +produce that particular plausible wrong answer. +""" + +from datetime import datetime + +import pytest + +from shopdb.core.models import Asset, AssetType, Building, MapLevel +from shopdb.core.api.mappositions import derive_transform, solve_axis + + +# -- the arithmetic, with no app or database involved ------------------------ + +def test_two_landmarks_give_identity_when_the_drawing_did_not_move(): + """A canvas that grew taller with the old floor untouched: scale 1, no offset. + + This is the assertion that rules out the dimension-derived answer. Nothing + about these landmarks mentions 2550 or 4000, and the result must not either. + """ + transform, problem = derive_transform([ + {'fromx': 100, 'fromy': 100, 'tox': 100, 'toy': 100}, + {'fromx': 3000, 'fromy': 2000, 'tox': 3000, 'toy': 2000}, + ]) + assert problem is None + assert transform['scalex'] == pytest.approx(1.0) + assert transform['scaley'] == pytest.approx(1.0) + assert transform['offsetx'] == pytest.approx(0.0) + assert transform['offsety'] == pytest.approx(0.0) + # The wrong answer, stated so a regression names itself. + assert transform['scaley'] != pytest.approx(4000 / 2550, abs=0.01) + + +def test_a_level_added_above_gives_a_pure_y_offset(): + """Existing floor pushed down by 1450px: identity scale, offset 1450.""" + transform, problem = derive_transform([ + {'fromx': 100, 'fromy': 100, 'tox': 100, 'toy': 1550}, + {'fromx': 3000, 'fromy': 2000, 'tox': 3000, 'toy': 3450}, + ]) + assert problem is None + assert transform['scaley'] == pytest.approx(1.0) + assert transform['offsety'] == pytest.approx(1450.0) + assert transform['scalex'] == pytest.approx(1.0) + + +def test_a_genuine_rescale_is_derived_per_axis(): + """X and Y can scale differently, which one uniform factor cannot express.""" + transform, _ = derive_transform([ + {'fromx': 0, 'fromy': 0, 'tox': 0, 'toy': 0}, + {'fromx': 1000, 'fromy': 1000, 'tox': 2000, 'toy': 1500}, + ]) + assert transform['scalex'] == pytest.approx(2.0) + assert transform['scaley'] == pytest.approx(1.5) + + +def test_a_third_landmark_averages_measurement_error(): + """Points picked by eye carry a few pixels of error; more points cancel it + rather than accumulating it.""" + transform, _ = derive_transform([ + {'fromx': 0, 'fromy': 0, 'tox': 2, 'toy': -2}, + {'fromx': 1000, 'fromy': 1000, 'tox': 999, 'toy': 1001}, + {'fromx': 2000, 'fromy': 2000, 'tox': 2001, 'toy': 1999}, + ]) + assert transform['scalex'] == pytest.approx(1.0, abs=0.01) + assert transform['scaley'] == pytest.approx(1.0, abs=0.01) + + +def test_landmarks_on_one_column_are_refused_not_guessed(): + """Two points with the same X cannot determine a horizontal scale. + + Returning 1.0 here would look like success and silently leave X unscaled, + which is the failure a user would discover after the write. + """ + transform, problem = derive_transform([ + {'fromx': 500, 'fromy': 100, 'tox': 600, 'toy': 100}, + {'fromx': 500, 'fromy': 2000, 'tox': 600, 'toy': 2000}, + ]) + assert transform is None + assert 'X' in problem + + +def test_one_landmark_is_not_enough(): + transform, problem = derive_transform([ + {'fromx': 1, 'fromy': 1, 'tox': 2, 'toy': 2}]) + assert transform is None + assert 'two landmarks' in problem + + +def test_solve_axis_reports_an_undetermined_axis(): + assert solve_axis([(5, 9), (5, 11)]) is None + assert solve_axis([(0, 0), (10, 20)]) == (2.0, 0.0) + + +# -- end to end, against the endpoints -------------------------------------- + +@pytest.fixture +def floor(db): + """One building, two levels sized like the real before and after.""" + assettype = AssetType.query.filter_by(assettype='machine').first() + if not assettype: + assettype = AssetType(assettype='machine') + db.session.add(assettype) + db.session.flush() + building = Building(buildingname='Main') + db.session.add(building) + db.session.flush() + ground = MapLevel(buildingid=building.buildingid, levelname='Ground floor', + sortorder=0, isdefault=True, mapwidth=3300, mapheight=2550) + second = MapLevel(buildingid=building.buildingid, levelname='Second floor', + sortorder=1, mapwidth=3308, mapheight=4000) + db.session.add_all([ground, second]) + db.session.flush() + assets = [] + for index, (x, y) in enumerate([(100, 100), (1500, 1200), (3200, 2500)]): + asset = Asset(assetnumber='M%03d' % index, assettypeid=assettype.assettypeid, + mapx=x, mapy=y, levelid=ground.levelid, + mapverifiedat=datetime(2026, 1, 1)) + db.session.add(asset) + assets.append(asset) + db.session.commit() + return {'ground': ground, 'second': second, 'assets': assets} + + +def _transform(client, headers, **body): + return client.post('/api/mappositions/transform', json=body, headers=headers) + + +def test_a_dry_run_writes_nothing(client, db, floor, auth_headers): + before = [(a.mapx, a.mapy, a.mapverifiedat) for a in floor['assets']] + resp = _transform(client, auth_headers, + levelid=floor['ground'].levelid, + landmarks=[{'fromx': 0, 'fromy': 0, 'tox': 0, 'toy': 1450}, + {'fromx': 1000, 'fromy': 1000, 'tox': 1000, 'toy': 2450}]) + assert resp.status_code == 200 + body = resp.get_json()['data'] + assert body['dryrun'] is True + assert body['assetcount'] == 3 + db.session.expire_all() + after = [(a.mapx, a.mapy, a.mapverifiedat) + for a in Asset.query.order_by(Asset.assetid).all()] + assert after == before + + +def test_applying_moves_markers_clears_review_and_snapshots( + client, db, floor, auth_headers): + resp = _transform(client, auth_headers, + levelid=floor['ground'].levelid, dryrun=False, + landmarks=[{'fromx': 0, 'fromy': 0, 'tox': 0, 'toy': 1450}, + {'fromx': 1000, 'fromy': 1000, 'tox': 1000, 'toy': 2450}]) + assert resp.status_code == 200 + body = resp.get_json()['data'] + assert body['dryrun'] is False + assert body['snapshotid'] + + db.session.expire_all() + moved = Asset.query.filter_by(assetnumber='M000').first() + assert (moved.mapx, moved.mapy) == (100, 1550) + # A transform is a guess, so the review state it had is gone. + assert moved.mapverifiedat is None + + +def test_markers_pushed_off_the_canvas_are_counted(client, db, floor, auth_headers): + """A marker at y=2500 offset by 1600 lands at 4100, past the 4000px canvas. + + Worth checking because a bad landmark pair is most likely to show up as + markers leaving the drawing entirely, and a preview that does not count them + lets that through. + """ + resp = _transform(client, auth_headers, + levelid=floor['ground'].levelid, + tolevelid=floor['second'].levelid, + landmarks=[{'fromx': 0, 'fromy': 0, 'tox': 0, 'toy': 1600}, + {'fromx': 1000, 'fromy': 1000, 'tox': 1000, 'toy': 2600}]) + body = resp.get_json()['data'] + assert body['outofboundscount'] == 1 + offender = next(m for m in body['moves'] if m['outofbounds']) + assert offender['toy'] > floor['second'].mapheight + + +def test_restore_puts_positions_and_review_state_back( + client, db, floor, auth_headers): + original = {a.assetnumber: (a.mapx, a.mapy, a.mapverifiedat) + for a in floor['assets']} + applied = _transform(client, auth_headers, + levelid=floor['ground'].levelid, dryrun=False, + landmarks=[{'fromx': 0, 'fromy': 0, 'tox': 0, 'toy': 1450}, + {'fromx': 1000, 'fromy': 1000, 'tox': 1000, 'toy': 2450}]) + snapshotid = applied.get_json()['data']['snapshotid'] + + resp = client.post('/api/mappositions/snapshots/%d/restore' % snapshotid, + headers=auth_headers) + assert resp.status_code == 200 + assert resp.get_json()['data']['restored'] == 3 + + db.session.expire_all() + for asset in Asset.query.all(): + assert (asset.mapx, asset.mapy, asset.mapverifiedat) == \ + original[asset.assetnumber] + + +def test_a_position_without_a_level_is_refused(client, db, floor, auth_headers): + """The rule the whole model rests on: a position with no level cannot be + stored, because it cannot be rendered.""" + resp = client.post('/api/mappositions/positions', json={ + 'positions': [{'assetid': floor['assets'][0].assetid, + 'mapx': 10, 'mapy': 20}]}, headers=auth_headers) + assert resp.status_code == 400 + assert 'levelid' in resp.get_json()['data']['error']['message'] + + +def test_placing_by_hand_counts_as_review(client, db, floor, auth_headers): + asset = floor['assets'][0] + client.post('/api/mappositions/verify', + json={'assetids': [asset.assetid], 'unverify': True}, + headers=auth_headers) + db.session.expire_all() + assert Asset.query.get(asset.assetid).mapverifiedat is None + + resp = client.post('/api/mappositions/positions', json={ + 'positions': [{'assetid': asset.assetid, 'mapx': 42, 'mapy': 43, + 'levelid': floor['second'].levelid}]}, + headers=auth_headers) + assert resp.status_code == 200 + db.session.expire_all() + moved = Asset.query.get(asset.assetid) + assert (moved.mapx, moved.mapy, moved.levelid) == (42, 43, floor['second'].levelid) + assert moved.mapverifiedat is not None diff --git a/tests/test_core/test_search_coverage.py b/tests/test_core/test_search_coverage.py new file mode 100644 index 00000000..3250e957 --- /dev/null +++ b/tests/test_core/test_search_coverage.py @@ -0,0 +1,201 @@ +"""What global search can reach, and what it must not return. + +Two plugins own records that are NOT assets - USB devices and printed items - +so the generic asset search cannot see them, and nothing else did either: those +records were unreachable from search entirely while their list pages existed and +their labels were printed on physical bins and sticks. + +The other half of this file is the retired-application rule. Hiding an article +about a decommissioned application from the plugin's own listing while global +search still returned it is not a rule at all - the article stayed two +keystrokes away, and the result printed the retired application as its subject. +A filter is only real if every path that reaches the row applies it. +""" + +import pytest + +from shopdb.core.models import Application + + +def search(client, query): + """Global search rows for a query.""" + resp = client.get(f'/api/search?q={query}') + assert resp.status_code == 200, resp.get_json() + return resp.get_json()['data']['results'] + + +def titles(client, query, resulttype=None): + return [r['title'] for r in search(client, query) + if resulttype is None or r['type'] == resulttype] + + +# --- USB devices (usb plugin, not an asset) --------------------------------- + +@pytest.fixture +def usbdevices(db): + """One live device and one deactivated one, distinctive enough that only the + USB searcher can match them.""" + from plugins.usb.models import USBDevice + live = USBDevice(serialnumber='ZZSERIAL1234', label='Loaner stick 4', + assetnumber='USB-0004', productname='Kingston DataTraveler', + isactive=True) + retired = USBDevice(serialnumber='ZZSERIAL9999', label='Dead stick', + assetnumber='USB-0009', productname='Kingston DataTraveler', + isactive=False) + db.session.add_all([live, retired]) + db.session.commit() + return {'live': live.usbdeviceid, 'retired': retired.usbdeviceid} + + +def test_a_usb_device_is_found_by_serial(client, usbdevices): + """The serial is what is printed on the stick in someone's hand.""" + rows = [r for r in search(client, 'ZZSERIAL1234') if r['type'] == 'usb_device'] + assert len(rows) == 1 + assert rows[0]['id'] == usbdevices['live'] + assert rows[0]['url'] == f"/usb/{usbdevices['live']}" + # An exact serial is the strongest possible match for this domain. + assert rows[0]['relevance'] == 100 + + +def test_a_usb_device_is_found_by_label_and_asset_tag(client, usbdevices): + assert 'Loaner stick 4' in titles(client, 'Loaner', 'usb_device') + assert 'Loaner stick 4' in titles(client, 'USB-0004', 'usb_device') + + +def test_a_deactivated_usb_device_is_not_found(client, usbdevices): + assert titles(client, 'ZZSERIAL9999', 'usb_device') == [] + # And it does not ride along on a query that matches both devices. + assert 'Dead stick' not in titles(client, 'Kingston', 'usb_device') + + +def test_a_usb_holder_is_not_searchable_by_name(client, db): + """currentusername records who holds a device. Making search a way to list + what a named person checked out is a different feature, deliberately not + built into this searcher.""" + from plugins.usb.models import USBDevice + db.session.add(USBDevice(serialnumber='ZZSERIAL5555', label='Held stick', + currentusername='Distinctivesurname', isactive=True)) + db.session.commit() + assert titles(client, 'Distinctivesurname', 'usb_device') == [] + + +# --- Printed items (printedparts plugin, not an asset) ---------------------- + +@pytest.fixture +def printeditems(db): + from plugins.printedparts.models import PrintedItem + live = PrintedItem(itemcode='3DP-8001', gagelabtag='WJRP8001', + itemname='Fixture clamp', itemdescription='Holds a part', + binlocation='Bin 12', quantityonhand=4, + lowstockthreshold=2, isactive=True) + retired = PrintedItem(itemcode='3DP-8009', gagelabtag='WJRP8009', + itemname='Obsolete clamp', quantityonhand=0, + lowstockthreshold=1, isactive=False) + db.session.add_all([live, retired]) + db.session.commit() + return {'live': live.printeditemid, 'retired': retired.printeditemid} + + +def test_a_printed_item_is_found_by_bin_code(client, printeditems): + """itemcode is the bin label someone reads off the shelf.""" + rows = [r for r in search(client, '3DP-8001') if r['type'] == 'printed_item'] + assert len(rows) == 1 + assert rows[0]['id'] == printeditems['live'] + assert rows[0]['url'] == f"/printedparts/{printeditems['live']}" + assert rows[0]['relevance'] == 100 + + +def test_a_printed_item_is_found_by_gage_lab_tag(client, printeditems): + rows = [r for r in search(client, 'WJRP8001') if r['type'] == 'printed_item'] + assert [r['title'] for r in rows] == ['Fixture clamp'] + + +def test_a_printed_item_is_found_by_name(client, printeditems): + assert 'Fixture clamp' in titles(client, 'Fixture clamp', 'printed_item') + + +def test_a_printed_item_subtitle_locates_it(client, printeditems): + """A hit is only useful if it says where to go and get the part.""" + rows = [r for r in search(client, '3DP-8001') if r['type'] == 'printed_item'] + assert 'Bin 12' in rows[0]['subtitle'] + + +def test_a_deactivated_printed_item_is_not_found(client, printeditems): + assert titles(client, '3DP-8009', 'printed_item') == [] + assert 'Obsolete clamp' not in titles(client, 'clamp', 'printed_item') + + +# --- The retired-application rule reaches global search too ---------------- + +@pytest.fixture +def kbarticles(db): + from plugins.knowledgebase.models import KnowledgeBase + live = Application(appname='Livesearchapp', isactive=True) + retired = Application(appname='Retiredsearchapp', isactive=False) + db.session.add_all([live, retired]) + db.session.flush() + db.session.add_all([ + KnowledgeBase(shortdescription='Zzrunbook for the live one', linkurl='u', + keywords='zzkeyword', appid=live.appid, isactive=True), + KnowledgeBase(shortdescription='Zzrunbook for the retired one', linkurl='u', + keywords='zzkeyword', appid=retired.appid, isactive=True), + KnowledgeBase(shortdescription='Zzrunbook with no topic', linkurl='u', + keywords='zzkeyword', appid=None, isactive=True), + ]) + db.session.commit() + + +def test_global_search_hides_an_article_about_a_retired_application(client, kbarticles): + found = titles(client, 'zzkeyword', 'knowledgebase') + assert 'Zzrunbook for the retired one' not in found + assert 'Zzrunbook for the live one' in found + + +def test_global_search_keeps_an_article_with_no_topic(client, kbarticles): + """A null topic is not a retired one.""" + assert 'Zzrunbook with no topic' in titles(client, 'zzkeyword', 'knowledgebase') + + +def test_global_search_hides_a_retired_application_itself(client, kbarticles): + """The application domain has always filtered isactive; pinned here so the + two rules stay together and cannot drift apart again.""" + assert titles(client, 'Retiredsearchapp', 'application') == [] + assert titles(client, 'Livesearchapp', 'application') == ['Livesearchapp'] + + +# --- The optional asset identifiers (ADR-001) ------------------------------- + +@pytest.fixture +def taggedmachine(client, db, auth_headers): + """A machine carrying both optional identifiers. + + A MACHINE on purpose, not a measuring tool: gaugelabreference was searchable + only through the measuring-tools searcher, so the same tag on a machine + matched nothing even though Settings offers the identifier for machines. + """ + from shopdb.core.models import AssetType, Asset + if not AssetType.query.filter_by(assettype='machine').first(): + db.session.add(AssetType(assettype='machine', pluginname='machines', + tablename='machines', description='m')) + db.session.commit() + resp = client.post('/api/machines', json={'assetnumber': 'ZZMACH01'}, + headers=auth_headers) + assert resp.status_code == 201, resp.get_json() + asset = Asset.query.filter_by(assetnumber='ZZMACH01').first() + asset.gaugelabreference = 'ZZGAUGE777' + asset.maintenancereference = 'ZZMAINT888' + db.session.commit() + return asset.assetid + + +def test_a_machine_is_found_by_its_gauge_lab_reference(client, taggedmachine): + rows = [r for r in search(client, 'ZZGAUGE777') if r['id'] == taggedmachine + or r['title'] == 'ZZMACH01'] + assert rows, 'a gauge-lab tag on a machine matched nothing' + + +def test_a_machine_is_found_by_its_maintenance_reference(client, taggedmachine): + """maintenancereference was searched by nothing at all, for any asset + type - a field the UI collects and search could not find.""" + rows = [r for r in search(client, 'ZZMAINT888') if r['title'] == 'ZZMACH01'] + assert rows, 'a maintenance reference matched nothing' diff --git a/tests/test_plugins/test_knowledgebase_visibility.py b/tests/test_plugins/test_knowledgebase_visibility.py new file mode 100644 index 00000000..3d8870a4 --- /dev/null +++ b/tests/test_plugins/test_knowledgebase_visibility.py @@ -0,0 +1,106 @@ +"""What a knowledge base article's topic decides about seeing the article. + +An article's topic is an Application. When that application is retired the +article describes something no longer in service, and showing it beside live +documentation reads as though it were current - so it does not show at all. + +`isactive` is the ONLY property of an application that decides this. In +particular `ishidden`, which governs whether an application appears on the tiles +page, says nothing about whether it can be the subject of an article. +""" + +import pytest + +from shopdb.core.models import Application +from plugins.knowledgebase.models import KnowledgeBase + + +@pytest.fixture +def library(db): + """One application of each interesting shape, with an article each.""" + applications = { + 'live': Application(appname='Live App', isinstallable=True, + ishidden=False, isactive=True), + 'notinstallable': Application(appname='Manual App', isinstallable=False, + ishidden=False, isactive=True), + 'hidden': Application(appname='Hidden App', isinstallable=False, + ishidden=True, isactive=True), + 'retired': Application(appname='Retired App', isinstallable=True, + ishidden=False, isactive=False), + } + db.session.add_all(applications.values()) + db.session.flush() + + articles = { + 'live': KnowledgeBase(shortdescription='Live runbook', linkurl='u', + appid=applications['live'].appid, clicks=3, + isactive=True), + 'hidden': KnowledgeBase(shortdescription='Hidden runbook', linkurl='u', + appid=applications['hidden'].appid, clicks=2, + isactive=True), + 'retired': KnowledgeBase(shortdescription='Retired runbook', linkurl='u', + appid=applications['retired'].appid, clicks=5, + isactive=True), + 'topicless': KnowledgeBase(shortdescription='General note', linkurl='u', + appid=None, clicks=1, isactive=True), + } + db.session.add_all(articles.values()) + db.session.commit() + return {'applications': applications, 'articles': articles} + + +def titles(client, url='/api/knowledgebase'): + return sorted(row['shortdescription'] + for row in client.get(url).get_json()['data']) + + +def test_an_article_about_a_retired_application_does_not_show(client, library): + assert 'Retired runbook' not in titles(client) + + +def test_an_article_with_no_topic_still_shows(client, library): + """A null topic is not a retired one. Not every article is about an + application, and those must not be collateral damage.""" + assert 'General note' in titles(client) + + +def test_a_hidden_application_is_still_a_valid_topic(client, library): + """ishidden keeps an application off the tiles page. It says nothing about + documentation, and isactive is the only filter that applies here.""" + assert 'Hidden runbook' in titles(client) + + +def test_searching_a_retired_application_name_finds_nothing(client, library): + """The topic search matched on application name without checking isactive, + which surfaced retired articles and printed the retired application as their + subject.""" + assert titles(client, '/api/knowledgebase?search=Retired App') == [] + + +def test_searching_a_title_does_not_resurrect_a_retired_article(client, library): + """Hiding it from the list and finding it by title would be no rule at all.""" + found = titles(client, '/api/knowledgebase?search=runbook') + assert 'Retired runbook' not in found + assert 'Live runbook' in found + + +def test_the_counts_agree_with_the_list(client, library): + """A total that includes articles nobody can see is a total nobody can + reconcile against the page.""" + visible = titles(client) + stats = client.get('/api/knowledgebase/stats').get_json()['data'] + total = stats.get('total_articles', stats.get('totalarticles')) + clicks = stats.get('total_clicks', stats.get('totalclicks')) + assert total == len(visible) == 3 + # 3 + 2 + 1 from the live, hidden-topic and topicless articles; the retired + # article's 5 clicks are excluded along with the article. + assert clicks == 6 + + +def test_the_topic_list_offers_every_active_application(client, library): + """What the article form's topic dropdown fetches. Installable or not, + hidden or not - only retired is excluded.""" + offered = sorted(row['appname'] for row in client.get( + '/api/applications?perpage=100&showhidden=true').get_json()['data']) + assert offered == ['Hidden App', 'Live App', 'Manual App'] + assert 'Retired App' not in offered diff --git a/tests/test_plugins/test_printer_install_list.py b/tests/test_plugins/test_printer_install_list.py index d6d315fc..188dfcee 100644 --- a/tests/test_plugins/test_printer_install_list.py +++ b/tests/test_plugins/test_printer_install_list.py @@ -66,7 +66,14 @@ def test_install_list_vendorname_resolves_via_model(client, db, auth_headers, def test_install_list_text_format(client, db, auth_headers, printer_assettype): """format=text returns one pipe-delimited line per printer (fixed field - order) so the Inno installers split() instead of parsing JSON.""" + order) so the Inno installers split() instead of parsing JSON. + + FIELD ORDER 0..7 IS FROZEN. The shipped installer reads them by index + (`GetField(Line, 6)` for mapx), so a field inserted anywhere but the end + silently shifts every later one: the installer would read a model number as + a coordinate and still run. levelid is appended as field 8, which installers + built before levels existed simply never read. + """ client.post('/api/printers', json={ 'assetnumber': 'CSF01-HP', 'windowsname': 'CSF01-HP', 'hostname': 'wjprn01', 'mapx': 120, 'mapy': 240, @@ -80,10 +87,14 @@ def test_install_list_text_format(client, db, auth_headers, printer_assettype): assert line is not None cols = line.split('|') # printerid|windowsname|vendorname|modelnumber|hostname|ipaddress|mapx|mapy - assert len(cols) == 8 + # |levelid + assert len(cols) == 9 assert cols[1] == 'CSF01-HP' # windowsname (falls back to assetnumber name) assert cols[4] == 'wjprn01' # hostname assert cols[6] == '120' and cols[7] == '240' + # levelid last, and empty rather than absent when the printer has no level: + # the field count must not vary between rows or a positional split breaks. + assert cols[8] == '' def test_install_list_excludes_usb_only_printer(client, db, auth_headers,