Scoped the content authoring API moderation policy to entity creation so drafts can be published.#245
Merged
Merged
Conversation
The 'ModerationPolicyHook' ran on every entity save, so it also clobbered a reviewer's later publish: any account holding 'use content authoring api' could create a draft but never publish it - the moderation state was silently coerced back to 'draft' on the update. The policy governs authoring, which is creation, so it now only applies when the entity is new; a subsequent publish (an update) is left untouched. Added a kernel test covering publish-after-authoring and a Behat scenario that publishes a draft through the editorial UI. The kernel test setup now installs the 'node_access' schema so a second (update) save can run.
📝 WalkthroughWalkthroughAdds a guard in ChangesModeration policy publish fix
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@web/modules/custom/do_content_api/tests/src/Kernel/Hook/ModerationPolicyHookTest.php`:
- Around line 180-204: Add a companion kernel test in ModerationPolicyHookTest
next to testApiPagePublishAfterAuthoringIsHonoured that covers the media path in
ModerationPolicyHook::entityPresave(). Create an API user with the content
authoring permission, create a media entity in draft as an authored save, then
update the same entity to published and assert the later save keeps the
published moderation_state and published status. Use the existing entityPresave
node/media branching as the target behavior to verify.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 964be520-6d30-4159-8763-d5be82222310
📒 Files selected for processing (4)
tests/behat/bootstrap/FeatureContext.phptests/behat/features/content_moderation_publish.featureweb/modules/custom/do_content_api/src/Hook/ModerationPolicyHook.phpweb/modules/custom/do_content_api/tests/src/Kernel/Hook/ModerationPolicyHookTest.php
Comment on lines
+180
to
+204
| /** | ||
| * Tests that an API actor can publish a page it previously authored. | ||
| */ | ||
| public function testApiPagePublishAfterAuthoringIsHonoured(): void { | ||
| $api_user = $this->createUser(['use content authoring api']); | ||
| $this->assertNotFalse($api_user); | ||
| $this->setCurrentUser($api_user); | ||
|
|
||
| // Authoring (create) is forced to draft by the policy. | ||
| $node = Node::create([ | ||
| 'type' => 'civictheme_page', | ||
| 'title' => '[TEST] Authored then published', | ||
| 'moderation_state' => 'draft', | ||
| ]); | ||
| $node->save(); | ||
| $this->assertSame('draft', $node->get('moderation_state')->value); | ||
|
|
||
| // A later publish is an update, not authoring, so it must be honoured. | ||
| $node->set('moderation_state', 'published'); | ||
| $node->save(); | ||
|
|
||
| $this->assertSame('published', $node->get('moderation_state')->value); | ||
| $this->assertTrue($node->isPublished()); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Missing test coverage for media update-after-authoring.
Given the guard in ModerationPolicyHook::entityPresave() (Line 53-55 of that file) applies to both the node and media branches, consider adding a companion test asserting the expected behavior when an API-authored media entity's moderation_state is changed on a subsequent update — this exercises the concern raised in ModerationPolicyHook.php.
🧰 Tools
🪛 PHPMD (2.15.0)
[error] 189-193: Avoid using static access to class '\Drupal\node\Entity\Node' in method 'testApiPagePublishAfterAuthoringIsHonoured'. (undefined)
(StaticAccess)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/modules/custom/do_content_api/tests/src/Kernel/Hook/ModerationPolicyHookTest.php`
around lines 180 - 204, Add a companion kernel test in ModerationPolicyHookTest
next to testApiPagePublishAfterAuthoringIsHonoured that covers the media path in
ModerationPolicyHook::entityPresave(). Create an API user with the content
authoring permission, create a media entity in draft as an authored save, then
update the same entity to published and assert the later save keeps the
published moderation_state and published status. Use the existing entityPresave
node/media branching as the target behavior to verify.
Member
Author
There was a problem hiding this comment.
Fixed in d5ff521. Added testApiMediaUpdateAfterAuthoringIsHonoured, which authors a media (coerced to published on create) then updates it to draft and asserts the later change is honoured - covering the media branch of the isNew guard alongside the node test.
This comment has been minimized.
This comment has been minimized.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #245 +/- ##
===========================================
+ Coverage 89.05% 89.16% +0.10%
===========================================
Files 15 15
Lines 201 203 +2
===========================================
+ Hits 179 181 +2
Misses 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Code coverage (threshold: 80%) Per-class coverage |
AlexSkrypnyk
added
the
Needs review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist before requesting a review
[#123] Verb in past tense.#123added to descriptionChangedsectionChanged
isNew()guard toModerationPolicyHook::entityPresave()so the policy that coerces moderation state only fires on entity creation (authoring), not on subsequent saves — allowing a reviewer to publish a draft authored through the API without the hook reverting the state back todraft.node_accessschema in the kernel testsetUp()so a secondsave()call (simulating the publish update) can execute cleanly inside the kernel test environment.testApiPagePublishAfterAuthoringIsHonoured()that creates a page as an API actor, asserts it is coerced todraft, then updates the moderation state topublishedand asserts the hook no longer reverts it.content_moderation_publish.featurethat logs in as a user with bothuse content authoring apiand editorial transition permissions, creates a draft page, publishes it through the editorial UI, and asserts the published state via a newFeatureContextstep.contentShouldBePublished()toFeatureContext— aThenstep that reloads the node from storage (bypassing the stale entity cache) and asserts both themoderation_statefield and theisPublished()flag.Screenshots
N/A — PHP hook, kernel test, and Behat feature; no template, CSS, or JS changes.
Before / After
Summary by CodeRabbit
New Features
Bug Fixes