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

When checking for existing phrase, check the group AND key. #155

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

caswd
Copy link
Contributor

@caswd caswd commented Feb 12, 2025

This fixed #111 for me.

@digitalcraftlabs maybe my issue is different from yours...

I have a structure like:

lang/
    en/
        authors.php
        posts.php

authors.php looks like this:

return [
    'page_title' => 'Authors',
];

posts.php looks like this:

return [
    'page_title' => 'Posts',
];

When I load it into my database with translations for 'en' and 'nl' (Dutch), the ltu_phrases table looks like:

id translation_id phrase_id key group
1 1 null page_title authors
2 1 null page_title posts
3 2 1 page_title authors

As you can see, I am missing the Dutch phrase for posts.page_title. This is because the query that creates the missing ltu_phrases in the database only check for key, and the key page_title does indeed already exist for the Dutch locale.

@caswd
Copy link
Contributor Author

caswd commented Feb 13, 2025

After my initial commit I had another issue. My table now looked like:

id translation_id phrase_id key group
1 1 null page_title authors
2 1 null page_title posts
3 2 1 page_title authors
4 2 1 page_title posts

Which is better.... But as you can see there is still a problem. The phrase_id of the fourth translation is incorrect. It was set to the first match by key, which not the first match by key and group.

My second commit fixes that.

I am wondering tho... Am I the first person to have a structure with many duplicate keys?

I did try the config option include_file_in_key but that open a whole other can of worms, I think that simply isn't compatible with .php translations.

@digitalcraftlabs
Copy link

@caswd i see the problem can you please add some test scenarios for this case and push it in the Branch

@caswd
Copy link
Contributor Author

caswd commented Feb 13, 2025

@digitalcraftlabs that's a good idea! I see none of the commands and actions are tested yet.

I don't have it checked out locally, but I will try to set it up in a Github Codespace to write some tests this weekend. For now, I'm simply using my local modifications in the vendor folder to get the job done 😅

@MohmmedAshraf
Copy link
Owner

Hi @caswd thank you for the PR, are you going to write tests for this or should i merge it?

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.

Missing phrases after import
3 participants