Problem/Motivation

When you currently enable a module, you'll notice that new local tasks are not visible. They only appear after clearing cache.
The problem was noticed in the field group module (#2848314: Add group button is not available at manage form display). But i notice that this is also the case for Field UI.

Steps to Reproduce:

  1. Go to the admin/modules
  2. Uninstall field ui
  3. Tasks are now gone on the 'Edit article content type' screen
  4. Install the module and refresh 'Edit article content type'. You'll notice that the tasks are not shown yet.
  5. Clear cache and refresh screen. The tabs are now correctly appearing.

Proposed resolution

Make sure that local tasks are also correctly rebuild when enabling modules.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

zuuperman created an issue. See original summary.

nils.destoop’s picture

Version: 8.3.x-dev » 8.4.x-dev
cilefen’s picture

Version: 8.4.x-dev » 8.3.x-dev
Priority: Minor » Normal
jmmarquez’s picture

Assigned: Unassigned » jmmarquez

I am working on it!!

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

geek-merlin’s picture

Assigned: jmmarquez » Unassigned

I expirienced this issue today.

@jmmarquez: any news?

tetranz’s picture

Here's a patch for field_ui. The problem is caused by local task caching.

I'm not sure if this is the ideal way of doing it. It feels like it should be more generalized but I haven't quite figured out how to do that. This works with Seven, Adminimal and Bartik as admin themes.

tetranz’s picture

Status: Active » Needs review
geek-merlin’s picture

Status: Needs review » Needs work

Hmm, this is indeed an ad-hoc hack that only had a chance to be committed with a *very very very* good justification.
So we need some investigation: Why is this? What else might be stale in cache?

My gut feeling is, that after module install, all caches should be cleared. So why is this stale?
Also (additional info not in the IS): When i experienced that problem, it only appeared on one (i suppose the first i viewed) content type.
We need some debugging here.

tetranz’s picture

Yeah, I tend to agree. I'm not sure if it's worth spending more time on. I did this much because I had some spare time and treated it as a learning exercise.

It happens for me on all content types. i.e., Basic Page, Article and others. It also happens on other other entity types, e.g., content blocks.

As far as I know, cache is not cleared when you install a module either via the UI or drush. Whether or not cache should be cleared is probably a matter for debate. I notice that drupal console (unlike drush) does clear cache when it installs a module and the problem does not occur.

If you're testing, you have the problem in reverse to really see the issue. After you uninstall field_ui you need to clear cache to observe the tabs disappearing.

Hmm ... I notice that content translation seems to avoid this issue. It uses the same block of local tasks so I might dive into that to see if it yields any helpful information. Once content translation is setup and two languages defined, you can open the edit page for a node, then toggle the translate on and off for that content type admin/config/regional/content-language, refresh the node edit and the Translate tab appears and disappears as expected without cache clear.

tetranz’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
582 bytes
1.23 KB

I was missing the forest for the trees with that first attempt :)

Here's a much better general solution. The local task blocks need to depend on cache tag local_task. When a module is installed or uninstalled MenuRouterRebuildSubscriber::onRouterRebuild is called which rebuilds the menu links and local tasks.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: invalidate-tasks-2861210-11.patch, failed testing. View results

tetranz’s picture

Sorry, I accidentally changed it to rtbc instead of review before. Looking at the failed test now.

tetranz’s picture

Status: Needs work » Needs review
FileSize
2.33 KB
1.77 KB
geek-merlin’s picture

This looks promising. Code is straightforward.

tetranz’s picture

I had a further play with this and, not surprisingly I guess, it fixes similar issues involving other modules, not just field_ui.

A couple of things I tested were:

Adding and removing a menu tab to a view.
Installing and uninstalling the contact module and watching the Contact tab on user profiles.

It should help anything that implements hook_menu_local_tasks_alter and then invalidates the local_task cache tag.

geek-merlin’s picture

Status: Needs review » Reviewed & tested by the community

OK code looks solid. That LocalTasksBlock should depend on cache tag local_task seems so obvious to me i dare set RTBC to get component maintainer's attention.

larowlan’s picture

Status: Reviewed & tested by the community » Needs review

Hi - can you upload two versions of the patch - a test-only patch and a test + fix patch.

The patch that only has tests in it should fail, so upload that one first.

This helps committers judge that the fix has test coverage.

Thanks!

tetranz’s picture

tetranz’s picture

A few unnecessary use statements crept in at the top. This should fail.

tetranz’s picture

The last submitted patch, 20: invalidate-tasks-2861210-20.patch, failed testing. View results

The last submitted patch, 19: invalidate-tasks-2861210-19.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +D8 cacheability

#18 is resolved in the #20/#21.

The actual fix is + $cacheability->addCacheTags(['local_task']); in \Drupal\Core\Menu\Plugin\Block\LocalTasksBlock::build, the rest are test-improvements / a new test.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/lib/Drupal/Core/Menu/Plugin/Block/LocalTasksBlock.php
@@ -85,6 +85,7 @@ public function defaultConfiguration() {
+    $cacheability->addCacheTags(['local_task']);

Let's get the cache tags from the plugin manager as this is where it comes from and what we need to stay in-sync with. So do $cacheability->addCacheableDependency($this->localTaskManager); instead.

borisson_’s picture

Status: Needs work » Needs review
FileSize
651 bytes
3.96 KB
borisson_’s picture

Status: Needs review » Reviewed & tested by the community

I'm pretty sure that #27 correctly resolves #26 and it was a really small fix. Since that was the only remark @alexpott had in #26 - I'm going to be bold and set it back to RTBC.

alexpott’s picture

Adding review credit.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 3fe7057fdb to 8.7.x and f55cb49475 to 8.6.x. Thanks!

  • alexpott committed 3fe7057 on 8.7.x
    Issue #2861210 by tetranz, borisson_, zuuperman, axel.rutz, alexpott:...

  • alexpott committed f55cb49 on 8.6.x
    Issue #2861210 by tetranz, borisson_, zuuperman, axel.rutz, alexpott:...
Wim Leers’s picture

Late to the party but …

+++ b/core/lib/Drupal/Core/Menu/Plugin/Block/LocalTasksBlock.php
@@ -85,6 +85,7 @@ public function defaultConfiguration() {
+    $cacheability->addCacheableDependency($this->localTaskManager);

👌 ❤️

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

liquidcms’s picture

Took a while trying to figure out why, when enabling field_group module, that Add Group button didn't show up on Manage form display. Turns out i needed to do a cr.

This is Drupal 8.7.8. Wondering why this is marked as closed (and sadly i can't modify status any more).