As I'm writing out my solution for this, I'm almost wondering if it's
time for the refactor I've been Theoretically Planning Someday, to move
items to a real `ItemAppearance` model in the database similar to
`PetState`… Hmm hmm hmm…
For now though, I'm taking a break!
This bug never made it into production I think, it was a consequence of
some of how I refactored stuff in the recent changes? I think??
But yeah, I refactor how we manage `SwfAsset#body_id`, to be a bit more
explicit about when and how it can change, instead of the weird
callbacks that tbqh have bit us too often…
Ah right, the callbacks in `ParentSwfAssetRelationship` don't get
called when Rails does automatic join-model management stuff. We need
the `Item` to call its `update_cached_fields` callback itself, too!
When fixing this, I found a new bug that arose, in how we infer
`body_id` for assets that fit all pets. Fixing that next!
This gives better output when they fail, and also avoids spurious
failures like when an array for `cached_compatible_body_ids` is replaced
by an identical one! (I'm running into this right now, and yeah, it
helps a lot lol)
Hmm, I think I made a mistake on `modeling_snapshot.rb:69`: I'm
assigning the *entire* `item.swf_assets` relation to *just* the assets
for the new model of it, which breaks all the other connections.
First, I'm disabling modeling. Then, I'll restore a backup. Then, I'll
write tests for that case, and fix it up!
Not actually touching alt style yet, just the very basic stuff about
how alt style can cause loading to fail in certain extremely rare cases
(specifically, if it's our first time seeing the underlying
color/species combo too, which… isn't gonna happen irl on DTI for a long
time if ever, I would guess, but hey!)
That is, if everything is the same as before, we don't need to change
anything in our database!
I also learned a bit more about RSpec syntax sugars, it's cute!
The NC Pet Styles sentence getting broken across two lines I think
makes it too hard to notice.
Design-wise, it would be nice to just call better attention to this
feature altogether in some higher-level design-language-y way, but!
Whatever!
If you check this box, it'll keep you in a mode where saving an alt
style redirects you to the *next* one that needs labeling, until
they're all done. Useful for big drops!
I want to not turn it off entirely, so that if there's a nasty one it
becomes visible, but we don't need all that vertical space for this
small test suite rn!
I forget, there was some tricky debugging about getting the fixtures
right, I think the previous commit doesn't *actually* pass from a clean
setting. Ah well, looks good now!
Just getting a basic foothold here. I'm thinking about moving this to
RSpec, cuz I feel like the assertions are gonna get pretty specific
and groupable.
I'm gonna work on adding modeling tests, and I want to not be breaking
them without realizing! The trade history ones are good to be checking
more often like this, too.
In 540ce08caa, I updated the Item class
to be more explicit about what fields are required, so this test would
fail in a more helpful way, instead of just crashing from `name` being
`nil` when trying to infer the Dyeworks info.
Now, we update the test to use Rails's standard "fixture" system to set
up a more-correct placeholder item, instead!
Catch missing fields in validation before sending it to the DB, and
skip the Dyeworks stuff if the name is missing.
I ran into this looking into `test/trade_activity_test.rb`, which fails
right now because we try to create a boring placeholder item with
minimal fields, which Dyeworks can't call `name.match()` on!
Now, the test fails with a more helpful error about the item being
invalid. Next, I'll fix that!
