Problem/Motivation

When we try to review a job from the job managment in /admin/tmgmt/jobs and then validate html tags from a translation that contains html tags, we get an error message about reciving an array instead of a string.

Steps to Recreate:
1.- Request a translation
2.- Go to jobs and pick manage in the dropdown.
3.- Select view/review in the job items.
4.- try to validate tags from an article with html tags.

Proposed resolution

If we get an array make it an string.
add tests

Remaining tasks

create patch, Review and commit.

User interface changes

error

Data model changes

Original report by [username]

Comments

juanse254 created an issue. See original summary.

juanse254’s picture

StatusFileSize
new608 bytes

Having a hard time with tests, for some reason is failing locally and passing with fix. But simpletest doesnt seem to have the error. Feedback apreciated.

juanse254’s picture

Issue summary: View changes
StatusFileSize
new266.44 KB
miro_dietiker’s picture

Status: Needs review » Needs work

Ah yeah, this is only with wysiwyg fields. They have that additional array nesting.
This really needs tests.

juanse254’s picture

Status: Needs work » Needs review
StatusFileSize
new1.76 KB
new1.88 KB
new2.48 KB

This should be it.
:)

The last submitted patch, 5: Validation_tags-2558625-TEST-ONLY-5.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: Validation_tags-2558625-5.patch, failed testing.

Status: Needs work » Needs review

The last submitted patch, 5: Validation_tags-2558625-TEST-ONLY-5.patch, failed testing.

LKS90’s picture

Status: Needs review » Needs work
+++ b/src/Form/JobItemForm.php
@@ -490,6 +490,12 @@ class JobItemForm extends TmgmtFormBase {
+    if (is_array($source)) {
+      $source = $source['value'];
+    }

What if $source or $translation is an empty array?

The Jenkins build ran after the issue was committed, I guess they aren't retested like the testbot ones.

Edit: I just found out why I insisted on tests: https://www.drupal.org/node/2538534#comment-10227397 :D

juanse254’s picture

StatusFileSize
new2.53 KB
new574 bytes

The test is right there, comment 5. Fixed the issue of empty array.

juanse254’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: Validation_tags-2558625-12.patch, failed testing.

juanse254’s picture

Status: Needs work » Needs review
LKS90’s picture

Status: Needs review » Needs work
+++ b/sources/content/src/Tests/ContentEntitySourceUiTest.php
@@ -35,12 +36,19 @@ class ContentEntitySourceUiTest extends EntityTestBase {
+    $filtered_html_format = FilterFormat::create(array(
+      'format' => 'filtered_html',
+      'name' => 'Filtered HTML',
+    ));
+    $filtered_html_format->save();
...
+      $filtered_html_format->getPermissionName(),

Unrelated changes (that don't do anything)?

juanse254’s picture

Status: Needs work » Needs review
StatusFileSize
new992 bytes
new1.71 KB

Right, this is it :).

miro_dietiker’s picture

Priority: Normal » Critical
Status: Needs review » Needs work

Yeah, looks fine now.
But tests pass before and still passing. So we need better test coverage for this case.

Critical because this is a repeated occurrence of the string VS array issue and leads to a fatal situation.

The last submitted patch, 5: Validation_tags-2558625-TEST-ONLY-5.patch, failed testing.

juanse254’s picture

Status: Needs work » Needs review
StatusFileSize
new1.2 KB

This is the test only.

juanse254’s picture

StatusFileSize
new1.85 KB
new866 bytes

This is the whole fix +tests.

The last submitted patch, 22: Validation_tags-2558625-22-TEST-ONLY.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 23: Validation_tags-2558625-22.patch, failed testing.

The last submitted patch, 22: Validation_tags-2558625-22-TEST-ONLY.patch, failed testing.

The last submitted patch, 23: Validation_tags-2558625-22.patch, failed testing.

miro_dietiker’s picture

Status: Needs work » Fixed

Thx, committed!

Status: Fixed » Closed (fixed)

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