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

Show deny reason to users after a failed login. Fixes #2191 #2206

Merged
merged 3 commits into from
Apr 19, 2022

Conversation

dessalines
Copy link
Member

@dessalines dessalines commented Apr 11, 2022

I tested this and it works, correctly shows the deny reason if you try to login.

if registration.deny_reason.is_some() {
return Err(LemmyError::from_message("registration_denied"));
if let Some(deny_reason) = registration.deny_reason {
return Err(LemmyError::from_message(&deny_reason));
Copy link
Member

Choose a reason for hiding this comment

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

Most of the deny reasons i see on lemmy.ml are really short, eg "Not an answer". Its going to be very confusing if that is shown as an error message after login. So this should have some message such as: "Registration denied: {{deny_reason}}".

Copy link
Member Author

Choose a reason for hiding this comment

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

I could add that, but it wouldn't be i18n compatible, it'd be always english. This is because the front end, if it can't find that i18n string, just outputs the whole text. So since it can't find the string : "registration_denied: X reason here", it would always be in english.

Copy link
Member

Choose a reason for hiding this comment

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

I mean do the localization on the backend, same way we do for email.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not fully familiar with how this works. In this build.rs, I'd need to add a 2nd src file with all those languages? Seems like there might be a namespace type conflict?

Copy link
Member

Choose a reason for hiding this comment

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

I would just add it to the same translation file we use for email, even if that makes the name accurate. So just add your string to the existing en.json and it should work.

Copy link
Member Author

Choose a reason for hiding this comment

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

K done.

@dessalines dessalines force-pushed the adding_deny_reason branch 2 times, most recently from cbcf8e1 to 163284b Compare April 13, 2022 18:15
@Nutomic Nutomic enabled auto-merge (squash) April 19, 2022 10:35
@Nutomic Nutomic merged commit 24be9f2 into main Apr 19, 2022
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