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

Ajout de la raison du bannissement à la connexion #6630

Merged
merged 4 commits into from
Sep 7, 2024

Conversation

leroivi
Copy link
Contributor

@leroivi leroivi commented Aug 5, 2024

Fix #6492

Contrôle qualité

  • Bannir un membre
  • Fournir une raison au bannissement (à noter quelque part pour vérification)
  • Tenter de se connecter à la place de l'utilisateur banni
  • Vérifier que le bandeau présente bien la raison du bannissement fourni précédemment
  • Enlever le bannissement de l'utilisateur
  • Le bannir à nouveau avec une raison différente
  • Tenter de se connecter à nouveau à sa place
  • Vérifier que la raison présentée est la seconde

@Arnaud-D Arnaud-D added the C-Back Concerne le back-end Django label Aug 5, 2024
Copy link
Contributor

@Arnaud-D Arnaud-D left a comment

Choose a reason for hiding this comment

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

Ça fonctionne, mais il y a quelques améliorations qu'on peut faire sur le code. Ce sont des petites choses, mais c'est important pour nous, parce qu'on a peu de ressources et ça allège la maintenance sur le long terme.

Tu as des tests qui ne passent pas non plus. Pour tester en local et voir d'où ça vient plus précisément qu'avec les logs de la CI : https://docs.zestedesavoir.com/guides/backend-tests.html . J'ai regardé rapidement et tu vas devoir modifier le test en tant que tel, parce que la méthode qu'il utilise pour simuler le bannissement est simpliste et ne crée pas d'objet Ban.

zds/member/forms.py Outdated Show resolved Hide resolved
zds/member/forms.py Outdated Show resolved Hide resolved
zds/member/forms.py Outdated Show resolved Hide resolved
zds/member/forms.py Outdated Show resolved Hide resolved
@leroivi
Copy link
Contributor Author

leroivi commented Aug 15, 2024

J'ai regardé un peu et j'ai bloqué sur la génération des fixtures du test pour generer le ban. Malheureusement, je ne serais pas disponible jusqu'au 27 août, le sujet va donc être en stand by d'ici là. J'y reviendrai si personne ne l'a achevé entre-temps.

@Arnaud-D
Copy link
Contributor

Le test devrait ressembler à quelque chose du genre :

    def test_banned_user(self):
        """
        Error case: correct username, activated user, correct password, but banned user.
        Expected: cannot log in, error associated with the ban.
        """

        # Simulate a banned user
        self.profile.can_read = False
        self.profile.save()
        note = "Raison du ban"
        Ban.object.create(user=self.profile.user, note=note, [...])  # à compléter, je ne sais pas quels champs sont obligatoires.

        result = self.client.post(
            self.login_url,
            {
                "username": self.correct_username,
                "password": self.correct_password,
                "remember": "remember",
            },
            follow=False,
        )
        self.assertContains(result, escape(LoginForm.error_messages["banned"].format(note))

Je n'ai pas testé, c'est pour montrer l'idée. Le code qu'on teste va chercher la raison du ban dans un objet Ban, sauf que le test actuel n'en crée pas.

On peut attendre quelques jours ton retour, on n'est pas pressés ici. ^^

@coveralls
Copy link

Coverage Status

coverage: 88.881%. remained the same
when pulling f1fc53f on leroivi:ban_rationale
into dfde405 on zestedesavoir:dev.

@Arnaud-D
Copy link
Contributor

Arnaud-D commented Sep 7, 2024

QA OK ✔️

@Arnaud-D Arnaud-D merged commit f3a93ab into zestedesavoir:dev Sep 7, 2024
12 checks passed
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
Archived in project
Development

Successfully merging this pull request may close these issues.

Afficher la raison du ban quand un membre banni tente de se connecter
3 participants