Remove the delete operation for localTask

+++ b/translators/tmgmt_local/tmgmt_local.module
@@ -296,6 +296,9 @@ function tmgmt_local_tmgmt_local_task_access(\Drupal\Core\Entity\EntityInterface
       return AccessResult::allowedIf(!empty($entity->tuid) && $entity->tuid == $account->id() && $account->hasPermission('provide translation services'));
+
+    case 'delete':
+      return AccessResult::forbidden();

This function shouldn't exist. It duplicates logic of LocalTaskAccessControlHandler and overrides what we have there.

We already have a delete implementation there, which limits delete access to the administer permission.

Actually not fully convinced if we should keep that behavior or not. Maybe together with a warning message on the delete form?

But, we should definitely kill those old access functions/hooks and make sure all logic is covered by the access control handlers (Those should be in an AccessControl namespace btw, not Controller)

And whatever we do should be covered with explicit test coverage.

Yeah, not permissing deletes is usually a requirement by workflow driven processes. However, Drupal usually does not allow this limitation for admins.

Not sure how much we want to add coded limitations. I still want to review the whole user scenarios when the local translator is clean again and the previously reported bugs have been removed.

1. +++ b/translators/tmgmt_local/src/Entity/Controller/LocalTaskAccessControlHandler.php
   @@ -23,19 +23,24 @@ class LocalTaskAccessControlHandler extends EntityAccessControlHandler {
   +    if (!$account) {
   +      global $user;
   +      $account = $user;
   +    }
   

   this makes no sense, you already use $account above. It can't be not set at this point.

2. +++ b/translators/tmgmt_local/src/Entity/Controller/LocalTaskAccessControlHandler.php
   @@ -23,19 +23,24 @@ class LocalTaskAccessControlHandler extends EntityAccessControlHandler {
   +    // @todo - probably need refinement when we introduce more module permissions.
   

   is that @todo ported over? I don't think we need this. It obviously needs to be refined when we introduce more features. We could say that about pretty much every line of code :)

3. +++ b/translators/tmgmt_local/tmgmt_local.module
   @@ -232,32 +232,6 @@ function tmgmt_local_task_delete_multiple(array $tltids) {
     * Access callback for the local task item entity.
     *
     * @param $op
   

   lets clean up item here too.

As discussed, we can simplify the access control handlers a lot.

+++ b/translators/tmgmt_local/src/Plugin/views/field/TaskItemOperations.php
@@ -33,7 +33,8 @@ class TaskItemOperations extends FieldPluginBase {
     }
-    if ($item->access('view') && $item->getTask()->getAssignee()->id() == \Drupal::currentUser()->id()) {
+    if ($item->access('view', \Drupal::currentUser(), TRUE)->isAllowed()
+      && $item->getTask()->getAssignee()->id() == \Drupal::currentUser()->id()) {
       $element = Link::fromTextAndUrl($item->isPending() ? t('Translate') : t('View'),

why this change?

Also please reroll, does not apply anymore.

Yeah with the current permissions, this makes sense.

However, we should create a followup to offer a separate permission if a translator can unassign at all... except if we would want to cover this transition limitation only by advanced workflow module integration that can easily cover this case.