entrouvert
/
passerelle
15
0
Fork 0
Code Issues Pull Requests 14 Releases Activity

toulouse-maelis: ajouter du traitement sur les indicateurs passés à la démarche d'inscription en crèche (#74445) #105

Merged
nroche merged 1 commits from wip/74445-parsifal-manage-ape-indicators into main 2023-03-29 10:27:56 +02:00
Conversation 8 Commits 1 Files Changed 2 +55 -8
nroche commented 2023-02-12 19:18:35 +01:00
Owner
Copy Link

Je découpe le référentiel des indicateurs APE en 3 référentiels semblables à ceux des indicateur enfants et adultes sur le DUI.
J'ai renommé la fonction qui valide les indicateurs (et calcule leur valeur booléenne).
Sur cette même fonction, j'ai remonté d'un niveau le nom du champ qui contient les indicateur dans le payload pour pouvoir l'utiliser également sur la réservation APE.

Je découpe le référentiel des indicateurs APE en 3 référentiels semblables à ceux des indicateur enfants et adultes sur le DUI. J'ai renommé la fonction qui valide les indicateurs (et calcule leur valeur booléenne). Sur cette même fonction, j'ai remonté d'un niveau le nom du champ qui contient les indicateur dans le payload pour pouvoir l'utiliser également sur la réservation APE.
nroche commented 2023-02-12 19:25:44 +01:00
Author
Owner
Copy Link

J'ai revert #74401 parce qu'il y a du cache sur l'objet request de w.c.s qui fait que l'on ne peut pas utiliser dans le même formulaire 2 sources de données différentes, servies par la même url, utilisant un dictionnaire (paramètre data) différent.

J'ai revert #74401 parce qu'il y a du cache sur l'objet request de w.c.s qui fait que l'on ne peut pas utiliser dans le même formulaire 2 sources de données différentes, servies par la même url, utilisant un dictionnaire (paramètre data) différent.
nroche changed title from toulouse-maelis: factoriser le traitement sur les indicateurs. to toulouse-maelis: factoriser le traitement sur les indicateurs (#74445) 2023-02-12 21:15:33 +01:00
fpeters commented 2023-02-13 07:34:55 +01:00
Owner
Copy Link

[...] parce qu'il y a du cache sur l'objet request de w.c.s qui fat que l'on ne peut pas utiliser dans le même formulaire 2 sources de données différentes, servies par la même url, utilisant un dictionnaire (paramètre data) différent.

Si je comprends correctement la description c'est un bug, j'ai créé https://dev.entrouvert.org/issues/74449 pour suivre ça.

> [...] parce qu'il y a du cache sur l'objet request de w.c.s qui fat que l'on ne peut pas utiliser dans le même formulaire 2 sources de données différentes, servies par la même url, utilisant un dictionnaire (paramètre data) différent. Si je comprends correctement la description c'est un bug, j'ai créé https://dev.entrouvert.org/issues/74449 pour suivre ça.
fpeters reviewed 2023-02-14 17:55:23 +01:00
fpeters left a comment
Owner
Copy Link

Si je comprends correctement la description c'est un bug, j'ai créé https://dev.entrouvert.org/issues/74449 pour suivre ça.

Ça a été https://dev.entrouvert.org/issues/74449 donc ça va être ok côté wcs, sans doute cette PR peut donc être annulée.

> Si je comprends correctement la description c'est un bug, j'ai créé https://dev.entrouvert.org/issues/74449 pour suivre ça. Ça a été https://dev.entrouvert.org/issues/74449 donc ça va être ok côté wcs, sans doute cette PR peut donc être annulée.
nroche commented 2023-02-14 19:15:35 +01:00
Author
Owner
Copy Link

Vu pour #74449, Merci Fred. Donc oui pas de revert de #74401.
Mais le propos ici c'est de factoriser les traitements sur les indicateurs,
parce qu'il est une source de confusion récurrente entre moi et Stéphane.
(Stéphane envisage par exemple de pouvoir les présenter sous forme de bloc de champs)

Vu pour #74449, Merci Fred. Donc oui pas de revert de #74401. Mais le propos ici c'est de factoriser les traitements sur les indicateurs, parce qu'il est une source de confusion récurrente entre moi et Stéphane. (Stéphane envisage par exemple de pouvoir les présenter sous forme de bloc de champs)
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from 2d25d967eb to 87b1e27676 2023-02-14 19:29:59 +01:00 Compare
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from 87b1e27676 to 8038351769 2023-02-14 19:32:08 +01:00 Compare
fpeters commented 2023-02-15 09:26:46 +01:00
Owner
Copy Link

Mais le propos ici c'est de factoriser les traitements sur les indicateurs,

Je ne capte en fait pas vraiment ce qui est entendu par "factoriser" ici (qui s'entend régulièrement comme une généralisation pour une réduction de code), ce que je ne trouve pas dans le commit en question.

parce qu'il est une source de confusion récurrente entre moi et Stéphane.

Tu peux éventuellement donner un exemple des confusions actuelles ?

(Stéphane envisage par exemple de pouvoir les présenter sous forme de bloc de champs)

Et/ou de comment ça aiderait pour ce point ? (en notant que je ne sais pas ce que voudrait dire "présenter un indicateur sous forme de bloc de champs").

> Mais le propos ici c'est de factoriser les traitements sur les indicateurs, Je ne capte en fait pas vraiment ce qui est entendu par "factoriser" ici (qui s'entend régulièrement comme une généralisation pour une réduction de code), ce que je ne trouve pas dans le commit en question. > parce qu'il est une source de confusion récurrente entre moi et Stéphane. Tu peux éventuellement donner un exemple des confusions actuelles ? > (Stéphane envisage par exemple de pouvoir les présenter sous forme de bloc de champs) Et/ou de comment ça aiderait pour ce point ? (en notant que je ne sais pas ce que voudrait dire "présenter un indicateur sous forme de bloc de champs").
nroche commented 2023-02-19 21:16:35 +01:00
Author
Owner
Copy Link

ce qui est entendu par "factoriser" ?

Dans create_nursery_demand, les indicateurs ne sont pas traités pour être transformés en Booléen. C'est un oubli et donc pour être plus lisible, j'aurais du ajouter du code ici pour le factoriser ensuite.

Tu peux éventuellement donner un exemple des confusions actuelles ?

L'utilisation de valeurs qui ne sont pas des booléens pour remplir les champ isActive (je ne me rappelle plus pourquoi mais on avait par exemple une chaîne 'False' ou '0' qui était prise pour True).

que voudrait dire "présenter un indicateur sous forme de bloc de champs") ?

Utiliser le filtre getlistdict pour envoyer un tableau au connecteur.
Ceci supposerait l’omission du champ isActive qui serait implicitement vrai.
(bien qu'ensuite on ne pourrait plus mettre à jours la demande en retirant des indicateurs)

> ce qui est entendu par "factoriser" ? Dans create_nursery_demand, les indicateurs ne sont pas traités pour être transformés en Booléen. C'est un oubli et donc pour être plus lisible, j'aurais du ajouter du code ici pour le factoriser ensuite. > Tu peux éventuellement donner un exemple des confusions actuelles ? L'utilisation de valeurs qui ne sont pas des booléens pour remplir les champ isActive (je ne me rappelle plus pourquoi mais on avait par exemple une chaîne 'False' ou '0' qui était prise pour True). > que voudrait dire "présenter un indicateur sous forme de bloc de champs") ? Utiliser le filtre getlistdict pour envoyer un tableau au connecteur. Ceci supposerait l’omission du champ isActive qui serait implicitement vrai. (bien qu'ensuite on ne pourrait plus mettre à jours la demande en retirant des indicateurs)
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from 8038351769 to ea797ce26e 2023-02-19 21:28:24 +01:00 Compare
fpeters commented 2023-02-20 10:11:05 +01:00
Owner
Copy Link

ce qui est entendu par "factoriser" ?

Dans create_nursery_demand, les indicateurs ne sont pas traités pour être transformés en Booléen. C'est un oubli et donc pour être plus lisible, j'aurais du ajouter du code ici pour le factoriser ensuite.

Par "factoriser" tu entends donc l'ajout de ces trois lignes en haut de la méthode create_nursery_demand ? :

        self.assert_indicator_payload_in_referential('ApeEnfIndicator', post_data, ['child_indicators'])
        self.assert_indicator_payload_in_referential('ApeFamIndicator', post_data, ['family_indicators'])
        self.assert_indicator_payload_in_referential('ApeResIndicator', post_data, ['demand_indicators'])

