Skip to content

Conversation

@orbitalbitmap
Copy link
Owner

No description provided.

…c and moved and renamed the indexPugInteractions.js file to the new folder, and deleted a dependency that was accidentally installed.
…ha, chai, sinon, and sinon-chai. Installed the dependencies: sequelize and mysql2.
…and populate the pokemons and pokemonTypes table with the first 2 generations of pokemon.
…o remove the setup.sql from commit history and put the file in the .gitignore file.
…sibility of a form that is an alternate form and didn't want possible future confusion.
…. Changed the password and username for the datbse no one could ue the info from previous commits of the setup.sql file.
…tter named, and added in more get path documentation.
…monForms to have an id column to allow for duplicate entries (because pokemon need to be overly complicated). Also fixed an error where a column was set to auto increment but wasn't supposed to have that.
@orbitalbitmap orbitalbitmap requested a review from jlcarmic May 28, 2020 18:54
Copy link
Collaborator

@jlcarmic jlcarmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good work overall. Just some struggled with the POST route and issues matching up docs with expectations.

Score: 86%

Comment on lines 72 to 85
div(class="method hidden" id="postPokemon")
h2 Method
h3(class="spacing post") POST
h2 Route
h3(class="spacing") http://localhost:1337/pokemon
h2 Headers
h3(class="spacing") Content-Type: application/json
h2 Body
h3(class="spacing") { "gen": "1", "name": "Bulbasaur", "Types": "['grass', 'poison']", "fromId": "null" }
h2 Decription
h3(class="spacing")
| Saves a new pokemon to the database. (Note: the fromId is if the pokemon is an evolution of
| another pokemon Bulbasuar has 'fromId: null' as its a base evolution where as Ivysaur has
| 'fromId: 1' as it evolves from Bulbasaur)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Comment on lines 22 to 30
div(class="method hidden" id="getGeneration")
h2 Method
h3(class="spacing get") GET
h2 Route
h3(class="spacing") http://localhost:1337/pokemon/generation/{genId}
h2 Headers
h3(class="spacing") None needed
h2 Decription
h3(class="spacing") Returns the list of pokemon from gemeration passed with {genId}.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

h2 Method
h3(class="spacing get") GET
h2 Route
h3(class="spacing") http://localhost:1337/pokemon/generation/{genId}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is because your example is wrong, works when you use the expected URL

Comment on lines 47 to 52
const {
name,
generationNumber,
fromId,
types
} = request.body
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match what is in your docs

image

fromId,
types
} = request.body
const isProtected = request.body.isPortected || 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That typo will cause issues

defaults: { generationNumber, fromId, isProtected }
})

Promise.resolve(savedPokemon)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary, you awaited the database call, savedPokemon is already resolved.


const [savedPokemon, created] = await models.Pokemons.findOrCreate({
where: { name },
defaults: { generationNumber, fromId, isProtected }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to clean up the fromId, you can't insert the string 'null' because it is a string not null.

image

})


const typesId = await Promise.all(promisedTypesId)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need a separate variable here. Promises resolve in place.

const typesId = types.map(async typeName => {
  const [type] = await models.Types.findOrCreate({ where: { name: typeName } })

  return type.id
})


await Promise.all(typesId)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants