-
Notifications
You must be signed in to change notification settings - Fork 0
Feat: Daymail routes #92
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
base: dev
Are you sure you want to change the base?
Conversation
…ow applied to classes instead of properties
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #92 +/- ##
==========================================
+ Coverage 83.19% 84.17% +0.97%
==========================================
Files 140 148 +8
Lines 2398 2565 +167
Branches 470 465 -5
==========================================
+ Hits 1995 2159 +164
- Misses 398 401 +3
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/assos/assos.service.ts
Outdated
| titleTranslation: { create: pick(title, 'fr', 'en', 'es', 'de', 'zh') }, | ||
| bodyTranslation: { create: pick(message, 'fr', 'en', 'es', 'de', 'zh') }, |
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.
Tu peux avoir d'autres propriétés sur une Translation ?
Et par la même occasion, pourquoi ne pas garder le type de TranslationReqDto au lieu de Translation ? (pareil pour update)
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.
Tu veux dire remplacer partout Translation par TranslationReqDto ? J'imagine pourquoi pas, en renamant TranslationReqDto en Translation then ?
Sinon, je pick pour m'assurer que le type est bien ce que je veux avoir (genre ya pas des fields qui traînent en plus pour je ne sais quelle raison). J'ai fait une liste pour store les languages du coup, tu me diras si ça te va
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.
Ce que je veux dire c'est que c'est juste un typage et en soit c'est encore ton dto à cet endroit là donc je trouve ça pas pertinent de le rename ici
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'ai viré le pick, j'avoue qu'après nouvelle réflexion c'était ptet en effet overkill
Par contre, non, à cet endroit là, on ne sait pas si ça vient d'un controller ou d'autre part, on est à l'intérieur de l'API, pas sûr la limite comme au niveau des controllers (ya 2-3 fois où on a give up la règle parce que chiant de redéfinir un type qui existe déjà)
POST /assos/:assoId/daymailHasSomeAmongis now applied to classes instead of properties