Problem/Motivation

The review form does not display the source and target language.
As a result you need to go back to the job to realise what the language pairs are.
That's unfortunate in some situations as it could lead to wrong languages prefilled and accepted in case of mapping errors or so.

Proposed resolution

Display source and target language in the review header area.

User interface changes

source and target language on review form.

CommentFileSizeAuthor
#7 interdiff-2685561-7.txt1.69 KBsanja_m
#7 2685561-7.patch1.86 KBsanja_m
#5 review_form.png61.71 KBsanja_m
#5 interdiff-2685561-5.txt2.19 KBsanja_m
#5 2685561-5.patch2.78 KBsanja_m
#3 2685561-3.patch1.5 KBsanja_m

Comments

miro_dietiker created an issue. See original summary.

sanja_m’s picture

sanja_m commented
Assigned: Unassigned » sanja_m

Assigning to me.

sanja_m’s picture

sanja_m commented
Status: Active » Needs review
StatusFileSize
new 2685561-3.patch1.5 KB

Uploaded patch.

Status: Needs review » Needs work

The last submitted patch, 3: 2685561-3.patch, failed testing.

sanja_m’s picture

sanja_m commented
Status: Needs work » Needs review
StatusFileSize
new 2685561-5.patch2.78 KB
new interdiff-2685561-5.txt2.19 KB
new review_form.png61.71 KB

Fixed failing tests.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

Code looks fine, just need improve the tests.

  1. +++ b/src/Tests/TMGMTUiReviewTest.php
@@ -19,7 +19,7 @@ use Drupal\node\Entity\Node;
 
-  public static $modules = ['ckeditor', 'tmgmt_content', 'image', 'node'];
+  public static $modules = ['ckeditor', 'tmgmt_content', 'image', 'node', 'language'];

    language shouldn't be needed in this list, we depend on that. 

  2. +++ b/src/Tests/TMGMTUiReviewTest.php
@@ -114,6 +116,13 @@ class TMGMTUiReviewTest extends EntityTestBase {
 
+    // Add predefined language.
+    $this->drupalGet('admin/config/regional/language/add');
+    $edit = array(
+      'predefined_langcode' => 'de',
+    );
+    $this->drupalPostForm(NULL, $edit, 'Add language');

    is this needed because this uses an undefined language?

    Use \Drupal\tmgmt\Tests\TMGMTTestBase::addLanguage for this. Faster, doesn't require a UI, so you don't need additional permissions. 

  3. +++ b/src/Tests/TMGMTUiTest.php
@@ -415,6 +415,10 @@ class TMGMTUiTest extends EntityTestBase {
+    // Test that source and target languages are displayed.
+    $this->assertText('Source language');
+    $this->assertText('Target language');

    We should also assert the actual languages that are being displayed. This would pass if we'd just print the labels.

sanja_m’s picture

sanja_m commented
Status: Needs work » Needs review
StatusFileSize
new 2685561-7.patch1.86 KB
new interdiff-2685561-7.txt1.69 KB

Updated patch. \Drupal\tmgmt\Tests\TMGMTTestBase::addLanguage was added in some other commits, so now there is no need to add that again.

  • Berdir committed a028516 on 8.x-1.x authored by sanja_m 
    Issue #2685561 by sanja_m: Display language from + to in review form
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Makes sense. We probably want to do the same for the local translator task item form, can you open a new issue for that?

Committed.

Status: Fixed » Closed (fixed)

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