Laravel-Backpack / community-forum

A workspace to discuss improvement and feature ideas, before they're actually implemented.
28 stars 0 forks source link

HasTranslations should listen for the `creating` instead of override `create` method #670

Closed promatik closed 2 weeks ago

promatik commented 1 year ago

Today I took a look at HasTranslations hand most probably it should have some upgrades.

It all started here; https://github.com/Laravel-Backpack/CRUD/issues/5312, @azenot-dev complained about a collision of the create method with another trait, there's nothing we could do regarding that, BUT, to override both create and update method isn't the best thing to do.

Can we move it to creating and updating events?


Also, I noticed this method;

    public function __call($method, $parameters)
    {
        switch ($method) {
            // translate all find methods
            case 'find':
            case 'findOrFail':
            case 'findMany':
            case 'findBySlug':
            case 'findBySlugOrFail':
                $translation_locale = \Request::input('_locale', \App::getLocale());

                if ($translation_locale) {
                    $item = parent::__call($method, $parameters);

                    if ($item instanceof \Traversable) {
                        foreach ($item as $instance) {
                            $instance->setLocale($translation_locale);
                        }
                    } elseif ($item) {
                        $item->setLocale($translation_locale);
                    }

                    return $item;
                }

                return parent::__call($method, $parameters);
                break;

                // do not translate any other methods
            default:
                return parent::__call($method, $parameters);
                break;
        }
    }

I found it strange that is is done only for find* methods. by doing Product::first()->name or Product::find(1)->name you may get different results 🤷‍♂️

So Instead of always translating (for finds only), maybe we should add some flag.

Model::translated()->findBySulg($slug);

or have it translated by default, but have a flag to disable it.

Model::untranslated()->findBySulg($slug);
pxpm commented 2 weeks ago

I don't think we will be doing this translated() and untranslated().

The reason why the find methods are translated it's because they are the methods that we (backpack) use. If you use the model outside of admin panel, you should take care to get the translations or not.

Cheers