OmgDef / yii2-multilingual-behavior

Yii2 port of the yii-multilingual-behavior.
146 stars 60 forks source link

Few bugs (repeated queries) #7

Closed andreCatita closed 9 years ago

andreCatita commented 9 years ago

Hello @OmgDef

First all, good job so far. I would like to point out a few things:

In your usages examples #1

//Assuming current language is english
$model = Post::findOne(1);
echo $model->title; //echo "English title"
//Now let's imagine current language is french 
$model = Post::findOne(1);
echo $model->title; //echo "Titre en Français"
$model = Post::find()->localized('en')->one();
echo $model->title; //echo "English title"
//Current language is still french here

This works, but from what I understood from a recent mail exchange we had, I thought the default Language wasn't supposed to go search for translation in PostLang, it would just use the Post for the default language. But I was able to fix that, in your MultilingualTrait, here is what I can't fix, or figure out why:

If after that example #1 of yours, I do the following:

$model = Post::find()->where(['id' => 1])->multilingual()->one();

The MultilingualTrait performs this query correctly, ie: SELECT * FROM PostLang WHERE post_id=1 It will also repeat the last find query, in your example #1, it means it would repeat a query to PostLang, searching for 'en'.

Here is my sequence with my 'Products' model:

$model = Products::findOne(1);
echo "<br>";
echo "findOne (default language pt) title: " . $model->title;
echo "<br>";
\Yii::$app->language = 'en';
$model = Products::findOne(1);
echo "<br>";
echo "findOne (default language en) title: " . $model->title;
echo "<br>";
\Yii::$app->language = 'fr';
$model = Products::findOne(1);
echo "<br>";
echo "findOne (default language fr) title: " . $model->title;
echo "<br>";
$model = Products::find()->where(['id_product' => 1])->multilingual()->one();
echo "<br>";
echo "findMultiLingual title: " . $model->title;
echo "<br>";
echo "findMultiLingual title_en: " . $model->title_en;
echo "<br>";
echo "findMultiLingual title_fr: " . $model->title_fr;

I hoped this would result in 4 queries. Without changing anything, by default your extension performs 6 queries. Taken I'm testing with 'pt', 'en' and 'fr'. He queries Products and ProductsLang for 'pt' ( 2 queries ) -> This one I was able to fix. He then queries ProductsLang for 'en' and 'fr' ( 2 queries ) * He then queries ProductsLang (without lang for multilingual ) ( 1 query) * At this point is when he repeats the query for the last findOne, which in this sequence was the 'fr' one..

OmgDef commented 9 years ago

Hi @andreCatita Look at this a160355d2f9b5c3655815899a299cb858fd2b517

Language wasn't supposed to go search for translation in PostLang, it would just use the Post for the default language.

In the near future I want to delete the duplicate columns from the parent table. So, we will always have two queries.

I hoped this would result in 4 queries

If you want to reduce number of queries you should use joinWith, but I don't think that it will be faster than additional query.

andreCatita commented 9 years ago

Thanks @OmgDef

I'm not testing it right now, but I hope that commit fixes the duplicated queries, when calling multilingual.

As for the two queries, if you delete the duplicated parent fields, I'm all for that approach, because it makes more sense in terms of DB design, but in the meanwhile, I'm just going to make it work by making sure your Trait doesn't fetch from the Lang table if we are in the the default language (I have set).

As for the result in fewer queries, I meant because of the bugs in duplicating it when using multilingual.

Another "bug" you might possibly try to fix, is that you allow the users to specify the column name, in my case it's lang and not language, while that works in the MultilingualBehaviour, it doesn't work in MultilingualTrait.php, which makes one have to go the vendor directly and change that there.

OmgDef commented 9 years ago

Another "bug" you might possibly try to fix, is that you allow the users to specify the column name, in my case it's lang and not language, while that works in the MultilingualBehaviour, it doesn't work in MultilingualTrait.php, which makes one have to go the vendor directly and change that there.

@andreCatita You can easily override $languageField property of the MultilingualTrait and set to lang

andreCatita commented 9 years ago

I know @OmgDef, I was just commenting on the fact that your ml configuration array, is not enough to change the column name and get it working.

I'll give feedback on the repeated queries tomorrow, thanks,

andreCatita commented 9 years ago

Well as far as I'm concerned the main issue here was fixed, which was the repeated queries with multilingual.

The language field was just a note, that the model configuration isn't enough AS IS.

And while we have the duplicated column system, I'm not going to use the lang for the default language, which is why I just added a quick if:

    public function localized($language = null) {
        if (!$language)
            $language = Yii::$app->language;

        if (\Yii::$app->params['defaultLanguage'] != $language) {
            if (!isset($this->with['translations'])) {
                $this->with(['translation' => function ($query) use ($language) {
                        $query->where([$this->languageField => substr($language, 0, 2)]);
                }]);
            }
        }
        return $this;
    }
andreCatita commented 9 years ago

Hey @OmgDef ,

I was trying to keep your extension up-to-date, but by doing so, I have a problem, because it's too late to go back on this project in terms of DB structure, and since "we" had duplicated columns in the "Post" table before, I just made sure, I ignored the query to get the PostLang relation for my 'default' language With this hack if (\Yii::$app->params['defaultLanguage'] != $language) {, mentioned in my previous comment. However in your most up-to-date commit, this no longer works.

When I perform findOne(1) in my default Language, I get 2 queries, when I was able to "tweak" it, to just receive 1 before for the defaultLanguage, but I get this:

SELECT * FROM shop_products WHERE id_product=1 SELECT * FROM shop_products_lang WHERE (lang='pt') AND (id_product='1')

The debugger says it's line: MultilingualBehavior.php (275)

Can you help me add my tweak to your "2.0 milestone", making me Ignore the relation table in my default language?

Thanks a lot!!

OmgDef commented 9 years ago

@andreCatita, You get the second query here https://github.com/OmgDef/yii2-multilingual-behavior/blob/master/src/MultilingualBehavior.php#L281

Why don't you want to use the first version? The main objective of the second version to remove duplicate columns

andreCatita commented 9 years ago

@OmgDef, I just figured there will be a bug fix at some point, which you will apply to 2.0 and move forward, so I thought I'd just tweak it. I'll check that tomorrow thanks.