Dynamic Enum Fix #547
Dynamic Enum Fix #547
Conversation
|
This needs tests still but I wanted to make sure others were ok with this change before doing so. |
app/collins/models/AssetMeta.scala
Outdated
| @@ -201,6 +201,7 @@ object AssetMeta extends Schema with AnormAdapter[AssetMeta] with AssetMetaKeys | |||
| } | |||
|
|
|||
| // Post enum fields, enum is not safe to extend with new values | |||
| type DynamicEnum = AssetMeta | |||
There was a problem hiding this comment.
will remove this... please ignore.
Adds support so that when a value that is of type Dynamic Enum is updated after initial intake the values are cleared from the initial run and the new ones are added.
Related to tumblr#547
|
@michaeljs1990 what kind of test cases can be added to prove behavior? |
|
I was going to first induct a machine and then update the asset with the same LSHW except for passing in a different value for BaseDescription, BaseProduct, BaseVendor, and BaseSerial in the xml. Currently it would show the original values from the initial induction process as well as having two entries in the database for each of the above attached to the asset. |
|
@michaeljs1990 if we can do the induction in a test, that would be ideal. That way we know nothing breaks now, and in the future with more changes that will be likely made to this codebase. |
Checks to ensure that we are properly updating assets in the database cleaning up old values and returning the new ones.
|
I think the added tests should cover all cases. Wasn't sure where to put this test though it makes use of the controller helpers a bunch but it's actually testing a sub section of AssetMeta. |
| // update hw details (lshw / lldp) - cannot proceed without this | ||
| val lshwData = getResource("lshw-basic.xml") | ||
| val lshwData = getResource(lshwXml) | ||
| val lldpData = getResource("lldpctl-two-nic.xml") |
There was a problem hiding this comment.
can this be parameterized, as well? While you are in there :)
There was a problem hiding this comment.
ha, for sure. I can get a second chance at travis not snuffing me on the solr test as well 😆
|
Ive kicked the tests again, because i think |
Allow for optionally overwritting the LLDP XML used when updating an asset for testing.
|
@defect @roymarantz can you take a look? This is good for landing |
|
Been running this in prod for a few weeks now without issue side by side with the latest GPU changes. |
|
If we dont hear anyone reject this in a few days, I'll land this. This is an awesome fix! |
|
🔥 🌶 🐝 |
Adds support so that when a value that is of type Dynamic Enum is updated
after initial intake the values are cleared from the initial run and the
new ones are added.
See #546