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

Supprime le bouton d’envoi de MP sur les profils bannis #3758

Merged
merged 3 commits into from
Aug 6, 2016
Merged

Conversation

Karnaj
Copy link
Contributor

@Karnaj Karnaj commented Aug 1, 2016

Q R
Type de modification évolution
Ticket(s) (issue(s)) concerné(s) #3672

QA

  • Bannir un membre.
  • Vérifier que le bouton « Envoyer un message privé » n’est pas présent sur le profil du membre banni.

@coveralls
Copy link

coveralls commented Aug 1, 2016

Coverage Status

Coverage remained the same at 87.677% when pulling a5978c8 on Karnaj:dev into c50283c on zestedesavoir:dev.

@vhf
Copy link
Contributor

vhf commented Aug 1, 2016

Top !

Est-ce que tu peux encore :

  1. Squasher tes 2 commits, et ce faisant modifier le message de commit pour le rendre compréhensible.
  2. Ajouter quelques tests ici :
    def test_details_member(self):

(Idéalement : tester l'affichage du bouton "Envoyer un MP" pour les combinaisons possibles de ([utilisateur déconnecté, utilisateur connecté], [membre existant, moi-même, membre banni]) )

@vhf vhf changed the title Fix #3672 : Supprime le bouton d’envoi de MP sur les profils bannis Supprime le bouton d’envoi de MP sur les profils bannis Aug 1, 2016
@GerardPaligot
Copy link
Member

En tout cas, je confirme que ça fonctionne. Manque plus que le test.

@Karnaj
Copy link
Contributor Author

Karnaj commented Aug 1, 2016

Pour le test, j’aimerais bien un peu d’aide vu que je n’en ai jamais fait.

@coveralls
Copy link

coveralls commented Aug 1, 2016

Coverage Status

Coverage remained the same at 87.677% when pulling e238bd1 on Karnaj:dev into c50283c on zestedesavoir:dev.

@vhf
Copy link
Contributor

vhf commented Aug 1, 2016

Je t'aide volontiers @Karnaj . Je te contacte les prochains jours.

@Karnaj
Copy link
Contributor Author

Karnaj commented Aug 2, 2016

OK, merci. Pour le moment, en m’aidant de la documentation de Django et des tests déjà présents, j’ai écrit ça.

user_ban = ProfileFactory()
user_ban.can_read = False
user_ban.can_write = False
user_ban.save()
user_1 = ProfileFactory()
user_2 = ProfileFactory()

phrase = "Envoyer un message privé"

result = self.client.get(reverse('member-detail', args=[user_1.username]), follow=False)
self.assertNotContains(result, phrase)

result = self.client.get(reverse('member-detail', args=[user_ban.username]), follow=False)
self.assertNotContains(result, phrase)

self.assertTrue(self.client.login(username=user_2.user.username, password='hostel77'))
result = self.client.get(reverse('member-detail', args=[user_1.username]), follow=False)
self.client.logout()
self.assertContains(result, phrase)

self.assertTrue(self.client.login(username=user_2.user.username, password='hostel77'))
result = self.client.get(reverse('member-detail', args=[user_ban.username]), follow=False)
self.client.logout()
self.assertNotContains(result, phrase)

self.assertTrue(self.client.login(username=user_1.user.username, password='hostel77'))
result = self.client.get(reverse('member-detail', args=[user_1.username]), follow=False)
self.client.logout()
self.assertNotContains(result, phrase)

@GerardPaligot
Copy link
Member

@Karnaj Ton test semble pas mal du tout. Tu l'as executé en local ?

@Karnaj
Copy link
Contributor Author

Karnaj commented Aug 6, 2016

@GerardPaligot Non, je ne l’ai pas exécuté en local (j’ai pas d’ordinateur avec Zds installé sous la main).

@GerardPaligot
Copy link
Member

@Karnaj Je t'ai fais une PR sur ta branche : https://github.com/Karnaj/zds-site/pull/1

Ton test était parfait hormis une mini faute : username est dans User et pas dans Profile. Mais sinon, c'était tout bon. Bravo ! :)

test(pm): Un membre banni n'est pas contactable.
@coveralls
Copy link

coveralls commented Aug 6, 2016

Coverage Status

Coverage increased (+0.08%) to 87.76% when pulling f4bc4a7 on Karnaj:dev into c50283c on zestedesavoir:dev.

@GerardPaligot
Copy link
Member

Comme la QA a déjà été faite, je merge ! :)

@GerardPaligot GerardPaligot merged commit c5a5f12 into zestedesavoir:dev Aug 6, 2016
@Karnaj
Copy link
Contributor Author

Karnaj commented Aug 6, 2016

Merci à @GerardPaligot pour sa correction.

@vhf vhf mentioned this pull request Aug 23, 2016
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.

None yet

4 participants