Fix track tests #120
Fix track tests #120
Conversation
Codecov Report
@@ Coverage Diff @@
## tracks #120 +/- ##
==========================================
+ Coverage 91.16% 92.79% +1.63%
==========================================
Files 21 21
Lines 645 722 +77
Branches 30 43 +13
==========================================
+ Hits 588 670 +82
+ Misses 57 52 -5
Continue to review full report at Codecov.
|
| @@ -1,12 +1,22 @@ | |||
| { | |||
| "initialArtist": { | |||
| "id": 1, | |||
There was a problem hiding this comment.
Estos ids los tenemos en varios pero creo que no esta bueno. No estoy segura de si se insertan asi o si no y despues los usamos asumiendo que quedaron con ese id. Nos complica mucho si guardamos las entries que devuelve knex una ve que creó y usamos ese id?
There was a problem hiding this comment.
Si le agregas el id y el mismo no esta repetido o es invalido la entry va a tener ese id seguro. Si queremos usar el id que devuelve knex independientemente de su valor habria que borrar esos campos de los json y no determinar un id valido de antemano en los json.
| res.body.track.should.have.property('artists') | ||
| .eql([ | ||
| artistsConstants.initialShortArtist, | ||
| artistsConstants.testShortArtist |
There was a problem hiding this comment.
Aca! mejor comparar contra lo que devovio knex y no contra la constante que suponemos que va a devolver, no?
src/test/routes/track.spec.js
Outdated
| dbHandler.general.createNewEntry(tables.artists, | ||
| [ | ||
| artistsConstants.initialArtist, | ||
| artistsConstants.testArtist |
There was a problem hiding this comment.
Si va a ser un initial siempre metele initialArtist2
This is a temporary fix until the artist field it's added to the GET /tracks endpoint
src/handlers/db/trackHandler.js
Outdated
| return artistTrackHandler.insertAssociations(insertedTrack[0].id, body.artists) | ||
| .then(() => insertedTrack); | ||
| }); | ||
| return db(tables.artists).whereIn('id', body.artists).then((artists) => { |
There was a problem hiding this comment.
Como hay que verificar tanto la existencia de los artistas como del album lo que hay que hacer es correr estos dos chequeos en paralelo y unificarlos en un Promise.all(promises)
src/handlers/db/trackHandler.js
Outdated
| }); | ||
| return db(tables.artists).whereIn('id', body.artists).then((artists) => { | ||
| if (artists.length < body.artists.length) { | ||
| logger.warn(`Req artists: ${JSON.stringify(body.artists)} vs DB artists: ${JSON.stringify(artists)}`); |
|
|
||
| const formatAlbumFromTrackJson = (album) => { | ||
| const formatAlbumShortJson = (album) => { | ||
| // TODO: catch null artist earlier | ||
| if (!album) { |
There was a problem hiding this comment.
album no deberia llegar nunca nulo
| album: formatAlbumFromTrackJson(track.album), | ||
| artists: track.artists.map((artist) => formatArtistFromTrackJson(artist)), | ||
| album: formatAlbumShortJson(track.album), | ||
| artists: (track.hasOwnProperty('artists')) ? track.artists.map((artist) => formatArtistShortJson(artist)) : [], |
There was a problem hiding this comment.
El campo artists siempre deberia existir y tener al menos un artista
| dbHandler.track.createNewTrackEntry(constants.initialTrack) | ||
| .then(() => done()) | ||
| .catch(error => done(error)); | ||
| db.migrate.rollback().then(() => { |
There was a problem hiding this comment.
Encapsular esta logica repetida (en este archivo y track.spec.js)
Note: album find function not yet implemented as the table it's not defined
#113