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

Update validate_zds_username using exists() #4568

Closed
wants to merge 2 commits into
base: dev
from

Conversation

Projects
None yet
8 participants
@cgabard
Member

cgabard commented Aug 21, 2017

Q R
Type de modification correction de bug / évolution
Ticket(s) (issue(s)) concerné(s) #4545

Bon j'ai tenté de corriger le bug #4545 mais je n'arrive pas à le reproduire en local et je ne comprends pas le bug en prod/beta. Quand j'aurais le temps je mettrais une base mySql en place pour tester. Dans tous les cas j'ai légèrement mit à jour la fonction qui pose problème : un count() est totalement inutile et un exists() est toujours au pire aussi rapide, tout en étant plus explicite. Avec un peu de chance ça corrigera magiquement le bug.

Au passage j'ai rajouté un test pour ce cas.

QA

  • Vérifier qu'on peut changer de pseudo
  • Vérifier qu'on peut changer de pseudo pour le même avec une casse différente
Update validate_zds_username using exists()
=> Avoid a count() when useless
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 21, 2017

Coverage Status

Coverage decreased (-33.9%) to 55.584% when pulling dcd6306 on cgabard:update_validate_pseudo into f11a134 on zestedesavoir:dev.

coveralls commented Aug 21, 2017

Coverage Status

Coverage decreased (-33.9%) to 55.584% when pulling dcd6306 on cgabard:update_validate_pseudo into f11a134 on zestedesavoir:dev.

@cgabard

This comment has been minimized.

Show comment
Hide comment
@cgabard

cgabard Aug 21, 2017

Member

Ha j'ai l'impression que Travis à le bug présent en prod, je vais peut être l'utiliser pour le traquer !

Member

cgabard commented Aug 21, 2017

Ha j'ai l'impression que Travis à le bug présent en prod, je vais peut être l'utiliser pour le traquer !

@motet-a motet-a added the C-Back label Aug 23, 2017

Update validators.py
Juste pour voir le résultat dans Travis
@cgabard

This comment has been minimized.

Show comment
Hide comment
@cgabard

cgabard Aug 24, 2017

Member

Ok donc :

User.objects.filter(username="DuMmY").all() me renvoi [<User: dummy>]. WTF ?

@vhf @SpaceFox ou n'importe qui, vous avez déjà entendu parlé de ce souci avec MySql ? Parce que je n'ai pas le souci avec Sqlite et manifestement MySql fait un select insensible à la casse !?

Member

cgabard commented Aug 24, 2017

Ok donc :

User.objects.filter(username="DuMmY").all() me renvoi [<User: dummy>]. WTF ?

@vhf @SpaceFox ou n'importe qui, vous avez déjà entendu parlé de ce souci avec MySql ? Parce que je n'ai pas le souci avec Sqlite et manifestement MySql fait un select insensible à la casse !?

@SpaceFox

This comment has been minimized.

Show comment
Hide comment
@SpaceFox

SpaceFox Aug 24, 2017

Member
Member

SpaceFox commented Aug 24, 2017

@vhf

This comment has been minimized.

Show comment
Hide comment
@vhf

vhf Aug 24, 2017

Member

Elle l'est pas, on a du utf8mb4_unicode_ci quasiment partout.

Member

vhf commented Aug 24, 2017

Elle l'est pas, on a du utf8mb4_unicode_ci quasiment partout.

@SpaceFox

This comment has been minimized.

Show comment
Hide comment
@SpaceFox

SpaceFox Aug 24, 2017

Member
Member

SpaceFox commented Aug 24, 2017

@cgabard

This comment has been minimized.

Show comment
Hide comment
@cgabard

cgabard Aug 24, 2017

Member

Je suis pas un pro des db et je ne suis pas surs de bien comprendre @vhf .

de ce que j'en lit utf8mb4_unicode_ci gère l'étendu des codes points compris (utf8mb4) et la façon de les trier (unicode_ci). Je ne vois nullpart que cela serait lié à la casse directement. J'ai raté quoi ?

Member

cgabard commented Aug 24, 2017

Je suis pas un pro des db et je ne suis pas surs de bien comprendre @vhf .

de ce que j'en lit utf8mb4_unicode_ci gère l'étendu des codes points compris (utf8mb4) et la façon de les trier (unicode_ci). Je ne vois nullpart que cela serait lié à la casse directement. J'ai raté quoi ?

@cgabard

This comment has been minimized.

Show comment
Hide comment
@cgabard

cgabard Aug 24, 2017

Member

J'ai rien dit, le je viens de comprendre le ci

Ok donc c'est un problème de configuration de table quoi.

Member

cgabard commented Aug 24, 2017

J'ai rien dit, le je viens de comprendre le ci

Ok donc c'est un problème de configuration de table quoi.

@cgabard

This comment has been minimized.

Show comment
Hide comment
@cgabard

cgabard Aug 24, 2017

Member

Bon vu la table que c'est, on va éviter de l'altérer. Je vais me débrouiller pour que ça fonctionne en prod et test. Merci à vous

Member

cgabard commented Aug 24, 2017

Bon vu la table que c'est, on va éviter de l'altérer. Je vais me débrouiller pour que ça fonctionne en prod et test. Merci à vous

@gcodeur

This comment has been minimized.

Show comment
Hide comment
@gcodeur

gcodeur Aug 26, 2017

Member

En fait, je suis en train de me demander si la solution ne serait pas simplement de passer lower sur les deux variables ici. Qu'en penses-tu ?

edit : fix lien

Member

gcodeur commented Aug 26, 2017

En fait, je suis en train de me demander si la solution ne serait pas simplement de passer lower sur les deux variables ici. Qu'en penses-tu ?

edit : fix lien

@gcodeur

This comment has been minimized.

Show comment
Hide comment
@gcodeur

gcodeur Oct 5, 2017

Member

Salut @cgabard, est-tu toujours intéressé par cette PR ?

Member

gcodeur commented Oct 5, 2017

Salut @cgabard, est-tu toujours intéressé par cette PR ?

@cgabard

This comment has been minimized.

Show comment
Hide comment
@cgabard

cgabard Oct 6, 2017

Member
Member

cgabard commented Oct 6, 2017

@rezemika

This comment has been minimized.

Show comment
Hide comment
@rezemika

rezemika Aug 8, 2018

Contributor

Salut ! Je viens de faire une reprise de la PR. Voir #5002. :)

Contributor

rezemika commented Aug 8, 2018

Salut ! Je viens de faire une reprise de la PR. Voir #5002. :)

@pierre-24

This comment has been minimized.

Show comment
Hide comment
@pierre-24

pierre-24 Aug 8, 2018

Member

oki :)

Member

pierre-24 commented Aug 8, 2018

oki :)

@pierre-24 pierre-24 closed this Aug 8, 2018

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