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

WIP: Email localization (fixes #500) #2053

Merged
merged 4 commits into from
Mar 24, 2022
Merged

WIP: Email localization (fixes #500) #2053

merged 4 commits into from
Mar 24, 2022

Conversation

Nutomic
Copy link
Member

@Nutomic Nutomic commented Jan 20, 2022

Needs some clarifications from upstream: baptiste0928/rosetta#5

@Nutomic Nutomic force-pushed the email-localization branch 3 times, most recently from 6be867c to e17cce7 Compare March 2, 2022 21:15
@Nutomic
Copy link
Member Author

Nutomic commented Mar 2, 2022

Updated, basic implementation is now done. For now it uses separate translation files (which need to get moved into lemmy-translations repo). After upstream changes we should be able to merge it with existing translation files.

I'm not really happy with the way the code is structured now, as email notification logic is split across crates utils, api, websocket. Ideally all of it would be together in the websocket crate (under name notifications), but that would create a circular dependency between api and websocket.

@Nutomic
Copy link
Member Author

Nutomic commented Mar 14, 2022

@dessalines Could you have a look at this?

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Looks good! Let me know when you're ready to take it out of draft and I can help test it a bit.

@@ -104,11 +105,16 @@ impl PerformCrud for CreatePrivateMessage {
LocalUserView::read_person(conn, recipient_id)
})
.await??;
let lang = get_user_lang(&local_recipient);
let inbox_link = format!("{}/inbox", context.settings().get_protocol_and_hostname());
Copy link
Member

Choose a reason for hiding this comment

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

Was confused at first, but yes this does make sense, its their lemmy-ui inbox page.

crates/api_crud/src/user/create.rs Outdated Show resolved Hide resolved
&inserted_local_user.email.expect("email was provided"),
&inserted_person.name,
&local_user_view,
&email,
Copy link
Member

Choose a reason for hiding this comment

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

Ah, iirc email is only in LocalUser, not the LocalUserSettings or Safe.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you mean, local_user_view contains LocalUser. And I dont think i changed any of the logic.

"notification_mentioned_by_body": "<h1>Person Mention</h1><br><div>{username} - {comment_text}</div><br><a href={inbox_link}>inbox</a>",
"notification_private_message_subject": "Private message from {username}",
"notification_private_message_body": "<h1>Private message</h1><br><div>{username} - {message_text}</div><br><a href={inbox_link}>inbox</a>"
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice, this is pretty easy, as long as its positional then.

Not super looking forward to setting up a third weblate for lemmy... but its pry gonna need one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, at least temporarily until the rosetta crate is updated.

}

pub fn get_user_lang(user: &LocalUserView) -> Lang {
let user_lang = LanguageId::new(user.local_user.lang.clone());
Copy link
Member

Choose a reason for hiding this comment

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

If this doesn't work correctly for things like pt-br, we might have to unfortunately create mappings. But lets not worry about that yet.

@Nutomic Nutomic marked this pull request as ready for review March 21, 2022 12:15
@Nutomic Nutomic force-pushed the email-localization branch 3 times, most recently from 1b41054 to 0a7474d Compare March 22, 2022 18:24
.gitmodules Show resolved Hide resolved
@dessalines dessalines enabled auto-merge (squash) March 24, 2022 01:47
@Nutomic
Copy link
Member Author

Nutomic commented Mar 25, 2022

I setup weblate translations.

https://weblate.yerbamate.ml/projects/lemmy/emails/

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.

2 participants