DEV: Improve API usage when creating * updating categories

The category model already has a default value for `color` and
`text_color` so they don't need to be required via the API. The ember UI
already requires that colors be selected.

The name of the category also doesn't need to be required when updating
the category either because we are already passing in the id for the
category we want to change.

These changes improve the api experience because you no longer have to
lookup the category name, color, or text color before updating a single
category attribute. When creating a category the name is still required.

https://meta.discourse.org/t/-/132424/2
This commit is contained in:
Blake Erickson 2020-08-12 12:28:29 -06:00
parent 70d4420c8e
commit c68563a281
2 changed files with 12 additions and 37 deletions

View File

@ -125,7 +125,7 @@ class CategoriesController < ApplicationController
@category = @category =
begin begin
Category.new(category_params.merge(user: current_user)) Category.new(required_create_params.merge(user: current_user))
rescue ArgumentError => e rescue ArgumentError => e
return render json: { errors: [e.message] }, status: 422 return render json: { errors: [e.message] }, status: 422
end end
@ -266,15 +266,18 @@ class CategoriesController < ApplicationController
end end
def required_param_keys def required_param_keys
[:name, :color, :text_color] [:name]
end
def required_create_params
required_param_keys.each do |key|
params.require(key)
end
category_params
end end
def category_params def category_params
@category_params ||= begin @category_params ||= begin
required_param_keys.each do |key|
params.require(key)
end
if p = params[:permissions] if p = params[:permissions]
p.each do |k, v| p.each do |k, v|
p[k] = v.to_i p[k] = v.to_i
@ -290,6 +293,9 @@ class CategoriesController < ApplicationController
result = params.permit( result = params.permit(
*required_param_keys, *required_param_keys,
:position, :position,
:name,
:color,
:text_color,
:email_in, :email_in,
:email_in_allow_strangers, :email_in_allow_strangers,
:mailinglist_mirror, :mailinglist_mirror,

View File

@ -120,16 +120,6 @@ describe CategoriesController do
expect(response.status).to eq(400) expect(response.status).to eq(400)
end end
it "raises an exception when the color is missing" do
post "/categories.json", params: { name: "hello", text_color: "fff" }
expect(response.status).to eq(400)
end
it "raises an exception when the text color is missing" do
post "/categories.json", params: { name: "hello", color: "ff0" }
expect(response.status).to eq(400)
end
describe "failure" do describe "failure" do
it "returns errors on a duplicate category name" do it "returns errors on a duplicate category name" do
category = Fabricate(:category, user: admin) category = Fabricate(:category, user: admin)
@ -314,27 +304,6 @@ describe CategoriesController do
expect(response).to be_forbidden expect(response).to be_forbidden
end end
it "requires a name" do
put "/categories/#{category.slug}.json", params: {
color: 'fff',
text_color: '0ff',
}
expect(response.status).to eq(400)
end
it "requires a color" do
put "/categories/#{category.slug}.json", params: {
name: 'asdf',
text_color: '0ff',
}
expect(response.status).to eq(400)
end
it "requires a text color" do
put "/categories/#{category.slug}.json", params: { name: 'asdf', color: 'fff' }
expect(response.status).to eq(400)
end
it "returns errors on a duplicate category name" do it "returns errors on a duplicate category name" do
other_category = Fabricate(:category, name: "Other", user: admin) other_category = Fabricate(:category, name: "Other", user: admin)
put "/categories/#{category.id}.json", params: { put "/categories/#{category.id}.json", params: {