Problem/Motivation

In the views there is a bunch of small errors, as they are to small we will do it in a single issue:

  • Some views have no empty message.
  • Rewrite: Word count, Item count, Loop count, Operation links => Words, Items, Loops, Operations
  • Button Save as completed redirects to Closed must go to translate overview
  • manage-translate/rejected lists assigned tasks

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Comments

edurenye created an issue. See original summary.

edurenye’s picture

Assigned: Unassigned » edurenye
edurenye’s picture

Status: Active » Needs review
StatusFileSize
new10.95 KB

First approach solving all those issues listed, still needs more manual testing to see if there something more that we should fix.

Status: Needs review » Needs work

The last submitted patch, 3: errors_in_views_in-2652678-3.patch, failed testing.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new10.73 KB
new637 bytes

Assigned where already right.

miro_dietiker’s picture

Status: Needs review » Fixed

Committing these improvements.

miro_dietiker’s picture

Status: Fixed » Needs work

Back to work. I still see views with over lengthy columns:
See: translate with "Word count " and "Item count" and that also affects all local tasks in that area.
Please recheck and provide further updates.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new18.13 KB

All the errors from the discussed list are fixed except one (Also fixed the thing of the previous comment):

  1. manage-translate/assigned lists unassigned closed task, remove
  2. formatted text didn't appear in LocalTaskItemForm
  3. caching error, invalidate tags in job when update jobitem and invalidate tags in task when update taskitem

Still need to fix button don't change when validating taskitem.

edurenye’s picture

Status: Needs review » Needs work

The last submitted patch, 9: errors_in_views_in-2652678-9.patch, failed testing.

miro_dietiker’s picture

+++ b/translators/tmgmt_local/src/Form/LocalTaskItemForm.php
@@ -148,21 +147,60 @@ class LocalTaskItemForm extends ContentEntityForm {
+          $form[$target_key]['source'] = array(
+            '#type' => 'text_format',
...
+          $form[$target_key]['source'] = array(

The two are almost identical. I would create the item unconditionally and redefine the two lines only in the if().
And there is a similar case below. Each case would result in 5..10 lines less code and better readability.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new14.28 KB
new3.85 KB

I created followups for the form related issues #2654062: Formattable data item do not show up in LocalTaskItemForm and #2654068: Item confirm buttons does not update properly with ajax.

So I reverted those changes here.

Status: Needs review » Needs work

The last submitted patch, 13: errors_in_views_in-2652678-13.patch, failed testing.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new13.8 KB
new902 bytes

Status: Needs review » Needs work

The last submitted patch, 15: errors_in_views_in-2652678-15.patch, failed testing.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new13.86 KB
new838 bytes

When we have a cart it first creates the JobItem, and then when we know the target language it creates a job.
So during this time we have a JobItem without a Job and the tests fail.
Fixed it.

miro_dietiker’s picture

Status: Needs review » Needs work

We should have some more test coverage (with the current process sequence) and check that the progress elements are updated.

edurenye’s picture

Yes, should be enough to delete the cache clear that are in the test, I'll work on that.

miro_dietiker’s picture

I committed the views label fixes.
The cache changes are not applied and awaiting test coverage.

edurenye’s picture

Resetting the cache make the test fail, so It's not enough.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new4.84 KB
new8.17 KB
new5.98 KB

I removed unneeded resetCache in the test but the test doesn't fail without the fix however it fails outside the tests.
Some of them I can't remove as I have to load the objects, and also because the cache is not invalidated between job item and task item, not sure but I think we don't need to change this.
I removed the Save after reviewing as was not needed, does nothing as we don't change anything, the confirm button must save it.
But I found there a small error, when formatted it was not correctly saved, so I fixed it.

The last submitted patch, 23: errors_in_views_in-2652678-23-test_only.patch, failed testing.

miro_dietiker’s picture

+++ b/translators/tmgmt_local/src/Tests/LocalTranslatorTest.php
@@ -534,12 +533,8 @@ class LocalTranslatorTest extends TMGMTTestBase {
-    $this->drupalPostForm(NULL, [], t('Save'));
-    $this->asserTaskItemProgress(1, '0/0/1');

It's still important to cover this submission and know where it redirects. Unsure if it is covered somewhere else already.

miro_dietiker’s picture

Status: Needs review » Needs work

Please check and report back.

edurenye’s picture

Status: Needs work » Needs review
StatusFileSize
new5.69 KB
new7.88 KB

We already check it in the line 491:

$this->drupalPostForm(NULL, $edit, t('Save'));
    $this->assertText('The translation for ' . $second_task_item->label() . ' has been saved.');
    // The first item is still completed, the second still untranslated.
    $this->assertRaw('icons/73b355/check.svg" title="Completed"');
    $this->assertRaw('tmgmt/icons/ready.svg" title="Untranslated"');

Rebased and adding a test_only that tests just the cache.

miro_dietiker’s picture

Status: Needs review » Fixed

Still committed. :-)

Status: Fixed » Closed (fixed)

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