Translations are not saved when using Baum\Node about laravel-translatable HOT 33 CLOSED

The solution provided by @freekmurze solved most of my issues - though I encountered a strange bug when running my integration tests when saving my translatable model.

When saving, the SQL query being generated was trying to save a column "translations", instead of saving it as a relation. I haven't looked into it very deeply, but Eloquent was mistaking the 'translations' relationship as an attribute.

This is my workaround, a slight variation on @freekmurze (note the $this->setRelation):

<?php 

trait Translatable
{
    use \Dimsav\Translatable\Translatable;

    public function save(array $options = array())
    {
        $tempTranslations = $this->translations;
        if ($this->exists)
        {
            if (count($this->getDirty()) > 0)
            {
                // If $this->exists and dirty, parent::save() has to return true. If not,
                // an error has occurred. Therefore we shouldn't save the translations.
                if (parent::save($options))
                {
                    $this->setRelation('translations', $tempTranslations);
                    return $this->saveTranslations();
                }
                return false;
            }
            else
            {
                // If $this->exists and not dirty, parent::save() skips saving and returns
                // false. So we have to save the translations
                $this->setRelation('translations', $tempTranslations);
                return $this->saveTranslations();
            }
        }
        elseif (parent::save($options))
        {
            // We save the translations only if the instance is saved in the database.
            $this->setRelation('translations', $tempTranslations);
            return $this->saveTranslations();
        }
        return false;
    }
}

from laravel-translatable.

I ended up overriding the save method in a trait of my own.

<?php namespace Spatie\Services\Translatable;

trait Translatable
{
    use \Dimsav\Translatable\Translatable;

    public function save(array $options = array())
    {
        $tempTranslations = $this->translations;
        if ($this->exists)
        {
            if (count($this->getDirty()) > 0)
            {
                // If $this->exists and dirty, parent::save() has to return true. If not,
                // an error has occurred. Therefore we shouldn't save the translations.
                if (parent::save($options))
                {
                    $this->translations = $tempTranslations;
                    return $this->saveTranslations();
                }
                return false;
            }
            else
            {
                // If $this->exists and not dirty, parent::save() skips saving and returns
                // false. So we have to save the translations
                $this->translations = $tempTranslations;
                return $this->saveTranslations();
            }
        }
        elseif (parent::save($options))
        {
            // We save the translations only if the instance is saved in the database.
            $this->translations = $tempTranslations;
            return $this->saveTranslations();
        }
        return false;
    }
}

Not the prettiest solution, but it'll work until Baum updates it's package. Haven't found the time to open an issue on that project though.

from laravel-translatable.

@michaelstivala Works like a charm, thanks for sharing!

from laravel-translatable.

@michaelstivala had the same error. Your solution worked. Thanks

from laravel-translatable.

@michaelstivala thanks for your solution! still working until today!

from laravel-translatable.

Hi @freekmurze,

as you can see in the Translatable trait, the translations are saved only if the parent class of Category returns true after saving.

You could dump parent::save($options) to see if true is returned or not.
If false is returned, then Baum/Node is not able to save probably because of a validation issue.

Keep me posted :)

Update: there is currently an issue causing the translations to not be saved. I am working on it now.

from laravel-translatable.

There was an issue with one of the latest commits of laravel 4.1 core. Model::The save() method now returns false if the object is not dirty. As a result the translations where not saved.

This is fixed in v4.1.1.

Please run a composer update and if the problem persists, let me know.

Thanks!

from laravel-translatable.

after updating to 4.1.1 I get an error regarding the foreign-key not being filled in the translation-table.

$category->getRelationKey() returns category_id

I've checked if parent::save($options) returns true and it does.

from laravel-translatable.

upgraded to the very fresh 4.2 version, but unfortunately the problem described above persists

from laravel-translatable.

Although parent::save() returns true there is something that went wrong with saving the object. After the call to parent::save() $this->translations was empty.

The problem can be fixed in a dirty way by storing the translations in a variable before parent::save() is called and restoring them afterwards

    public function save(array $options = array())
    {
        if (count($this->getDirty()) > 0)
        {
            $tempTranslations = $this->translations;
            if (parent::save($options))
            {
                $this->translations = $tempTranslations;
                return $this->saveTranslations();
            }
            return false;
        }
        else
        {
            return $this->saveTranslations();
        }
    }

I'm not 100% sure, but I think the problem does not lie with your component, but with Baum\Node.

from laravel-translatable.

The screenshot above shows that the category_id is null. Can you see if the category is created before saving the translations?

from laravel-translatable.

Regarding the screeshot: the category was not saved in the db because a category itself had no properties besides id. When no properties are set on a object, it was not saved. If described this in ticket #26. I believe this is a bug introduced in version 4.2.

However when creating a dummy field on category and setting it to a value, the category is saved in the db, but my original problem (the translations not being saved) persist.

from laravel-translatable.

Update: Closing as duplicate of #26 not related to this package . Thanks for the feedback.

from laravel-translatable.

Though I think it is correct that you close this issue as the real problem lies within Baum/Node, you should note that the issue was not a duplicate of #26

from laravel-translatable.

Ok, feel free to open an issue if you see something wrong with laravel-translatable.

from laravel-translatable.

Hello guys i faced this problem anyone solved this?

