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

Renvoi d'email de confirmation #2660

Merged
merged 2 commits into from
Jun 16, 2015
Merged

Renvoi d'email de confirmation #2660

merged 2 commits into from
Jun 16, 2015

Conversation

DevHugo
Copy link
Contributor

@DevHugo DevHugo commented May 11, 2015

Q R
Correction de bugs ? oui
Nouvelle Fonctionnalité ? non
Tickets (issues) concernés #2655

QA :

  • Créé un compte sans le valider
  • Aller sur la page de connexion, cliquer sur le lien et suivez la procédure.

@GerardPaligot
Copy link
Member

  1. Ne faut-il pas invalider le procéder token s'il existe ?
  2. Tu pourrais ajouter un TU ?

@firm1
Copy link
Contributor

firm1 commented May 11, 2015

Je trouve le scénario un peu bizarre. Si j'en crois le code que je lis, lorsque le compte n'est pas activé, on renvoit un mail automatiquement lors d'une connexion. J'aurai préféré une demande explicite de l'utilisateur, car là, on peut très facilement se retrouver avec 3-4 mails d'activations sans trop le remarquer, et au final il y'en a que 1/4 qui soit bon.

Bref, y'a pas moyen de rajouter dans la même veine que "J'ai oublié mon mot de passe", un lien "Me renvoyer le mail d'activation" ?

@GerardPaligot
Copy link
Member

@firm1 +1

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.01% when pulling 5624bad on DevHugo:renvoi_email_validation into 25ebb65 on zestedesavoir:dev.

@DevHugo
Copy link
Contributor Author

DevHugo commented May 11, 2015

J'avais pensé à ça, pour éviter de créé une page et du code uniquement pour un cas « extréme », qui est pas censé ce produire.

Ne faut-il pas invalider le procéder token s'il existe ?
Le token est valide pour une heure, on peut le supprimer directement aussi.

@DevHugo DevHugo changed the title Renvoi email de confirmation lors de la connexion. Renvoi d'email de confirmation lors de la connexion. May 11, 2015
@DevHugo DevHugo changed the title Renvoi d'email de confirmation lors de la connexion. Renvoi d'email de confirmation May 11, 2015
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.00% when pulling b57ec7b on DevHugo:renvoi_email_validation into 25ebb65 on zestedesavoir:dev.

@DevHugo
Copy link
Contributor Author

DevHugo commented May 11, 2015

J'était motivé, j'ai créé une page + Rebase + Tests.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.00% when pulling c3941b3 on DevHugo:renvoi_email_validation into 25ebb65 on zestedesavoir:dev.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling a4c7903 on DevHugo:renvoi_email_validation into 25ebb65 on zestedesavoir:dev.

@WinXaito
Copy link
Contributor

Selon moi, je dirais de mettre un lien "Renvoyer l'email de confirmation" juste après le premier envoie (En cas de bug par exemple), et également si l'utilisateur tente de se connecter sans avoir activer son compte (On me un lien pour le mail.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 81.54% when pulling a4c7903 on DevHugo:renvoi_email_validation into 25ebb65 on zestedesavoir:dev.

@DevHugo
Copy link
Contributor Author

DevHugo commented May 11, 2015

Pour l'instant, j'ai ajouté le lien (ce qui fait l'unanimité) et fait une page dédiée. Le renvoi d'email après la connexion, ne fait l'unanimité (cf conv audessus), j'ai donc supprimé cette partie.

La revue de code et la QA peuvent être faite.

@Eskimon
Copy link
Contributor

Eskimon commented May 12, 2015

Un utilisateur déjà activé NE DOIS PAS pouvoir être réactivé. On a pas envie qu'un petit malin s'amuse à spammer tout les compte en rentrant tout les pseudos ;)

{% blocktrans %}
<p>
Vous n'avez pas reçu le mail de confirmation ? nous pouvons vous le renvoyer, nous avons
besoin des informations suivantes : saisissez votre nom d'utilisateur ou votre adresse de courriel
Copy link
Contributor

Choose a reason for hiding this comment

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

"Nous pouvons..." (majuscule)

@Eskimon
Copy link
Contributor

Eskimon commented May 12, 2015

A part mes deux petits commentaires c'est du tout bon :)

@amorison
Copy link

Dans la même veine que le commentaire précédent, "Envoi d'un email de
confirmation", il ne faut pas de "s" à Envoi.

2015-05-12 18:22 GMT+02:00 Eskimon notifications@github.com:

A part mes deux petits commentaires c'est du tout bon :)


Reply to this email directly or view it on GitHub
#2660 (comment)
.

@DevHugo
Copy link
Contributor Author

DevHugo commented May 12, 2015

Comme url, je met quoi ? je suis pas très inspiré ce soir.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.00% when pulling b2cb559 on DevHugo:renvoi_email_validation into 25ebb65 on zestedesavoir:dev.

@Eskimon
Copy link
Contributor

Eskimon commented May 13, 2015

Comme url, je met quoi ? je suis pas très inspiré ce soir.

URL pour quoi ? (et attention un TU est cassé)

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling 862388c on DevHugo:renvoi_email_validation into 85e62f9 on zestedesavoir:release-v15.5.1.

