-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Tableau de bord] endpoint pour récupérer les informations du tableau de bord à partir d'une géométrie WKT #1695
[Tableau de bord] endpoint pour récupérer les informations du tableau de bord à partir d'une géométrie WKT #1695
Conversation
override val updatedAtUtc: Instant? = null, | ||
override val withVHFAnswer: Boolean? = null, | ||
override val isInfractionProven: Boolean, | ||
@Formula("created_at + INTERVAL '1 hour' * validity_time") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note : On a séparé en plusieurs classes à cause du @Formula
qui fait planter les natives queries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
WHERE reporting.mission.id = :missionId | ||
""", | ||
) | ||
fun findByMissionId( | ||
missionId: Int, | ||
): List<ReportingModel> | ||
): List<ReportingModelJpa> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Du coup ReportingModelJpa
est utilisé pour le JPQL et ReportingModel
pour 1) les native queries et 2) transformer le reporting en entité métier ?
ça peut être bien de le spécifier (nommage / javadoc) pour savoir quel type utiliser lorsqu'on ajoute une query ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour le 1) Oui, sinon pour le mapping en entité métier c'est dans la classe abstraite et il n'y a pas d'écart entre les 2 models
Allez un coup de doc pour être plus claire. Après la requete plante si tu utilise le ReportingModelJPA en nativequery
private val vigilanceAreaRepository: IVigilanceAreaRepository, | ||
) { | ||
|
||
fun execute(geometry: Geometry): ExtractedAreaEntity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Petite note : il y a 5 seq. scan dans ce use-case (car pas d'index sur la geometry
).
J'avais compris que ce use-case n'était pas utilisé très fréquemment, mais c'est à garder en tête si la requête est longue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes on a fait un ticket à la fin de la feature pour revenir sur la perf. Quitte a changer l'infra si ya trop de lenteur
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hors NamedEntity où je n'ai pas la compétence pour pouvoir review, good job sur cette PR :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'aime bien le fait d'avoir les fixtures juxtaposées aux dtos, c'est le genre de patterns que je suis habituellement. Je les aurais simplement plutôt mises dedans plutôt que dehors (dtos/fixtures/...). On pourrait aussi le faire pour les entités d'ailleurs (suivre ce pattern de manière générale je veux dire).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour moi, les fixtures sont exclusivement utilisées simplifier les tests (d'ou sa présence dans le package test) et ne doivent pas être utilisées en prod. Ca nous permettrait de revoir le typage des entités pour supprimer les null
par défaut dans un futur proche
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'avais lu trop vite en effet, je n'avais pas vu le dossier test/
dans le path (caché par les ellipsis).
Création de l'api permettant de récupérer le département (un seul pour le moment), les zones reg, amp, zones de vigilances et signalements à partir d'une zone créée.
Related Pull Requests & Issues