This is very similar to #2921844: Allow auto-generating local actions for entities but for local tasks.

The "default" local task provider checks whether there are routes for "View", "Edit" and "Delete" (in that order) and if there are at least two of those, it adds a set of local tasks with the base route of the first one found. That way it can be used for both entities with and without a "View" path (i.e. config entities).

Other than that I really stole everything from that issue including the title-translation trick and the (basic) test.

CommentFileSizeAuthor
#16 2994349-16.patch14.08 KBtstoeckler
#16 2994349-11-16-interdiff.txt2.49 KBtstoeckler
#11 2994349-11.patch16.5 KBtstoeckler
#11 2994349-9-11-interdiff.txt2.11 KBtstoeckler
#9 2994349-9.patch14.39 KBtstoeckler
#9 2994349-7-9-interdiff.txt569 byteststoeckler
#7 2994349-7.patch14.49 KBtstoeckler
#7 2994349-2-7-interdiff.txt7.01 KBtstoeckler
#2 2994349-2.patch12.3 KBtstoeckler

Comments

tstoeckler created an issue. See original summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
StatusFileSize
new 2994349-2.patch12.3 KB

Here it is.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Oh right: One thing that is weird is the current "Revisions" Task that automatically gets added to all entity types. It's kind of unfortunate to have two different methods of adding tasks to entities, but I guess it would be a BC-break to convert this to a local task provider because that would inherently make it opt-in.

I mean theoretically we could also add an appropriate local task provider in hook_entity_type_build() for the revisions task, but not sure...

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Related issues: +#2921844: Allow auto-generating local actions for entities
bojanz’s picture

bojanz commented
Status: Needs review » Fixed

Let's do this.

bojanz’s picture

bojanz commented
Status: Fixed » Needs review

Meant to post that in #2989556: Make EntityCollectionLocalActionProvider add a destination query parameter.

Regarding #3, could we just merge RevisionsOverviewDeriver into EntityLocalTask?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
StatusFileSize
new 2994349-2-7-interdiff.txt7.01 KB
new 2994349-7.patch14.49 KB

OK, so something like this? I think this would certainly be the cleanest solution. On the one hand people might not be happy about their Revisions tab disappearing, on the other hand, we can tell them "Hey, add these 3 lines to your annotation and you can delete *all* of your local tasks" so...

I renamed the provider, somehow I couldn't get that to show up properly in the interdiff.

Status: Needs review » Needs work

The last submitted patch, 7: 2994349-7.patch, failed testing. View results

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new 2994349-7-9-interdiff.txt569 bytes
new 2994349-9.patch14.39 KB

Oops, I'm not quite up-to-date, it seems ;-)

Status: Needs review » Needs work

The last submitted patch, 9: 2994349-9.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs work » Needs review
StatusFileSize
new 2994349-9-11-interdiff.txt2.11 KB
new 2994349-11.patch16.5 KB

And I guess a bit trigger-happy today. Sorry for the noise.

Not sure what to do with the RevisionOverviewTest. Removing it for now, not sure what others think.

mjpa’s picture

mjpa commented

Found this patch while trying to work out how to stop this module from enforcing the revisions local task... works fine for that bit :D

bojanz’s picture

bojanz commented

I'm fine with the approach to the version-history, but I'm confused by the EntityLocalTask class.

+  /**
+   * {@inheritdoc}
+   */
+  public function getTitle(Request $request = NULL) {
+    switch ($this->rel) {
+      case 'canonical':
+        return $this->t('View');
+
+      case 'edit-form':
+        return $this->t('Edit');
+
+      case 'delete-form':
+        return $this->t('Delete');
+
+      case 'version-history':
+        return $this->t('Revisions');
+
+      default:
+        return parent::getTitle($request);
+    }
+  }

It feels very unusual for a plugin class to hardcode every possible title
What's wrong with using t() in the DefaultEntityTasksProvider?

Another unusual thing is that we provide a local task for version-history, but we don't provide a route for it.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

What's wrong with using t() in the DefaultEntityTasksProvider?

As far as I know the local tasks are cached in a way that running the translation at build time would not result in the proper runtime translation being applied. I did not investigate this deeply, though, I just copied the pattern from what we do with route providers, where it is actually necessary to do this. Will test and investigate though, to see if we can avoid this.

Another unusual thing is that we provide a local task for version-history, but we don't provide a route for it.

Sorry, I don't understand. As far as I can tell the route is generated by \Drupal\entity\Routing\RevisionRouteProvider::getRevisionHistoryRoute().

morenstrat’s picture

morenstrat
he/him
Primary language German
Location Bochum, Germany
commented
Status: Needs review » Reviewed & tested by the community

I applied the patch, added the local_task_provider handler to my custom entity type and the local tasks appeared.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
StatusFileSize
new 2994349-11-16-interdiff.txt2.49 KB
new 2994349-16.patch14.08 KB

So I looked into the translation thing and it is indeed not necessary because local tasks are cached by langcode. See LocalTaskManager::__construct():

    $this->setCacheBackend($cache, 'local_task_plugins:' . $language_manager->getCurrentLanguage()->getId(), ['local_task']);

So here's a simplified patch. The interdiff doesn't show the removed EntityLocalTask because I accidentally deleted that in PhpStorm.

I will now go and look if I did that workaround for the local action thing unnecessarily, as well...

Leaving at RTBC, since it's basically just removed code, hope that's OK here.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Ahem... #3028768: Remove unnecessary translation workaround in local action plugin

bojanz’s picture

bojanz commented
Status: Reviewed & tested by the community » Fixed

Removed an unused use statement from EntityLocalTaskTest and added a one-line note to the README about the new feature.

Thank you!

Followup: #3041435: Provide an automatic local task for the Duplicate operation.

  • bojanz committed 0f8da70 on 8.x-1.x authored by tstoeckler 
    Issue #2994349 by tstoeckler: Allow auto-generating local tasks for...

Status: Fixed » Closed (fixed)

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