fix(directory): #IMPULS-5881 add usefull link feature in directory - #1127
Conversation
| private Sql sql = Sql.getInstance(); | ||
|
|
||
| @Override | ||
| public Future<LinkOperationError> createLink(LinkDTO link, String userId) { |
There was a problem hiding this comment.
Est-ce une bonne pratique d'utiliser des DTOs dans les signatures des méthodes des services ?
Le DTO en théorie est réservé à la [de]sérialization des données transférées.
Là, si on n'a pas de DTO , on ne peut pas appeler le service.
Pas vraiment bloquant, mais je me posais la question... 🤔
There was a problem hiding this comment.
Merci, la remarque m'a fait corriger deux choses.
pour le `deleteLink, le contrôleur n'a qu'un id de path param et devait
fabriquer un LinkDTO aux deux tiers vide . C'est devenu deleteLink(UUID linkId, String userId)
Il y un problème, côté retour. LinkOperationError transportait des codes HTTP (200/409/400) depuis le service, et un lien introuvable était un Future réussi portant 400. Remplacé par (CreateLinkResult, DeleteLinkResult)
En revanche je garde createLink(LinkDTO link, String userId). L'alternative createLink(String name, String url, String userId) donne
deux String adjacents interchangeables et l'autre solution est un type de domaine à côté du DTO, pour trois champs sans comportement, c'est une classe de plus, et une conversion pour reproduire les mêmes champs.
Le fait de ne pas avoir cette DTO n'est pas un problème, le service appartient à directory donc tu as forcement la DTO, tu peux appeler en HTTP sans DTO. Une DTO n'est pas réservé à la desiralization / ser des données, mais au transport. Si tu prends Wikipedia Un DTO (Data Transfer Object, ou objet de transfert de données en français) est un patron de conception (design pattern) utilisé en programmation pour transporter des données entre différentes couches ou sous-systèmes d'une application. Reste l'argument de la séparation des couches, dans notre stack technique sans contrat, je ne le trouve pas rentable ou pertinent
| sql.prepared(" INSERT INTO directory.user_link(id, user_id, name, url, \"position\") " + | ||
| " SELECT ?::UUID, ?, ?, ?, free.pos " + | ||
| " FROM (SELECT s AS pos " + | ||
| " FROM generate_series(0, " + (MAX_LINKS_PER_USER - 1) + ") AS s " + |
There was a problem hiding this comment.
generate_series : je ne connaissais pas
jcbe-ode
left a comment
jcbe-ode
left a comment
There was a problem hiding this comment.
2-3 questions + il manque l'opération Update.
Pourrait être nécessaire un jour.
| @Override | ||
| public Future<List<LinkDTO>> getLinks(String userId) { | ||
| Promise<List<LinkDTO>> promise = Promise.promise(); | ||
| sql.prepared("SELECT id, user_id, name, url FROM directory.user_link WHERE user_id = ? ORDER BY lower(name) ASC NULLS LAST, id", |
There was a problem hiding this comment.
Pourquoi ne pas trier par position plutôt ?
There was a problem hiding this comment.
D'ailleurs, on ne peut pas réordonner les links entre eux ?
Il semble manquer le U de CRUD
There was a problem hiding this comment.
Pour le tri c'est au fonctionnel de dire ce qu'il veut comme tri. Pour l'update, c'est une bonne question, je vais rajouter le verbe
| CONSTRAINT user_link_user_position_key UNIQUE ("user_id", "position") | ||
| ); | ||
|
|
||
| COMMENT ON COLUMN directory.user_link."position" IS 'Index de capacite, pas un ordre d''affichage : le CHECK 0..9 et la contrainte UNIQUE (user_id, position) sont ce qui plafonne un utilisateur a 10 liens. L''affichage est trie alphabetiquement sur name.'; |
86f1cdb to
13e84e0
Compare
d1f8a4f to
91ad7e0
Compare
6acc7ad to
98f223b
Compare
|
…1127) * fix(directory): #IMPULS-5881 add usefull link feature in directory * PR review * PR review * add update link endpoint
…uvelle page d'accueil (#1132) * fix(directory): #IMPULS-5881 add usefull link feature in directory (#1127) * fix(directory): #IMPULS-5881 add usefull link feature in directory * PR review * PR review * add update link endpoint * feat(timeline): #IMPULS-6168 affiche le widget Liens utiles sur la nouvelle page d'accueil Le widget (UsefulLinksContainer) vient de @edifice.io/react/homepage. La disposition en 2 colonnes de la zone "moreWidgets" sera traitée dans un ticket suivant. --------- Co-authored-by: vbillard91 <vincent.billard@edifice.io>
…uvelle page d'accueil (#1132) * fix(directory): #IMPULS-5881 add usefull link feature in directory (#1127) * fix(directory): #IMPULS-5881 add usefull link feature in directory * PR review * PR review * add update link endpoint * feat(timeline): #IMPULS-6168 affiche le widget Liens utiles sur la nouvelle page d'accueil Le widget (UsefulLinksContainer) vient de @edifice.io/react/homepage. La disposition en 2 colonnes de la zone "moreWidgets" sera traitée dans un ticket suivant. --------- Co-authored-by: vbillard91 <vincent.billard@edifice.io>
2 participants
Description
add usefull link functionnality, add endpoints in directory and configuration to manage directory schema in postgresql
Fixes
https://edifice-community.atlassian.net/browse/IMPULS-5881
Type of change
Please check options that are relevant.
Which packages changed?
Please check the name of the package you changed
Tests
Reminder
Security flaws
Performance impacts (think bulk !)
Unit tests were replayed
Unit tests were added and/or changed
I have updated the reminder for the version including my modifications
All done ! 😃