From 5ae60ba9ab856959e7a4474f672210c95d558542 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Wed, 30 Jan 2019 17:43:43 -0500 Subject: [PATCH 1/2] Remove references to Category When we reference category, we are really talking about a classification --- app/controllers/api/categories_controller.rb | 4 ++-- app/controllers/api/tags_controller.rb | 2 +- spec/requests/categories_spec.rb | 17 +++++++------- spec/requests/collections_spec.rb | 4 ++-- spec/requests/tags_spec.rb | 24 ++++++++++---------- 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/app/controllers/api/categories_controller.rb b/app/controllers/api/categories_controller.rb index 17b7cf7d94..5e3b868184 100644 --- a/app/controllers/api/categories_controller.rb +++ b/app/controllers/api/categories_controller.rb @@ -5,12 +5,12 @@ class CategoriesController < BaseController before_action :set_additional_attributes, :only => [:index, :show, :update] def edit_resource(type, id, data = {}) - raise ForbiddenError if Category.find(id).read_only? + raise ForbiddenError if Classification.is_category.find(id).read_only? super end def delete_resource(type, id, data = {}) - raise ForbiddenError if Category.find(id).read_only? + raise ForbiddenError if Classification.is_category.find(id).read_only? super end diff --git a/app/controllers/api/tags_controller.rb b/app/controllers/api/tags_controller.rb index 19738fdd94..0b7f12b3d3 100644 --- a/app/controllers/api/tags_controller.rb +++ b/app/controllers/api/tags_controller.rb @@ -47,7 +47,7 @@ def fetch_category(data) unless category_id raise BadRequestError, "Category id, href or name needs to be specified for creating a new tag resource" end - Category.find_by(:id => category_id) + Classification.is_category.find_by(:id => category_id) end def destroy_tag_and_classification(tag_id) diff --git a/spec/requests/categories_spec.rb b/spec/requests/categories_spec.rb index cb16d48199..0c4b23f368 100644 --- a/spec/requests/categories_spec.rb +++ b/spec/requests/categories_spec.rb @@ -1,6 +1,7 @@ RSpec.describe "categories API" do it "can list all the categories" do categories = FactoryBot.create_list(:category, 2) + FactoryBot.create(:classification_tag, :parent => categories.first) api_basic_authorize collection_action_identifier(:categories, :read, :get) get api_categories_url @@ -78,7 +79,7 @@ expect do post api_categories_url, :params => { :name => "test", :description => "Test" } - end.to change(Category, :count).by(1) + end.to change(Classification, :count).by(1) expect(response).to have_http_status(:ok) end @@ -107,7 +108,7 @@ api_basic_authorize collection_action_identifier(:categories, :create) post api_categories_url, :params => { :name => "test", :description => "Test" } - category = Category.find(response.parsed_body["results"].first["id"]) + category = Classification.is_category.find(response.parsed_body["results"].first["id"]) expect(category.tag.name).to eq("/managed/test") end @@ -130,7 +131,7 @@ expect do post api_category_url(nil, category), :params => gen_request(:delete) - end.to change(Category, :count).by(-1) + end.to change(Classification, :count).by(-1) expect(response).to have_http_status(:ok) end @@ -141,7 +142,7 @@ expect do delete api_category_url(nil, category) - end.to change(Category, :count).by(-1) + end.to change(Classification, :count).by(-1) expect(response).to have_http_status(:no_content) end @@ -153,7 +154,7 @@ expect do post api_category_url(nil, category), :params => gen_request(:delete) - end.not_to change(Category, :count) + end.not_to change(Classification, :count) expect(response).to have_http_status(:forbidden) end @@ -176,7 +177,7 @@ expect do post api_categories_url, :params => { :name => "test", :description => "Test" } - end.not_to change(Category, :count) + end.not_to change(Classification, :count) expect(response).to have_http_status(:forbidden) end @@ -198,7 +199,7 @@ expect do post api_category_url(nil, category), :params => gen_request(:delete) - end.not_to change(Category, :count) + end.not_to change(Classification, :count) expect(response).to have_http_status(:forbidden) end @@ -209,7 +210,7 @@ expect do delete api_category_url(nil, category) - end.not_to change(Category, :count) + end.not_to change(Classification, :count) expect(response).to have_http_status(:forbidden) end diff --git a/spec/requests/collections_spec.rb b/spec/requests/collections_spec.rb index 5d5c1b5793..d425ea298d 100644 --- a/spec/requests/collections_spec.rb +++ b/spec/requests/collections_spec.rb @@ -74,7 +74,7 @@ def test_collection_bulk_query(collection, collection_url, klass, id = nil) it "query Categories" do FactoryBot.create(:category) - test_collection_query(:categories, api_categories_url, Category) + test_collection_query(:categories, api_categories_url, Classification.is_category) end it "query Chargebacks" do @@ -412,7 +412,7 @@ def test_collection_bulk_query(collection, collection_url, klass, id = nil) it "bulk query Categories" do FactoryBot.create(:category) - test_collection_bulk_query(:categories, api_categories_url, Category) + test_collection_bulk_query(:categories, api_categories_url, Classification.is_category) end it "bulk query Chargebacks" do diff --git a/spec/requests/tags_spec.rb b/spec/requests/tags_spec.rb index 89f50f2a0b..c02ac7201d 100644 --- a/spec/requests/tags_spec.rb +++ b/spec/requests/tags_spec.rb @@ -23,14 +23,14 @@ context "with an appropriate role" do it "can create a tag with category by href" do api_basic_authorize collection_action_identifier(:tags, :create) - category = FactoryBot.create(:category) + category = FactoryBot.create(:classification) options = {:name => "test_tag", :description => "Test Tag", :category => {:href => api_category_url(nil, category)}} expect { post api_tags_url, :params => options }.to change(Tag, :count).by(1) result = response.parsed_body["results"].first tag = Tag.find(result["id"]) - tag_category = Category.find(tag.category.id) + tag_category = tag.category expect(tag_category).to eq(category) expect(result["href"]).to include(api_tag_url(nil, tag)) expect(response).to have_http_status(:ok) @@ -38,14 +38,14 @@ it "can create a tag with a category by id" do api_basic_authorize collection_action_identifier(:tags, :create) - category = FactoryBot.create(:category) + category = FactoryBot.create(:classification) expect do post api_tags_url, :params => { :name => "test_tag", :description => "Test Tag", :category => {:id => category.id} } end.to change(Tag, :count).by(1) tag = Tag.find(response.parsed_body["results"].first["id"]) - tag_category = Category.find(tag.category.id) + tag_category = tag.category expect(tag_category).to eq(category) expect(response).to have_http_status(:ok) @@ -53,14 +53,14 @@ it "can create a tag with a category by name" do api_basic_authorize collection_action_identifier(:tags, :create) - category = FactoryBot.create(:category) + category = FactoryBot.create(:classification) expect do post api_tags_url, :params => { :name => "test_tag", :description => "Test Tag", :category => {:name => category.name} } end.to change(Tag, :count).by(1) tag = Tag.find(response.parsed_body["results"].first["id"]) - tag_category = Category.find(tag.category.id) + tag_category = tag.category expect(tag_category).to eq(category) expect(response).to have_http_status(:ok) @@ -68,13 +68,13 @@ it "can create a tag as a subresource of a category" do api_basic_authorize collection_action_identifier(:tags, :create) - category = FactoryBot.create(:category) + category = FactoryBot.create(:classification) expect do post(api_category_tags_url(nil, category), :params => { :name => "test_tag", :description => "Test Tag" }) end.to change(Tag, :count).by(1) tag = Tag.find(response.parsed_body["results"].first["id"]) - tag_category = Category.find(tag.category.id) + tag_category = tag.category expect(tag_category).to eq(category) expect(response).to have_http_status(:ok) @@ -91,7 +91,7 @@ it "can update a tag's name" do api_basic_authorize action_identifier(:tags, :edit) classification = FactoryBot.create(:classification_tag) - category = FactoryBot.create(:category, :children => [classification]) + category = FactoryBot.create(:classification, :children => [classification]) tag = classification.tag expect do @@ -104,7 +104,7 @@ it "can update a tag's description" do api_basic_authorize action_identifier(:tags, :edit) classification = FactoryBot.create(:classification_tag) - FactoryBot.create(:category, :children => [classification]) + FactoryBot.create(:classification, :children => [classification]) tag = classification.tag expect do @@ -160,7 +160,7 @@ api_basic_authorize action_identifier(:tags, :delete) classification1 = FactoryBot.create(:classification_tag) classification2 = FactoryBot.create(:classification_tag) - category = FactoryBot.create(:category, :children => [classification1, classification2]) + category = FactoryBot.create(:classification, :children => [classification1, classification2]) tag1 = classification1.tag tag2 = classification2.tag @@ -183,7 +183,7 @@ api_basic_authorize action_identifier(:tags, :delete) classification1 = FactoryBot.create(:classification_tag) classification2 = FactoryBot.create(:classification_tag) - category = FactoryBot.create(:category, :children => [classification1, classification2]) + category = FactoryBot.create(:classification, :children => [classification1, classification2]) tag1 = classification1.tag tag2 = classification2.tag body = gen_request(:delete, [{:name => tag1.name}, {:name => tag2.name}]) From ae15101bc7f4258926a6c1accf42672c50a8aaee Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Fri, 1 Feb 2019 11:53:11 -0500 Subject: [PATCH 2/2] Use classification instead of category for api WIP: This causes some test failures. I'm wondering if we created Category to get around limitations in the api --- config/api.yml | 2 +- spec/requests/collections_spec.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/config/api.yml b/config/api.yml index 5df387fddc..1ee1d665b6 100644 --- a/config/api.yml +++ b/config/api.yml @@ -346,7 +346,7 @@ :options: - :collection :verbs: *gpd - :klass: Category + :klass: Classification :subcollections: - :tags :collection_actions: diff --git a/spec/requests/collections_spec.rb b/spec/requests/collections_spec.rb index d425ea298d..96a01811e1 100644 --- a/spec/requests/collections_spec.rb +++ b/spec/requests/collections_spec.rb @@ -73,7 +73,8 @@ def test_collection_bulk_query(collection, collection_url, klass, id = nil) end it "query Categories" do - FactoryBot.create(:category) + category = FactoryBot.create(:category) + FactoryBot.create(:classification_tag, :parent => category) test_collection_query(:categories, api_categories_url, Classification.is_category) end @@ -411,7 +412,8 @@ def test_collection_bulk_query(collection, collection_url, klass, id = nil) end it "bulk query Categories" do - FactoryBot.create(:category) + category = FactoryBot.create(:category) + FactoryBot.create(:classification_tag, :parent => category) # should not come back test_collection_bulk_query(:categories, api_categories_url, Classification.is_category) end