Pull Request for User Resource only #75
Conversation
Also contains user database migration and database seed.
…with command 'rspec spec/requests/api/v1/users_spec.rb' in the server directory.
There was a problem hiding this comment.
I think you can revisit the api controller logic in light of the agreement that the rails app should publish a REST API for consumption by the front-end team. I am by far not an expert in Ruby and Rails, but to me this article looks good (I might be wrong) regarding details how to make an API controller in Rails. It might be worth a read ;)
As for designing REST APIs, this article might be of interest as a starting point. A point that often is missed is that responses different than 2xx are quite important and need to be considered for every endpoint.
…so that I can call individual accounts via API
…formation form this ApiUser database, also created a series of tests for these Apis.
|
Will resubmit with a different pull request |
CRUD APIs have been created A series of tests in Spec have been created for the CRUD APIs
|
@sibsmc I think it would be great to continue on this pull request, if you haven't started re-implementing this completely from scratch. A pull request in my opinion is a place for discussion, to share opinions, suggestions for improvement and to reach to a stage where both reviewer and author have maybe learnt something and have agreed that the state is good, whatever that means. In that sense, keeping the history of this discussion and progression is something valuable - the journey is as important as the destination. Having this point of view in mind, consider whether to open a new PR or re-open this one. |
|
Hi |
|
Creates a user database called 'ApiUser' I started to look into authentication, hence these three files: Test can be run by running command 'bundle exec rspec spec' |
In general, I usually say that if something doesn't serve a purpose, it shouldn't be in the code base in the first place. This applies also to the worse case when something doesn't work and the worst case when something works wrong/partially. The reason for this is that anything that is in the codebase calls for maintenance and when working in a team anyone can decide it is what they need (unfortunately not everyone reads docs, even if they exist) and this can either cause them headaches or even completely throw them off and slow down overall progress. On higher product level it is like releasing a partially done feature with the docs in code playing the role of explicit marking that the feature is experimental. When this is done, any user can start using it and depending on it so it becomes something that there might be market pressure to keep there, maybe even working in the broken way it was working in the first place. What I stated above pictures some of the grimmest scenarios that something like that can lead to. The risk with your change is low, I just wanted to illustrate the idea. Enough with general talk, on the practical side, I would recommend to remove the files that are not used in this review. You can stash them or add them to another branch locally (or remotely) so that you don't lose them - you've done some work after all in a direction that is needed. FYI, there are several authentication-related stories: #8 #13 #32 #33 #34 (I grouped them under a label). So you can consider continue working on auth as part of one of them (what you have here is probably more in scope of #8 #13 as the rest are integrations of libraries/products) |
Add User resource and browser version Create, Edit, Read and Delete function.
Also contains user database migration and database seed.