Et ces lignes, loin de "assert", servent en fait à transformer les données ?

Tu peux éventuellement donner un exemple des confusions actuelles ?

L'utilisation de valeurs qui ne sont pas des booléens pour remplir les champ isActive (je ne me rappelle plus pourquoi mais on avait par exemple une chaîne 'False' ou '0' qui était prise pour True).

Ok c'est compliqué de parler de "champs", pour moi ça évoque les champs des démarches. Ici le cadre c'est "w.c.s. envoie des données, passerelle doit les interpréter", et le truc est qu'on voudrait pour le booléen "isActive" pouvoir envoyer aussi bien un booléen qu'une chaine qui dirait 'Oui', ou un entier qui dirait 1, ou à l'opposé "Non" ou 0.

que voudrait dire "présenter un indicateur sous forme de bloc de champs") ?

Utiliser le filtre getlistdict pour envoyer un tableau au connecteur.
Ceci supposerait l’omission du champ isActive qui serait implicitement vrai.
(bien qu'ensuite on ne pourrait plus mettre à jours la demande en retirant des indicateurs)

(je laisse ça de côté).

--

Donc le problème de ce patch ce serait plus général c'est dans ce connecteur appeler "assert" quelque chose qui modifie les données.

Et cette branche dans un commit fait en sorte de traiter le paramètre isActive. Et dans un autre divise l'endpoint read_ape_indicators_list en trois ?

Mais ces deux sujets n'ont pas de rapport ?

> > ce qui est entendu par "factoriser" ? > > Dans create_nursery_demand, les indicateurs ne sont pas traités pour être transformés en Booléen. C'est un oubli et donc pour être plus lisible, j'aurais du ajouter du code ici pour le factoriser ensuite. Par "factoriser" tu entends donc l'ajout de ces trois lignes en haut de la méthode create_nursery_demand ? : ``` self.assert_indicator_payload_in_referential('ApeEnfIndicator', post_data, ['child_indicators']) self.assert_indicator_payload_in_referential('ApeFamIndicator', post_data, ['family_indicators']) self.assert_indicator_payload_in_referential('ApeResIndicator', post_data, ['demand_indicators']) ``` Et ces lignes, loin de "assert", servent en fait à transformer les données ? > > Tu peux éventuellement donner un exemple des confusions actuelles ? > > L'utilisation de valeurs qui ne sont pas des booléens pour remplir les champ isActive (je ne me rappelle plus pourquoi mais on avait par exemple une chaîne 'False' ou '0' qui était prise pour True). Ok c'est compliqué de parler de "champs", pour moi ça évoque les champs des démarches. Ici le cadre c'est "w.c.s. envoie des données, passerelle doit les interpréter", et le truc est qu'on voudrait pour le booléen "isActive" pouvoir envoyer aussi bien un booléen qu'une chaine qui dirait 'Oui', ou un entier qui dirait 1, ou à l'opposé "Non" ou 0. > > que voudrait dire "présenter un indicateur sous forme de bloc de champs") ? > > Utiliser le filtre getlistdict pour envoyer un tableau au connecteur. > Ceci supposerait l’omission du champ isActive qui serait implicitement vrai. > (bien qu'ensuite on ne pourrait plus mettre à jours la demande en retirant des indicateurs) (je laisse ça de côté). -- Donc le problème de ce patch ce serait plus général c'est dans ce connecteur appeler "assert" quelque chose qui modifie les données. Et cette branche dans un commit fait en sorte de traiter le paramètre isActive. Et dans un autre divise l'endpoint read_ape_indicators_list en trois ? Mais ces deux sujets n'ont pas de rapport ?
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from ea797ce26e to 753c572b7a 2023-02-21 14:14:40 +01:00 Compare
nroche commented 2023-02-22 14:32:08 +01:00
Author
Owner
Copy Link

Par "factoriser" tu entends donc l'ajout de ces trois lignes

