Appeler l'API notifications tout le temps ou jamais lors des événements de contenu #4874

Open
opened 1 year ago by rastapopoulos · 7 comments
Owner

Actuellement le noyau de SPIP appelle l'API notifications uniquement dans objet_instituer() donc uniquement si changement de statut ou de date.

Par cohérence, ça devrait soit être jamais et c'est à des plugins d'ajouter des appels, soit si on le fait, à chaque event, pas que les statuts. Parce que là des fois c'est déjà là (mais avec un nom un peu pourri d'ailleurs), des fois faut s'insérer dans des pipelines juste pour ajouter des appels.

Je propose qu'on ajoute donc un appel dans objet_inserer et dans objet_modifier.

Et je propose aussi de changer le nom pour mieux normer les appels, pour celui d'instituer (en gardant l'ancien appel pour rétro compat). En effet, le nom actuel est "instituer$objet" tout collé, c'est super moche et c'est pas très normé pour avoir des automatismes. Je propose que toutes les notifications qui concernent un objet déclaré soit de la forme "patate_nom".

Donc on aurait :

  • $obet_inserer
  • $objet_modifier
  • $objet_instituer
  • instituer$objet pour compat
Actuellement le noyau de SPIP appelle l'API notifications uniquement dans objet_instituer() donc uniquement si changement de statut ou de date. Par cohérence, ça devrait soit être jamais et c'est à des plugins d'ajouter des appels, soit si on le fait, à chaque event, pas que les statuts. Parce que là des fois c'est déjà là (mais avec un nom un peu pourri d'ailleurs), des fois faut s'insérer dans des pipelines juste pour ajouter des appels. Je propose qu'on ajoute donc un appel dans objet_inserer et dans objet_modifier. Et je propose aussi de changer le nom pour mieux normer les appels, pour celui d'instituer (en gardant l'ancien appel pour rétro compat). En effet, le nom actuel est "instituer$objet" tout collé, c'est super moche et c'est pas très normé pour avoir des automatismes. Je propose que toutes les notifications qui concernent un objet déclaré soit de la forme "patate_nom". Donc on aurait : - $obet_inserer - $objet_modifier - $objet_instituer - instituer$objet pour compat
b_b commented 1 year ago
Owner

+1, on tente ça pour SPIP 4.1 ?

/me tente d'attribuer un jalon aux tickets du core :)

+1, on tente ça pour SPIP 4.1 ? /me tente d'attribuer un jalon aux tickets du core :)
Poster
Owner

Oui c'est 4 lignes à ajouter à priori.

Oui c'est 4 lignes à ajouter à priori.
rastapopoulos added this to the 4.1 milestone 1 year ago
Owner

Je pense un peu tout haut…
Pas d’objection pour la proposition.

https://git.spip.net/spip/spip/src/branch/master/ecrire/action/editer_objet.php#L471

En fait donc tu parles d’un événement de notification qui appelle ensuite inc_notification_dist("instituer$objet", ...).

Chez Sf, pour les événement (https://symfony.com/doc/current/components/event_dispatcher.html) (et d’autres trucs), ils utilisent le point comme séparateur, avec à la fin le verbe de l’action réalisée (au passé pour le coup), mais bon, disons à l’infinitif pour nous. De même chez Laravel (

M’est avis qu’il faudrait (pas forcément là) à un moment distinguer aussi le séparateur utilisé (ici un souligné) car le souligné peut être présent dans le nom d’objet. On pourrait alors avoir truc_muche.instituer comme nom d’événement.

Cependant par défaut inc_notification_dist($quoi, ...) tente de trouver une fonction php correspondante au nom passé (if ($notification = charger_fonction($quoi, 'notifications', true)) {), et ça ne marcherait pas avec le . donc.

Bref.

Je pense un peu tout haut… Pas d’objection pour la proposition. https://git.spip.net/spip/spip/src/branch/master/ecrire/action/editer_objet.php#L471 En fait donc tu parles d’un événement de notification qui appelle ensuite `inc_notification_dist("instituer$objet", ...)`. Chez Sf, pour les événement (https://symfony.com/doc/current/components/event_dispatcher.html) (et d’autres trucs), ils utilisent le point comme séparateur, avec à la fin le verbe de l’action réalisée (au passé pour le coup), mais bon, disons à l’infinitif pour nous. De même chez Laravel ( M’est avis qu’il faudrait (pas forcément là) à un moment distinguer aussi le séparateur utilisé (ici un souligné) car le souligné peut être présent dans le nom d’objet. On pourrait alors avoir `truc_muche.instituer` comme nom d’événement. Cependant par défaut `inc_notification_dist($quoi, ...)` tente de trouver une fonction php correspondante au nom passé (`if ($notification = charger_fonction($quoi, 'notifications', true)) {`), et ça ne marcherait pas avec le `.` donc. Bref.
Poster
Owner

Oui dans SPIP on essaye souvent de trouver des fonctions (et des fichiers) correspondants au nom d'un param, c'est comme pour autoriser. L'API notification cherche une fonction implémentant le nom, et en cascade pareil pour notifs avancées qui cherche notifications_$quoi_destinataires, etc.

Pour connaitre le nom d'objet ya une fonction qui se trompe à peu près jamais :
https://git.spip.net/spip-contrib-extensions/notifications_avancees/src/branch/master/notifavancees_pipelines.php#L494

Le dernier mot n'est jamais dans l'objet car on considère que yora pas de notif avec uniquement le nom d'un objet seul, toujours au moins un mot en suffixe, et dans ce qui reste, on cherche un objet déclaré officiellement. Ça marche parfaitement pour "truc_muche_instituer" ou même "truc_muche_instituer_bis".

Donc en attendant mieux, c'est le plus simple il me semble pour normer les noms, et permettre des automatismes (savoir que tel notif est autour de tel objet). :)

Oui dans SPIP on essaye souvent de trouver des fonctions (et des fichiers) correspondants au nom d'un param, c'est comme pour autoriser. L'API notification cherche une fonction implémentant le nom, et en cascade pareil pour notifs avancées qui cherche notifications_$quoi_destinataires, etc. Pour connaitre le nom d'objet ya une fonction qui se trompe à peu près jamais : https://git.spip.net/spip-contrib-extensions/notifications_avancees/src/branch/master/notifavancees_pipelines.php#L494 Le dernier mot n'est jamais dans l'objet car on considère que yora pas de notif avec uniquement le nom d'un objet seul, toujours au moins un mot en suffixe, et dans ce qui reste, on cherche un objet déclaré officiellement. Ça marche parfaitement pour "truc_muche_instituer" ou même "truc_muche_instituer_bis". Donc en attendant mieux, c'est le plus simple il me semble pour normer les noms, et permettre des automatismes (savoir que tel notif est autour de tel objet). :)
rastapopoulos added the
APIs
label 1 year ago
Poster
Owner

En attendant que ce soit dans le core, j'ai mis des appels normés dans le master de Notifs avancées 👍:
https://git.spip.net/spip-contrib-extensions/notifications_avancees/commit/7cb81432

Ce qui permet de relire, tester IRL (sur un site en stable), et ça pourra être recopié dans le core et viré du plugin.

En attendant que ce soit dans le core, j'ai mis des appels normés dans le master de Notifs avancées 👍: https://git.spip.net/spip-contrib-extensions/notifications_avancees/commit/7cb81432 Ce qui permet de relire, tester IRL (sur un site en stable), et ça pourra être recopié dans le core et viré du plugin.
b_b added the
amélioration
label 1 year ago
b_b commented 4 weeks ago
Owner

@rastapopoulos après 12 mois de test, tu penses que ça peut être introduit pour la 4.2 ?

@rastapopoulos après 12 mois de test, tu penses que ça peut être introduit pour la 4.2 ?
b_b modified the milestone from 4.1 to 4.2 4 weeks ago
Poster
Owner

Et donc la PR #5304

Et donc la PR https://git.spip.net/spip/spip/pulls/5304
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.