From e591658f48d80e6029ccb83908693a8470652cd7 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 31 Jul 2019 14:18:28 +0100 Subject: [PATCH 01/10] Keep only used api/taxons index action, delete all others as not required right now --- .../spree/api/taxons_controller.rb | 39 ----------- .../spree/api/taxons_controller_spec.rb | 66 ------------------- 2 files changed, 105 deletions(-) diff --git a/app/controllers/spree/api/taxons_controller.rb b/app/controllers/spree/api/taxons_controller.rb index a6fe754184..d66af7dd71 100644 --- a/app/controllers/spree/api/taxons_controller.rb +++ b/app/controllers/spree/api/taxons_controller.rb @@ -25,51 +25,12 @@ module Spree show end - def create - authorize! :create, Taxon - @taxon = Taxon.new(params[:taxon]) - @taxon.taxonomy_id = params[:taxonomy_id] - taxonomy = Taxonomy.find_by_id(params[:taxonomy_id]) - - if taxonomy.nil? - @taxon.errors[:taxonomy_id] = I18n.t(:invalid_taxonomy_id, scope: 'spree.api') - invalid_resource!(@taxon) && return - end - - @taxon.parent_id = taxonomy.root.id unless params[:taxon][:parent_id] - - if @taxon.save - respond_with(@taxon, status: 201, default_template: :show) - else - invalid_resource!(@taxon) - end - end - - def update - authorize! :update, Taxon - if taxon.update_attributes(params[:taxon]) - respond_with(taxon, status: 200, default_template: :show) - else - invalid_resource!(taxon) - end - end - - def destroy - authorize! :delete, Taxon - taxon.destroy - respond_with(taxon, status: 204) - end - private def taxonomy return if params[:taxonomy_id].blank? @taxonomy ||= Taxonomy.find(params[:taxonomy_id]) end - - def taxon - @taxon ||= taxonomy.taxons.find(params[:id]) - end end end end diff --git a/spec/controllers/spree/api/taxons_controller_spec.rb b/spec/controllers/spree/api/taxons_controller_spec.rb index 03ec795ea1..9c76e2ae71 100644 --- a/spec/controllers/spree/api/taxons_controller_spec.rb +++ b/spec/controllers/spree/api/taxons_controller_spec.rb @@ -64,72 +64,6 @@ module Spree response["attr"].should eq("name" => taxon2.name, "id" => taxon2.id) response["state"].should eq("closed") end - - it "can learn how to create a new taxon" do - api_get :new, taxonomy_id: taxonomy.id - expect(json_response["attributes"]).to eq(attributes.map(&:to_s)) - required_attributes = json_response["required_attributes"] - expect(required_attributes).to include("name") - end - - it "cannot create a new taxon if not an admin" do - api_post :create, taxonomy_id: taxonomy.id, taxon: { name: "Location" } - assert_unauthorized! - end - - it "cannot update a taxon" do - api_put :update, taxonomy_id: taxonomy.id, - id: taxon.id, - taxon: { name: "I hacked your store!" } - assert_unauthorized! - end - - it "cannot delete a taxon" do - api_delete :destroy, taxonomy_id: taxonomy.id, id: taxon.id - assert_unauthorized! - end - end - - context "as an admin" do - let(:current_api_user) { build(:admin_user) } - - it "can create" do - api_post :create, taxonomy_id: taxonomy.id, taxon: { name: "Colors" } - - expect(attributes.all? { |a| json_response.include? a }).to be true - expect(response.status).to eq(201) - - expect(taxonomy.reload.root.children.count).to eq 2 - - expect(Spree::Taxon.last.parent_id).to eq taxonomy.root.id - expect(Spree::Taxon.last.taxonomy_id).to eq taxonomy.id - end - - it "cannot create a new taxon with invalid attributes" do - api_post :create, taxonomy_id: taxonomy.id, taxon: {} - expect(response.status).to eq(422) - expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.") - errors = json_response["errors"] - - expect(taxonomy.reload.root.children.count).to eq 1 - end - - it "cannot create a new taxon with invalid taxonomy_id" do - api_post :create, taxonomy_id: 1000, taxon: { name: "Colors" } - expect(response.status).to eq(422) - expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.") - - errors = json_response["errors"] - expect(errors["taxonomy_id"]).not_to be_nil - expect(errors["taxonomy_id"].first).to eq "Invalid taxonomy id." - - expect(taxonomy.reload.root.children.count).to eq 1 - end - - it "can destroy" do - api_delete :destroy, taxonomy_id: taxonomy.id, id: taxon2.id - expect(response.status).to eq(204) - end end end end From 367932a7674716794f6298fb8f614877d8961ac1 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 31 Jul 2019 14:27:16 +0100 Subject: [PATCH 02/10] Make spree/api/taxons_controller use AMS serializer instead of rabl --- .../spree/api/taxons_controller.rb | 6 ++++-- .../spree/api/taxons_controller_spec.rb | 20 +++++++++++-------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/app/controllers/spree/api/taxons_controller.rb b/app/controllers/spree/api/taxons_controller.rb index d66af7dd71..d56c7a2b63 100644 --- a/app/controllers/spree/api/taxons_controller.rb +++ b/app/controllers/spree/api/taxons_controller.rb @@ -1,8 +1,10 @@ module Spree module Api - class TaxonsController < Spree::Api::BaseController + class TaxonsController < ::Api::BaseController respond_to :json + skip_authorization_check only: :index + def index if taxonomy @taxons = taxonomy.root.children @@ -13,7 +15,7 @@ module Spree @taxons = Taxon.ransack(params[:q]).result end end - respond_with(@taxons) + render json: @taxons, each_serializer: ::Api::TaxonSerializer end def show diff --git a/spec/controllers/spree/api/taxons_controller_spec.rb b/spec/controllers/spree/api/taxons_controller_spec.rb index 9c76e2ae71..fa7068ccd3 100644 --- a/spec/controllers/spree/api/taxons_controller_spec.rb +++ b/spec/controllers/spree/api/taxons_controller_spec.rb @@ -26,20 +26,24 @@ module Spree api_get :index, taxonomy_id: taxonomy.id expect(json_response.first['name']).to eq taxon.name - children = json_response.first['taxons'] - expect(children.count).to eq 1 - expect(children.first['name']).to eq taxon2.name - expect(children.first['taxons'].count).to eq 1 + + # WIP maybe not needed + #children = json_response.first['taxons'] + #expect(children.count).to eq 1 + #expect(children.first['name']).to eq taxon2.name + #expect(children.first['taxons'].count).to eq 1 end it "gets all taxons" do api_get :index expect(json_response.first['name']).to eq taxonomy.root.name - children = json_response.first['taxons'] - expect(children.count).to eq 1 - expect(children.first['name']).to eq taxon.name - expect(children.first['taxons'].count).to eq 1 + + # WIP maybe not needed + #children = json_response.first['taxons'] + #expect(children.count).to eq 1 + #expect(children.first['name']).to eq taxon.name + #expect(children.first['taxons'].count).to eq 1 end it "can search for a single taxon" do From 4ca8feeef1353358b26b42dd6dfcbaae9e3daad2 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 1 Aug 2019 14:21:33 +0100 Subject: [PATCH 03/10] Move api/taxons route and controller out of spree namespace into /api and move the ctrl spec as well --- app/controllers/{spree => }/api/taxons_controller.rb | 0 config/routes/api.rb | 2 ++ spec/controllers/{spree => }/api/taxons_controller_spec.rb | 0 3 files changed, 2 insertions(+) rename app/controllers/{spree => }/api/taxons_controller.rb (100%) rename spec/controllers/{spree => }/api/taxons_controller_spec.rb (100%) diff --git a/app/controllers/spree/api/taxons_controller.rb b/app/controllers/api/taxons_controller.rb similarity index 100% rename from app/controllers/spree/api/taxons_controller.rb rename to app/controllers/api/taxons_controller.rb diff --git a/config/routes/api.rb b/config/routes/api.rb index 99c7f76eda..4a193fec7f 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -29,5 +29,7 @@ Openfoodnetwork::Application.routes.draw do resources :enterprise_fees, only: [:destroy] post '/product_images/:product_id', to: 'product_images#update_product_image' + + resources :taxons, :only => [:index] end end diff --git a/spec/controllers/spree/api/taxons_controller_spec.rb b/spec/controllers/api/taxons_controller_spec.rb similarity index 100% rename from spec/controllers/spree/api/taxons_controller_spec.rb rename to spec/controllers/api/taxons_controller_spec.rb From 7a652fd67b62539f9c7221ae1a53b908e1c9cf32 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 1 Aug 2019 14:25:02 +0100 Subject: [PATCH 04/10] Adapt api/taxons controller to new namespace outside Spree: remove Spree module and add Spree namespace to Taxons and Taxonomy classes --- app/controllers/api/taxons_controller.rb | 50 ++++----- .../controllers/api/taxons_controller_spec.rb | 106 +++++++++--------- 2 files changed, 75 insertions(+), 81 deletions(-) diff --git a/app/controllers/api/taxons_controller.rb b/app/controllers/api/taxons_controller.rb index d56c7a2b63..c20097e0f6 100644 --- a/app/controllers/api/taxons_controller.rb +++ b/app/controllers/api/taxons_controller.rb @@ -1,38 +1,36 @@ -module Spree - module Api - class TaxonsController < ::Api::BaseController - respond_to :json +module Api + class TaxonsController < Api::BaseController + respond_to :json - skip_authorization_check only: :index + skip_authorization_check only: :index - def index - if taxonomy - @taxons = taxonomy.root.children + def index + if taxonomy + @taxons = taxonomy.root.children + else + if params[:ids] + @taxons = Spree::Taxon.where(id: params[:ids].split(",")) else - if params[:ids] - @taxons = Taxon.where(id: params[:ids].split(",")) - else - @taxons = Taxon.ransack(params[:q]).result - end + @taxons = Spree::Taxon.ransack(params[:q]).result end - render json: @taxons, each_serializer: ::Api::TaxonSerializer end + render json: @taxons, each_serializer: Api::TaxonSerializer + end - def show - @taxon = taxon - respond_with(@taxon) - end + def show + @taxon = taxon + respond_with(@taxon) + end - def jstree - show - end + def jstree + show + end - private + private - def taxonomy - return if params[:taxonomy_id].blank? - @taxonomy ||= Taxonomy.find(params[:taxonomy_id]) - end + def taxonomy + return if params[:taxonomy_id].blank? + @taxonomy ||= Spree::Taxonomy.find(params[:taxonomy_id]) end end end diff --git a/spec/controllers/api/taxons_controller_spec.rb b/spec/controllers/api/taxons_controller_spec.rb index fa7068ccd3..884fc17bec 100644 --- a/spec/controllers/api/taxons_controller_spec.rb +++ b/spec/controllers/api/taxons_controller_spec.rb @@ -1,73 +1,69 @@ require 'spec_helper' -module Spree - describe Api::TaxonsController do - render_views +describe Api::TaxonsController do + render_views - let(:taxonomy) { create(:taxonomy) } - let(:taxon) { create(:taxon, name: "Ruby", taxonomy: taxonomy) } - let(:taxon2) { create(:taxon, name: "Rails", taxonomy: taxonomy) } - let(:attributes) { - ["id", "name", "pretty_name", "permalink", "position", "parent_id", "taxonomy_id"] - } + let(:taxonomy) { create(:taxonomy) } + let(:taxon) { create(:taxon, name: "Ruby", taxonomy: taxonomy) } + let(:taxon2) { create(:taxon, name: "Rails", taxonomy: taxonomy) } + let(:attributes) { + ["id", "name", "pretty_name", "permalink", "position", "parent_id", "taxonomy_id"] + } - before do - allow(controller).to receive(:spree_current_user) { current_api_user } + before do + allow(controller).to receive(:spree_current_user) { current_api_user } - taxon2.children << create(:taxon, name: "3.2.2", taxonomy: taxonomy) - taxon.children << taxon2 - taxonomy.root.children << taxon + taxon2.children << create(:taxon, name: "3.2.2", taxonomy: taxonomy) + taxon.children << taxon2 + taxonomy.root.children << taxon + end + + context "as a normal user" do + let(:current_api_user) { build(:user) } + + it "gets all taxons for a taxonomy" do + api_get :index, taxonomy_id: taxonomy.id + + expect(json_response.first['name']).to eq taxon.name + + children = json_response.first['taxons'] + expect(children.count).to eq 1 + expect(children.first['name']).to eq taxon2.name + expect(children.first['taxons'].count).to eq 1 end - context "as a normal user" do - let(:current_api_user) { build(:user) } + it "gets all taxons" do + api_get :index - it "gets all taxons for a taxonomy" do - api_get :index, taxonomy_id: taxonomy.id + expect(json_response.first['name']).to eq taxonomy.root.name - expect(json_response.first['name']).to eq taxon.name + children = json_response.first['taxons'] + expect(children.count).to eq 1 + expect(children.first['name']).to eq taxon.name + expect(children.first['taxons'].count).to eq 1 + end - # WIP maybe not needed - #children = json_response.first['taxons'] - #expect(children.count).to eq 1 - #expect(children.first['name']).to eq taxon2.name - #expect(children.first['taxons'].count).to eq 1 - end + it "can search for a single taxon" do + api_get :index, q: { name_cont: "Ruby" } - it "gets all taxons" do - api_get :index + expect(json_response.count).to eq(1) + expect(json_response.first['name']).to eq "Ruby" + end - expect(json_response.first['name']).to eq taxonomy.root.name + it "gets a single taxon" do + api_get :show, id: taxon.id, taxonomy_id: taxonomy.id - # WIP maybe not needed - #children = json_response.first['taxons'] - #expect(children.count).to eq 1 - #expect(children.first['name']).to eq taxon.name - #expect(children.first['taxons'].count).to eq 1 - end + expect(json_response['name']).to eq taxon.name + expect(json_response['taxons'].count).to eq 1 + end - it "can search for a single taxon" do - api_get :index, q: { name_cont: "Ruby" } + it "gets all taxons in JSTree form" do + api_get :jstree, taxonomy_id: taxonomy.id, id: taxon.id - expect(json_response.count).to eq(1) - expect(json_response.first['name']).to eq "Ruby" - end - - it "gets a single taxon" do - api_get :show, id: taxon.id, taxonomy_id: taxonomy.id - - expect(json_response['name']).to eq taxon.name - expect(json_response['taxons'].count).to eq 1 - end - - it "gets all taxons in JSTree form" do - api_get :jstree, taxonomy_id: taxonomy.id, id: taxon.id - - response = json_response.first - response["data"].should eq(taxon2.name) - response["attr"].should eq("name" => taxon2.name, "id" => taxon2.id) - response["state"].should eq("closed") - end + response = json_response.first + response["data"].should eq(taxon2.name) + response["attr"].should eq("name" => taxon2.name, "id" => taxon2.id) + response["state"].should eq("closed") end end end From ece0652ca3d3a95d9734446b948ff121625ec7d3 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 1 Aug 2019 20:46:05 +0100 Subject: [PATCH 05/10] Adapt spree/admin/shared/_routes.html.erb to new location of the api/taxons routes --- app/views/spree/admin/shared/_routes.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/shared/_routes.html.erb b/app/views/spree/admin/shared/_routes.html.erb index 056a49cf0c..b84c691ea9 100644 --- a/app/views/spree/admin/shared/_routes.html.erb +++ b/app/views/spree/admin/shared/_routes.html.erb @@ -1,7 +1,7 @@