@@ -40,6 +40,7 @@
url(r'^deconnexion/$', 'zds.member.views.logout_view'),
url(r'^inscription/$', RegisterView.as_view(), name='register-member'),
url(r'^reinitialisation/$', 'zds.member.views.forgot_password'),
url(r'^email_validation/$', SendValidationEmailView.as_view(), name='send-validation-email'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cette adresse la ^^

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling 7a91af0 on DevHugo:renvoi_email_validation into 85e62f9 on zestedesavoir:release-v15.5.1.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling 521e208 on DevHugo:renvoi_email_validation into 7571e0d on zestedesavoir:dev.

@Eskimon
Copy link
Contributor

Eskimon commented May 15, 2015

Hello !

Tu pourrais rebaser ta branche avec celle de dev' pour que les tests front repassent ?

Sinon pour l'url pourquoi pas simplement validation ?

@DevHugo
Copy link
Contributor Author

DevHugo commented May 15, 2015

"validation", ça me parait bien !

@landscape-bot
Copy link

Code Health
Repository health increased by 0.00% when pulling 8ed3e64 on DevHugo:renvoi_email_validation into 5ee998a on zestedesavoir:dev.

@Eskimon
Copy link
Contributor

Eskimon commented May 15, 2015

Si je met un nom de compte ou un email qui n'existe pas je me prend une 404. Ca serait surement plus cool d'avoir une erreur sur le formulaire :)

# Fetch the user
usr = None
if username:
usr = get_object_or_404(User, Q(username=username))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Le problème, il est la, bien vu.

@Situphen Situphen added Evolution C-Back Concerne le back-end Django labels May 19, 2015
@@ -40,6 +40,15 @@
<div class="content-col-2">
<h2>{% trans "Connexion classique" %}</h2>
{% crispy form %}
<p>
<a href="{% url "zds.member.views.forgot_password" %}" class="form-sub-link">
Mot de passe oublié ?
Copy link
Member

Choose a reason for hiding this comment

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

Il faut utiliser {% trans "" %} :)

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.01% when pulling 3649a39 on DevHugo:renvoi_email_validation into 93559e3 on zestedesavoir:dev.

@DevHugo
Copy link
Contributor Author

DevHugo commented May 23, 2015

Merci Situphen ^^

@coveralls
Copy link

Coverage Status

Coverage increased (+1.88%) to 83.8% when pulling 3649a39 on DevHugo:renvoi_email_validation into 93559e3 on zestedesavoir:dev.

@GerardPaligot
Copy link
Member

J'ai tiré ta branche, j'ai mis à jour ma base de données, mes dépendances et j'ai regénéré le front mais j'obtiens la page suivant sur la page d'inscription :

capture d ecran 2015-05-23 a 18 20 40

@DevHugo
Copy link
Contributor Author

DevHugo commented May 23, 2015

J'ai vu, j'ai aucun temps avant la semaine prochaine (samedi mn)

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.01% when pulling c263411 on DevHugo:renvoi_email_validation into 1e1c05e on zestedesavoir:dev.

@DevHugo
Copy link
Contributor Author

DevHugo commented May 30, 2015

Suffisait de rebase sur dev, je me suis souvenu de ce bug, j'avais même fait la QA et ça avait été corrigé par Eskimon mais je retrouve plus le ticket.

QA can continue !

@Eskimon
Copy link
Contributor

Eskimon commented Jun 3, 2015

Pour un compte deja active, si on redemande un token d'activation:

  • via le pseudo = on est prevenu que c'est deja actif = OK
  • via l'email = on revient sur le formulaire sans avertissement = Pas OK

sinon a part ce tout petit point tout marche tres bien :)


return usr

def get_form(self, form_class):
Copy link
Member

Choose a reason for hiding this comment

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

Je me demande si c'est pas l'implémentation par défaut. Il faudrait donc vérifier si c'est nécessaire.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pas compris là.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.03% when pulling e7ca6a2 on DevHugo:renvoi_email_validation into 112f349 on zestedesavoir:dev.

@Eskimon
Copy link
Contributor

Eskimon commented Jun 3, 2015

landscape dit : Relative import 'forms', should be 'zds.member.forms'

@DevHugo
Copy link
Contributor Author

DevHugo commented Jun 3, 2015

J'ai pas encore tous corrigé, j'ai été interrompu, je continu ce soir. Je corrige avec ça.

Edit: C'est bon tout est corrigé ! ^^

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.00% when pulling 61235dd on DevHugo:renvoi_email_validation into 1038dc4 on zestedesavoir:dev.

@pierre-24
Copy link
Member

Rapport de QA: OK (étonnant qu'une PR aussi vieille puisse être mergée, mais puisque c'est le cas, je fonce !)

pierre-24 added a commit that referenced this pull request Jun 16, 2015
@pierre-24 pierre-24 merged commit 1963a31 into zestedesavoir:dev Jun 16, 2015
@pierre-24 pierre-24 added this to the Version de développement milestone Jun 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Back Concerne le back-end Django
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants