Fix model photo upload, and give network devices the model link the page assumed

Three faults around vendor-model photos, found while looking at why an uploaded
image did not appear.

Saving a model was blocked after uploading a photo. The Image URL field was
type="url", and an upload sets it to an application path such as
/api/models/image/model-120.png. Native url validation demands an absolute URL
with a scheme, so the browser refused to submit the form with "Please enter a
URL" for a value the page had just written itself. The field is now type="text",
which is what it always needed to be: it holds either a full web address or a
path on this server. documentationurl stays type="url".

The upload button did not appear when adding a model, only when editing one.
That was deliberate - the photo is stored as model-<id>.<ext>, so it cannot be
sent before the record has an id - but it reads as a missing feature, and the
hint explaining it was easy to miss. A photo chosen while creating is now held
and uploaded as soon as the model is saved, and it is dropped if the dialog is
cancelled, so it cannot land on the next model created in the same session.

Network devices could never show a photo. NetworkDeviceDetail.vue binds its hero
image to networkdevice.imageurl, but networkdevices carried only vendorid, with
no link to a catalog model, so nothing could populate it - a feature that looked
present and could not work. Machines, PCs and printers have carried
modelnumberid since July. This adds the same column and relationship, the
to_dict branch that exposes modelname and imageurl, the field on the API, and a
Model selector on the form so the link can actually be set.

The migration is guarded the same way employees0002photo is: on a fresh database
the tables come from the SQLAlchemy models, which already declare the column, so
an unconditional add fails with "duplicate column name". The foreign key is
created only on databases that can add one by ALTER; routing it through
batch_alter_table made Alembic's column sort raise "Circular dependency
detected" on the fresh-database test.

Deploying this needs `flask db upgrade` and `flask plugin upgrade-all` on the
server, not just a file copy.

frontend/src/views/settings/ModelsList.vue

@@ -138,7 +138,14 @@
               <div class="image-manage">
                 <img v-if="form.imageurl" :src="withBase(form.imageurl)" alt="Model image" class="image-thumb" />
                 <div class="image-actions">
-                  <template v-if="editingModel">
+                  <!--
+                    Offered when CREATING too. The photo is stored as
+                    model-<id>.<ext>, so it cannot be sent before the model has
+                    an id - but making the operator save, reopen and come back
+                    just to add a picture reads as a missing feature. On a new
+                    model the file is held here and uploaded immediately after
+                    the model is created.
+                  -->
                   <input
                     ref="imageFileInput"
                     type="file"
@@ -147,25 +154,35 @@
                     @change="onImageSelected"
                   />
                   <button type="button" class="btn btn-secondary btn-sm" :disabled="uploadingImage" @click="triggerImageUpload">
-                      {{ uploadingImage ? 'Uploading...' : (form.imageurl ? 'Replace' : 'Upload') }}
+                    {{ uploadingImage ? 'Uploading...' : ((form.imageurl || pendingImageFile) ? 'Replace' : 'Upload') }}
                   </button>
-                    <button v-if="form.imageurl" type="button" class="btn btn-danger btn-sm" @click="removeImage">Remove</button>
-                  </template>
-                  <small v-else class="text-muted">Save the model first, then upload a photo.</small>
+                  <button v-if="form.imageurl || pendingImageFile" type="button" class="btn btn-danger btn-sm" @click="removeImage">Remove</button>
+                  <small v-if="pendingImageFile" class="text-muted pending-note">
+                    {{ pendingImageFile.name }} will be uploaded when you save.
+                  </small>
                 </div>
               </div>
             </div>
 
             <div class="form-group">
               <label for="imageurl">Image URL</label>
+              <!--
+                type="text", NOT type="url". An uploaded photo sets this field to
+                an application path such as /api/models/image/model-120.png, and
+                native url validation demands an absolute URL with a scheme - so
+                the browser refused to save the form with "Please enter a URL"
+                for a value this page had just written itself.
+              -->
               <input
                 id="imageurl"
                 v-model="form.imageurl"
-                type="url"
+                type="text"
                 class="form-control"
-                placeholder="https://..."
+                placeholder="https://... or /api/models/image/..."
               />
-              <small class="text-muted">Manual alternative. Uploading a photo overwrites this URL.</small>
+              <small class="text-muted">
+                Set automatically when you upload a photo. Accepts a full web address or a path on this server.
+              </small>
             </div>
 
             <div class="form-group">
@@ -224,6 +241,8 @@ const perPage = ref(20)
 
 const showModal = ref(false)
 const editingModel = ref(null)
+// A photo chosen while CREATING a model, uploaded once the model has an id.
+const pendingImageFile = ref(null)
 const saving = ref(false)
 const error = ref('')
 
@@ -337,6 +356,9 @@ function openModal(m = null) {
 function closeModal() {
   showModal.value = false
   editingModel.value = null
+  // Or a photo picked for a model that was never saved would be uploaded onto
+  // the NEXT model created in this session.
+  pendingImageFile.value = null
 }
 
 async function saveModel() {
@@ -351,7 +373,20 @@ async function saveModel() {
     if (editingModel.value) {
       await modelsApi.update(editingModel.value.modelnumberid, data)
     } else {
-      await modelsApi.create(data)
+      const created = await modelsApi.create(data)
+      // Send a photo chosen before the model existed. A failure here must not
+      // read as "the model was not saved" - it was.
+      if (pendingImageFile.value) {
+        const newId = created?.data?.data?.modelnumberid
+        if (newId) {
+          try {
+            await modelsApi.uploadImage(newId, pendingImageFile.value)
+          } catch (imgErr) {
+            console.error('Error uploading image:', imgErr)
+            toast.error(apiError(imgErr, 'Model saved, but the photo could not be uploaded'))
+          }
+        }
+      }
     }
     closeModal()
     loadModels()
@@ -369,7 +404,14 @@ function triggerImageUpload() {
 
 async function onImageSelected(event) {
   const file = event.target.files?.[0]
-  if (!file || !editingModel.value) return
+  if (!file) return
+  // No model id yet, so hold the file and send it once saveModel has created
+  // the record. Nothing is written to the server until the operator saves.
+  if (!editingModel.value) {
+    pendingImageFile.value = file
+    if (imageFileInput.value) imageFileInput.value.value = ''
+    return
+  }
   uploadingImage.value = true
   try {
     const response = await modelsApi.uploadImage(editingModel.value.modelnumberid, file)
@@ -387,7 +429,12 @@ async function onImageSelected(event) {
 }
 
 async function removeImage() {
-  if (!editingModel.value) return
+  // On a new model there is nothing on the server yet; just drop the choice.
+  if (!editingModel.value) {
+    pendingImageFile.value = null
+    form.value.imageurl = ''
+    return
+  }
   if (!confirm('Remove this model image?')) return
   try {
     await modelsApi.removeImage(editingModel.value.modelnumberid)

plugins/network/api/routes.py

@@ -346,7 +346,7 @@ def create_network_device():
 
     Optional fields:
     - name, serialnumber, statusid, locationid, businessunitid
-    - networkdevicetypeid, vendorid, hostname
+    - networkdevicetypeid, vendorid, modelnumberid, hostname
     - firmwareversion, portcount, ispoe, ismanaged, rackunit
     - mapx, mapy, notes
     """
@@ -408,6 +408,7 @@ def create_network_device():
         assetid=asset.assetid,
         networkdevicetypeid=data.get('networkdevicetypeid'),
         vendorid=data.get('vendorid'),
+        modelnumberid=data.get('modelnumberid'),
         hostname=data.get('hostname'),
         firmwareversion=data.get('firmwareversion'),
         portcount=data.get('portcount'),
@@ -495,7 +496,7 @@ def update_network_device(device_id: int):
             setattr(asset, key, data[key])
 
     # Update network device fields
-    netdev_fields = ['networkdevicetypeid', 'vendorid', 'hostname',
+    netdev_fields = ['networkdevicetypeid', 'vendorid', 'modelnumberid', 'hostname',
                      'firmwareversion', 'portcount', 'ispoe', 'ismanaged', 'rackunit']
     for key in netdev_fields:
         if key in data:

plugins/network/frontend/views/NetworkDeviceForm.vue

@@ -133,6 +133,23 @@
                 </option>
               </select>
             </div>
+            <div class="form-group">
+              <!--
+                The catalog model is where the device photo comes from: the
+                detail page's hero image reads the model's imageurl, exactly as
+                machines, PCs and printers do.
+              -->
+              <label for="modelnumberid">Model</label>
+              <select id="modelnumberid" v-model="form.modelnumberid" class="form-control">
+                <option value="">Select Model</option>
+                <option v-for="m in models" :key="m.modelnumberid" :value="m.modelnumberid">
+                  {{ m.modelnumber }}
+                </option>
+              </select>
+            </div>
+          </div>
+
+          <div class="form-row">
             <div class="form-group">
               <label for="firmwareversion">Firmware Version</label>
               <input
@@ -251,6 +268,7 @@ import { useRoute, useRouter } from 'vue-router'
 import {
   networkApi,
   vendorsApi,
+  modelsApi,
   locationsApi,
   assetsApi,
   businessunitsApi
@@ -284,6 +302,7 @@ const form = ref({
   hostname: '',
   networkdevicetypeid: '',
   vendorid: '',
+  modelnumberid: '',
   firmwareversion: '',
   portcount: null,
   rackunit: '',
@@ -296,6 +315,7 @@ const form = ref({
 
 const deviceTypes = ref([])
 const vendors = ref([])
+const models = ref([])
 const locations = ref([])
 const statuses = ref([])
 const businessUnits = ref([])
@@ -329,6 +349,8 @@ async function loadVendors() {
   try {
     const response = await vendorsApi.list({ perpage: 100 })
     vendors.value = response.data.data || []
+    const modelResponse = await modelsApi.list({ perpage: 500 })
+    models.value = modelResponse.data.data || []
   } catch (err) {
     console.error('Error loading vendors:', err)
   }
@@ -385,6 +407,7 @@ async function loadDevice() {
       form.value.hostname = data.networkdevice.hostname || ''
       form.value.networkdevicetypeid = data.networkdevice.networkdevicetypeid || ''
       form.value.vendorid = data.networkdevice.vendorid || ''
+      form.value.modelnumberid = data.networkdevice.modelnumberid || ''
       form.value.firmwareversion = data.networkdevice.firmwareversion || ''
       form.value.portcount = data.networkdevice.portcount
       form.value.rackunit = data.networkdevice.rackunit || ''
@@ -414,6 +437,7 @@ async function submitForm() {
       hostname: form.value.hostname || null,
       networkdevicetypeid: form.value.networkdevicetypeid || null,
       vendorid: form.value.vendorid || null,
+      modelnumberid: form.value.modelnumberid || null,
       firmwareversion: form.value.firmwareversion || null,
       portcount: form.value.portcount || null,
       rackunit: form.value.rackunit || null,

plugins/network/migrations/versions/0002_networkdevice_model.py

@@ -0,0 +1,58 @@
+"""Link a network device to a catalog model.
+
+Machines, PCs and printers all carry modelnumberid, which is what lets an asset
+detail page show the photo uploaded against its vendor model. Network devices
+carried only vendorid, so NetworkDeviceDetail.vue rendered a hero image bound to
+`networkdevice.imageurl` that nothing could ever populate - a feature that
+looked present and could not work.
+
+Nullable, with no backfill: a device whose model is not recorded is normal, and
+guessing one from the vendor would put wrong photos on real hardware.
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = 'network0002model'
+down_revision = 'network0001anchor'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # Guarded exactly like employees0002photo. On a FRESH database the tables
+    # are built from the SQLAlchemy models, which already declare this column,
+    # so an unconditional add fails with "duplicate column name". On an existing
+    # server the column really is missing and gets added.
+    bind = op.get_bind()
+    insp = sa.inspect(bind)
+    if 'networkdevices' not in insp.get_table_names():
+        return
+    cols = {c['name'] for c in insp.get_columns('networkdevices')}
+    if 'modelnumberid' not in cols:
+        op.add_column('networkdevices', sa.Column('modelnumberid', sa.Integer(), nullable=True))
+        # Only where it is possible. SQLite has no ALTER TABLE ADD CONSTRAINT,
+        # and routing this through batch_alter_table - which rebuilds the table -
+        # made Alembic's column sort raise "Circular dependency detected". The
+        # column is what the relationship needs; the constraint is integrity on
+        # the real database.
+        if bind.dialect.name != 'sqlite':
+            op.create_foreign_key(
+                'fk_networkdevices_modelnumberid',
+                'networkdevices', 'models',
+                ['modelnumberid'], ['modelnumberid'],
+            )
+
+
+def downgrade():
+    bind = op.get_bind()
+    insp = sa.inspect(bind)
+    if 'networkdevices' not in insp.get_table_names():
+        return
+    cols = {c['name'] for c in insp.get_columns('networkdevices')}
+    if 'modelnumberid' in cols:
+        if bind.dialect.name != 'sqlite':
+            op.drop_constraint('fk_networkdevices_modelnumberid', 'networkdevices',
+                               type_='foreignkey')
+        op.drop_column('networkdevices', 'modelnumberid')

plugins/network/models/network_device.py

@@ -49,6 +49,13 @@ class NetworkDevice(BaseModel):
     )
 
     # Vendor
+    modelnumberid = db.Column(
+        db.Integer,
+        db.ForeignKey('models.modelnumberid'),
+        nullable=True,
+        comment='Catalog model, which is where the device photo comes from'
+    )
+
     vendorid = db.Column(
         db.Integer,
         db.ForeignKey('vendors.vendorid'),
@@ -97,6 +104,7 @@ class NetworkDevice(BaseModel):
     )
     networkdevicetype = db.relationship('NetworkDeviceType', backref='networkdevices')
     vendor = db.relationship('Vendor', backref='network_devices')
+    model = db.relationship('Model', backref='network_devices')
 
     __table_args__ = (
         db.Index('idx_netdev_type', 'networkdevicetypeid'),
@@ -116,5 +124,11 @@ class NetworkDevice(BaseModel):
             result['networkdevicetypename'] = self.networkdevicetype.networkdevicetype
         if self.vendor:
             result['vendorname'] = self.vendor.vendor
+        # Same shape as machines, PCs and printers: the detail page's hero image
+        # binds to imageurl, and it comes from the catalog model, not the device.
+        if self.model:
+            result['modelname'] = self.model.modelnumber
+            if self.model.imageurl:
+                result['imageurl'] = self.model.imageurl
 
         return result

tests/test_plugin_migrations.py

@@ -58,6 +58,9 @@ EXPECTED_HEAD_REVISION['machines'] = 'machines0002rename'
 EXPECTED_HEAD_REVISION['employees'] = 'employees0002photo'
 # usb drops the dead usbcheckouts.machineid column on top of its anchor.
 EXPECTED_HEAD_REVISION['usb'] = 'usb0002dropmachineid'
+# network links a device to a catalog model, which is where its photo comes
+# from - machines, PCs and printers already had that link.
+EXPECTED_HEAD_REVISION['network'] = 'network0002model'
 # printedparts is post-cutover: its 0001 really creates its tables; 0004 adds
 # the per-transaction revision column.
 EXPECTED_HEAD_REVISION['printedparts'] = 'printedparts0004txnrev'