From c68563a281b64e00affd8b56e1d06528697a6e4e Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Wed, 12 Aug 2020 12:28:29 -0600
Subject: [PATCH] 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
---
 app/controllers/categories_controller.rb    | 18 ++++++++----
 spec/requests/categories_controller_spec.rb | 31 ---------------------
 2 files changed, 12 insertions(+), 37 deletions(-)

diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index a0cbe54bb82..32ad2df84d1 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -125,7 +125,7 @@ class CategoriesController < ApplicationController
 
     @category =
       begin
-        Category.new(category_params.merge(user: current_user))
+        Category.new(required_create_params.merge(user: current_user))
       rescue ArgumentError => e
         return render json: { errors: [e.message] }, status: 422
       end
@@ -266,15 +266,18 @@ class CategoriesController < ApplicationController
   end
 
   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
 
   def category_params
     @category_params ||= begin
-      required_param_keys.each do |key|
-        params.require(key)
-      end
-
       if p = params[:permissions]
         p.each do |k, v|
           p[k] = v.to_i
@@ -290,6 +293,9 @@ class CategoriesController < ApplicationController
       result = params.permit(
         *required_param_keys,
         :position,
+        :name,
+        :color,
+        :text_color,
         :email_in,
         :email_in_allow_strangers,
         :mailinglist_mirror,
diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb
index 6a605770c89..65cd0a7dbed 100644
--- a/spec/requests/categories_controller_spec.rb
+++ b/spec/requests/categories_controller_spec.rb
@@ -120,16 +120,6 @@ describe CategoriesController do
         expect(response.status).to eq(400)
       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
         it "returns errors on a duplicate category name" do
           category = Fabricate(:category, user: admin)
@@ -314,27 +304,6 @@ describe CategoriesController do
         expect(response).to be_forbidden
       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
         other_category = Fabricate(:category, name: "Other", user: admin)
         put "/categories/#{category.id}.json", params: {