kenepa / translation-manager

Manage your application's translations in Filament.
MIT License
86 stars 27 forks source link

Quick translate offsets issue #19

Closed alanablett closed 11 months ago

alanablett commented 1 year ago

https://github.com/kenepa/translation-manager/blob/407442cb3a8ea342434ad84b5b903cc90c61a94c/src/Pages/QuickTranslate.php#L71-L109

First of all thanks for the great plugin 👏

There seems to be an issue when quick saving translations. It makes sense that offsets are used for users to skip records, but in the code there is also a whereNull check done to fetch the first available record that requires translating. In the case of a user saving and continuing, this means that after the first record is translated the offset is set to one, and therefore the first record is skipped.

Assuming there are 4 records to translate:

User translates the first record found, hits save and continue, the offset is set to one.

LanguageLine::whereNull('text->' . $this->selectedLocale)
    ->offset($this->offset)
    ->first();

Since whereNull is used, the first translated record is disregarded, but the offset is also set to one, so the next logical record is also disregarded and so we are shown record three.

I'm not sure but it might make more sense that instead of calling $this->next() in the saveAndContinue() function, instead just call $this->update() since the whereNull will load the next available translation anyway.

Jehizkia commented 11 months ago

Nice catch! Thank you. I've implemented the fix you suggested, and the previous solution was indeed skipping an untranslated record due to the offset.

Jehizkia commented 11 months ago

Also, I appreciate the detailed description and explanation. It really helps out a lot! 🔥

alanablett commented 11 months ago

Also, I appreciate the detailed description and explanation. It really helps out a lot! 🔥

No problem. Thanks again for the plugin 👏

alanablett commented 11 months ago

Sorry just another slight issue with the fix #23 Since the next method has been removed you may also want to remove the button to Skip this translation. Cheers

Jehizkia commented 11 months ago

Thanks again, I'll fix it!