- π§πͺBelgium kristiaanvandeneynde Antwerp, Belgium
I am currently facing this issue in the Group module where I feel I have to reject a patch that sets content entities to syncing because the interface specifically points out this is mainly about data imports (config import, migrate, ...): π Node update hook is triggered upon group content create Needs work
They are correctly figuring out that workspaces will not create a new revision on syncing content and therefore set the flag to prevent that from happening. It would be cool if we kept this flag (isSyncing) for the intended purpose as documented on the interface, but also introduce a new flag that can only be checked for updates: isInternalUpdate.
When this flag is set you could also choose not to create a revision, but at least you'd have the choice to differentiate between imported content and content that was (updated with internal data only. Either way, I think it's dangerous to repurpose an existing interface as you cannot know what contrib has already done with it.
So maybe leave SynchronizableInterface as is, but come up with a new interface for internal data changes such as metadata, certain flags, adding something to a group, updating nested set values (left and right), ...
- π³πΏNew Zealand john pitcairn
+1 to the above. Useful in update hooks where you're fixing bad data, updating data formats, etc. Setting
isSyncing
before save when doing so just seems like an abuse of the flag for that purpose. - πΊπΈUnited States dww
This is no longer postponed on anything, so removing the "[PP-2]" prefix. Curious about how this unfolds.
I'm basically in agreement with #13, although one underlying problem is that this existing interface isn't well documented / defined. So even if we "leave it alone", I support trying to improve the docs and clarify what it was supposed to mean. π
But yes, perhaps adding a new interface (with clear docs and scope) for "internal updates" or whatever would be prudent.
Thanks,
-Derek