Bulk Editing Custom Fields

[spencerrlongg]

Description

Happy to add more on the UI side if it's wanted, but I think this works pretty well and seems fairly obvious what's going on.

The warning at the top of the page now mentions how many different models you're editing in edition to the number of assets:

And the custom fields are separated with their corresponding model at the bottom as well.

Not much else changes on the UI side, let me know if this seems good.

Fixes #3054

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I've tested locally and everything seems good, could probably use a second set of eyes to make sure I'm not missing anything.

Test Configuration:

• PHP version: 8.0.28
• MySQL version 8.0.32

[what-the-diff]

PR Summary

• Added Custom Fields Logic for Bulk Edit
  This change will allow users to edit multiple items with their custom fields simultaneously, improving efficiency and user experience.

• Updated View to Display Models and Custom Fields
  The view has been updated to display all available models and their respective custom fields, making it easier for users to access and manage the custom fields for different models.

[snipe]

This looks like a great start, Spencer. How do we (or do we?) account for fields that appear in multiple field sets/models?

[spencerrlongg]

Ah, good one. Should work on the backend, but no callout on the UI side. Moving back to draft and I'll put some thought into how to display that.

[spencerrlongg]

Alright, changed to only list a custom field that is present on multiple models/fieldsets once, and to include which models every field is present on.

Achieved this through a bunch of trial and error, and eventually adding new methods to a couple of models.

CustomField::find(1)->assetModels() will now return all asset models that have that CustomField

AssetModel::find(1)->customFields is more or less a shortcut for AssetModel::find(1)->fieldset->fields

It's a little ugly, but I think everything works as intended from my testing - would be happy to have some more eyes on it though.

[marcusmoore]

Following the queries in here made my Thursday afternoon brain 🫠 (as queries sometimes do) so I plan on looking at this with fresh eyes later but a couple things stood out at first pass.

Along with the inline comments: when I loaded the bulk edit page with 4 assets and 2 model types, Debugbar showed there were 21 duplicate queries on the page. I have a feeling it is from the new methods in AssetModel and CustomField calling relationships via () instead of having them eager loaded. I've run into this problem many times and it's not always easy to solve. When my brain is less melty I can help debug it.

[marcusmoore]

FYI: me hitting "Request Changes" on this is more for the translation string than the duplicate queries. We can always circle back and clean that up.

[spencerrlongg]

Ok, this should be ready for review/more testing now @snipe - this is what the validation error looks like:

[marcusmoore]

(I'm gonna wrap up my review tomorrow 👍🏾)

[marcusmoore]

Nice work! I know it wasn't easy to wire this all up.

I left a couple inline comments and also noticed that the MAC address field that we seed in development didn't behave the way I expected. Unlike the IMEI field that also has regex validation, when I submitted an invalid MAC address the operation went through successfully without displaying an error message. The other fields I included were updated but MAC address was left blank. The IEMI field kicked back with an error message.

[spencerrlongg]

I left a couple inline comments and also noticed that the MAC address field that we seed in development didn't behave the way I expected. Unlike the IMEI field that also has regex validation, when I submitted an invalid MAC address the operation went through successfully without displaying an error message. The other fields I included were updated but MAC address was left blank. The IEMI field kicked back with an error message.

Hm - this makes almost no sense to me. Functionally IMEI and MAC should be the exact same thing. But I am seeing the same thing. IMEI works as expected and MAC doesn't. It does work fine when it's given a valid MAC address.

Can't find anything that's different about it vs any other custom fields. Open to any ideas about it.

Also just realized something else, certain fields should probably never be bulk-editable, like anything marked 'unique' - MAC address isn't marked unique in this instance but probably should be.

[marcusmoore]

@spencerrlongg that's a great catch. I'm not sure if there are other scenarios to consider but fields marked unique should definitely be un-editable here. Like we talked about elsewhere it probably makes sense to show the field but disable it so users can't modify it.

I'm gonna do a little more digging on what's up with the MAC address validation.

[marcusmoore]

Ha! You just pushed the commit for unique fields 😄

[spencerrlongg]

Ok, unique fields is done - looks like this:

I only did it for <textarea> and <input type="text"> for now because I can't see a scenario in which the other field types could or would be unique.

[spencerrlongg]

Ok, everything resolved except for commas I guess? Haven't gotten my head wrapped around that one yet.

[spencerrlongg]

Alright @snipe - commas resolved and we should be good.

[marcusmoore]

One more request that would be a good improvement 😅

[spencerrlongg]

@snipe was there a reason this never got merged?

[snipe]

@spencerrlongg Because I'm bad at my job

[snipe]

Really nice work here, @spencerrlongg - I think we'll probably end up tweaking this a bit down the line, but this is a terrific place to start!