Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
To reproduce:
* Publish node 123 tagged with taxonomy term 'example'
* Visit the taxonomy term view page for 'example'
* Note that the view contains node 123
* Save a new draft for node 123
* Revisit the 'example' page and it no longer contains node 123, even though node 123 still has a published revision
Under the hood, I see that saving a draft removes node 123 from the taxonomy_index table. Happy to help there's a good way to chip in.
Comment | File | Size | Author |
---|---|---|---|
#17 | 2708735-17.patch | 4.23 KB | amateescu |
Comments
Comment #2
eeeschwartz CreditAttribution: eeeschwartz commentedComment #3
Crell CreditAttribution: Crell at Palantir.net for Acquia commentedHm. This could be a core bug; it shouldn't be removing items if the default revision is published. Basically, the code that manages that table is not forward-revision aware.
Reassigning to core, where I believe the bug needs to be fixed.
Comment #4
jibranHow can we reproduce this in core?
Comment #5
Crell CreditAttribution: Crell at Palantir.net for Acquia commentedI doubt it could be replicated exclusively with core, because nothing in core creates forward revisions. With core's facilities, default revision == most recent revision is always true. That means this situation could not be created. It's only when using modules such as Workbench Moderation, which create forward revisions, that such issues would be noticed.
Comment #6
dawehnerHere is a test, I just wanted to see what is going on, and yeah this is totally reproducible.
Comment #7
Crell CreditAttribution: Crell at Palantir.net for Acquia commentedComment #9
unstatu CreditAttribution: unstatu as a volunteer commentedComment #10
unstatu CreditAttribution: unstatu as a volunteer commentedDawehner told me in the DrupalCampES that the Core API implements the forward revisions, but not the UI for handling them.
Modules like Workbench Moderation provide the needed UI.
I have done some research and I think the problem is in the Context Filter defined in the Taxonomy Term view (admin/structure/views/view/taxonomy_term).
Comment #11
eeeschwartz CreditAttribution: eeeschwartz commented@unstatu I wouldn't _think_ the issue would be at the view level.
As far as I can tell the view is behaving correctly: it looks at taxonomy_index for published nodes related to a given tid. The problem seems to be that saving a draft removes the published nid from the taxonomy_index table.
Comment #12
catchThis is a difference between the forward revision approach of drafty (which was done only out of necessity) vs. the one in core.
Drafty first saves the draft revision, then re-saves the live revision - resulting in all hook_entity_*() hooks being fired again, so that any tables which aren't revision-aware get restored to their pre-draft/published state. This means that a revision-unaware hook like taxonomy_node_update() in 7.x with drafty doesn't have to do anything special (and it can't even if we wanted it to due to lack of forward revision support in core).
Core 8.x saves the new revision without touching the live revision at all, but then taxonomy_node_update() needs to 1. know it's getting a draft revision, not a live one 2. return early.
Short version of that:
taxonomy_node_update() should check $entity->isDefaultRevision(). But we should also consider adding docs to the entity CRUD hooks to point out they fire both when saving default and non-default revisions.
Comment #13
markdorisonImplemented a basic version of @catch's suggestion in #12 to verify the previously submitted test.
Comment #15
jeqqComment #17
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedI think this assertion is wrong, the taxonomy index should not change when we're not dealing with the default revision. That's the whole point of this issue :)
Fixed the point above in this patch and also cleaned it up a little.
Comment #19
jibranLooks good now.
Comment #20
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedForgot the tag :)
Comment #23
catchCommitted/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!