Skip to content

Conversation

@nattsw
Copy link
Contributor

@nattsw nattsw commented May 13, 2025

This PR fixes the bug where we were double-cooking when saving PostLocalization when the AI provider is used.

The reason is due to the fact that we used to translate raw, then cook, and save the cooked. But now, we are required to save both the raw and cooked into PostLocalization.

This PR also splits translate_translatable into translate_post and translate_topic. Originally they were taking the same route, but it was proving to be a bit annoying because post uses raw and cooked, whereas topic uses title. It makes understanding any issues with a single model (e.g. Posts) easier to understand if it is a single code path. Lastly, PR also fixes an Amazon provider bug as it was never tested.

@nattsw nattsw force-pushed the dont-double-cook branch from 0498c1e to d98a5fb Compare May 13, 2025 05:42
def self.translate_translatable!(translatable, target_locale_sym = I18n.locale)
detected_lang = detect(translatable)
def self.translate_post!(post, target_locale_sym = I18n.locale, opts = {})
raw = opts.key?(:raw) ? opts[:raw] : !opts[:cooked]
Copy link
Member

@SamSaffron SamSaffron May 13, 2025

Choose a reason for hiding this comment

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

generally if we can get away without this pattern I think it is better..

def self.translate_post!(post, locale: I18n.locale, type: :raw)

something a bit clearer sig wise

@SamSaffron
Copy link
Member

overall I approve, it is well tested, but the interface can be a bit more readable, avoiding opts and being deliberate about params makes stuff a bit clearer

@nattsw
Copy link
Contributor Author

nattsw commented May 13, 2025

Yeah I agree about the interface.. the first step here was to expand to a specific method per model (post + topic) so there would be less opts thrown about.

@nattsw nattsw merged commit 7507795 into main May 13, 2025
6 checks passed
@nattsw nattsw deleted the dont-double-cook branch May 13, 2025 06:39
nattsw added a commit that referenced this pull request May 23, 2025
In #203 we introduced a bug for the amazon provider, which was fixed in #297.

This commit is a migration to purge the incorrect translations that have been stored. The translation will look like the following as the full response is saved instead of the specific attribute.

```
{translated_text: ...
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants