Model photo upload fixes; network devices linked to catalog models

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>
+                  <button v-if="form.imageurl || pendingImageFile" type="button" class="btn btn-danger btn-sm" @click="removeImage">Remove</button>
-                  </template>
+                  <small v-if="pendingImageFile" class="text-muted pending-note">
-                  <small v-else class="text-muted">Save the model first, then upload a photo.</small>
+                    {{ 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'