Create a default task menu link for content overview so contrib doesn't have to

Thanks @bkosborne you have explained the problem and the solution perfectly. This is exactly what we need in Scheduler. Can you provide a patch here?

Hi joegraduate,
If you are going to work on this, I can point you towards the code I added in Scheduler to create the task.
Or from the issue summary: "We can just move the definition for this task out of content moderation and into node."

Jonathan

MR !1805 moves the 'Overview' local task from content_moderation.links.task.yml to system.links.task.yml.

I verified the following in the Tugboat live preview:

• The top-level 'Content' task provided by the system module still works (with and without the content_moderation module installed).
• The 'Moderated content' local task provided by the content_moderation module still works when the module is installed.

Without content_moderation:

With content_moderation:

Adding patches for use in composer projects.

The attached 9.4.x patch is a copy of the current changes from MR !1805 and should be compatible with 9.4.x or 9.3.x.

Also included a 9.2.x compatible patch.

If I understand this correctly, this just fixes a glitch, and any module relying on system.admin_content as a parent route will still function.
Unless they were relying on it being disabled until the workflow module was enabled, in which case it could cause problems, but I also think that is fine. RTBC

@trackleft2 thanks for the feedback and for raising the question

Unless they were relying on it being disabled until the workflow module was enabled, in which case it could cause problems

I think that it is very unlikely that a 3rd-party module would rely on something not existing unless another module is enabled without taking the precaution of making sure of the situation. For exampe in Scheduler we added the missing parent only if Content Moderation was not enabled. If that module was enabled we did not add the parent. Even if the 3rd-party module did not do that check, the worst that would happen is that you get the 'overview' local task link unnecessarily repeated.

@joegraduate, nice work. Marking this RTBC from me, but obviously needs a core maintainer to review it too.

The thunder_article module also provides:

# We need an 'Overview' task also when content_moderation is not enabled
content_moderation.content:
  title: 'Overview'
  route_name: system.admin_content
  parent_id: system.admin_content

... this is very similar code to the scheduler module...

content_moderation.content:
  # Use content_moderation.content which is the same key as is used in the core
  # Content Moderation module. If that modules is enabled this avoids two
  # 'Overview' links. If https://www.drupal.org/project/drupal/issues/3199682
  # gets committed then this route could be removed from here.
  title: 'Overview'
  route_name: system.admin_content
  parent_id: system.admin_content

Sites which use these modules are going to see additional links until the modules update. And then the new versions will only work best with Drupal 9.4

I'm not sure how to proceed with respect to BC. The right option here might be proceed and accept that some sites might have duplicate overview links until the modules are updated. However, regardless of that, because at least those two modules need changing and I'd guess that this is more - we need a change record for this issue which tells people what to do.

I also wonder if this behaviour can be improved and when a child task is added there could be automatically a child task created for the parent. I could be something like:

system.admin_content:
  title: Content
  route_name: system.admin_content
  base_route: system.admin_content
  child:
    title: Overview
    weight: -100

If the child key was present and there were children then we could automatically create a child using this info. I think in the long run this would make it much easier to work out BC implications. But I'm not sure...

The right option here might be proceed and accept that some sites might have duplicate overview links until the modules are updated.

Yes the simple solution is to accept that the sites where modules have added their own get duplicate local task links. The timing of core changes vs contrib would never work precisely. I tried to avoid duplicates in Scheduler's DynamicLocalTasks by checking for the existence of the known module (eg content moderation or media) which add their own. But it would be better to check the existence of the local task, not the module (a module) which creates it. However I cannot find how to access this within the DynamicLocalTasks. I can get the plugin.manager.menu.local_task but running getLocalTasks('system.admin_content') always returns one item, regardless of whether the patches above are applied or not.

So an alternative idea is to use hook_local_tasks_alter to remove the additional task if a duplicate is provided by core. This seems clumsy because Scheduler adds the task then removes it later if the duplicate is found. But it does mean that sites running Scheduler would not have two 'Overview' links whenever the change gets committed to core. Other contrib modules could do likewise.

This is needs review now? Looks great to me! So many contrib modules need to try to add this, not knowing if another module already has; having core do it would be so much better!

#14 mentions a change record so moving to NW for that.

Thanks for keeping this one going.

Added a draft change record https://www.drupal.org/node/3468153, Hopefully I understand the issue well enough.

Think that's probably one of the most detailed CR's I've seen nice!

Tested the MR to make sure no task appears when just 1 and everything still works when a module like content moderation is installed.

LGTM

Thank you for pushing this forward. Just a few points:

1. The Change Record credits me, Joegraduate and Catch. But I can't see any mention of Catch here in the issue. In fact I did not know that CRs had credits
2. The Issue is crediting Joegraduate and Bkosborne. I think Bkosborne should also be listed in the Change Record, if anyone is
3. What discussion do we need regarding back-porting to Drupal 10? It should be possible, and would be highly desirable. Then modules such as Scheduler and others that created their own task would be more able to do the update to remove their duplicate

1 + 2 Deleted the credits on the change record.
3. Whatever you all decide is fine with me in terms of backporting. If we are willing to make this kind of change in a patch release for 11.x why not for 10.x

Thanks for the details change record!

I read the issue summary, comments, MR and CR. Everything is in good order. There is just one question here, about BC, mentioned in #15. There is nothing about this is the 'how to deprecate'.

I updated the credit here.

There's been a couple of mentions of BC here for sites using contrib modules or profiles that provide their own version of this.

I think we can easily add code to \Drupal\Core\Menu\LocalTaskManager::getDefinitions that filters the definitions where parent_id = system.admin_content, title = 'Overview' and route_name: = system.admin_content. If it finds more than one, it can trigger a deprecation error.

Question is, is that overkill?

Setting back to NR for second opinions on this.

Probably the more "proper" way to officially throw a deprecation error.