Tracks #114
Tracks #114
Conversation
src/handlers/db/generalHandler.js
Outdated
| @@ -8,9 +8,13 @@ const findAllEntries = (tableName) => { | |||
|
|
|||
| const findEntryWithId = (tableName, id) => { | |||
| logger.info(`Searching for entry ${id}`); | |||
| return db(tableName).where('id', id); | |||
| return db(tableName).where('id', id).first('*'); | |||
There was a problem hiding this comment.
A createNewEntry seguro que no, si no cuando alguien cree mas de un entry (como en los tests) no tenes manera de ver que se creo mas alla de la primer entry. Con los otros casos puede que sea igual.
src/handlers/db/generalHandler.js
Outdated
|
|
||
| const findAllWithAttributes = (tableName, attributes) => { | ||
| logger.info(`Querying table ${tableName} for entries with attributes ${JSON.stringify(attributes, null, 4)}`); | ||
| return db(tableName).where(attributes); |
There was a problem hiding this comment.
Pero hay que ser consistente y seguir abstrallendonos de knex (? 👮
src/handlers/db/trackHandler.js
Outdated
| .then((insertedTrack) => { | ||
| logger.info(`Inserted track: ${JSON.stringify(insertedTrack, null, 4)}`); | ||
| return artistHandler.selectAllArtistsIdsWithNames(track.artists) | ||
| .then(ids => artistTrackHandler.insertAssociations(insertedTrack[0].id, ids)) | ||
| return artistTrackHandler.insertAssociations(insertedTrack[0].id, body.artists) |
There was a problem hiding this comment.
Que pasa si se inserta mas de un track? (Por ej, en los tests)
There was a problem hiding this comment.
yyyy pero para los tests habria que hacer otra funcion si es que necesitamos, no tendriamos que codear pensando en los tests. Si hay algun caso de uso en el que te puedan dar de alta mas de un track en un mismo request si, pero segun la doc creo que no 🤔
There was a problem hiding this comment.
Eso te iba a decir, hay que abrir un issue para crearle al handler funciones especificas para los tests y no crear conflictos con el comportamiento esperado por la doc.
There was a problem hiding this comment.
Abrilo y vamos comentando las que necesitaria
| return generalHandler.updateEntryWithId(tables.tracks, id, track) | ||
| .then((updatedTrack) => { | ||
| logger.info(`Updated track: ${JSON.stringify(updatedTrack, null, 4)}`); | ||
| return artistTrackHandler.updateAssociations(updatedTrack[0].id, body.artists) |
There was a problem hiding this comment.
Ídem insertedTrack, aunque no vi que hagamos un update de a varios en los tests aún.
src/handlers/db/trackHandler.js
Outdated
| }) | ||
| .then((result) => { | ||
| if (result.length) { | ||
| logger.info(`User ${userId} already liked track ${trackId}`); |
There was a problem hiding this comment.
Conviene poner esto como un warn
src/handlers/response.js
Outdated
| }; | ||
| }; | ||
|
|
||
| const formatAlbumFromTrackJson = (album) => { |
src/handlers/response.js
Outdated
| }; | ||
|
|
||
| const formatAlbumFromTrackJson = (album) => { | ||
| // TODO: catch null artist earlier |
There was a problem hiding this comment.
Si el id del artista que se pasa al crear un track no existe, la asociación se crea igual y cuando pedís todos los artists con los id's del track te llega un array vacío.
En definitiva, esto viene por otro quilombo: no estamos chequeando que los artists que nos pasa el nuevo track existan.
src/routes/track.js
Outdated
| db.general.findEntryWithId(tables.tracks, req.params.id) | ||
| .then((track) => { | ||
| if (!respond.entryExists(req.params.id, track, res)) return; | ||
| const getArtistsInfo = () => { |
There was a problem hiding this comment.
👮👮👮👮👮
(bien ahí igual, me olvide de moverlas despues de chequear que funcionen)
src/routes/track.js
Outdated
| Promise.all(getters) | ||
| .then((results) => { | ||
| logger.info(`Results: ${JSON.stringify(results, null, 4)}`); | ||
| logger.info(`Pre fetch Track: ${JSON.stringify(track, null, 4)}`); |
src/test/routes/track.constants.json
Outdated
| "initialTrack": { | ||
| "name": "Despacito", | ||
| "duration": 240, | ||
| "artists": ["Luis Fonsi", "Papa Baute"] |
There was a problem hiding this comment.
Se actualizo la creación de asociaciones junto con este cambio? Antes se buscaba el id del artista según el nombre y después se lo insertaba en la tabla de asociación.
There was a problem hiding this comment.
Si, mas arriba comente que la funcion que buscaba los ids de artistas con x nombre ya no se usa. Mire la doc y llegan ids directo.
This is a temporary fix until the artist field it's added to the GET /tracks endpoint
Note: album find function not yet implemented as the table it's not defined
Fix track tests
Closes #23