Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in dropdown translation #953

Closed
yllen opened this issue Sep 9, 2016 · 14 comments
Closed

Bug in dropdown translation #953

yllen opened this issue Sep 9, 2016 · 14 comments
Labels
Milestone

Comments

@yllen
Copy link
Collaborator

yllen commented Sep 9, 2016

http://forum.glpi-project.org/viewtopic.php?id=150158

Introduce by 1f0fcdc#diff-a801c52129f44d50ab6d1db9f049c4b7

Dropdown list is never translated (because only use by super-admin).
Only dropdown in item is translated

@yllen yllen added the bug label Sep 9, 2016
@yllen yllen added this to the 0.90.6 milestone Sep 9, 2016
yllen added a commit that referenced this issue Sep 9, 2016
@tomolimo
Copy link
Contributor

tomolimo commented Sep 9, 2016

Could it be reverted?
Or should I commit a revert?

@tomolimo
Copy link
Contributor

tomolimo commented Sep 9, 2016

Oups, sorry you've done it already, didn't you?

@yllen
Copy link
Collaborator Author

yllen commented Sep 9, 2016

Yes i do the revert and now all is OK: list of dropdown is not translated but item in a dropdown is translated in an object (tested with many objects like statuses, itil categories...).
It's OK for you too?

@tomolimo
Copy link
Contributor

tomolimo commented Sep 9, 2016

OK for not doing the translation, I agree with you that when editing master data, we can edit them in default language.

@yllen
Copy link
Collaborator Author

yllen commented Sep 9, 2016

OK i do the same in master branch

yllen added a commit that referenced this issue Sep 9, 2016
@yllen
Copy link
Collaborator Author

yllen commented Sep 9, 2016

OK for you in master too?

@tomolimo
Copy link
Contributor

tomolimo commented Sep 9, 2016

I've no time to test, but I trust yours, so it's ok for me

@trasher trasher modified the milestones: 9.1, 0.90.6 Sep 15, 2016
@orthagh
Copy link
Contributor

orthagh commented Sep 15, 2016

tests ok for me, i close.

@trasher
Copy link
Contributor

trasher commented Dec 15, 2016

Sounds like there is still an issue in current 9.1/bugfixes and master branch, apparently due to some changes in a984704.

Adding a status using dropdowns is working fine, but displaying the list produces the following error:

  *** MySQL query error:
  SQL: SELECT \'glpi\' AS currentuser,
                        `glpi_states`.`entities_id`, `glpi_states`.`is_recursive`,  `glpi_states`.`completename` AS `ITEM_0`,
                        `glpi_states`.`id` AS `ITEM_0_id`,
                        GROUP_CONCAT(DISTINCT CONCAT(IFNULL(IFNULL(`glpi_states_trans`.`value`,\'__NULL__\') , \'__NULL__\'),
                                                          \'$#$\',`glpi_states`.`id`)
                                          SEPARATOR \'$$##$$\')
                                  AS `ITEM_0_trans`, 
                        `glpi_entities`.`completename` AS `ITEM_1`,  `glpi_states`.`id` AS id  FROM `glpi_states`LEFT JOIN `glpi_entities` 
                                          ON (`glpi_states`.`entities_id` = `glpi_entities`.`id`
                                              ) WHERE   (  1 )  ORDER BY ITEM_0 ASC  LIMIT 0, 20
  Error: Unknown column 'glpi_states_trans.value' in 'field list'
  Backtrace :
  inc/search.class.php:921                           
  inc/search.class.php:97                            Search::constructDatas()
  inc/search.class.php:80                            Search::showList()
  front/dropdown.common.php:52                       Search::show()
  front/state.php:41                                 include()

There is no table named glpi_states_trans (in facts, there is no table named *_trans at all in the whole database).

As a quick fix, I've done:

 % git diff inc/search.class.php 
diff --git a/inc/search.class.php b/inc/search.class.php
index 6e759e1..7abf925 100644
--- a/inc/search.class.php
+++ b/inc/search.class.php
@@ -2533,12 +2533,12 @@ class Search {
                            $ADDITONALFIELDS";
                }
                $TRANS = '';
-               if (Session::haveTranslations(getItemTypeForTable($table), $field)) {
+               /*if (Session::haveTranslations(getItemTypeForTable($table), $field)) {
                    $TRANS = "GROUP_CONCAT(DISTINCT CONCAT(IFNULL($tocomputetrans, '".self::NULLVALUE."'),
                                                           '".self::SHORTSEP."',$tocomputeid)
                                           SEPARATOR '".self::LONGSEP."')
                                   AS `".$NAME."_".$num."_trans`, ";
-               }
+               }*/
                return " $tocompute AS `".$NAME."_$num`,
                         `$table$addtable`.`id` AS `".$NAME."_".$num."_id`,
                         $TRANS
@@ -2553,12 +2553,12 @@ class Search {
                   || isset($searchopt[$ID]["computationgroupby"])
                      && $searchopt[$ID]["computationgroupby"]))) { // Not specific computation
          $TRANS = '';
-         if (Session::haveTranslations(getItemTypeForTable($table), $field)) {
+         /*if (Session::haveTranslations(getItemTypeForTable($table), $field)) {
             $TRANS = "GROUP_CONCAT(DISTINCT CONCAT(IFNULL($tocomputetrans, '".self::NULLVALUE."'),
                                                    '".self::SHORTSEP."',$tocomputeid) SEPARATOR '".self::LONGSEP."')
                                   AS `".$NAME."_".$num."_trans`, ";
 
-         }
+         }*/
          return " GROUP_CONCAT(DISTINCT CONCAT(IFNULL($tocompute, '".self::NULLVALUE."'),
                                                '".self::SHORTSEP."',$tocomputeid) SEPARATOR '".self::LONGSEP."')
                               AS `".$NAME."_$num`,
@@ -2566,10 +2566,10 @@ class Search {
                   $ADDITONALFIELDS";
       }
       $TRANS = '';
-      if (Session::haveTranslations(getItemTypeForTable($table), $field)) {
+      /*if (Session::haveTranslations(getItemTypeForTable($table), $field)) {
          $TRANS = $tocomputetrans." AS `".$NAME."_".$num."_trans`, ";
 
-      }
+      }*/
       return "$tocompute AS `".$NAME."_$num`, $TRANS $ADDITONALFIELDS";
    }

This is maybe not the way to go, but doing this, I can now see my statuses list and translate them. I'm pretty unsure what should be done here.

@trasher
Copy link
Contributor

trasher commented Dec 15, 2016

Still working on this one... The _trans stuff is just an alias added into a left join in the search class; displaying item list that permit to display translated dropdown values.
So maybe a solution would be to add also the join when coming from dropdown values list, do not know yet if this is possible.

@trasher
Copy link
Contributor

trasher commented Dec 15, 2016

A fix proposal:
trasher@ee2d9d9

@trasher
Copy link
Contributor

trasher commented Dec 16, 2016

The proposed solution add the missing query parts; retrieve translated value from DB, but this is not used (in administration, listed values should be what is exactly stored in database).

A better solution would be not to translate at all ; but as soon as we've got a datatype => 'itemlink' for something translated, the translation query will be played.

I'm not able to understand in which case the original code should be used; and so if the proposal breaks something or not.

I give up.

trasher added a commit to trasher/glpi that referenced this issue Dec 16, 2016
@trasher
Copy link
Contributor

trasher commented Dec 16, 2016

Saying that... I had an idea and I think I've finally find a fix...

orthagh pushed a commit that referenced this issue Dec 19, 2016
orthagh pushed a commit that referenced this issue Dec 19, 2016
@orthagh
Copy link
Contributor

orthagh commented Dec 19, 2016

Checked and merged

@orthagh orthagh closed this as completed Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants