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

Notifications sur des sujets auxquels on n'a plus accès #6186

Closed
ChantyTaguan opened this issue Oct 4, 2021 · 10 comments · Fixed by #6196
Closed

Notifications sur des sujets auxquels on n'a plus accès #6186

ChantyTaguan opened this issue Oct 4, 2021 · 10 comments · Fixed by #6196
Labels
S-BUG Corrige un problème

Comments

@ChantyTaguan
Copy link
Contributor

Description du bug

Lorsqu'un nouveau sujet est créé dans le forum "Staff" ou lorsqu'une réponse est posté dans un sujet du forum "staff" auquel j'ai répondu, je reçois une notification. N'étant plus membre du staff, je n'ai plus accès aux forum staff et ces notification mènent donc à une 401.

Comment reproduire ?

La liste des étapes qui permet de reproduire le bug :

  1. En temps que Staff, suivre le forum staff, ou un sujet de ce forum
  2. Perdre les droits "staff"
  3. Quelqu'un répond au sujet suivi ou crée un sujet
  4. Notification reçue mais impossible à lire

Comportement attendu

On ne devrait pas recevoir de notifications pour un sujet de forum auquel on n'a pas accès

@ChantyTaguan ChantyTaguan added the S-BUG Corrige un problème label Oct 4, 2021
@SpaceFox
Copy link
Contributor

SpaceFox commented Oct 4, 2021

Tiens, j’étais persuadé qu’il était corrigé depuis longtemps celui-là !

@ChantyTaguan
Copy link
Contributor Author

@SpaceFox j'ai encore régulièrement des notifs sur des sujets staff...

Je prendrais bien le bug, je profite d'hacktober pour essayer de me remettre au développement python.

A priori je vois 2 manières de résoudre le problème :

  1. à chaque fois que des notifications sont émises suite à un nouveau post sur un sujet, ou suite à un nouveau sujet, on vérifie d'abord si l'utilisateur ayant souscrit au sujet/forum a bien accès au forum en question. Si oui, on crée la notification, si non, on désactive l'abonnement au sujet/forum ;
  2. lors de la perte de droits donnant accès à un forum particulier (y e na d'autres que "staff" ?), on parcourt la liste des abonnement du membre et on désactive celles qui sont devenues caduques

L'avantage de la solution 1, c'est qu'elle est facile et rapide à mettre en place. Par contre ça va créer pas mal de vérifications de droits à chaque post/sujet créé.

La solution 2 est plus propre, mais il faut prévoir de repasser sur tous les abonnements actuels pour désactiver ceux qui sont devenus caduques avant la correction.

@ChantyTaguan
Copy link
Contributor Author

Ou alors ça a été corrigé via la solution 2 mais sans prendre en compte les abo déjà caduques, dont les miens ? A vérifier, j'ai pas encore trouvé à quel endroit du code les droits étaient gérés

@ChantyTaguan
Copy link
Contributor Author

Ah ! C'est bien ça, la solution 2 est implémentée, on vire les abonnements caduques quand on retire un user d'un groupe. Mais les vieux abo qui trainaient avant ça sont toujours là !

@Situphen
Copy link
Member

Situphen commented Oct 4, 2021

Effectivement, la solution actuellement implémentée empêche ce soucis de se produire mais n'a pas eu d'effet rétroactif sur les soucis déjà présents. Dans l'idéal, il faudrait créer un petit script ou une petite procédure pour lister les abonnements qui lient un membre à un objet qui lui est inaccessible pour résoudre pour de bon ce problème. En attendant j'ai effectué la technique de "t'ajouter puis te retirer juste après du groupe Staff" pour que tu résoudre le soucis pour toi.

@ChantyTaguan
Copy link
Contributor Author

Ah, merci @Situphen !

Cela dit, en vérifiant le code de la correction, je pense qu'il manque le désabonnement au forum même. Je teste, et le cas échéant je rajoute ça. Avec aussi un ptit script pour virer les anciens trucs qui trainent.

@ChantyTaguan
Copy link
Contributor Author

D'après mes tests, on est bien désabonné du forum même, mais je ne comprends absolument pas à quel endroit du code ça a lieu. Je vois bien le désabonnement aux sujets, mais je ne vois rien pour le forum. Je suis perplexe...

@Situphen
Copy link
Member

Situphen commented Oct 5, 2021

Les deux types de désabonnement sont gérées dans la fonction remove_group_subscription_on_quitting_groups du fichier zds/notification/receivers.py. On crée une liste vide subscriptions et pour chaque forum, on ajoute dans cette liste l'abonnement au forum s'il existe (NewTopicSubscription), les abonnements aux différents sujets s'ils existent ( TopicAnswerSubscription), puis on supprime tous les abonnements de cette liste.

@ChantyTaguan
Copy link
Contributor Author

Ah punaise merci ! Mais du coup le code qui chipote aux abonnements dans settings_promote dans le fichier zds/member/views.py, il ne sert à rien ? Enfin il fait le taf une 2e fois pour les abonnements aux sujets quoi. Autant le virer.

@Situphen
Copy link
Member

Situphen commented Oct 6, 2021

Oui en effet, on peut virer ce bout de code !

Ce serait bien d'ajouter quelques tests unitaires d'ailleurs pour s'assurer que ce bug ne réapparaisse pas dans quelques années, dans un nouveau fichier dans zds/notification/tests/. Il y a un petit guide sur l'écriture des tests dans la doc (https://docs.zestedesavoir.com/guides/write-backend-tests.html) et sinon tu peux t'inspirer des tests existants pour les notifications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-BUG Corrige un problème
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants