From d1081547ec21ac754030ca80812026dfee12dbea Mon Sep 17 00:00:00 2001 From: GermanDZ Date: Wed, 6 May 2026 22:59:55 +0200 Subject: [PATCH] feat(api): allow creating categories via API (#1676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(api): allow creating categories via API Adds POST /api/v1/categories so external integrations (e.g. bulk classification scripts that import already-categorized data from another system) can create categories without going through the web UI. Mirrors the existing tags create endpoint: requires the read_write scope, accepts name/color/icon/parent_id, auto-suggests an icon when omitted, and rejects parent_ids from other families. Also adds Minitest behavioural coverage, an rswag docs spec, a CategoryCreateRequest schema, and regenerates docs/api/openapi.yaml. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(api): address review feedback on POST /api/v1/categories - Re-raise ActionController::ParameterMissing in #create so the BaseController rescue_from handles it as a 400 instead of the generic 500 from the broad rescue inside the action. - Add a 403 'insufficient scope' response block to the rswag POST example so the generated OpenAPI documents read-only key rejection. - Switch the new create-action Minitest cases to API key auth via X-Api-Key + api_headers (using the existing api_keys fixtures), matching the project's API endpoint consistency rule. - Add Minitest coverage for two more 4xx paths: rejecting third-level nesting (parent_id pointing at a depth-2 subcategory) and rejecting requests without the category payload (400). Co-Authored-By: Claude Opus 4.7 (1M context) * refactor(test): migrate categories API index/show tests to X-Api-Key The pre-existing index and show tests in this file authenticated via Doorkeeper bearer tokens. Per the project's API endpoint consistency rule (CLAUDE.md, .cursor/rules/api-endpoint-consistency.mdc) Minitest controller tests under test/controllers/api/v1/ must use ApiKey + X-Api-Key auth. Drops the Doorkeeper application/access-token setup and routes every request through the existing api_keys fixtures and the api_headers helper, matching the create-action tests already in this file (and the pattern used in sync/users/family_settings tests). No behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(api): address second-round review on POST /api/v1/categories - Add a 400 response block to the POST rswag example so the generated OpenAPI documents the missing-category-payload contract that BaseController#handle_bad_request already returns. Regenerate docs/api/openapi.yaml. - Replace fixture-backed read_write_api_key / read_only_api_key helpers with explicit ApiKey.create! calls (matching the pattern in sync_controller_test, users_controller_test, and family_settings_controller_test). Setup now destroys active keys for the test user so the one-active-key-per-source validation does not collide with fixtures. Co-Authored-By: Claude Opus 4.7 (1M context) * test(api): tighten 422 create-category cases - Pass color and icon explicitly in the duplicate-name and third-level-nesting tests so each case is self-documenting about which validation it isolates (the model's color presence check is satisfied by the column default today, but reviewers — human and bot — flagged the implicit reliance). - Assert the JSON error envelope (error key + present message) on every 422 path so the response shape stays consistent and a regression in the rendered error body is caught uniformly. Co-Authored-By: Claude Opus 4.7 (1M context) * fix(api): tighten POST /api/v1/categories per review - Drop the no-op `rescue ActionController::ParameterMissing; raise` and the broad `rescue => e` from the create action. The BaseController already has rescue_from ActionController::ParameterMissing → 400, and unexpected exceptions are best left to Rails' default 500 handling (which logs identically). Keeps the action focused on its happy path and the two real error branches. - Stop accepting `lucide_icon` as a request key. The OpenAPI schema documents only `icon`; the dual permit was undocumented and pointless. `icon` is now the single canonical request key, mapped to `lucide_icon` on the model in category_params. - Migrate the Minitest helpers to the project's documented API key pattern: ApiKey.generate_secure_key + api_key.plain_key in the X-Api-Key header (matching the rswag spec in this PR and the rule in .cursor/rules/api-endpoint-consistency.mdc), instead of hand-built display_key strings. Co-Authored-By: Claude Opus 4.7 (1M context) * Botched conflict merge --------- Signed-off-by: Juan José Mata Co-authored-by: Claude Opus 4.7 (1M context) Co-authored-by: Juan José Mata Co-authored-by: Juan José Mata --- .../api/v1/categories_controller.rb | 38 +++- config/routes.rb | 6 +- docs/api/openapi.yaml | 71 ++++++ spec/requests/api/v1/categories_spec.rb | 87 +++++++ spec/swagger_helper.rb | 16 ++ .../api/v1/categories_controller_test.rb | 213 ++++++++++++++---- 6 files changed, 382 insertions(+), 49 deletions(-) diff --git a/app/controllers/api/v1/categories_controller.rb b/app/controllers/api/v1/categories_controller.rb index fa30d53d4..302b97225 100644 --- a/app/controllers/api/v1/categories_controller.rb +++ b/app/controllers/api/v1/categories_controller.rb @@ -3,7 +3,8 @@ class Api::V1::CategoriesController < Api::V1::BaseController include Pagy::Backend - before_action :ensure_read_scope + before_action :ensure_read_scope, only: %i[index show] + before_action :ensure_write_scope, only: :create before_action :set_category, only: :show def index @@ -45,6 +46,30 @@ class Api::V1::CategoriesController < Api::V1::BaseController }, status: :internal_server_error end + def create + family = current_resource_owner.family + attrs = category_params + + if attrs[:parent_id].present? && !family.categories.exists?(id: attrs[:parent_id]) + return render json: { + error: "unprocessable_entity", + message: "Parent must be a category in your family" + }, status: :unprocessable_entity + end + + @category = family.categories.new(attrs) + @category.lucide_icon = Category.suggested_icon(@category.name) if @category.lucide_icon.blank? + + if @category.save + render :show, status: :created + else + render json: { + error: "unprocessable_entity", + message: @category.errors.full_messages.join(", ") + }, status: :unprocessable_entity + end + end + private def set_category @@ -61,6 +86,17 @@ class Api::V1::CategoriesController < Api::V1::BaseController authorize_scope!(:read) end + def ensure_write_scope + authorize_scope!(:read_write) + end + + def category_params + permitted = params.require(:category).permit(:name, :color, :icon, :parent_id) + icon = permitted.delete(:icon) + permitted[:lucide_icon] = icon if icon.present? + permitted + end + def apply_filters(query) # Filter for root categories only (no parent) if params[:roots_only].present? && ActiveModel::Type::Boolean.new.cast(params[:roots_only]) diff --git a/config/routes.rb b/config/routes.rb index 847ed6133..9f8cd8973 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -428,13 +428,13 @@ Rails.application.routes.draw do resources :balances, only: [ :index, :show ] resources :budgets, only: [ :index, :show ] resources :budget_categories, only: [ :index, :show ] - resources :categories, only: [ :index, :show ] - resources :merchants, only: %i[index show] + resources :categories, only: [ :index, :show, :create ] + resources :merchants, only: [ :index, :show ] resources :rules, only: [ :index, :show ] resources :rule_runs, only: [ :index, :show ] resources :securities, only: [ :index, :show ] resources :security_prices, only: [ :index, :show ] - resources :tags, only: %i[index show create update destroy] + resources :tags, only: [ :index, :show, :create, :update, :destroy ] resources :transactions, only: [ :index, :show, :create, :update, :destroy ] resources :trades, only: [ :index, :show, :create, :update, :destroy ] diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 3ff0ffae3..52a24d3ba 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -942,6 +942,33 @@ components: "$ref": "#/components/schemas/CategoryDetail" pagination: "$ref": "#/components/schemas/Pagination" + CategoryCreateRequest: + type: object + required: + - category + properties: + category: + type: object + required: + - name + properties: + name: + type: string + description: Category name (required, unique within family) + color: + type: string + description: 'Hex color code (e.g. #22c55e). Defaults to #6172F3 if + omitted; subcategories inherit parent color.' + icon: + type: string + description: Lucide icon name (e.g. "coffee"). Auto-suggested from the + name when omitted. + parent_id: + type: string + format: uuid + nullable: true + description: Parent category ID. Must belong to the same family. Categories + support up to 2 levels of nesting. Merchant: type: object required: @@ -3411,6 +3438,50 @@ paths: application/json: schema: "$ref": "#/components/schemas/CategoryCollection" + post: + summary: Create category + tags: + - Categories + security: + - apiKeyAuth: [] + parameters: [] + responses: + '201': + description: subcategory created with parent + content: + application/json: + schema: + "$ref": "#/components/schemas/CategoryDetail" + '422': + description: validation error - duplicate name + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '403': + description: forbidden - api key missing read_write scope + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '400': + description: bad request - missing category payload + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + '401': + description: unauthorized - missing api key + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + requestBody: + content: + application/json: + schema: + "$ref": "#/components/schemas/CategoryCreateRequest" + required: true "/api/v1/categories/{id}": parameters: - name: id diff --git a/spec/requests/api/v1/categories_spec.rb b/spec/requests/api/v1/categories_spec.rb index 90e06d5c2..d74e76b56 100644 --- a/spec/requests/api/v1/categories_spec.rb +++ b/spec/requests/api/v1/categories_spec.rb @@ -96,6 +96,93 @@ RSpec.describe 'API V1 Categories', type: :request do run_test! end end + + post 'Create category' do + tags 'Categories' + security [ { apiKeyAuth: [] } ] + consumes 'application/json' + produces 'application/json' + parameter name: :body, in: :body, required: true, schema: { + '$ref' => '#/components/schemas/CategoryCreateRequest' + } + + response '201', 'category created' do + schema '$ref' => '#/components/schemas/CategoryDetail' + + let(:body) do + { + category: { + name: 'Imported / Coffee', + color: '#22c55e', + icon: 'coffee' + } + } + end + + run_test! + end + + response '201', 'subcategory created with parent' do + schema '$ref' => '#/components/schemas/CategoryDetail' + + let(:body) do + { + category: { + name: 'Imported / Espresso', + color: '#22c55e', + icon: 'coffee', + parent_id: parent_category.id + } + } + end + + run_test! + end + + response '422', 'validation error - duplicate name' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:body) { { category: { name: parent_category.name } } } + + run_test! + end + + response '403', 'forbidden - api key missing read_write scope' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:read_only_api_key) do + key = ApiKey.generate_secure_key + ApiKey.create!( + user: user, + name: 'API Docs Read Key', + key: key, + scopes: %w[read], + source: 'mobile' + ) + end + let(:'X-Api-Key') { read_only_api_key.plain_key } + let(:body) { { category: { name: 'Anything' } } } + + run_test! + end + + response '400', 'bad request - missing category payload' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:body) { {} } + + run_test! + end + + response '401', 'unauthorized - missing api key' do + schema '$ref' => '#/components/schemas/ErrorResponse' + + let(:'X-Api-Key') { nil } + let(:body) { { category: { name: 'Anything' } } } + + run_test! + end + end end path '/api/v1/categories/{id}' do diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index f3e9b07cf..886c0cf24 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -535,6 +535,22 @@ RSpec.configure do |config| pagination: { '$ref' => '#/components/schemas/Pagination' } } }, + CategoryCreateRequest: { + type: :object, + required: %w[category], + properties: { + category: { + type: :object, + required: %w[name], + properties: { + name: { type: :string, description: 'Category name (required, unique within family)' }, + color: { type: :string, description: 'Hex color code (e.g. #22c55e). Defaults to #6172F3 if omitted; subcategories inherit parent color.' }, + icon: { type: :string, description: 'Lucide icon name (e.g. "coffee"). Auto-suggested from the name when omitted.' }, + parent_id: { type: :string, format: :uuid, nullable: true, description: 'Parent category ID. Must belong to the same family. Categories support up to 2 levels of nesting.' } + } + } + } + }, Merchant: { type: :object, required: %w[id name], diff --git a/test/controllers/api/v1/categories_controller_test.rb b/test/controllers/api/v1/categories_controller_test.rb index c60fcacc1..12d30fef8 100644 --- a/test/controllers/api/v1/categories_controller_test.rb +++ b/test/controllers/api/v1/categories_controller_test.rb @@ -8,17 +8,10 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest @other_family_user = users(:family_member) @other_family_user.update!(family: families(:empty)) - @oauth_app = Doorkeeper::Application.create!( - name: "Test API App", - redirect_uri: "https://example.com/callback", - scopes: "read read_write" - ) - - @access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @user.id, - scopes: "read" - ) + # Fixtures pre-create active keys for family_admin; clear them so we can + # create scoped keys per-test without tripping the one-active-key-per-source + # validation. + @user.api_keys.active.destroy_all @category = categories(:food_and_drink) @subcategory = categories(:subcategory) @@ -35,9 +28,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should return user's family categories successfully" do - get "/api/v1/categories", params: {}, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories", params: {}, headers: api_headers(read_only_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -53,15 +44,15 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should not return other family's categories" do - access_token = Doorkeeper::AccessToken.create!( - application: @oauth_app, - resource_owner_id: @other_family_user.id, - scopes: "read" + other_family_api_key = ApiKey.create!( + user: @other_family_user, + name: "Other Family Read Key", + key: ApiKey.generate_secure_key, + scopes: %w[read], + source: "web" ) - get "/api/v1/categories", params: {}, headers: { - "Authorization" => "Bearer #{access_token.token}" - } + get "/api/v1/categories", params: {}, headers: api_headers(other_family_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -72,9 +63,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should return proper category data structure" do - get "/api/v1/categories", params: {}, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories", params: {}, headers: api_headers(read_only_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -96,9 +85,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should include parent information for subcategories" do - get "/api/v1/categories", params: {}, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories", params: {}, headers: api_headers(read_only_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -112,9 +99,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should handle pagination parameters" do - get "/api/v1/categories", params: { page: 1, per_page: 2 }, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories", params: { page: 1, per_page: 2 }, headers: api_headers(read_only_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -125,9 +110,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should filter for roots only" do - get "/api/v1/categories", params: { roots_only: true }, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories", params: { roots_only: true }, headers: api_headers(read_only_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -138,9 +121,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should sort categories alphabetically" do - get "/api/v1/categories", params: {}, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories", params: {}, headers: api_headers(read_only_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -152,9 +133,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest # Show action tests test "should return a single category" do - get "/api/v1/categories/#{@category.id}", params: {}, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories/#{@category.id}", params: {}, headers: api_headers(read_only_api_key) assert_response :success response_body = JSON.parse(response.body) @@ -166,9 +145,7 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest end test "should return 404 for non-existent category" do - get "/api/v1/categories/00000000-0000-0000-0000-000000000000", params: {}, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories/00000000-0000-0000-0000-000000000000", params: {}, headers: api_headers(read_only_api_key) assert_response :not_found response_body = JSON.parse(response.body) @@ -182,10 +159,156 @@ class Api::V1::CategoriesControllerTest < ActionDispatch::IntegrationTest classification_unused: "expense" ) - get "/api/v1/categories/#{other_family_category.id}", params: {}, headers: { - "Authorization" => "Bearer #{@access_token.token}" - } + get "/api/v1/categories/#{other_family_category.id}", params: {}, headers: api_headers(read_only_api_key) assert_response :not_found end + + # Create action tests + + test "create requires authentication" do + post "/api/v1/categories", params: { category: { name: "Anything" } } + assert_response :unauthorized + end + + test "create rejects api key without read_write scope" do + post "/api/v1/categories", + params: { category: { name: "Coffee Runs", color: "#22c55e", icon: "coffee" } }, + headers: api_headers(read_only_api_key) + + assert_response :forbidden + end + + test "create returns 201 with full attributes" do + post "/api/v1/categories", + params: { category: { name: "Coffee Runs", color: "#22c55e", icon: "coffee" } }, + headers: api_headers(read_write_api_key) + + assert_response :created + body = JSON.parse(response.body) + assert body["id"].present? + assert_equal "Coffee Runs", body["name"] + assert_equal "#22c55e", body["color"] + assert_equal "coffee", body["icon"] + assert_nil body["parent"] + assert_equal 0, body["subcategories_count"] + + persisted = @user.family.categories.find(body["id"]) + assert_equal "coffee", persisted.lucide_icon + end + + test "create auto-suggests icon when omitted" do + post "/api/v1/categories", + params: { category: { name: "Groceries Imported", color: "#407706" } }, + headers: api_headers(read_write_api_key) + + assert_response :created + body = JSON.parse(response.body) + assert body["icon"].present? + assert_not_equal "", body["icon"] + end + + test "create attaches parent when provided" do + post "/api/v1/categories", + params: { category: { name: "Imported Subcategory", color: "#22c55e", icon: "shapes", parent_id: @category.id } }, + headers: api_headers(read_write_api_key) + + assert_response :created + body = JSON.parse(response.body) + assert_equal @category.id, body.dig("parent", "id") + assert_equal @category.name, body.dig("parent", "name") + end + + test "create returns 422 on duplicate name within family" do + post "/api/v1/categories", + params: { category: { name: @category.name, color: "#22c55e", icon: "shapes" } }, + headers: api_headers(read_write_api_key) + + assert_response :unprocessable_entity + body = JSON.parse(response.body) + assert_equal "unprocessable_entity", body["error"] + assert body["message"].present? + end + + test "create returns 422 on invalid color" do + post "/api/v1/categories", + params: { category: { name: "Bad Color", color: "not-a-hex" } }, + headers: api_headers(read_write_api_key) + + assert_response :unprocessable_entity + body = JSON.parse(response.body) + assert_equal "unprocessable_entity", body["error"] + assert body["message"].present? + end + + test "create returns 422 when parent_id belongs to another family" do + other_family_category = families(:empty).categories.create!( + name: "External Parent", + color: "#FF0000", + classification_unused: "expense" + ) + + post "/api/v1/categories", + params: { category: { name: "Should Fail", color: "#22c55e", icon: "shapes", parent_id: other_family_category.id } }, + headers: api_headers(read_write_api_key) + + assert_response :unprocessable_entity + body = JSON.parse(response.body) + assert_equal "unprocessable_entity", body["error"] + assert body["message"].present? + end + + test "create returns 422 when nesting exceeds two levels" do + child = @user.family.categories.create!( + name: "Existing Child", + color: "#22c55e", + lucide_icon: "shapes", + parent: @category + ) + + post "/api/v1/categories", + params: { category: { name: "Grandchild", color: "#22c55e", icon: "shapes", parent_id: child.id } }, + headers: api_headers(read_write_api_key) + + assert_response :unprocessable_entity + body = JSON.parse(response.body) + assert_equal "unprocessable_entity", body["error"] + assert body["message"].present? + end + + test "create returns 400 when category payload is missing" do + post "/api/v1/categories", + params: {}, + headers: api_headers(read_write_api_key) + + assert_response :bad_request + body = JSON.parse(response.body) + assert_equal "bad_request", body["error"] + end + + private + + def read_write_api_key + @read_write_api_key ||= ApiKey.create!( + user: @user, + name: "Test RW Key", + key: ApiKey.generate_secure_key, + scopes: %w[read_write], + source: "web" + ) + end + + def read_only_api_key + @read_only_api_key ||= ApiKey.create!( + user: @user, + name: "Test RO Key", + key: ApiKey.generate_secure_key, + scopes: %w[read], + source: "mobile" + ) + end + + def api_headers(api_key) + { "X-Api-Key" => api_key.plain_key } + end end