Skip to content

Feat/Create Hardcoded API with Schema#3

Open
leonardostsouza wants to merge 14 commits into
masterfrom
feat/create-hardcoded-api-with-schema
Open

Feat/Create Hardcoded API with Schema#3
leonardostsouza wants to merge 14 commits into
masterfrom
feat/create-hardcoded-api-with-schema

Conversation

@leonardostsouza
Copy link
Copy Markdown
Contributor

No description provided.

@leonardostsouza leonardostsouza changed the title WIP: Feat/Create Hardcoded API with Schema Feat/Create Hardcoded API with Schema Oct 1, 2019
Copy link
Copy Markdown

@bamorim bamorim left a comment

Choose a reason for hiding this comment

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

The spec is not as we discussed.
Also, the API is not set correctly (content-type wise).
Also, some of the responses are weird (returning 201 on a failed request is a completely no-no)

Comment thread src/routes/api-routes.js Outdated
Comment thread src/routes/api-routes.js Outdated
Comment thread src/routes/api-routes.js
Comment thread src/routes/api-routes.js Outdated
Comment thread src/routes/api-routes.js
Comment thread src/routes/api-routes.js Outdated
Comment thread src/routes/api-routes.js
Comment thread src/routes/api-routes.js Outdated
Comment thread src/routes/api-routes.js Outdated
Copy link
Copy Markdown

@bamorim bamorim left a comment

Choose a reason for hiding this comment

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

In general, it looks better. The only problem I think is the content type.
So adding the content type assertion to the tests is nice. Also, moving the utils and event store to a diferent file with corresponding unit tests (not passing through the API) would improve things.

Summing up

  • Add assertions about content-type
  • Move isValidSize and isBase64 to utils and add unit tests to them.
  • Move event store related to something like event_store/in_memory.js and adding unit tests.

Comment thread src/routes/api-routes.js Outdated
Comment thread src/routes/api-routes.js Outdated
Comment thread src/routes/api-routes.js Outdated
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