feat(profils): add profils search full text to api #329

hlecuyer · 2024-10-28T14:35:44Z

No description provided.

vmttn

ok 1ère passe ! c'est cool

je n'ai pas vraiment fait la revue de code côté api

avant, est-ce que tu peux ajouter un test dédié, et quelques tests cases ? (sans tester la logique websearch).

pipeline/dbt/models/staging/sources/soliguide/_soliguide__models.yml

pipeline/dbt/models/intermediate/sources/reseau_alpha/int_reseau_alpha__services.sql

pipeline/dbt/models/intermediate/sources/soliguide/int_soliguide__services.sql

pipeline/dbt/models/intermediate/sources/france_travail/int_france_travail__services.sql

pipeline/dbt/models/intermediate/sources/agefiph/int_agefiph__services.sql

pipeline/dbt/models/intermediate/sources/action_logement/int_action_logement__services.sql

pipeline/dbt/models/intermediate/sources/dora/int_dora__services.sql

deployment/MIGRATION.md

vperron

Je verrais bien une petite discussion autour du design de cette PR car je ne suis étonné par certains choix:

introduction d'un nouveau query parameter et son utilisation pour filtrer
introduction d'un nouveau champ et choix des valeurs possible pour ce dernier

Par ailleur, attention aux refactors purement de doc, de SQLFluff et celui concernant Soliguide, qui me praissent tous mériter leur propre commit voire PR.

deployment/MIGRATION.md

pipeline/dbt/models/intermediate/sources/agefiph/int_agefiph__services.sql

pipeline/dbt/models/intermediate/sources/data_inclusion/int_data_inclusion__services.sql

pipeline/dbt/models/intermediate/sources/dora/int_dora__services.sql

api/src/alembic/versions/20241028_172223_c947102bb23f_add_profils_autres_field_in_service.py

vperron · 2024-11-13T11:36:56Z

api/src/data_inclusion/api/inclusion_data/models.py

