Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Translation] [Loco] Fix idempotency of LocoProvider write method#44187

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:5.3fromwelcoMattic:fix/loco-provider
Nov 29, 2021

Conversation

@welcoMattic
Copy link
Member

@welcoMatticwelcoMattic commentedNov 21, 2021
edited
Loading

QA
Branch?5.3
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#43953
LicenseMIT
Doc PR

This PR fix the exception thrown when we push translations twice, with existing translations on Loco.

Explanation of the fix:

To translate an "asset" (a translation key in Loco vocabulary), we need its Locoid. But Loco does not allow us to retrieve an id from a key. So, we fetch all ids for the current domain. Then, we build a map withkey as key, andid as value. After that, we can intersect the keys of this map and our messages array to get all Loco ids of the currently processed messages.

foreach ($catalogue->all()as$domain =>$messages) {$keysIdsMap = [];foreach ($this->getAssetsIds($domain)as$id) {$keysIdsMap[$this->retrieveKeyFromId($id,$domain)] =$id;    }$ids =array_intersect_key($keysIdsMap,$messages);$this->translateAssets(array_combine(array_values($ids),array_values($messages)),$locale);}

Todo:

  • Make some real tests to be 100% sure there is no BC
  • Add tests to guarantee the fix

Kocal reacted with eyes emoji
@welcoMatticwelcoMattic changed the title[Translation] [Loco] Fix idempotency of translation:push on Loco[Translation] [Loco] Fix idempotency of LocoProvider write methodNov 26, 2021
@welcoMattic
Copy link
MemberAuthor

Ready for review /cc@nicolas-grekas@OskarStark


$ids =array_intersect_key($keysIdsMap,$messages);

$this->translateAssets(array_combine(array_values($ids),array_values($messages)),$locale);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

what if some keys don't have an asset id ? This would fail inarray_combine AFAICT due to the different number of items.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The case could not happen, Loco generate an asset for every asset.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

As we just create assets for new keys, they mandatory have an asset id

@nicolas-grekas
Copy link
Member

Thank you@welcoMattic.

@nicolas-grekasnicolas-grekas merged commitc19bc94 intosymfony:5.3Nov 29, 2021
This was referencedNov 29, 2021
@fabpotfabpot mentioned this pull requestDec 29, 2021
@welcoMatticwelcoMattic deleted the fix/loco-provider branchJuly 6, 2022 09:55
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof approved these changes

@OskarStarkOskarStarkOskarStark approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.3

Development

Successfully merging this pull request may close these issues.

5 participants

@welcoMattic@nicolas-grekas@stof@OskarStark@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp