Skip to content

Conversation

@jjfreund
Copy link
Contributor

… request paradigm

Copy link
Contributor

@schroerbrian schroerbrian left a comment

Choose a reason for hiding this comment

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

Looks great. There are some test failures, but I assume you are working on those

config/routes.rb Outdated
Comment on lines 51 to 52
post :create
put :update
Copy link
Member

Choose a reason for hiding this comment

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

I think these routes are already handled automatically by the resources method, according to https://guides.rubyonrails.org/routing.html#crud-verbs-and-actions. In fact, I think you're actually adding different routes compared to the Rails builtin ones, since this will create the create route at /notes/create, whereas the builtin is just at /notes (with a POST method), and yours will create the update route at /notes/update, whereas the builtin is /notes/:id/, where :id is the ID of the resource in the URL itself rather than as a parameter.

Copy link
Member

@richardxia richardxia left a comment

Choose a reason for hiding this comment

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

Changes mostly LGTM, but you're failing some Postman tests.

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.

3 participants