Create Basic Content Publishing recipe

If this is going to be something that applies to all content types, may I suggest that we put the workflow into the drupal_cms_content_type_base recipe, which is meant to be a foundational recipe for things that are used by all content types? Reducing the number of recipes will help us distribute our components more effectively, and will also help keep our offerings consistent.

Got it, Adam, makes sense.

recalculating ...

We might need to run the following as well after creating new content types, or maybe just at the end after all recipes are run.

config:
  actions:
    strict: false
    workflows.workflow.editorial:
      addNodeTypes: '*'
    core.entity_form_display.node.*.default:
      setComponents:
        - name: moderation_state
          options:
            type: moderation_state_default
            region: content

rajab natshah → changed the visibility of the branch 0.x to hidden.

I think this is great work. Very clear!

I think my main issue here is that the architecture gets a bit sketchy when the content types want to opt themselves in to moderation and scheduling, rather than simply having the drupal_cms_publishing recipe opt all existing content types in. If we did it the latter way, this would be simpler and cause less weirdness in the dependency graph. I feel fairly strongly about this.

Beyond that, the only thing this is really missing is test coverage. We need, at minimum, a ComponentValidationTest (you can pretty much copy and paste it from any other recipe) to prove idempotency.

Option 1 - MR252
Option 2 - MR254

Having the ComponentValidationTest in the recipe

Both MRs have tests, so removing the tag.

Thanks for splitting these two approaches into two MRs, @rajab natshah. I was skeptical of that, but it actually makes the difference clear. I discussed the choices with @tim.plunkett and we agreed that both approaches are clean from a technical perspective. Which one we should choose, then, comes down to what would be best for the product. That's a @pameeela decision.

So I'll break it down for her to analyze.

First way: the content types opt themselves into the publishing tools
The main architectural difference here is that the content_type_base recipe will supply the moderation workflow and scheduling stuff. Nothing will be opted into it by default. So if no content types opt into moderation, then the workflow will just sit there uselessly. If a site builder creates a new content type, it will not be opted into moderation by default, and they will have to go through the configuration UI to do that.

Second way: the recipe opts all extant content types into the publishing tools
In this scenario, the moderation workflow is provided by the publishing tools recipe, and it opts all existing content types into it. This has the benefit of being consistent. There is also a simple way to get moderation and scheduling on a content type you've created: reapply the publishing recipe. The major downside is that, if you have a content type that has opted out of the moderation and/or scheduling, and you reapply the recipe, it will get opted back in. I can see that being very frustrating for site builders, since they will have to undo what the recipe did.

Personally, I think #1 is the way to go. Forcing site builders to potentially undo what a recipe did is a big no-no, and an example of a recipe exerting an unhelpful opinion. It could potentially generate hours of extra work for a site builder, depending on how familiar they are with Drupal. Having to opt new content types into publishing tools is annoying, yes, but much better from a "don't take away the site builder's agency" perspective. Additionally, it may be the case before long that config actions will be flexible enough to apply the same set of actions to an arbitrarily chosen content type. (Recipes can't do that now, but @alexpott is on board for it and I have relaxed some of my initial skepticism about it.) So there is a future path to make it more convenient, whereas it will never NOT be inconvenient to force a site builder to undo what a recipe did.

Final decision is Pam's.

Leaning toward option 2 because I think wanting these tools is the clear 80% use case. I understand there are folks who won't want it, as with everything, but as long as they are in the minority, I think that's OK. To me, having it available but not enabled for any content types is pretty pointless because turning it on is hard to find (and also super annoying that you have to do it each time you add one -- I never fail to forget to do this when working on sites!).

I can't really see a downside to having drafts and scheduling available by default, so it's hard to picture a *common* scenario where someone had to actively remove them. Even if moderation/workflow is enabled, you can still publish and unpublish your content right away using the transitions.

Still need to manually test this, which I will do later, but just wanted to post my initial thoughts.

After looking at the recipe I'm now a bit confused by the scope, because it doesn't match what's in the IS. It includes content lock and access unpublished, which were both specified for the advanced recipe. Not sure where the wires got crossed and I'm sorry that I didn't see it until now.

If we are going to have the content types opt in to moderation/workflows and scheduling, then that should be moved into content_type_base. Diff module isn't mentioned in the proposal, so I would propose we leave it out for now. We had some concerns about the UX and didn't expect to include it although the Diff Plus module improves this significantly. But we can move this to a follow up.

Then we can create a publishing tools recipe for the extras to align with the proposal for advanced.

