WIP: Implement JSON API #230

timurvafin · 2018-09-12T12:17:31Z

Token request:

curl "https://fs-rails-base-api-pr-230.herokuapp.com/v1/tokens" -d '{"data":{"type":"token_requests","attributes":{"email":"[email protected]","password":"123456"}}}' -X POST \
  -H "Content-Type: application/vnd.api+json" \
  -H "Accept: application/vnd.api+json"

Users requests (specify token from Tokens request):

curl -g "https://fs-rails-base-api-pr-230.herokuapp.com/v1/users" -X GET \
  -H "Content-Type: application/vnd.api+json" \
  -H "Accept: application/vnd.api+json" \
  -H "Authorization: Bearer TOKEN"

Closes #208

* Update Ruby to 2.5.1 * Replace factory_girl -> factory_bot * Add JWT * Generate markdown API docs only * Errors refactor * Update README and remove unused gems

After upgrading rails to version 5, database_cleaner became redundant * https://github.com/teamcapybara/capybara#transactions-and-database-setup * thoughtbot/suspenders@44e984f

ArthurZaharov · 2018-09-12T19:20:02Z

README.md

@@ -55,39 +54,6 @@ Status of the API could be checked at [http://localhost:5000/docs](http://localh
 * `bin/ci` - should be used in the CI or locally
 * `bin/server` - to run server locally


Should we add bin/doc script description here?

Also we can remove bin/foreman.

ArthurZaharov · 2018-09-12T19:27:46Z

README.md

+* [knock](https://github.com/nsarno/knock) – seamless JWT authentication
+* [puma](https://github.com/puma/puma) as Rails web server
+* [rack-cors](https://github.com/cyu/rack-cors) for [CORS](http://en.wikipedia.org/wiki/Cross-origin_resource_sharing)
+* [responders](https://github.com/plataformatec/responders) - DRY controllers


We do not use them as I see. Is it planned for future using in admin panel for example?

ArthurZaharov · 2018-09-12T19:31:12Z

app/controllers/v1/users_controller.rb

+module V1
+  class UsersController < V1::BaseController
+    expose :user
+    expose :users, -> { User.all }


Should we add pagination?

We can consider https://github.com/davidcelis/api-pagination gem to mimic pagination like User.page(params[:page][:number]).per(params[:page][:size]) in controller as I think.

ArthurZaharov · 2018-09-12T19:46:57Z

config/environments/production.rb

-  config.log_level = :info
+  # Use the lowest log level to ensure availability of diagnostic information
+  # when problems arise.
+  config.log_level = :debug


Should we use :info log level in production env to reduce logs size? If not then we can comment or remove this line (it is default).

ArthurZaharov · 2018-09-12T19:54:56Z

README.md

@@ -1,52 +1,51 @@
-# Skeleton for new Rails 4 application for REST API
+# Skeleton for new Rails 5 application for JSON API


Should we update doc/README_TEMPLATE.md accordingly?

ArthurZaharov · 2018-09-12T20:01:21Z

spec/api/v1/tokens_spec.rb

+    context "with invalid password" do
+      let(:password) { "invalid" }
+
+      example "Create Token with invalid password" do


And here...

ArthurZaharov · 2018-09-12T20:01:50Z

spec/api/v1/users_spec.rb

+    context "with invalid id" do
+      let(:id) { 0 }
+
+      example "Retrive User with invalid id" do


And here...

ArthurZaharov · 2018-09-12T20:05:27Z

spec/support/shared_examples/api_endpoint_with_authorization.rb

@@ -0,0 +1,12 @@
+# rubocop:disable RSpec/EmptyExampleGroup
+shared_examples "API endpoint with authorization" do
+  context "without authorization headers", document: false do


document: false does not work on context as I know. It should be written like:

context "..." do example "...", document: false do do_request ... end end

ArthurZaharov · 2018-09-12T20:06:28Z

spec/support/shared_examples/failure_interactor.rb

+  it "fails" do
+    interactor.run
+    expect(context).to be_failure
+    expect(context.message).to eql(params[:message]) if params


Our interactor returns code on failure. Should we check it here?

ArthurZaharov · 2018-09-12T20:07:17Z

spec/support/shared_examples/success_interactor.rb

@@ -0,0 +1,6 @@
+shared_examples "success interactor" do
+  it "success" do


it "succeeds"

DamirNurgaliev · 2018-12-11T17:40:47Z

app/controllers/v1/tokens_controller.rb

+    skip_before_action :authenticate_user!
+
+    def create
+      result = CreateJwt.call(authentication_params)


Should we really introduce custom logic for generating a token? It is already provided by a gem. Also creating a separate JwtToken model not necessary. Looks like you are doing it only to response in JSON API format, but maybe we can return token here in JSON API manually?

class V1::TokensController < Knock::AuthTokenController skip_before_action :verify_authenticity_token, only: :create def create render json: response_data, status: :created end private def response_data { data: { id: auth_token.token, type: "tokens" } } end def entity_class User end def auth_params jsonapi_params(only: %i[email password]) end end

In this case, we can remove JwtToken model, CreateJwt interactor and JwtTokenSerializer. Any thoughts?

DamirNurgaliev · 2018-12-11T17:46:56Z

app/models/user.rb

-  devise :database_authenticatable, :registerable,
-    :recoverable, :trackable, :validatable
+  validates :email, presence: true
+  validates :password, length: { minimum: 6 }


To update a user's attribute he always needs to pass a password. Was it intended? Looks like if you need to update a full_name for example, you don't need to provide a password.

validates :password, length: { minimum: 6 }, if: -> { new_record? || password_digest_changed? }

Implement JSON API

1476e7d

* Update Ruby to 2.5.1 * Replace factory_girl -> factory_bot * Add JWT * Generate markdown API docs only * Errors refactor * Update README and remove unused gems

timurvafin temporarily deployed to fs-rails-base-api-pr-230 September 12, 2018 12:17 Inactive

timurvafin mentioned this pull request Sep 12, 2018

[WIP] Update ruby/rails #228

Closed

timurvafin added 5 commits September 12, 2018 15:18

Remove securerandom

0a6482b

Remove yarn bin

c5c03eb

Filter :token in logs

3f63e7c

Rename factory_bot for generators

30d7967

Do not wrap params for json

645dcf5

timurvafin temporarily deployed to fs-rails-base-api-pr-230 September 12, 2018 12:25 Inactive

timurvafin added 6 commits September 12, 2018 15:26

Set TargetRubyVersion 2.5 for Rubocop

7be0e85

Remove unsed bin/update

78734d0

Remove unsed application_controller_renderer.rb initializer

a4e9355

Restore locales

bddde7f

Restore secrets

9120098

Remove DatabseCleaner

072ed46

After upgrading rails to version 5, database_cleaner became redundant * https://github.com/teamcapybara/capybara#transactions-and-database-setup * thoughtbot/suspenders@44e984f

timurvafin deployed to fs-rails-base-api-pr-230 September 12, 2018 12:39 View deployment

ArthurZaharov reviewed Sep 12, 2018

View reviewed changes

DamirNurgaliev reviewed Dec 11, 2018

View reviewed changes

timurvafin mentioned this pull request Apr 17, 2019

Updated for Ruby and some gems to prevent security issues #234

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Implement JSON API #230

WIP: Implement JSON API #230

timurvafin commented Sep 12, 2018 •

edited

Loading

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 13, 2018

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 12, 2018 •

edited

Loading

ArthurZaharov Sep 12, 2018

ArthurZaharov Sep 12, 2018

DamirNurgaliev Dec 11, 2018

DamirNurgaliev Dec 11, 2018

		@@ -55,39 +54,6 @@ Status of the API could be checked at [http://localhost:5000/docs](http://localh
		* `bin/ci` - should be used in the CI or locally
		* `bin/server` - to run server locally

		@@ -1,52 +1,51 @@
		# Skeleton for new Rails 4 application for REST API
		# Skeleton for new Rails 5 application for JSON API

		@@ -0,0 +1,6 @@
		shared_examples "success interactor" do
		it "success" do

WIP: Implement JSON API #230

Are you sure you want to change the base?

WIP: Implement JSON API #230

Conversation

timurvafin commented Sep 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArthurZaharov Sep 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timurvafin commented Sep 12, 2018 •

edited

Loading

ArthurZaharov Sep 12, 2018 •

edited

Loading