Problem/Motivation

TaskItemOperation throws an exception when trying to get translator when it doesn't have any assigned:
Fatal error: Call to a member function id() on null in modules/tmgmt/translators/tmgmt_local/src/Plugin/views/field/TaskItemOperations.php on line 33

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

edurenye created an issue. See original summary.

edurenye’s picture

Status: Active » Needs review
StatusFileSize
new924 bytes

Not sure if this is the proper solution, or we should check if we can translate it, and show the link, and then in case we click assign it automatically to ourself. But I think is better to make the user assign it manually to himself to not get confused, but not sure.

miro_dietiker’s picture

+++ b/translators/tmgmt_local/src/Plugin/views/field/TaskItemOperations.php
@@ -30,7 +30,7 @@ class TaskItemOperations extends FieldPluginBase {
+    if ($item->access('view') && $item->getTask()->getTranslator() && $item->getTask()->getTranslator()->id() == \Drupal::currentUser()->id()) {

Yeah that fix works. This issue is not about changing the workflow at all.
There are many ways to improve the whole UX of the local translator, most importantly guide the user through the whole process, but that's a followup.

I would still prefer two if()s or an early exit if no translator available.

miro_dietiker’s picture

Status: Needs review » Needs work

Oops, in any case, use hasTranslator() to check if there is a translator!

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new847 bytes
new1001 bytes

We don't have hasTranslator() but we can use isUnassigned().
This affects this other issue #2646592: Fix duplicated task when saving local task, I have this error in the tests that I'm adding there.

miro_dietiker’s picture

Status: Needs review » Needs work
+++ b/translators/tmgmt_local/src/Plugin/views/field/TaskItemOperations.php
@@ -30,6 +30,9 @@ class TaskItemOperations extends FieldPluginBase {
+    if ($item->getTask()->isUnassigned()) {
...
     if ($item->access('view') && $item->getTask()->getTranslator()->id() == \Drupal::currentUser()->id()) {

Oh man, "getTranslator()" is about the account that translates... It clashes somehow withour usual getTranslator that is fetching the translator plugin from job / item...
Should we change this API to getAssignee() or so? If we have a more complex workflow (from translator to reviewer, ...) we will use the same property / getter.

I just wanted to commit as is with followup work, but we also need a failing test for this!

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new732 bytes
new1.54 KB

Yes, I agree. Assignee is a better name to not be mistaken with that. I'll open a followup for that.
Here is the test. I'll extend that test in this issue #2646592: Fix duplicated task when saving local task.

edurenye’s picture

Issue created for renaming translator to assignee #2648678: Use Assignee instead of translator in tmgmt_local

The last submitted patch, 7: exception_in-2646582-7-test_only.patch, failed testing.

  • Berdir committed 556c81a on 8.x-1.x authored by edurenye
    Issue #2646582 by edurenye: Exception in TaskItemOperation trying to get...
berdir’s picture

Status: Needs review » Fixed

Ok, committed.

Status: Fixed » Closed (fixed)

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