Whoa, okay, I hadn't realized that the MRs as implemented include things that are not in the agreed-upon scope.

We need to significantly dial this back. Let's start a completely new MR which does the following:

• Adds scheduler and scheduler_content_moderation_integration to drupal_cms_content_type_base.
• Adds a completely new workflow to drupal_cms_content_type_base. Let's call it basic_editorial. It should not be built upon core's editorial_workflow recipe. It should implement the following states and transitions (copied from the plan linked in the issue summary):
  

  Draft → Initial state where content is created, or a draft is created from existing content.
  Published → Content becomes live.
  Unpublished → Content becomes unpublished or archived.

• All existing content types (which, as you know, depend on drupal_cms_content_type_base should opt into scheduling (via third party settings in the node.type.whatever file -- that would be preferable to config actions, in this case), and opt themselves into the basic_editorial workflow.

So this means that there will not be an additional recipe added for this initial scope.

Thanks, Pamela and Adam, for reviewing and providing feedback!
I'm with less recipes.

If we move these, there won’t be a need for a Basic Publishing recipe it will be built-in.

What direction should we take for publishing recipes? Should we have two separate recipes (drupal_cms_publishing_basic and drupal_cms_publishing_advanced) or combine them into a single drupal_cms_publishing recipe?

Open to moving any configurations or actions to drupal_cms_content_type_base:

• The publishing dashboard, or replace it with the default dashboard.
• The publishing views and blocks could be incorporated into the default dashboard (following Pamela's style and format for consistency).

If we do this we do not need the Basic Publishing recipe, it will be built-in.

I’ll address all feedback and directions in the 3486774-publishing-recipe branch via MR182

Please, have a look on your time.

Made all the requested changes in the 3486774-publishing-recipe branch. You can check them out here: MR182.

What are your thoughts on moving blocks from the Publishing dashboard to the Default dashboard?

Moved the Publishing content, and Site Overview view to the starter Drupal CMS recipe.

Please have a look at your time.

Looks pretty good to me, honestly - that seems like the agreed-upon scope and the placement in content_type_base guarantees availability to all content types. A couple of pretty small things and then I'd be okay RTBCing this and sending it to Pam for review.

pameeela → changed the visibility of the branch 3486774-publishing-option2 to hidden.

pameeela → changed the visibility of the branch 3486774-publishing-option1 to hidden.

I'm happy with this overall but I'd like to move the dashboard changes into a separate issue/MR so that we can review them specifically, and make sure the dashboard folks are across it. This can be in a follow up that gets merged post-RC. We can leave the views in here, just shouldn't add them to the dashboard.

I made the minor changes I asked for, just to get this over the line. Pam tested and OKed it, with the proviso that the dashboard changes be removed for now and get done in a separate issue, coordinating with @penyaskito. Tagging for a follow-up to deal with that. Also re-titling for clarity about what I'm actually going to commit.

I'll merge this when it passes tests. Sorry there was so much back-and-forth on this one; it's great to have this settled.

Looks like there are a couple of bugs here, so we need to get those tests fixed. Chances are just re-exporting a couple of content entities and views will do the trick.

I think we should set a deadline for merging this of COB Friday. It’s too much functionally to add at the very last minute, as I’m sure it will need some fine tuning. Any later and we won’t have time to test it properly.

Took some doing but I traced the test failure to this:

          <body>The website encountered an unexpected error. Try again later.<br><br><em class="placeholder">Symfony\Component\Routing\Exception\RouteNotFoundException</em>: Route "view.scheduler_scheduled_content.overview" does not exist. in <em class="placeholder">Drupal\Core\Routing\RouteProvider-&gt;getRouteByName()</em> (line <em class="placeholder">211</em> of <em class="placeholder">core/lib/Drupal/Core/Routing/RouteProvider.php</em>). <pre class="backtrace">Drupal\Core\Menu\LocalTaskDefault-&gt;getRouteParameters(Object) (Line: 310)
Drupal\Core\Menu\LocalTaskManager-&gt;getTasksBuild('system.admin_content', Object) (Line: 358)
Drupal\Core\Menu\LocalTaskManager-&gt;getLocalTasks('system.admin_content', 0) (Line: 106)
Drupal\Core\Menu\Plugin\Block\LocalTasksBlock-&gt;build() (Line: 171)
Drupal\block\BlockViewBuilder::preRender(Array)
call_user_func_array('Drupal\block\BlockViewBuilder::preRender', Array) (Line: 107)
Drupal\Core\Render\Renderer-&gt;doTrustedCallback('Drupal\block\BlockViewBuilder::preRender', Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. See https://www.drupal.org/node/2966725', 'exception', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 825)
Drupal\Core\Render\Renderer-&gt;doCallback('#pre_render', 'Drupal\block\BlockViewBuilder::preRender', Array) (Line: 387)
Drupal\Core\Render\Renderer-&gt;doRender(Array) (Line: 459)
Drupal\Core\Render\Renderer-&gt;doRender(Array, ) (Line: 203)
Drupal\Core\Render\Renderer-&gt;render(Array) (Line: 484)
Drupal\Core\Template\TwigExtension-&gt;escapeFilter(Object, Array, 'html', NULL, 1) (Line: 58)
__TwigTemplate_94a632f1d47fc84265a5cf7b588a9a20-&gt;doDisplay(Array, Array) (Line: 387)
Twig\Template-&gt;yield(Array, Array) (Line: 343)
Twig\Template-&gt;display(Array) (Line: 358)
Twig\Template-&gt;render(Array) (Line: 35)
Twig\TemplateWrapper-&gt;render(Array) (Line: 33)
twig_render_template('core/themes/claro/templates/page.html.twig', Array) (Line: 348)
Drupal\Core\Theme\ThemeManager-&gt;render('page', Array) (Line: 446)
Drupal\Core\Render\Renderer-&gt;doRender(Array, ) (Line: 203)
Drupal\Core\Render\Renderer-&gt;render(Array) (Line: 484)
Drupal\Core\Template\TwigExtension-&gt;escapeFilter(Object, Array, 'html', NULL, 1) (Line: 91)
__TwigTemplate_a7ac531d6aaadb8a53e1ee0793e59a90-&gt;doDisplay(Array, Array) (Line: 387)
Twig\Template-&gt;yield(Array, Array) (Line: 343)
Twig\Template-&gt;display(Array) (Line: 358)
Twig\Template-&gt;render(Array) (Line: 35)
Twig\TemplateWrapper-&gt;render(Array) (Line: 33)
twig_render_template('core/themes/claro/templates/classy/layout/html.html.twig', Array) (Line: 348)
Drupal\Core\Theme\ThemeManager-&gt;render('html', Array) (Line: 446)
Drupal\Core\Render\Renderer-&gt;doRender(Array, ) (Line: 203)
Drupal\Core\Render\Renderer-&gt;render(Array) (Line: 158)
Drupal\Core\Render\MainContent\HtmlRenderer-&gt;Drupal\Core\Render\MainContent\{closure}() (Line: 593)
Drupal\Core\Render\Renderer-&gt;executeInRenderContext(Object, Object) (Line: 153)
Drupal\Core\Render\MainContent\HtmlRenderer-&gt;renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber-&gt;onViewRenderArray(Object, 'kernel.view', Object) (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}(Object, 'kernel.view', Object) (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher-&gt;callListeners(Array, 'kernel.view', Object) (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher-&gt;dispatch(Object, 'kernel.view') (Line: 188)
Symfony\Component\HttpKernel\HttpKernel-&gt;handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel-&gt;handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session-&gt;handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle-&gt;handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength-&gt;handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength-&gt;handle(Object, 1, 1) (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache-&gt;pass(Object, 1, 1) (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache-&gt;handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware-&gt;handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware-&gt;handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState-&gt;handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel-&gt;handle(Object, 1, 1) (Line: 709)
Drupal\Core\DrupalKernel-&gt;handle(Object) (Line: 19)
</pre></body>

The salient part:

Route "view.scheduler_scheduled_content.overview" does not exist.

This is almost certainly the router not being rebuilt when a recipe is applied - a known issue.

@pameeela signed off on this. The stupid Cypress test is driving me nuts and rather than keep fighting it, I just decided to convert it to a PHPUnit functional test. There was nothing in the Cypress test that specifically relied on the presence of JavaScript anyway.

This was hard to get done - it turns out that changing foundational aspects of Drupal CMS's content handling is tricky! Who coulda guessed?

Nice work all around here. Thanks for sticking with this long odyssey of a patch; merged into 1.x. Thank you!

Now this piece of code needs to be added to all default content items:

moderation_state:
    -
      value: published

Because now all of them are unpublished

An issue already exists for that: 📌 Update default content to add moderation state Active

@a.dmitriiev, please open the MR against the other issue, not this one. :)

a.dmitriiev → closed merge request !325