+    searchable_index_profils_precisions: Mapped[str | None] = mapped_column(
+        TSVECTOR,
+        Computed(
+            "to_tsvector('french', COALESCE(profils_precisions, ''))", persisted=True


tiens, pourquoi ici on ne prend pas en compte profils contrairement à plus haut ? Ca me parait mériter un commentaire a minima.

Bien vu...
Je ne sais pas si ca change grand chose, je ne maitrise pas les arcannes de sqlalchemy mais c'est un oublie de ma part...

api/src/data_inclusion/api/inclusion_data/services.py

api/tests/e2e/api/test_inclusion_data.py

vperron · 2024-11-18T14:01:34Z

Je voudrai bien refaire une dernière passe ces prochains jours une fois les commits réorganisés comme suggéré !

vperron

Nice, quelques petites remarques !

Je n'approuve pas encore car:

besoin de discuter notamment sur la question des profils
quelques petits trucs mineurs a verifier que j'ai relevé
faut qu'on teste bien tout ça en staging

Chaud pour voir ce que donne aussi ce nouveau champ profil_precisions in situ.

vperron · 2024-11-19T17:22:41Z

pipeline/dbt/models/intermediate/sources/soliguide/int_soliguide__services.sql

+        ('prison', 'personne sortant de prison', 'sortants-de-detention'),
+        ('student', 'étudiant', 'etudiants'),
+        ('ukraine', 'ukraine', 'personnes-de-nationalite-etrangere')
+    ) AS di_mapping (category, traduction, profils) ON publics.value = di_mapping.category


jolie syntaxe, je connaissais pas

vperron · 2024-11-19T17:26:13Z

pipeline/dbt/models/intermediate/sources/soliguide/int_soliguide__services.sql

+        ('handicap', 'personne en situation d''handicap', 'personnes-en-situation-de-handicap'),
+        ('lgbt', 'personne LGBT+', NULL),
+        ('hiv', 'vih personne séropositive', NULL),
+        ('prostitution', 'personne en situation de prostitution', NULL),


on a pas genre "minorité" ou "défavorisé" dans nos profils ? pour info. Autant je comprends que "regular" ou "men" on ait pas de catégorie spéciale, autant pour "lgbt" je suis étonné qu'on ait rien alors qu'on a "femmes" ^^

Enfin, je chipote. Techniquement rien à dire.

vperron · 2024-11-19T17:26:21Z

pipeline/dbt/models/intermediate/sources/soliguide/int_soliguide__services.sql

+        ('isolated', 'isolé', NULL),
+        ('family', 'famille', 'familles-enfants'),
+        ('couple', 'couple', 'familles-enfants'),
+        ('pregnent', 'enceinte', 'familles-enfants'),


pregnAnt non ?

vperron · 2024-11-19T17:26:54Z

pipeline/dbt/models/intermediate/sources/soliguide/int_soliguide__services.sql

+profils AS (
+    SELECT
+        publics.lieu_id,
+        ARRAY_TO_STRING(ARRAY_AGG(DISTINCT di_mapping.traduction), ',') AS traduction,


ici on joint avec une virgule, a d'autres endroits c'est virgule puis espace. on fait partout pareil ?

j'ai mis partout pareil , au cas ou on l'affiche. Ca ne change rien a la recherche full text.

vperron · 2024-11-19T17:27:27Z

pipeline/dbt/models/intermediate/sources/soliguide/int_soliguide__services.sql

-        NULL::TEXT []                                                 AS "profils",
-        NULL::TEXT []                                                 AS "pre_requis",
+        CASE
+            WHEN lieux.publics__accueil IN (0, 1) THEN ARRAY_APPEND(profils.profils, 'tout-publics')


sur les tout publics je sens qu'il faut provoquer une discussion avec Antoine & Valentin.

Oui je crois que j'ai rate l'hebdo ou on devait en parler :(. Je me note d'en parler au daily jeudi!

vperron · 2024-11-19T17:28:22Z

pipeline/dbt/models/staging/sources/soliguide/_soliguide__models.yml

+          - not_null
+          - dbt_utils.not_empty_string
+          - accepted_values:
+              values: ['regular', 'asylum', 'refugee', 'undocumented']


j'ai pas vu refugee dans ton mapping plus haut ?

Bien vu, il est bien dans la doc, je le rajoute au mapping

vperron · 2024-11-19T17:29:21Z

pipeline/dbt/models/staging/sources/soliguide/_soliguide__models.yml

+                - 'prostitution'
+                - 'prison'
+                - 'student'
+                - 'ukraine'


nit: c'est cool de les mettre dans l'ordre alphabétique tant qu'a faire !

J'ai mis dans le meme ordre que la documentation. Je trouve ca plus facile a comparer si jamis ils rajoute des valeurs non ?

vmttn · 2024-12-16T14:18:50Z

J'ai rebased pour les migrations alembic

hlecuyer force-pushed the hlecuyer/feat/profiles/search-full-text branch 3 times, most recently from d1a3259 to 6210edf Compare October 30, 2024 10:12

vmttn requested changes Oct 31, 2024

View reviewed changes

vperron requested changes Nov 13, 2024

View reviewed changes

vperron reviewed Nov 14, 2024

View reviewed changes

api/tests/e2e/api/test_inclusion_data.py Outdated Show resolved Hide resolved

hlecuyer force-pushed the hlecuyer/feat/profiles/search-full-text branch 2 times, most recently from 69ad921 to b7ff346 Compare November 19, 2024 16:47

hlecuyer marked this pull request as ready for review November 19, 2024 16:48

hlecuyer force-pushed the hlecuyer/feat/profiles/search-full-text branch from b7ff346 to 3f6fe40 Compare November 19, 2024 17:15

vperron reviewed Nov 19, 2024

View reviewed changes

hlecuyer added the deploy-to-staging Permet d'activer le déploiement de la PR en staging. label Nov 19, 2024

hlecuyer force-pushed the hlecuyer/feat/profiles/search-full-text branch from 3f6fe40 to 8cd7122 Compare November 20, 2024 10:18

hlecuyer temporarily deployed to staging November 20, 2024 11:24 — with GitHub Actions Inactive

hlecuyer added deploy-to-staging Permet d'activer le déploiement de la PR en staging. and removed deploy-to-staging Permet d'activer le déploiement de la PR en staging. labels Nov 27, 2024

hlecuyer temporarily deployed to staging November 27, 2024 11:46 — with GitHub Actions Inactive

hlecuyer added deploy-to-staging Permet d'activer le déploiement de la PR en staging. and removed deploy-to-staging Permet d'activer le déploiement de la PR en staging. labels Dec 2, 2024

hlecuyer force-pushed the hlecuyer/feat/profiles/search-full-text branch 2 times, most recently from 1839739 to f998dfb Compare December 2, 2024 16:04

hlecuyer temporarily deployed to staging December 2, 2024 16:11 — with GitHub Actions Inactive

vperron removed the deploy-to-staging Permet d'activer le déploiement de la PR en staging. label Dec 9, 2024

vmttn force-pushed the hlecuyer/feat/profiles/search-full-text branch from a1f330a to f0e1483 Compare December 16, 2024 14:17

hlecuyer force-pushed the hlecuyer/feat/profiles/search-full-text branch from f0e1483 to 6636864 Compare December 17, 2024 08:42

vmttn added the deploy-to-staging Permet d'activer le déploiement de la PR en staging. label Dec 18, 2024

hlecuyer added 2 commits December 18, 2024 10:54

feat(soliguide): add profil and profil precisions

5480003

feat(profils_precisions): Add profils precesions for other sources

6ea8f1a

hlecuyer and others added 5 commits December 18, 2024 10:54

fix(sqlfluff): formating with sqlfluff

6228d06

feat(profil_precision): add filter profil_precisions in api

2774770

fix(tests): fix test test_list_services_all payload

acbd8c5

fix(profils): fix after review victor and Antoine

d65b76e

chore(api): rebase migration

8e2ed13

vmttn force-pushed the hlecuyer/feat/profiles/search-full-text branch from 6636864 to 8e2ed13 Compare December 18, 2024 10:01

hlecuyer added deploy-to-staging Permet d'activer le déploiement de la PR en staging. and removed deploy-to-staging Permet d'activer le déploiement de la PR en staging. labels Dec 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(profils): add profils search full text to api #329

feat(profils): add profils search full text to api #329

hlecuyer commented Oct 28, 2024

vmttn left a comment

vperron left a comment

vperron Nov 13, 2024

hlecuyer Nov 14, 2024

vperron commented Nov 18, 2024

vperron left a comment

vperron Nov 19, 2024

vperron Nov 19, 2024

vperron Nov 19, 2024

vperron Nov 19, 2024

hlecuyer Dec 10, 2024

vperron Nov 19, 2024

hlecuyer Dec 10, 2024

vperron Nov 19, 2024

hlecuyer Dec 10, 2024

vperron Nov 19, 2024

hlecuyer Dec 10, 2024

vmttn commented Dec 16, 2024

feat(profils): add profils search full text to api #329

Are you sure you want to change the base?

feat(profils): add profils search full text to api #329

Conversation

hlecuyer commented Oct 28, 2024

vmttn left a comment

Choose a reason for hiding this comment

vperron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vperron commented Nov 18, 2024

vperron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vmttn commented Dec 16, 2024