From 0b17976256ee8eb9a4ccd15d5aabd024674e859c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Elvis=20Serr=C3=A3o?= <elvis.serrao1@gmail.com>
Date: Thu, 30 Jan 2025 14:35:30 -0300
Subject: [PATCH] Don't allow a subcategory to be assigned to another
 subcategory to ensure 1 level of nesting max (#1730)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Improve category level limit validation

* Set categories list only for non parents

* Disable select field

* Add info about the disabled select

* Don’t render a select input for parent categories

* Handle correctly turbo_stream request format

* Add turbo_stream format to requests on create and update action's tests

* Remove no_content status from update action

* Revert "Remove no_content status from update action"

This reverts commit 866140c196337d0f38ecf42fc3808fee3d502708.

* Revert "Add turbo_stream format to requests on create and update action's tests"

This reverts commit c6bf21490f79c657acbdda58c4cece7ba25d999a.

* Add correct redirect url for both html and turbo_stream formats

* Remove useless turbo_frame_tag
---
 app/controllers/categories_controller.rb | 26 +++++++++++++++++++-----
 app/models/category.rb                   |  7 ++++++-
 app/views/categories/_form.html.erb      |  6 ++++--
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index 03752869..a9d3d105 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -2,6 +2,7 @@ class CategoriesController < ApplicationController
   layout :with_sidebar
 
   before_action :set_category, only: %i[edit update destroy]
+  before_action :set_categories, only: %i[update edit]
   before_action :set_transaction, only: :create
 
   def index
@@ -10,7 +11,7 @@ class CategoriesController < ApplicationController
 
   def new
     @category = Current.family.categories.new color: Category::COLORS.sample
-    @categories = Current.family.categories.alphabetically.where(parent_id: nil).where.not(id: @category.id)
+    set_categories
   end
 
   def create
@@ -27,19 +28,26 @@ class CategoriesController < ApplicationController
         format.turbo_stream { render turbo_stream: turbo_stream.action(:redirect, redirect_target_url) }
       end
     else
-      @categories = Current.family.categories.alphabetically.where(parent_id: nil).where.not(id: @category.id)
+      set_categories
       render :new, status: :unprocessable_entity
     end
   end
 
   def edit
-    @categories = Current.family.categories.alphabetically.where(parent_id: nil).where.not(id: @category.id)
   end
 
   def update
-    @category.update! category_params
+    if @category.update(category_params)
+      flash[:notice] = t(".success")
 
-    redirect_back_or_to categories_path, notice: t(".success")
+      redirect_target_url = request.referer || categories_path
+      respond_to do |format|
+        format.html { redirect_back_or_to categories_path, notice: t(".success") }
+        format.turbo_stream { render turbo_stream: turbo_stream.action(:redirect, redirect_target_url) }
+      end
+    else
+      render :edit, status: :unprocessable_entity
+    end
   end
 
   def destroy
@@ -59,6 +67,14 @@ class CategoriesController < ApplicationController
       @category = Current.family.categories.find(params[:id])
     end
 
+    def set_categories
+      @categories = unless @category.parent?
+        Current.family.categories.alphabetically.roots.where.not(id: @category.id)
+      else
+        []
+      end
+    end
+
     def set_transaction
       if params[:transaction_id].present?
         @transaction = Current.family.transactions.find(params[:transaction_id])
diff --git a/app/models/category.rb b/app/models/category.rb
index ebeadbf9..d0ccaa60 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -15,6 +15,7 @@ class Category < ApplicationRecord
   validate :nested_category_matches_parent_classification
 
   scope :alphabetically, -> { order(:name) }
+  scope :roots, -> { where(parent_id: nil) }
   scope :incomes, -> { where(classification: "income") }
   scope :expenses, -> { where(classification: "expense") }
 
@@ -91,6 +92,10 @@ class Category < ApplicationRecord
     end
   end
 
+  def parent?
+    subcategories.any?
+  end
+
   def subcategory?
     parent.present?
   end
@@ -121,7 +126,7 @@ class Category < ApplicationRecord
 
   private
     def category_level_limit
-      if subcategory? && parent.subcategory?
+      if (subcategory? && parent.subcategory?) || (parent? && subcategory?)
         errors.add(:parent, "can't have more than 2 levels of subcategories")
       end
     end
diff --git a/app/views/categories/_form.html.erb b/app/views/categories/_form.html.erb
index 8927c0bf..884d5509 100644
--- a/app/views/categories/_form.html.erb
+++ b/app/views/categories/_form.html.erb
@@ -1,7 +1,7 @@
 <%# locals: (category:, categories:) %>
 
 <div data-controller="color-avatar">
-  <%= styled_form_with model: category, class: "space-y-4", data: { turbo_frame: :_top } do |f| %>
+  <%= styled_form_with model: category, class: "space-y-4" do |f| %>
     <section class="space-y-4">
       <div class="w-fit m-auto">
         <%= render partial: "shared/color_avatar", locals: { name: category.name, color: category.color } %>
@@ -34,7 +34,9 @@
       <div class="space-y-2">
         <%= f.select :classification, [["Income", "income"], ["Expense", "expense"]], { label: "Classification" }, required: true %>
         <%= f.text_field :name, placeholder: t(".placeholder"), required: true, autofocus: true, label: "Name", data: { color_avatar_target: "name" } %>
-        <%= f.select :parent_id, categories.pluck(:name, :id), { include_blank: "(unassigned)", label: "Parent category (optional)" } %>
+        <% unless category.parent? %>
+          <%= f.select :parent_id, categories.pluck(:name, :id), { include_blank: "(unassigned)", label: "Parent category (optional)" }, disabled: category.parent? %>
+        <% end %>
       </div>
     </section>