It's impossible to search for "Strings not translated to language ..." [#1664528]

Steps to repeat:
1. Set up a site with base (English) and one other language (Swedish for example)
2. Visit admin/config/regional/translate/translate and search for

String contains: "" (empty)
Language: "Swedish"
Search in: "Only untranslated strings"
Limit search to: "Built-in interface"

Expected Results:
I expect to receive a list of strings that haven't been translated into Swedish.

Actual Results:
"No strings available."

Lengthy description:
I wanted to keep the steps to repeat fairly short, so here goes a longer description.

I'm assuming that my expectation is correct - the "Language" filter is meant to filter on target language, and in combination with the "Search in" filter we should be able to find either strings that have been translated into that language, or strings that haven't been translated into that language.

The way it currently works, the "Only untranslated strings" filter will only return strings when paired with the "English" or "All languages" settings for the Language filter. And in that case it will return "all strings that haven't been translated into any language". Which doesn't seem like a reasonable use case to me. Thus bug.

Comment	File	Size	Author
#12	drupal-fix_searching_for_untranslated_strings-1664528-12-complete.patch	2.69 KB	Carl Johan
#12
#12	drupal-fix_searching_for_untranslated_strings-1664528-12-test.patch	1.44 KB	Carl Johan
#12
#9	drupal-fix_searching_for_untranslated_strings-1664528-9-test.patch	1002 bytes	Carl Johan
#9
#9	drupal-fix_searching_for_untranslated_strings-1664528-9-complete.patch	2.23 KB	Carl Johan
#9
#6	drupal-fix_searching_for_untranslated_strings-1664528-6.patch	1.25 KB	Carl Johan
#6
#5	drupal-fix_searching_for_untranslated_strings-1664528-5.patch	2.58 KB	Carl Johan
#5
#1	drupal-fix_searching_for_untranslated_strings-1664528-1.patch	1.69 KB	Carl Johan
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Carl Johan CreditAttribution: Carl Johan commented 28 June 2012 at 17:48

File	Size
drupal-fix_searching_for_untranslated_strings-1664528-1.patch	1.69 KB

Moving the language condition into the join solves the problem.

It ain't pretty. I'm not sure what it breaks. But it works for me?

Comment #2

Carl Johan CreditAttribution: Carl Johan commented 28 June 2012 at 17:50

Status:

Active

» Needs review

Comment #3

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 28 June 2012 at 19:45

Version:	7.x-dev	» 8.x-dev
Issue tags:		+D8MI, +language-ui

Sounds like indeed it would not work because you are filtering based on values in the translation table, but empty translations are currently not stored in Drupal (I believe they were stored before). Applies to Drupal 8, which means we need to fix there first based on our process.

Comment #4

Carl Johan CreditAttribution: Carl Johan commented 28 June 2012 at 21:33

Version:

8.x-dev

» 7.x-dev

But it looks like this was fixed for 8.x in #1452188: New UI for string translation.

In pretty much the same way, too. Just slightly more elegant because you can't search for english/base language strings in 8.x it seems.

The new interface looks great, btw :)

Comment #5

Carl Johan CreditAttribution: Carl Johan commented 29 June 2012 at 12:28

File	Size
drupal-fix_searching_for_untranslated_strings-1664528-5.patch	2.58 KB

Here's another patch that looks a bit more like the 8.x code.

Comment #6

Carl Johan CreditAttribution: Carl Johan commented 29 June 2012 at 12:43

File	Size
drupal-fix_searching_for_untranslated_strings-1664528-6.patch	1.25 KB

There seems to have been a problem with the encoding..

Comment #7

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 1 July 2012 at 16:30

Issue tags:

+Needs tests

Indeed, this is fixed in Drupal 8 with the change pointed out. We did not realize this was a bug in Drupal 7 when we fixed it in Drupal 8 and it was part of a larger improvement there.

The code looks good, however, to get it committed, we'd ideally need automated tests and/or manual testing from others.

