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

Corrige les erreurs 500 liées à l'attribution des casquettes #4572

Merged
merged 2 commits into from Aug 25, 2017
Merged

Conversation

gllmc
Copy link
Member

@gllmc gllmc commented Aug 22, 2017

Q R
Type de modification correction de bug
Ticket(s) (issue(s)) concerné(s) https://sentry.sandhose.fr/zeste-de-savoir/beta-backend/issues/1006/

L'erreur est causée par le fait que unique=True supporte mal l'UTF-8. Je l'enlève car le get_or_create fait déjà cette vérification. Par ailleurs, j'empêche les espaces au début et à la fin qui n'ont aucune raison d'être. ^^

QA

  • Ajouter une casquette "Staff" à un membre puis ajouter "Staff " (notez l'espace) à un autre membre. Vérifier que les deux membres reçoivent "Staff".
  • Ajouter deux casquettes UTF-8 ( et ) à un membre et vérifier qu'elles sont correctement attribuées.
  • Code review

@gllmc gllmc added C-Back Concerne le back-end Django S-BUG Corrige un problème labels Aug 22, 2017
@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage remained the same at 89.49% when pulling e957c9e on gcodeur:fix-hats into 5c73352 on zestedesavoir:release-v25.

@coveralls
Copy link

Coverage Status

Coverage decreased (-15.9%) to 73.599% when pulling e957c9e on gcodeur:fix-hats into 5c73352 on zestedesavoir:release-v25.

@@ -800,7 +800,7 @@ def add_hat(request, user_pk):

user = get_object_or_404(User, pk=user_pk)

hat_name = request.POST.get('hat', None)
hat_name = request.POST.get('hat', None).strip()
Copy link
Member

Choose a reason for hiding this comment

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

du coup vaut mieux remplacer le None par '' sinon tu vas avoir un AttributeError.

Copy link
Member Author

Choose a reason for hiding this comment

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

Bien vu !

@@ -19,7 +19,7 @@ def handle(self, *args, **options):
except Group.DoesNotExist:
raise CommandError('Group {} does not exist.'.format(options['group']))

hat_name = options['hat']
hat_name = options['hat'].strip()
Copy link
Member

Choose a reason for hiding this comment

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

options.get('hat', '').strip()

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est moins important ici, mais ça vaut le coup quand même, je m'en occupe.

@@ -480,7 +480,7 @@ class Hat(models.Model):
that a moderation message was posted by a staff member.
"""

name = models.CharField('Casquette', max_length=40, unique=True)
name = models.CharField('Casquette', max_length=40)
Copy link
Member

Choose a reason for hiding this comment

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

du coup je suis pas sûr que supprimer le unique soit une bonne idée.

Copy link
Member Author

Choose a reason for hiding this comment

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

C'est nécessaire pour résoudre le problème des smileys (deux casquettes identiques avec un caractère UTF-8 différent).

@coveralls
Copy link

coveralls commented Aug 23, 2017

Coverage Status

Coverage remained the same at 89.493% when pulling ac8cc9a on gcodeur:fix-hats into f5f76b5 on zestedesavoir:release-v25.

@artragis artragis self-assigned this Aug 25, 2017
@artragis artragis merged commit 9ef9aa9 into zestedesavoir:release-v25 Aug 25, 2017
@artragis artragis added this to the v25 milestone Aug 25, 2017
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 S-BUG Corrige un problème
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants