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

Découple le module de MP des notifications #5971

Merged

Conversation

Arnaud-D
Copy link
Contributor

@Arnaud-D Arnaud-D commented Oct 17, 2020

Contexte

Pour #5940, je vais avoir besoin d'utiliser le module de notification, mais la manière dont il est fait à tendance à créer des imbroglio avec les dépendances. La solution que j'envisage est de découpler mieux le module de notification des modules qui envoie des signaux.

Le problème

Actuellement, on a :

  • le module de notification qui définit des signaux ;
  • les autres modules qui importent ces signaux pour les envoyer au moment opportun (et parfois importent carrément le module de notification pour créer des abonnements) ;
  • le module de notification qui reçoit ces signaux et importe ce qu'il faut des autres modules pour gérer les notifications associées.

On a donc des imports dans les deux sens, ce qui fait un couplage assez fort.

Une solution

Un design plus pratique est de rendre le module de notification entièrement invisible pour tous les autres modules. Seul le module de notification aurait connaissance des autres modules. Les autres modules se contentent d'envoyer des signaux pour informer les récepteurs de ce qu'ils sont entrain de faire.

Mon but est d'aller vers cette architecture :

  • chaque module définit lui-même les signaux qui sont utiles pour observer son comportement (création, ajout de message, suppression, ajout ou retrait de participants, etc.) ;
  • le module de notification écoute ces signaux et gère tout ce qui concerne les notif (abonnements et notif elles-mêmes).

La présente PR

Cette PR est une première étape, qui s'occupe uniquement du module de MP. Les autres viendront plus tard.

On a ici :

  • la création de signaux directement dans le module de MP pour remplacer ceux précédemment importés du module de notification ;
  • la mise à jour des tests existants pour vérifier le bon envoi des signaux ;
  • la mise à jour du module de notification pour écouter les signaux du module de MP au lieu de ses propres signaux comme auparavant.

Notez que certaines choses ne sont pas vraiment testées dans les MP (les grosses fonctions utilitaires par exemple), et que je ne souhaite pas ajouter de tests pour les signaux afférants dans le cadre de cette PR.

Contrôle qualité

  • vérifier que toutes les notifications marchent correctement pour les MP (réception de MP, marquage non lu, ...)
  • test unitaires

@coveralls
Copy link

coveralls commented Oct 17, 2020

Coverage Status

Coverage increased (+0.002%) to 86.85% when pulling 50a8283 on Arnaud-D:rendre_à_César_ce_qui_est_à_César_1 into 849094b on zestedesavoir:dev.

@Arnaud-D Arnaud-D force-pushed the rendre_à_César_ce_qui_est_à_César_1 branch from 3be94ad to 58938c5 Compare October 17, 2020 12:35
@Arnaud-D Arnaud-D marked this pull request as ready for review October 17, 2020 13:05
@firm1
Copy link
Contributor

firm1 commented Oct 18, 2020

Rapport de QA

QA : OK

Je n'ai pas fait de code review par contre.

@artragis
Copy link
Member

Review OK

@artragis artragis merged commit a20043a into zestedesavoir:dev Oct 18, 2020
@Arnaud-D Arnaud-D deleted the rendre_à_César_ce_qui_est_à_César_1 branch October 18, 2020 14:55
viki53 pushed a commit to viki53/zds-site that referenced this pull request Jul 7, 2022
* Découple le module de MP des notifications

* Ajoute test signal message_unread

* Ajoute test signal topic_read

* Ajoute tests signals gestion des participants

* Lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants