Problem/Motivation

After setting a JobItem to needs review the message displayed shows the link with the html tags instead of the link.

Proposed resolution

Fix it.
For the test coverage just fixing the test added in #2646554: Add a message to tmgmt job from receiving a translation will be fine I think, and add a test for tmgmt_file also.

Remaining tasks

User interface changes

Data model changes

Original report by [username]

CommentFileSizeAuthor
#7 interdiff-html_tags_showing-2650130-5-7.txt1.07 KBedurenye
#7 html_tags_showing-2650130-7.patch4.11 KBedurenye
#5 interdiff-html_tags_showing-2650130-3-5.txt952 bytesedurenye
#5 html_tags_showing-2650130-5.patch4.18 KBedurenye
#3 html_tags_showing-2650130-3.patch4.23 KBedurenye
#3 html_tags_showing-2650130-3-test_only.patch2.07 KBedurenye
Screenshot from 2016-01-14 11-26-42.png8.55 KBedurenye

Comments

edurenye created an issue. See original summary.

edurenye’s picture

edurenye commented
Issue summary: View changes
Parent issue: » #2646554: Add a message to tmgmt job from receiving a translation
edurenye’s picture

edurenye commented
Status: Active » Needs review
StatusFileSize
new html_tags_showing-2650130-3-test_only.patch2.07 KB
new html_tags_showing-2650130-3.patch4.23 KB

Maybe it's better if we just fix it first and then we add the message with the other test in the parent issue as this test is failing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
  1. +++ b/src/Entity/Message.php
@@ -96,10 +97,10 @@ class Message extends ContentEntityBase implements MessageInterface {
     if ($this->variables->first()->toArray()) {
-      return t($text, $this->variables->first()->toArray());
+      return new TranslatableMarkup($text, $this->variables->first()->toArray());

    Functionally, this doesn't make a difference, that's what t( returns. 

  2. +++ b/translators/tmgmt_file/src/Tests/FileTranslatorTest.php
@@ -470,9 +473,10 @@ class FileTranslatorTest extends TMGMTTestBase {
     foreach ($job->getMessages() as $message) {
-      if ($message->getMessage() == t('Failed to validate semantic integrity of %key element. Please check also the HTML code of the element in the review process.', array('%key' => 'dummy][deep_nesting'))) {
+      $untranslated_string = new TranslatableMarkup('Failed to validate semantic integrity of %key element. Please check also the HTML code of the element in the review process.', array('%key' => 'dummy][deep_nesting'));
+      if ((strpos($message->getMessage()->getUntranslatedString(), $untranslated_string->getUntranslatedString()) !== FALSE)) {

    This is confusing, why create an object only to get the string out a again? This also means we don't test the value of the placeholder anymore.

    Those objects can be casted to a string, why didn't that work?

edurenye’s picture

edurenye commented
Status: Needs work » Needs review
StatusFileSize
new html_tags_showing-2650130-5.patch4.18 KB
new interdiff-html_tags_showing-2650130-3-5.txt952 bytes

In the first point, I did that because I had a warning saying that only literals should be passed to t() when possible.
Maybe this counts as an impossible case? Not sure.

Fixed the second point.

Status: Needs review » Needs work

The last submitted patch, 5: html_tags_showing-2650130-5.patch, failed testing.

edurenye’s picture

edurenye commented
Status: Needs work » Needs review
StatusFileSize
new html_tags_showing-2650130-7.patch4.11 KB
new interdiff-html_tags_showing-2650130-5-7.txt1.07 KB

Fixed the error.

berdir’s picture

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

Thanks, committed.

  • Berdir committed a007eb9 on 8.x-1.x authored by edurenye 
    Issue #2650130 by edurenye: Html tags showing instead of link in...

Status: Fixed » Closed (fixed)

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