Oui, c'était bien ici que je voulais faire appel à l'existant (en le modifiant un peu pour que ce soit possible).
(et je comprend que je ne dois pas utiliser le terme "factoriser" ici, j'aurais du dire rationaliser ou rendre générique par exemple.)

Donc le problème de ce patch ce serait plus général c'est dans ce connecteur appeler "assert" quelque chose qui modifie les données.

Historiquement, je n'avais qu'à contrôler le payload, puis est apparu le problème d'encodage des booléens.
Les champs étant tous parcourus sur le contrôle, j'y ai glissé cet encodage.
Il conviendrait donc de renommer les fonctions assert_payload_ en validate_and_adapt_payload_ et je fais ça de suite dans un autre ticket.

Et cette branche dans un commit fait en sorte de traiter le paramètre isActive.
Et dans un autre divise l'endpoint read_ape_indicators_list en trois ?
Mais ces deux sujets n'ont pas de rapport ?

Jusqu'à présent la définition des indicateurs nous était toujours fournie via un référentiel.
Mon idée était d'utiliser le même traitement sur les indicateurs (via les référentiel) que celui utilisé jusqu'à présent.
Pour cela le plus simple était de diviser les indicateurs APE en 3 référentiels distincts.
Je ne sais pas comment tu fais pour avoir le nez creux à ce point, mais il se trouve que sur les inscriptions il est à présent prévu de passer de nouveaux indicateurs, qui ne seront pas définis via un référentiel.
J'abandonne ici l'idée de généraliser le traitement des indicateurs et j'ajoute simplement du code spécifique.

> Par "factoriser" tu entends donc l'ajout de ces trois lignes Oui, c'était bien ici que je voulais faire appel à l'existant (en le modifiant un peu pour que ce soit possible). (et je comprend que je ne dois pas utiliser le terme "factoriser" ici, j'aurais du dire rationaliser ou rendre générique par exemple.) > Donc le problème de ce patch ce serait plus général c'est dans ce connecteur appeler "assert" quelque chose qui modifie les données. Historiquement, je n'avais qu'à contrôler le payload, puis est apparu le problème d'encodage des booléens. Les champs étant tous parcourus sur le contrôle, j'y ai glissé cet encodage. Il conviendrait donc de renommer les fonctions assert_payload_ en validate_and_adapt_payload_ et je fais ça de suite dans un autre ticket. > Et cette branche dans un commit fait en sorte de traiter le paramètre isActive. > Et dans un autre divise l'endpoint read_ape_indicators_list en trois ? > Mais ces deux sujets n'ont pas de rapport ? Jusqu'à présent la définition des indicateurs nous était toujours fournie via un référentiel. Mon idée était d'utiliser le même traitement sur les indicateurs (via les référentiel) que celui utilisé jusqu'à présent. Pour cela le plus simple était de diviser les indicateurs APE en 3 référentiels distincts. Je ne sais pas comment tu fais pour avoir le nez creux à ce point, mais il se trouve que sur les inscriptions il est à présent prévu de passer de nouveaux indicateurs, qui ne seront pas définis via un référentiel. J'abandonne ici l'idée de généraliser le traitement des indicateurs et j'ajoute simplement du code spécifique.
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from 753c572b7a to 14f59b52ca 2023-02-22 18:08:29 +01:00 Compare
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from 14f59b52ca to 26896e57eb 2023-02-24 14:52:58 +01:00 Compare
nroche commented 2023-03-01 15:19:30 +01:00
Author
Owner
Copy Link

J'ai mis à jour le titre de la PR pour refléter tous ces changements.
(il n'est plus question de factoriser quoi que ce soit)

J'ai mis à jour le titre de la PR pour refléter tous ces changements. (il n'est plus question de factoriser quoi que ce soit)
nroche changed title from toulouse-maelis: factoriser le traitement sur les indicateurs (#74445) to toulouse-maelis: ajouter du traitement sur les indicateurs passés à la démarche d'inscription en crèche (#74445) 2023-03-01 15:44:54 +01:00
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from 26896e57eb to 3779347a85 2023-03-24 10:04:34 +01:00 Compare
smihai approved these changes 2023-03-28 16:24:27 +02:00
nroche force-pushed wip/74445-parsifal-manage-ape-indicators from 3779347a85 to 761391c484 2023-03-29 10:13:11 +02:00 Compare
nroche merged commit 761391c484 into main 2023-03-29 10:27:56 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
smihai
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/passerelle#105
No description provided.
Delete Branch "wip/74445-parsifal-manage-ape-indicators"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?