- ๐บ๐ธUnited States smustgrave
Moving to NW for the suggestions made by @longwave.
- ๐บ๐ธUnited States bradjones1 Digital Nomad Life
Marking this NR - I haven't chased my tail like this in quite a while but I think this ended up in a decent place with a minimum amount of change to logic. Helps reveal where we were depending on cloned properties rather than being explicit.
- ๐บ๐ธUnited States bradjones1 Digital Nomad Life
OK, a few hours of my life gone into the depths of the meaning of "original" in various contexts around Drupal.
The genesis of this issue appears to be getting away from a dynamic property, but it's morphed into a larger problem space occupied by related issues such as ๐ Impossible to update an entity revision if the field value you are updating matches the default revision. Fixed . There's also a heavy dose of translations weirdness here.
Overall, the patch is pretty simple in that it replaces gets and sets of $entity->original with new getOriginal/setOriginal methods accessing what will soon be a protected property of the entity. Simple enough to understand that.
Even though that feels like it should go very smoothly with no regressions, test failures cropped up in code that must use the now-deprecated original property to do all sorts of magic like deciding when a translation has changed. I'm not entirely sure what broke this in every instance but I think it's related to the particulars of cloning the "parent" entity into the translations array.
This feels overall like the right cleanup to do, but the collateral damage is gnarly (as mentioned way back in #5. As you can see from the recent commit history, I went back and forth on various hacks and had myself questioning "how does any of this even work right now?" but we're down to a few failing tests which is pretty manageable.
One failing test in particular has an ominous link to a recently-closed issue relating to the behavior of the original property with revisions. @Berdir mentioned at #2859042-54: Impossible to update an entity revision if the field value you are updating matches the default revision. โ that the method introduced in this issue is the correct path forward in smoothing out these special cases. So let's do that?
This one is pretty gnarly but I think this is a move in the right direction and I'm happy to see mostly green with a limited amount of hack.