Comment #7.0

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 1 July 2012 at 16:30

Issue summary:

View changes

Emphasis, wording.

Comment #8

Carl Johan CreditAttribution: Carl Johan commented 1 July 2012 at 23:10

It should be pretty easy to manually test/verify this using the steps in the description.

I was a bit unsure how to tackle adding an automated test for this, but it seems like LocaleTranslationFunctionalTest would be a good place to start.

Comment #9

Carl Johan CreditAttribution: Carl Johan commented 2 July 2012 at 22:38

File	Size
drupal-fix_searching_for_untranslated_strings-1664528-9-complete.patch	2.23 KB

drupal-fix_searching_for_untranslated_strings-1664528-9-test.patch	1002 bytes

All right, let's see if this test is done right.

Comment #10

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 3 July 2012 at 08:21

Looks good. I'm wondering why this existing test does not expose the problem for us (which you have seem to copied)?

    // Ensure untranslated string appears if searching on 'only untranslated
    // strings'.
    $search = array(
      'string' => $name,
      'language' => 'all',
      'translation' => 'untranslated',
      'group' => 'all',
    );
    $this->drupalPost('admin/config/regional/translate/translate', $search, t('Filter'));
    $this->assertNoText(t('No strings available.'), t('Search found the string.'));

Comment #11

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 3 July 2012 at 12:06

Status:

Needs review

» Needs work

We discussed on IRC that the existing test works differently because when looking at all languages, the language condition is not in the query, and therefore the bugos filtering of results does not happen. The patch moves the language condition into the left join to make the same behavior happen even if language is there. We agreed that the existing comment needs update on the code that I quoted. Otherwise it looks good.

Comment #12

Carl Johan CreditAttribution: Carl Johan commented 4 July 2012 at 20:33

File	Size
drupal-fix_searching_for_untranslated_strings-1664528-12-test.patch	1.44 KB

drupal-fix_searching_for_untranslated_strings-1664528-12-complete.patch	2.69 KB

Tried to explain the different intents of the tests, original comment was:

Ensure untranslated string appears if searching on 'only untranslated
strings'.

Now the two different ones are:

Ensure untranslated string appears if searching on 'only untranslated
strings' in "all" (hasn't been translated to any language).

and

Ensure untranslated string appears if searching on 'only untranslated
strings' in the custom language (hasn't been translated to that specific language).

Comment #13

Carl Johan CreditAttribution: Carl Johan commented 4 July 2012 at 20:34

Status:

Needs work

» Needs review

Comment #14

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 4 July 2012 at 21:49

Status:

Needs review

» Reviewed & tested by the community

IMHO this looks perfect now. As noted above it was already fixed for Drupal 8 in the same way in #1452188: New UI for string translation (because language 'all' was removed from this screen and we figured out that other languages were buggy, but did not work to backport the fix, which this issue does).

Thanks Carl for sticking to this and polishing it up!

Comment #15

David_Rothstein CreditAttribution: David_Rothstein commented 9 July 2012 at 03:50

Status:

Reviewed & tested by the community

» Fixed

Makes sense to me... and it looks like in addition to the bug fix, a similar test already exists in Drupal 8 also (at least I think).

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/efe6bdf

Any chance this needs to be backported to Drupal 6 too?

Comment #16

David_Rothstein CreditAttribution: David_Rothstein commented 9 July 2012 at 04:13

Issue tags:

+7.15 release notes

Actually, this seems important enough to mention in the release notes, and I added it to CHANGELOG.txt also: http://drupalcode.org/project/drupal.git/commit/828cd90

Comment #17

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 9 July 2012 at 07:48

Issue tags:

-Needs tests

It does not seem to apply to Drupal 6, the condition was part of the join there, as this patch fixed for Drupal 7, and we earlier fixed for Drupal 8. http://drupalcode.org/project/drupal.git/blob/refs/heads/6.x:/includes/l...