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

Add macro for $this->translations() in factories #382

Merged
merged 5 commits into from
Apr 20, 2023

Conversation

bram-pkg
Copy link
Contributor

@bram-pkg bram-pkg commented Mar 24, 2023

Context

Because factory definitions do not go through a Model's accessors/mutators, old definitions break when migrating columns to use the translatable trait. This PR helps users overcome this hurdle, by adding a macro to the factory that outputs valid JSON for the translatable columns.

Proposal

This macro enables users to do the following in factory definitions, where description is a translatable column:

Scenario 1:

return [
  'description' => $this->translatable('en', 'English description'),
];

// output: {"en":"English description"}

Scenario 2:

return [
  'description' => $this->translatable(['en', 'nl'], 'English and Dutch description'),
];

// output: {"en":"English and Dutch description","nl":"English and Dutch description"}

Scenario 3:

return [
  'description' => $this->translatable(
    ['en', 'nl'],
    [
      'English description',
      'Dutch description',
    ]
  ),
];

// output: "{"en":"English description","nl":"Dutch description"}"

The translatable function can also be used statically, using Factory::translatable

Documentation

I would appreciate some guidance on the documentation, specifically what page this should be documented on.

@freekmurze
Copy link
Member

Thanks!

Could you also ad a test to make sure that this works.

As for docs, I would add a page in the "Advanced" section, titled "Usage with factories". On this page, explain how you can use the macro in a few sentences, and provide a clear example.

@bram-pkg
Copy link
Contributor Author

Yep, of course I can do that.

I will write some documentation for this functionality.

Currently I am quite busy with a family emergency, that might take a few weeks.
Just letting you know (@freekmurze), also because of the Nova translatable activity.

@moham96
Copy link
Contributor

moham96 commented Apr 7, 2023

This would be great, but it doesn't work,

The macro produces this:

{
  "en": "\u0647\u0627\u0646\u064a \u0627\u0644\u062c\u0647\u0646\u064a",
  "ar": "\u0623\u0631\u0648\u0649 \u0645\u0643\u064a"
}

but when the factory is used in a seeder it will write the above string as a translation in it self, so the results in the database is the following:

{
  "ar": "{\"en\":\"\\u0647\\u0627\\u0646\\u064a \\u0627\\u0644\\u062c\\u0647\\u0646\\u064a\",\"ar\":\"\\u0623\\u0631\\u0648\\u0649 \\u0645\\u0643\\u064a\"}"
}

@bram-pkg
Copy link
Contributor Author

Can you give me a code snippet for this?
Factories use the model depending on how you call them.
Seeders might not, once again depending on how you call them.

@moham96
Copy link
Contributor

moham96 commented Apr 12, 2023

Can you give me a code snippet for this? Factories use the model depending on how you call them. Seeders might not, once again depending on how you call them.

This is a basic example:

Migration: create_elinks_table.php

<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
    /**
     * Run the migrations.
     */
    public function up(): void
    {
        Schema::create('elinks', function (Blueprint $table) {
            $table->id();
            $table->timestamps();
            $table->json('title');
            $table->longText('url');
        });
    }

    /**
     * Reverse the migrations.
     */
    public function down(): void
    {
        Schema::dropIfExists('elinks');
    }
};

ElinkFactory.php

<?php

namespace Database\Factories;

use Illuminate\Database\Eloquent\Factories\Factory;

/**
 * @extends \Illuminate\Database\Eloquent\Factories\Factory<\App\Models\Elink>
 */
class ElinkFactory extends Factory
{
    /**
     * Define the model's default state.
     *
     * @return array<string, mixed>
     */
    public function definition(): array
    {
        return [
            "title" => $this->translatable(['en'], [$this->faker->realText()]),
            "url" => $this->faker->url(),
        ];
    }
}

Calling the factory from a seeder or from tinker like this: \App\Models\Elink::factory()->create();

The result in the database is this:

{"ar":"{\"en\":\"\\u0635\\u063a\\u064a\\u0631\\u0629 \\u062c\\u062f\\u0627\\u064b\\u060c \\u0645\\u0646\\u0642\\u0633\\u0645\\u0629 \\u0628\\u0642\\u0633\\u0645\\u064a\\u0646\\u060c \\u0628\\u064a\\u0646\\u0647\\u0627 \\u062d\\u062c\\u0627\\u0628 \\u0631\\u0642\\u064a\\u0642\\u060c \\u0645\\u0645\\u062a\\u0644\\u0626\\u0629 \\u0628\\u062c\\u0633\\u0645 \\u0644\\u0637\\u064a\\u0641 \\u0647\\u0648\\u0627\\u0626\\u064a \\u0641\\u064a \\u063a\\u0627\\u064a\\u0629 \\u0627\\u0644\\u0648\\u062b\\u0627\\u0642\\u0629\\u060c \\u0648\\u0627\\u0644\\u0631\\u062b\\u0629 \\u0645\\u0637\\u064a\\u0641\\u0629 \\u0628\\u0647 \\u0645\\u0646 \\u0627\\u0644\\u0639\\u0631\\u064a \\u0648\\u0639\\u062f\\u0645 \\u0627\\u0644\\u0633\\u0644\\u0627\\u062d\\u060c \\u0648\\u0636\\u0639\\u0641 \\u0627\\u0644\\u0639\\u062f\\u0648\\u060c \\u0648\\u0642\\u0644\\u0629 \\u0627\\u0644\\u0628\\u0637\\u0634\\u060c \\u0639\\u0646\\u062f\\u0645\\u0627 \\u0643\\u0627\\u0646\\u062a \\u062a\\u0646\\u0627\\u0632\\u0639\\u0647 \\u0627\\u0644\\u0648\\u062d\\u0648\\u0634 \\u0623\\u0643\\u0644 \\u0627\\u0644\\u062b\\u0645\\u0631\\u0627\\u062a\\u060c \\u0648\\u062a\\u0633\\u062a\\u0628\\u062f \\u0628\\u0647\\u0627.\"}"}

@moham96
Copy link
Contributor

moham96 commented Apr 12, 2023

BTW ths solution for the problem is very simple, don't return a json string, instead return an array in a format the laravel-translatable can understand, so the function should look like this:

Factory::macro('translatable', function (string | array $locales, mixed $value) {
            return
            is_array($value)
            ? array_combine((array) $locales, $value)
            : array_fill_keys((array) $locales, $value);

        });

It works but I haven't done much testing

@bram-pkg
Copy link
Contributor Author

Let me do some testing on your suggestion, returning an array might be the solution.

@bram-pkg
Copy link
Contributor Author

@freekmurze I have added tests and docs :)

@freekmurze
Copy link
Member

All clear now. Nice! One final remark: could you rename translatable to translations everywhere? That feels a tad more human and rememberable.

@bram-pkg
Copy link
Contributor Author

Sure, I will rename.

@bram-pkg bram-pkg changed the title Add macro for $this->translatable() in factories Add macro for $this->translations() in factories Apr 20, 2023
@freekmurze freekmurze merged commit 732e46b into spatie:main Apr 20, 2023
11 checks passed
@freekmurze
Copy link
Member

Thanks!

@bram-pkg bram-pkg deleted the patch-1 branch May 9, 2023 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants