Problem/Motivation

If you enable page cache for anonymous users, the page cache isn't always cleared when a moderation state change impacts a published revision.
That is; when node_save() saves the node instead of a regular node form.
And if you don't have an extra module like Purge configured to clear certain caches on node_save().

Proposed resolution

Introduce an option to cache_clear_all() when a published revision changes.
Set the option off by default to provide seamless backwards compatibility.

Depend on #1436260: Saving nodes outside Workbench Moderation leads to incorrect state transitions (e.g., "needs review" appearing as published) because that issue provides a patch that adds a function that's only called when a published revision changes. And removes a node_save() call with it.

Extend an existing Simpletest test to cover the use case explained earlier.

User interface changes

None.

API changes

A new variable, called workbench_moderation_cache_clear_all

CommentFileSizeAuthor
#23 workbench_moderation-option_cache_clear_all-2377423-23.patch3.31 KBhmdnawaz
#20 workbench_moderation-option_cache_clear_all-2377423-18.patch3.29 KBhmdnawaz
#17 workbench_moderation-option_cache_clear_all-2377423-17.patch3.29 KBhmdnawaz
#16 workbench_moderation-option_cache_clear_all-2377423-16.patch3.83 KBhmdnawaz
#14 workbench_moderation-option_cache_clear_all-2377423-14.patch3.29 KBangel.h
#12 workbench_moderation-option_cache_clear_all-2377423-12.patch6.57 KBiSoLate
#10 workbench_moderation-option_cache_clear_all-2377423-10.patch703 bytesiSoLate
#6 combined_with_1436260_and_against_1.3-2377201-6.patch18.77 KBPieterDC
#5 combined_with_1436260_and_against_1.3-2377201-5.patch9.68 KBPieterDC
#2 interdiff.txt3.28 KBPieterDC
#2 clear_page_cache_after_node_save-2377201-2.patch10.23 KBPieterDC
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

PieterDC’s picture

PieterDC
Primary language Dutch
Location Ghent
CreditAttribution: PieterDC commented

Numerous caching-related issues already exist, but none of them clearly defined a use case or reproduction steps, matching mine.

So, I opted to create a new issue.

PieterDC’s picture

PieterDC
Primary language Dutch
Location Ghent
CreditAttribution: PieterDC commented
Assigned: PieterDC » Unassigned
FileSize
clear_page_cache_after_node_save-2377201-2.patch10.23 KB
interdiff.txt3.28 KB

Attaching patch.

bertramakers’s picture

bertramakers CreditAttribution: bertramakers commented

I reviewed the interdiff with #1436260: Saving nodes outside Workbench Moderation leads to incorrect state transitions (e.g., "needs review" appearing as published) as part of a bigger review of an internal ticket at the place where I work with PieterDC.
I can't seem to find any problems with the changes, there's backwards compatibility provided so it won't break anything existing either.

I'm marking this as RTBC but obviously it will need to wait until #1436260: Saving nodes outside Workbench Moderation leads to incorrect state transitions (e.g., "needs review" appearing as published) is committed before this can be committed.

bertramakers’s picture

bertramakers CreditAttribution: bertramakers commented
Status: Needs review » Reviewed & tested by the community
PieterDC’s picture

PieterDC
Primary language Dutch
Location Ghent
CreditAttribution: PieterDC commented
FileSize
combined_with_1436260_and_against_1.3-2377201-5.patch9.68 KB

Rerolled the combined patch against 7.x-1.3 (for use in our distribution).

PieterDC’s picture

PieterDC
Primary language Dutch
Location Ghent
CreditAttribution: PieterDC commented
FileSize
combined_with_1436260_and_against_1.3-2377201-6.patch18.77 KB

Forgot to add the new files in the patch. Added now. New patch attached.

colan’s picture

colan
he/him
Primary language English
Location Toronto 🇨🇦
CreditAttribution: colan commented
Status: Reviewed & tested by the community » Needs work

No longer applies cleanly; needs a re-roll.

iSoLate’s picture

iSoLate CreditAttribution: iSoLate commented

.

iSoLate’s picture

iSoLate CreditAttribution: iSoLate commented
Status: Needs work » Closed (duplicate)
iSoLate’s picture

iSoLate CreditAttribution: iSoLate commented
Version: 7.x-1.x-dev » 7.x-1.4
Status: Closed (duplicate) » Needs review
FileSize
workbench_moderation-option_cache_clear_all-2377423-10.patch703 bytes

Created a patch against 1.4...

Status: Needs review » Needs work

The last submitted patch, 10: workbench_moderation-option_cache_clear_all-2377423-10.patch, failed testing.

iSoLate’s picture

iSoLate CreditAttribution: iSoLate commented
Status: Needs work » Needs review
FileSize
workbench_moderation-option_cache_clear_all-2377423-12.patch6.57 KB

Updated the test code to comply.

PieterDC’s picture

PieterDC
Primary language Dutch
Location Ghent
CreditAttribution: PieterDC at Randstad Digital commented
Version: 7.x-1.4 » 7.x-1.x-dev
Status: Needs review » Needs work

Needs work. Reroll the patch against 7.x-1.x

  • Remove the unrelated test fixes. They got fixed in commit 7536599 of #2454131: Fix failing tests Commit f5abf2f contains more test fixes alike. Not included in 1.4, but currently only in 1.x
  • Re-add the extra test coverage the original patch of this ticket added; see the interdiff from the second comment: https://www.drupal.org/files/issues/interdiff_8046.txt to discover what this issue is actually about. I understand the initial patch could be a bit daunting because it included a patch from another issue. But that's what the interdiff is for ;-)
angel.h’s picture

angel.h
Primary language English
Location Plovdiv
CreditAttribution: angel.h commented
Status: Needs work » Needs review
FileSize
workbench_moderation-option_cache_clear_all-2377423-14.patch3.29 KB

Patch rerolled for 7.x-1.x.

PieterDC’s picture

PieterDC
Primary language Dutch
Location Ghent
CreditAttribution: PieterDC at Randstad Digital commented
hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
FileSize
workbench_moderation-option_cache_clear_all-2377423-16.patch3.83 KB

Patch rerolled for 7.x-3.0

hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
FileSize
workbench_moderation-option_cache_clear_all-2377423-17.patch3.29 KB

The last submitted patch, 16: workbench_moderation-option_cache_clear_all-2377423-16.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 17: workbench_moderation-option_cache_clear_all-2377423-17.patch, failed testing. View results

hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
FileSize
workbench_moderation-option_cache_clear_all-2377423-18.patch3.29 KB
hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 20: workbench_moderation-option_cache_clear_all-2377423-18.patch, failed testing. View results

hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
FileSize
workbench_moderation-option_cache_clear_all-2377423-23.patch3.31 KB

Updated the patch for 7.x-3.0 to pass the test.

hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 23: workbench_moderation-option_cache_clear_all-2377423-23.patch, failed testing. View results

hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
Version: 7.x-1.x-dev » 7.x-3.x-dev
Status: Needs work » Needs review
hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented

Status: Needs review » Needs work

The last submitted patch, 23: workbench_moderation-option_cache_clear_all-2377423-23.patch, failed testing. View results

hmdnawaz’s picture

hmdnawaz CreditAttribution: hmdnawaz commented
Status: Needs work » Needs review

The test is passed for the 7.x-3.x branch.