from laravel-translatable.

@freekmurze thanks man it is worked :) Now i have problem with dynamic nested combobox with Translatable column :D Did you solve that ??

from laravel-translatable.

Nope, haven't come accross that problem.

from laravel-translatable.

Found the problem in baum,
sadly it's in a core function so I don't think it will be 'fixed' for us....
I think the proposed sulution above it's less problematic than anything else

from laravel-translatable.

Hey, can you send a link with the lines including the bug?

Thanks

from laravel-translatable.

I repeat it's not a bug, just how Baum works.
When a model is saved, baum hooks into the saved event and therefore calls this method

https://github.com/etrepat/baum/blob/master/src/Baum/Extensions/Eloquent/Model.php#L18-36

from laravel-translatable.

Thanks @freekmurze for giving a temporary solution! 😄 I just came across that same issue.

from laravel-translatable.

Sorry for jumping in so late on this.

There's no bug/issue in Baum. It just happens to hook on save events and perform several actions for keeping the table structure (housekeeping).

Maybe we can find a way to make the packages compatible... @dimsav could you please give me some insight on why Baum is interfering with your save hooks?

from laravel-translatable.

@etrepat basically this package saves the translations after the parent model is saved.
after it calls this method that loops over the $this->translations relationship.

but that property is empty because when your package save the parent model, the package relods the model from db and, obviously, the translations are not yet in db so that property doesn't exists

from laravel-translatable.

Thanks @Fed03.

So, if I understand correctly (please, correct me if I'm wrong)...

It happens that when saving a Baum\Node instance, that node gets reloaded (this happens in two situations: the first being when that node is moved, and the second, when recomputing the node's depth) before the Translatable::save() extended method can perform its work. I understand that this is because Eloquent dispatches the saved event (which Baum hooks into) before returning from its save method, so Translatable just happens to go afterwards in the inheritance chain. The problem arises then, because the saveTranslations() method cannot find any "dirty" translations (because that instance has been freshened from the db) and so, the changes on them do not get saved (there's no change on them anyway at this point).

As I see it, it is indeed not a bug on neither side but just an incompatible way of dealing with model saving.

I can't come up with any obvious solution to this right now, because I have not looked into this package implementation very much, but maybe I can think of something to make the packages compatible.

from laravel-translatable.

Same problem with another nested set package, and I think its a general problem:
https://github.com/lazychaser/laravel-nestedset/blob/master/src/Node.php#L117
the 'saving' event is not fired, in Laravel model the saving event is always called:

public function save(array $options = array())
    {
        $query = $this->newQueryWithoutScopes();
        // If the "saving" event returns false we'll bail out of the save and return
        // false, indicating that the save failed. This provides a chance for any
        // listeners to cancel save operations if validations fail or whatever.
        if ($this->fireModelEvent('saving') === false)
        {
            return false;
        }

after that the 'saved' event is called.
in translatable, the 'saved' event is sometimes not called;

also

// If $this->exists and not dirty, parent::save() skips saving and returns
                // false. So we have to save the translations
                if ($saved = $this->saveTranslations()) {
                    $this->fireModelEvent('saved', false);
                }

save returns true, not false; so you can just

public function save(array $options = [])
    {
        if (parent::save($options)) {
            return $this->saveTranslations();
        }
        return false;
    }

from laravel-translatable.

I had the same problem with Baum and I resolved with the solution of @freekmurze #25 (comment)

@dimsav can you add @freekmurze solution in master branch?

from laravel-translatable.

@etrepat @dimsav Any news on this issue? Could we find a way to make compatible the projects?

from laravel-translatable.

@dimsav I agree with @studiomado , can you add @freekmurze solution in master branch ?

Temporary i saved my own trait:

<?php 
namespace App\Http\Classes;

trait TranslateTrait{
    use \Dimsav\Translatable\Translatable;
public function save(array $options = array())
    {
   //here i pasted  function from comment #25
...

}

from laravel-translatable.

Glad that my solution has helped! I'm still using it too :)

from laravel-translatable.

@michaelstivala
Thank you! Working great here as well ...

from laravel-translatable.

When i save these categories i am getting a lot of queries, can it be quite optimized ? Or it is normal behavior ? I used Trait published above by @freekmurze (Baum + Translatable)

$c1 = Category::create([
            'ru' => [
                'title' => 'Root category'
            ]
        ]);
        $c2 = Category::create([
            'ru' => [
                'title' => 'Test1'
            ]
        ]);
        $c3 = Category::create([
            'ru' => [
                'title' => 'Root2 category'
            ]
        ]);
        $c4 = Category::create([
            'ru' => [
                'title' => 'Subcat'
            ]
        ]);
        $c5 = Category::create([
            'ru' => [
                'title' => 'Subcat2'
            ]
        ]);
        $c6 = Category::create([
            'ru' => [
                'title' => 'SubSubcat'
            ]
        ]);
        $c4->makeChildOf($c3);
        $c5->makeChildOf($c3);

        $c6->makeChildOf($c5);

        $c2->makeChildOf($c1);

from laravel-translatable.

FYI , i used this package https://github.com/lazychaser/laravel-nestedset and it works great, more stable than Baum, and there is good feature to rebuilding a tree from array

from laravel-translatable.