This isssue is a spin-off from #2798689: Scheduler integration with core Content moderation as it is a general problem, not solely to do with content moderation. Here are the comments relating to this problem:
#43 Steps to repoduce:
- enable scheduling and content moderation on content type article
- create an article, published
- edit the article and save the draft
- edit the draft, add a publishing date and set publish state to "published"
- run cron when the publishing date is passed
- result : the draft is not published and there is still a publishing date programmed (indicated in the scheduler fieldset)
#51 I can confirm the same bug as in #43 above. If you have an existing published node, create a new draft and set a publishing date, the new draft is not transitioned into published. It does print the message "This post is unpublished and will be published..." but it doesn't show up on Scheduled tab and after cron runs the scheduled date remains populated in the fieldset. It seems to only move the latest revision through the state transitions, but not pending drafts when there is an existing published revision. Publishing drafts that do not have any published revisions before it is working correctly.
#170 Interdiff for latest revision fix
#178 Probably affects everyone using revisions, content_translation springs into mind.
Loading of latest revision is clumsy - the revision information is returned by entityQuery, then thrown away in $nids = array_unique(array_merge($nids, $this->nidList($action))); and later corrected by loadNodes).
#190 LoadMultiple() will not return latest revision, merging the values from hook_scheduler_nid_list() is not the problem, even though EntityQuery returns valid nids, loadMultiple() will possibly still get the wrong revision.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3049070-14.load_latest_revision_for_1.0_not_dev.patch | 2.56 KB | jonathan1055 |
| #2 | 3049070-1.load_latest_revision.patch | 2.5 KB | jonathan1055 |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
compare
Comments
Comment #2
jonathan1055 commentedHere are the changes for the Latest Revision fix, extracted from the patch #191/192.
This code change needs test coverage, and for that we need a programmable method of demonstrating the fault so that we can write a test which fails without this correction.
Comment #3
nsciaccaI can confirm that the issue appears fixed by this patch. The only thing missing from the UI was the Draft of the current Published node did not show up on the /admin/content/scheduled page.
I looked at the existing /scheduler_content_moderation_integration/test/src/Kernel/ModeratedContentSchedulingTest.php and it seemed to have this case in the "testPublishOfDraft" (Tests publish scheduling for a draft of a published node.) function. Is that not adequate for demonstrating the fault and the fix? It should fail without the patch and pass with the patch correct?
I don't have much experience with tests, but willing to help to get a stable release so let me know if there's anything I can take on.
Comment #4
nsciaccaHere's a patch for the Scheduled Content admin pages Views - instead of basing it off the Content base table, it bases off the Content Revision table, filtering by the latest revision that has a "publish on" or "unpublish on" value.
Comment #5
nsciaccaComment #7
justpro commentedThe first patch is working great on 8.x-1.x-dev branch.
Is there a way to run the patch against 8.x-1.0 and not 8.x-1.x-dev. Thanks, and I am following the issue.
Comment #8
nsciaccaUpdated patch to fix failed tests. Also, if you think this should be in a different issue I can move it.
Comment #9
nsciaccaComment #10
jonathan1055 commentedHi nsciacca,
Thanks for your comments in #3
For the content view, have you seen #3021005: Scheduler view should be based on the latest revision which I think is probably the same problem as you are reporting. You could add your patch there instead, and compare against the patch already added.
Jonathan
Comment #11
nsciaccaokay thanks - will check out that issue, feel free to clean this issue up and remove it. Sorry for mucking it up!
Comment #12
jonathan1055 commentedThat's no problem. I'm happy that you provided a patch.
Comment #13
jonathan1055 commentedHi justpro,
Thanks for the feedback. Here is a patch which makes the same changes but should apply to 8.x-1.0 not -dev. There will be offsets when applying but that won't affect the result.
Are you using this with Scheduler Content Moderation Integration? If not, I would be interested to know what you are needing the patch for, as that will help with the new tests I am going to try to write for this fix.
Comment #14
jonathan1055 commentedSlight update to the patch for 1.0 having actually tested it on 1.0 with Scheduler Content Integration Moderation module. It needed
$this->entityManager(which is the property in use in 1.0) instead of$this->entityTypeManagerwhich is what we now have in -dev.edit: This patch has now been used sucessfully in the test run on #3049346-17: Implement scheduler hooks and event listeners to remove hard-coding in Scheduler module
Comment #16
jonathan1055 commentedCommitted. Thank you to all who contributed here and on #2798689: Scheduler integration with core Content moderation - in particular gambry, volkerk, chr.fritsch and seanpclark
Comment #17
jonathan1055 commentedTest coverage for this change is provided in the Scheduler Content Moderation Integration sub-module. Those tests fail without this, and that is reasonable coverage.