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

feat: log synthese route manage params #2345

Closed

Conversation

andriacap
Copy link
Contributor

@andriacap andriacap commented Feb 17, 2023

Bonjour,

J'ai apporté les modifications basées sur ce commentaire #2337 (review).

Modifs réalisées

  • J'ai supprimé le fichier utils/routes.py qui semblait superflu.
  • J'ai rassemblé les fonctions de "query' liées au model SyntheseLogEntry.
  • J'ai ajouté la possibilité d'utiliser plusieurs sort en params en me basant sur ce qui a été dit dans le ticket (ref: [Dev] Bonnes pratiques API RESTful : query string pour le tri #2273)
  • Il est possible de filtrer sur les colonnes de type "Date" (ex: meta_last_action_date) en utilisant la syntaxe mentionnée par @TheoLechemia ?meta_last_action_date:gt=<date iso 8601> à l'exception que la date doit être au format 'YYYY-mm-dd' . J'utilise une fonction permettant de convertir la date au bon format
  • J'ai supprimé le @json_resp (remplacé par jsonify)

Concernant la comparaison de date , je me base pour l'instant sur la syntaxe : gt, lt, gte, lte (> , <, >=, <=). Comme mentioné ici par Elie (ref: #2273), le sujet reste ouvert pour qu'il y ait un accord sur la bonne pratique à adopter.

Travail ajoutés:

  • Test trigger ON DELETE Synthese INSERT in t_log_synthese
  • Test route /log
  • Test sort list query function (column:asc and column:desc)
  • Test sort params on route /log (?sort=last_action:asc&sort=meta_last_action_date:asc)

Travail restant

  • Test sur gt, lt, gte, lte (comparator date)

Merci pour votre lecture.

hypsug0 and others added 13 commits February 13, 2023 12:17
Improving PR  PnX-SI#1835
According to this comment :
Supprimer l’usage de GenericQuery : la manière fléché par SQLAlchemy pour cela serait d’utiliser query_class ; on peut envisager de faire des fonctions de filtrage générique de cette manière mais il faut que cela soit bien réfléchit et bien couvert par les tests.
Supprimer la vue v_log_synthese (plus difficile à maintenir) au profit d’une requête d’union directement dans la route

Reviewed_by:andriac
[Refs_PR]: PnX-SI#1835
Change down revision of `add_synthese_log_history` to target head of GN
Branch develop

Reviewed-by: andriacap
Remove `unique_id_sinp` from `t_log_synthese`
It seems useless to keep ``unique_id_sinp`

Reviewed-by: andriacap
Remove unused function in _get_model and _get_entity
by usign call directly the model `Synthese`

Reviewed-by: andriacap
- Add query_class for model SyntheseLogEntry
- Manage params :  multiple sort , multiple comparator on columns model
  (ref: PnX-SI#2273)

Reviewed-by: andriacap
[Refs PR]: PnX-SI#2337
@bouttier bouttier changed the base branch from feat/log-history to develop February 20, 2023 09:22
Test trigger ON DELETE Synthese INSERT in t_log_synthese
Test route /log
Test sort list query function (column:asc and column:desc)
Test sort params on route /log (?sort=last_action:asc&sort=meta_last_action_date:asc)

WIP: Test on gt, lt, gte, lte (comparator date)

Reviewed-by: andriac
@camillemonchicourt camillemonchicourt added this to the 2.12 milestone Feb 28, 2023
@bouttier
Copy link
Contributor

PR reprise et intégré dans #2337

@bouttier bouttier closed this Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants