| [08:05:32] | <symbioquine[m]> | <mstenta[m]> "symbioquine: ready for review..." <- 🏸 |
| [08:12:40] | <mstenta[m]> | symbioquine: thanks! good points... forgot about equipment and inventory :-/ |
| [08:12:51] | <mstenta[m]> | i'll review and respond in more detail in the PR... |
| [11:47:59] | <symbioquine[m]> | <mstenta[m]> "symbioquine: thanks! good points..." <- I'm afk, but I wonder if the set of fields to be checked should be populated by a hook implemented in each module which adds log fields... |
| [11:48:51] | <mstenta[m]> | Mm yea perhaps |
| [11:49:04] | <mstenta[m]> | I'm looking into merging all of these into a single constraint right now, as a first step |
| [11:49:07] | <symbioquine[m]> | E.g. the location module (or perhaps a submodule thereof) would "tell" the constraint logic to check the location field. |
| [11:49:18] | <mstenta[m]> | Adding a hook would be a simple next step (fixup) |
| [11:50:30] | <symbioquine[m]> | The nice thing about a submodule is that it could be separately disabled if admins want cross-farm locations. |
| [11:50:40] | <mstenta[m]> | The inventory one is trickier... but I'll look into that once I get the others working |
| [11:51:03] | <mstenta[m]> | Interesting idea re: submodule... |
| [11:51:24] | <mstenta[m]> | I think we should give cross-farm relationships more thought before we provide any option to enable it though |
| [11:51:29] | <symbioquine[m]> | symbioquine[m]: Maybe that's not sufficient granulatity of control though so maybe it needs more thought... |
| [11:51:36] | <mstenta[m]> | (too much in my head to consider that right now) |
| [11:51:38] | <symbioquine[m]> | s/granulatity/granularity/ |
| [11:52:41] | <mstenta[m]> | I'm going to put all fields into the one constraint first, and make it an entity-level constraint (instead of field-level) |
| [11:53:27] | <symbioquine[m]> | I'll be afk for the rest of the day probably (unless I get really lucky with kiddo naps), but should be on early again tomorrow. |
| [11:53:54] | <mstenta[m]> | Cool thanks for the input symbioquine! |
| [11:54:04] | <mstenta[m]> | I'll try to get something ready for review by tomorrow |
| [11:54:33] | <mstenta[m]> | (doing all fixup commits right now... so you can follow changes, but ultimately might be easiest to review as a single squashed commit - we'll see) |
| [12:30:15] | <mstenta[m]> | <mstenta[m]> "Adding a hook would be a..." <- This might have some complexities/considerations to it... maybe we should just focus on the core fields for now, hard code them, and then reassess as a refactor/feature request post-4.0.0 |
| [12:49:16] | <mstenta[m]> | (For one, I'm not sure if it's possible to inject services into constraint validators... 🤔) |
| [12:49:39] | <mstenta[m]> | (Also, the inventory_asset check on quantity requires completely different logic than the simple field names) |
| [12:50:15] | <mstenta[m]> | I think I have the inventory_asset logic sketched up... adding tests now to confirm it works |
| [13:05:28] | <mstenta[m]> | symbioquine: OK I think it's ready for review again. Didn't do the hook piece, but `asset`, `equipment`, `group`, `location`, and `quantity->inventory_asset` fields are all being checked now (with automated tests) |