Produits Entr'ouvert » Combo

Je n'ai pas touché du tout aux APIs externe (à part le serializer pour changer les valeurs par défaut, None -> ''). Mais il y aurait certainement des choses à faire, typiquement Ack devrait prendre une liste d'id parce que typiquement en cliquant sur le badge on devrait acker toutes les notifications visibles et ne pas faire n appels.

À voir si le acked=False sur un renotify est vraiment le comportement souhaité, il me semble que si.

(ceci n'est pas une relecture)

cliquant sur le badge

En l'espèce on n'a pas de clic sur le badge, les clics se font sur les notifications, individuelles.

• (user, external_id) is now unique, no need to user .first() / .last()

J'ai l'impression qu'on doit aussi inclure l'origin dans le tuple d'unicité, ou on convient de donner comme consigne aux appelants de namespacer l'external_id.

Frédéric Péters a écrit :

(ceci n'est pas une relecture)

cliquant sur le badge

En l'espèce on n'a pas de clic sur le badge, les clics se font sur les notifications, individuelles.

En discutant avec Thomas il sous-entendait que c'était en cliquant sur le badge qu'un encart contenant les notifications s'ouvrait et que certainement à ce moment les notifications étaient ackés mais je ne pense pas qu'il ait déjà vu une vrai intégration lui non plus.

Est-ce que tu peux me pointer vers une intégration existante appelant les WS ack et forget dans combo je ne vois rien ?

Est-ce que tu peux me pointer vers une intégration existante appelant les WS ack et forget dans combo je ne vois rien ?

gnm, https://git.entrouvert.org/publik-base-theme.git/tree/templates/variants/grandlyon-gnm/combo/notificationscell.html &

    var ack_url = $(this).data('combo-notification-ack-url');
    $.ajax({
         url: ack_url,
         success: function(html) {
           $(cell).removeClass('new').addClass('acked');
...

dans le js.

1. origin n'étant jamais nul ça me va, mais est-ce qu'on impose au niveau du WS (voir au niveau de l'API interne aussi) d'avoir forcément un origin si on a un id fourni ? Ça me paraîtrait bien mais je ne voudrai pas casser une utilisation existante.

Je ne pense pas que ça casse (à gnm ça serait utilisé pour avoir la commune traitant dans le champ origine mais ça n'est pas encore présent).

2. est-ce qu'on imposerait d'avoir au moins un caractère non alpha-numérique dans un id fourni via le web-service ? pour éviter les collisions si jamais un client voulait envoyer un entier et que par hasard ça matchait une autre notification existante.

Je pense qu'on pourrait imposer la forme namespace:whatever, [\w-]+:[\w]+ (voire même imposer plusieurs caractères pour le namespace ?). (et si on fait ça je pense on peut laisser origin hors des critères d'unicité, juste une info utile pour l'affichage).

Frédéric Péters a écrit :

1. origin n'étant jamais nul ça me va, mais est-ce qu'on impose au niveau du WS (voir au niveau de l'API interne aussi) d'avoir forcément un origin si on a un id fourni ? Ça me paraîtrait bien mais je ne voudrai pas casser une utilisation existante.

Je ne pense pas que ça casse (à gnm ça serait utilisé pour avoir la commune traitant dans le champ origine mais ça n'est pas encore présent).

Après réflexion si on rend origin+external_id unique alors il faut demander l'origine sur les appels à ack et forget.

2. est-ce qu'on imposerait d'avoir au moins un caractère non alpha-numérique dans un id fourni via le web-service ? pour éviter les collisions si jamais un client voulait envoyer un entier et que par hasard ça matchait une autre notification existante.

Je pense qu'on pourrait imposer la forme namespace:whatever, [\w-]+:[\w]+ (voire même imposer plusieurs caractères pour le namespace ?). (et si on fait ça je pense on peut laisser origin hors des critères d'unicité, juste une info utile pour l'affichage).

Ok.

Dernière suggestion: je me demande si le retour created par notify() a vraiment un intérêt, ici il ne sert qu'une fois, pour la réponse au WS add, je ne sais pas trop ce que ça apporte.

Dans les derniers patchs seulement 5, 6 et 7 ont bougé.

Dernière suggestion: je me demande si le retour created par notify() a vraiment un intérêt, ici il ne sert qu'une fois, pour la réponse au WS add, je ne sais pas trop ce que ça apporte.

Il est arrivé dans https://dev.entrouvert.org/issues/21189

Vraiment l'origine on s'en fout pour l'unicité, pour tout le reste aussi, juste une information d'affichage; et donc je n'ai pas du être clair, dans 0007, la validation, ça devrait pour moi juste être en cas d'id une vérif sur le format "<namespace>:<whatever>" (la partie regex manque du patch ?). (et si cette validation peut se monter pour se faire aussi dans l'API interne, c'est super).

(ça fait trop de patchs tu pousserais ça dans une branche ?)

Frédéric Péters a écrit :

Dernière suggestion: je me demande si le retour created par notify() a vraiment un intérêt, ici il ne sert qu'une fois, pour la réponse au WS add, je ne sais pas trop ce que ça apporte.

Il est arrivé dans https://dev.entrouvert.org/issues/21189

Clairement ça ne sert à rien je vais le virer, les notifications sont nouvelles si acked == False rien à voir avec la présence ou pas en base.

Vraiment l'origine on s'en fout pour l'unicité, pour tout le reste aussi, juste une information d'affichage; et donc je n'ai pas du être clair, dans 0007, la validation, ça devrait pour moi juste être en cas d'id une vérif sur le format "<namespace>:<whatever>" (la partie regex manque du patch ?). (et si cette validation peut se monter pour se faire aussi dans l'API interne, c'est super).

D'ac je virer l'index et la contrainte pour avoir origin, je vais voir pour l'API interne.

(ça fait trop de patchs tu pousserais ça dans une branche ?)

Oui.

http://git.entrouvert.org/combo.git/log/?h=wip/22390-Ajouter-un-unique-together-user-external-id-sur-le-modele-Notification

Dans le patch: http://git.entrouvert.org/combo.git/commit/?h=wip/22390-Ajouter-un-unique-together-user-external-id-sur-le-modele-Notification&id=3749fd7dfb2535d20d6d0812f143f81bb978bb18 je mettrais:

id = unicode(id)

Dans http://git.entrouvert.org/combo.git/commit/?h=wip/22390-Ajouter-un-unique-together-user-external-id-sur-le-modele-Notification&id=3749fd7dfb2535d20d6d0812f143f81bb978bb18 l'origin ne semble pas être éxigé lors de l'appel à l'api, contrairement à ce que dit le message du commit.

(vraiment utiliser la branche plutôt que poser un tas de patchs).

0008 est à discuter ailleurs il me semble.

Et pousse déjà 0001→0003, qui n'ont pas de rapport.

Ack pour ma part pour les patchs 0001-0007.
Le 0008, comme dit Frédéric, peut-être traité dans un autre ticket.