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

[fix] Improve the message w.a. when admins tries to create users with a reserved Email Address (Issue 1216) #553

Merged
merged 8 commits into from Oct 31, 2018

Conversation

Projects
None yet
3 participants
@frju365
Member

frju365 commented Oct 7, 2018

The problem

  • Error message is not useful to inform user that email adress will be created by default ( fixes YunoHost/issues#1216)

Solution

  • Add a custom Message.

PR Status

Finished. Not tested.

How to test

  • Install Yunohost (ynh-dev)
  • Modify app.py
  • Try to create a user and add an already-used alias for this email

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :

frju365 added some commits Oct 7, 2018

@frju365 frju365 closed this Oct 7, 2018

@frju365 frju365 reopened this Oct 7, 2018

@@ -122,6 +122,7 @@
"mail_alias_remove_failed": "Impossible de supprimer l'alias courriel « {mail:s} »",
"mail_domain_unknown": "Le domaine « {domain:s} » du courriel est inconnu",
"mail_forward_remove_failed": "Impossible de supprimer le courriel de transfert « {mail:s} »",
"mail_unavailable": "Adresse mail indisponible",

This comment has been minimized.

@alexAubin

alexAubin Oct 7, 2018

Member

(We are not supposed to edit fr.json, this usually breaks weblate :s)

This comment has been minimized.

@frju365

frju365 Oct 7, 2018

Member

well... it's because weblate is already broken that I did that. But ok. Will be removed. Only english will stay.

This comment has been minimized.

@frju365
@alexAubin

alexAubin approved these changes Oct 7, 2018 edited

Not tested but LGTM (though it'd be nice if somebody could test this as it's a critical piece of code)

(But please name your PRs with something more explicit than 'Fix issue #[randomnumber]' 😅 )

@@ -255,6 +255,7 @@
"mail_domain_unknown": "Unknown mail address domain '{domain:s}'",
"mail_forward_remove_failed": "Unable to remove mail forward '{mail:s}'",
"mailbox_used_space_dovecot_down": "Dovecot mailbox service need to be up, if you want to get mailbox used space",
"mail_unavailable": "Mail adress unavailable",

This comment has been minimized.

@alexAubin

alexAubin Oct 7, 2018

Member

I think this message could be improved, e.g. by clarifying that those addresses are reserved and automatically assigned to the first user created

This comment has been minimized.

@frju365

frju365 Oct 7, 2018

Member

done.

frju365 added some commits Oct 7, 2018

@frju365 frju365 changed the title from Try to fix issue 1216 to [fix] Message when admins tries to create users with a reserved Email Address Oct 7, 2018

@frju365 frju365 changed the title from [fix] Message when admins tries to create users with a reserved Email Address to [fix] Improve the message w.a. when admins tries to create users with a reserved Email Address Oct 7, 2018

@frju365 frju365 changed the title from [fix] Improve the message w.a. when admins tries to create users with a reserved Email Address to [fix] Improve the message w.a. when admins tries to create users with a reserved Email Address (Issue 1216) Oct 7, 2018

@zamentur

I suggest to check if this reserved mail is already assigned.
If not, we should allow the admin to set up this mail on this user...

I volunteer to implement it, but feel free to do it, if you prefer.

@alexAubin

This comment has been minimized.

Member

alexAubin commented Oct 24, 2018

@zamentur : not sure what you mean : those emails are to be allocated to the first user anyway, so ?

@alexAubin

Tested and works okay on my side

@alexAubin

This comment has been minimized.

Member

alexAubin commented Oct 27, 2018

Proposing to merge in a few days

@alexAubin alexAubin merged commit f5a8113 into YunoHost:stretch-unstable Oct 31, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@frju365 frju365 deleted the frju365:patch-7 branch Oct 31, 2018

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