From 77def1db404457c32e46797e68a420cc53819216 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 20 Dec 2024 11:37:26 -0500 Subject: [PATCH] Nested Categories (#1561) * Prepare entry search for nested categories * Subcategory implementation * Remove caching for test stability --- app/controllers/categories_controller.rb | 17 +++- app/controllers/registrations_controller.rb | 1 - app/controllers/transactions_controller.rb | 10 +-- app/models/account/entry.rb | 46 +--------- app/models/account/entry_search.rb | 59 +++++++++++++ app/models/account/trade.rb | 20 +---- app/models/account/transaction.rb | 47 +--------- app/models/account/transaction_search.rb | 42 +++++++++ app/models/account/valuation.rb | 10 --- app/models/category.rb | 86 +++++++++++++------ app/models/demo/generator.rb | 5 ++ app/views/account/trades/_header.html.erb | 4 +- app/views/categories/_badge.html.erb | 2 +- app/views/categories/_category.html.erb | 6 +- app/views/categories/_form.html.erb | 13 ++- app/views/categories/_menu.html.erb | 2 +- app/views/categories/edit.html.erb | 2 +- app/views/categories/index.html.erb | 28 ++++-- app/views/categories/new.html.erb | 2 +- app/views/category/dropdowns/_row.html.erb | 3 + app/views/category/dropdowns/show.html.erb | 17 +++- config/locales/models/transaction/en.yml | 12 --- config/locales/views/categories/en.yml | 13 ++- config/routes.rb | 2 + .../20241219174803_add_parent_category.rb | 6 ++ db/schema.rb | 6 +- .../controllers/categories_controller_test.rb | 14 ++- .../registrations_controller_test.rb | 9 -- test/fixtures/categories.yml | 7 +- test/models/account/entry_test.rb | 4 - test/models/category_test.rb | 38 +++----- 31 files changed, 298 insertions(+), 235 deletions(-) create mode 100644 app/models/account/entry_search.rb create mode 100644 app/models/account/transaction_search.rb delete mode 100644 config/locales/models/transaction/en.yml create mode 100644 db/migrate/20241219174803_add_parent_category.rb diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index ddc1ecaae96..2d1882f44ae 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -10,6 +10,7 @@ def index 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) end def create @@ -17,19 +18,21 @@ def create if @category.save @transaction.update(category_id: @category.id) if @transaction - redirect_back_or_to transactions_path, notice: t(".success") + + redirect_back_or_to categories_path, notice: t(".success") else - redirect_back_or_to transactions_path, alert: t(".failure", error: @category.errors.full_messages.to_sentence) + 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 - redirect_back_or_to transactions_path, notice: t(".success") + redirect_back_or_to categories_path, notice: t(".success") end def destroy @@ -38,6 +41,12 @@ def destroy redirect_back_or_to categories_path, notice: t(".success") end + def bootstrap + Current.family.categories.bootstrap_defaults + + redirect_back_or_to categories_path, notice: t(".success") + end + private def set_category @category = Current.family.categories.find(params[:id]) @@ -50,6 +59,6 @@ def set_transaction end def category_params - params.require(:category).permit(:name, :color) + params.require(:category).permit(:name, :color, :parent_id) end end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 128b309cb81..0d8d6e92674 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -24,7 +24,6 @@ def create if @user.save @invitation&.update!(accepted_at: Time.current) - Category.create_default_categories(@user.family) unless @invitation @session = create_session_for(@user) redirect_to root_path, notice: t(".success") else diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index acceab79dc4..664c308040d 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -3,13 +3,13 @@ class TransactionsController < ApplicationController def index @q = search_params - result = Current.family.entries.account_transactions.search(@q).reverse_chronological - @pagy, @transaction_entries = pagy(result, limit: params[:per_page] || "50") + search_query = Current.family.transactions.search(@q).includes(:entryable).reverse_chronological + @pagy, @transaction_entries = pagy(search_query, limit: params[:per_page] || "50") @totals = { - count: result.select { |t| t.currency == Current.family.currency }.count, - income: result.income_total(Current.family.currency).abs, - expense: result.expense_total(Current.family.currency) + count: search_query.select { |t| t.currency == Current.family.currency }.count, + income: search_query.income_total(Current.family.currency).abs, + expense: search_query.expense_total(Current.family.currency) } end diff --git a/app/models/account/entry.rb b/app/models/account/entry.rb index 4d7334fbb75..8d6a8f402f1 100644 --- a/app/models/account/entry.rb +++ b/app/models/account/entry.rb @@ -60,6 +60,10 @@ def display_name end class << self + def search(params) + Account::EntrySearch.new(params).build_query(all) + end + # arbitrary cutoff date to avoid expensive sync operations def min_supported_date 30.years.ago.to_date @@ -141,49 +145,7 @@ def expense_total(currency = "USD") Money.new(total, currency) end - def search(params) - query = all - query = query.where("account_entries.name ILIKE ?", "%#{sanitize_sql_like(params[:search])}%") if params[:search].present? - query = query.where("account_entries.date >= ?", params[:start_date]) if params[:start_date].present? - query = query.where("account_entries.date <= ?", params[:end_date]) if params[:end_date].present? - - if params[:types].present? - query = query.where(marked_as_transfer: false) unless params[:types].include?("transfer") - - if params[:types].include?("income") && !params[:types].include?("expense") - query = query.where("account_entries.amount < 0") - elsif params[:types].include?("expense") && !params[:types].include?("income") - query = query.where("account_entries.amount >= 0") - end - end - - if params[:amount].present? && params[:amount_operator].present? - case params[:amount_operator] - when "equal" - query = query.where("ABS(ABS(account_entries.amount) - ?) <= 0.01", params[:amount].to_f.abs) - when "less" - query = query.where("ABS(account_entries.amount) < ?", params[:amount].to_f.abs) - when "greater" - query = query.where("ABS(account_entries.amount) > ?", params[:amount].to_f.abs) - end - end - - if params[:accounts].present? || params[:account_ids].present? - query = query.joins(:account) - end - - query = query.where(accounts: { name: params[:accounts] }) if params[:accounts].present? - query = query.where(accounts: { id: params[:account_ids] }) if params[:account_ids].present? - - # Search attributes on each entryable to further refine results - entryable_ids = entryable_search(params) - query = query.where(entryable_id: entryable_ids) unless entryable_ids.nil? - - query - end - private - def entryable_search(params) entryable_ids = [] entryable_search_performed = false diff --git a/app/models/account/entry_search.rb b/app/models/account/entry_search.rb new file mode 100644 index 00000000000..c561765b557 --- /dev/null +++ b/app/models/account/entry_search.rb @@ -0,0 +1,59 @@ +class Account::EntrySearch + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :search, :string + attribute :amount, :string + attribute :amount_operator, :string + attribute :types, :string + attribute :accounts, :string + attribute :account_ids, :string + attribute :start_date, :string + attribute :end_date, :string + + class << self + def from_entryable_search(entryable_search) + new(entryable_search.attributes.slice(*attribute_names)) + end + end + + def build_query(scope) + query = scope + + query = query.where("account_entries.name ILIKE :search OR account_entries.enriched_name ILIKE :search", + search: "%#{ActiveRecord::Base.sanitize_sql_like(search)}%" + ) if search.present? + query = query.where("account_entries.date >= ?", start_date) if start_date.present? + query = query.where("account_entries.date <= ?", end_date) if end_date.present? + + if types.present? + query = query.where(marked_as_transfer: false) unless types.include?("transfer") + + if types.include?("income") && !types.include?("expense") + query = query.where("account_entries.amount < 0") + elsif types.include?("expense") && !types.include?("income") + query = query.where("account_entries.amount >= 0") + end + end + + if amount.present? && amount_operator.present? + case amount_operator + when "equal" + query = query.where("ABS(ABS(account_entries.amount) - ?) <= 0.01", amount.to_f.abs) + when "less" + query = query.where("ABS(account_entries.amount) < ?", amount.to_f.abs) + when "greater" + query = query.where("ABS(account_entries.amount) > ?", amount.to_f.abs) + end + end + + if accounts.present? || account_ids.present? + query = query.joins(:account) + end + + query = query.where(accounts: { name: accounts }) if accounts.present? + query = query.where(accounts: { id: account_ids }) if account_ids.present? + + query + end +end diff --git a/app/models/account/trade.rb b/app/models/account/trade.rb index 70b0c8f32c2..7d4976ba162 100644 --- a/app/models/account/trade.rb +++ b/app/models/account/trade.rb @@ -8,26 +8,8 @@ class Account::Trade < ApplicationRecord validates :qty, presence: true validates :price, :currency, presence: true - class << self - def search(_params) - all - end - - def requires_search?(_params) - false - end - end - - def sell? - qty < 0 - end - - def buy? - qty > 0 - end - def unrealized_gain_loss - return nil if sell? + return nil if qty.negative? current_price = security.current_price return nil if current_price.nil? diff --git a/app/models/account/transaction.rb b/app/models/account/transaction.rb index 6b8f4995ee5..afe5a568227 100644 --- a/app/models/account/transaction.rb +++ b/app/models/account/transaction.rb @@ -12,52 +12,7 @@ class Account::Transaction < ApplicationRecord class << self def search(params) - query = all - if params[:categories].present? - if params[:categories].exclude?("Uncategorized") - query = query - .joins(:category) - .where(categories: { name: params[:categories] }) - else - query = query - .left_joins(:category) - .where(categories: { name: params[:categories] }) - .or(query.where(category_id: nil)) - end - end - - query = query.joins(:merchant).where(merchants: { name: params[:merchants] }) if params[:merchants].present? - - if params[:tags].present? - query = query.joins(:tags) - .where(tags: { name: params[:tags] }) - .distinct - end - - query + Account::TransactionSearch.new(params).build_query(all) end - - def requires_search?(params) - searchable_keys.any? { |key| params.key?(key) } - end - - private - - def searchable_keys - %i[categories merchants tags] - end end - - def eod_balance - entry.amount_money - end - - private - def account - entry.account - end - - def daily_transactions - account.entries.account_transactions - end end diff --git a/app/models/account/transaction_search.rb b/app/models/account/transaction_search.rb new file mode 100644 index 00000000000..f61fae6906b --- /dev/null +++ b/app/models/account/transaction_search.rb @@ -0,0 +1,42 @@ +class Account::TransactionSearch + include ActiveModel::Model + include ActiveModel::Attributes + + attribute :search, :string + attribute :amount, :string + attribute :amount_operator, :string + attribute :types, array: true + attribute :accounts, array: true + attribute :account_ids, array: true + attribute :start_date, :string + attribute :end_date, :string + attribute :categories, array: true + attribute :merchants, array: true + attribute :tags, array: true + + # Returns array of Account::Entry objects to stay consistent with partials, which only deal with Account::Entry + def build_query(scope) + query = scope + + if categories.present? + if categories.exclude?("Uncategorized") + query = query + .joins(:category) + .where(categories: { name: categories }) + else + query = query + .left_joins(:category) + .where(categories: { name: categories }) + .or(query.where(category_id: nil)) + end + end + + query = query.joins(:merchant).where(merchants: { name: merchants }) if merchants.present? + + query = query.joins(:tags).where(tags: { name: tags }) if tags.present? + + entries_scope = Account::Entry.account_transactions.where(entryable_id: query.select(:id)) + + Account::EntrySearch.from_entryable_search(self).build_query(entries_scope) + end +end diff --git a/app/models/account/valuation.rb b/app/models/account/valuation.rb index 93ebf5ffb1f..219ecd9005f 100644 --- a/app/models/account/valuation.rb +++ b/app/models/account/valuation.rb @@ -1,13 +1,3 @@ class Account::Valuation < ApplicationRecord include Account::Entryable - - class << self - def search(_params) - all - end - - def requires_search?(_params) - false - end - end end diff --git a/app/models/category.rb b/app/models/category.rb index 4a2d63614f8..6f50070bf91 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -1,12 +1,16 @@ class Category < ApplicationRecord has_many :transactions, dependent: :nullify, class_name: "Account::Transaction" has_many :import_mappings, as: :mappable, dependent: :destroy, class_name: "Import::Mapping" + belongs_to :family + has_many :subcategories, class_name: "Category", foreign_key: :parent_id + belongs_to :parent, class_name: "Category", optional: true + validates :name, :color, :family, presence: true validates :name, uniqueness: { scope: :family_id } - before_update :clear_internal_category, if: :name_changed? + validate :category_level_limit scope :alphabetically, -> { order(:name) } @@ -14,30 +18,55 @@ class Category < ApplicationRecord UNCATEGORIZED_COLOR = "#737373" - DEFAULT_CATEGORIES = [ - { internal_category: "income", color: COLORS[0] }, - { internal_category: "food_and_drink", color: COLORS[1] }, - { internal_category: "entertainment", color: COLORS[2] }, - { internal_category: "personal_care", color: COLORS[3] }, - { internal_category: "general_services", color: COLORS[4] }, - { internal_category: "auto_and_transport", color: COLORS[5] }, - { internal_category: "rent_and_utilities", color: COLORS[6] }, - { internal_category: "home_improvement", color: COLORS[7] } - ] - - def self.create_default_categories(family) - if family.categories.size > 0 - raise ArgumentError, "Family already has some categories" + class Group + attr_reader :category, :subcategories + + delegate :name, :color, to: :category + + def self.for(categories) + categories.select { |category| category.parent_id.nil? }.map do |category| + new(category, category.subcategories) + end + end + + def initialize(category, subcategories = nil) + @category = category + @subcategories = subcategories || [] + end + end + + class << self + def bootstrap_defaults + default_categories.each do |name, color| + find_or_create_by!(name: name) do |category| + category.color = color + end + end end - family_id = family.id - categories = self::DEFAULT_CATEGORIES.map { |c| { - name: I18n.t("transaction.default_category.#{c[:internal_category]}"), - internal_category: c[:internal_category], - color: c[:color], - family_id: - } } - self.insert_all(categories) + private + def default_categories + [ + [ "Income", "#e99537" ], + [ "Loan Payments", "#6471eb" ], + [ "Bank Fees", "#db5a54" ], + [ "Entertainment", "#df4e92" ], + [ "Food & Drink", "#c44fe9" ], + [ "Groceries", "#eb5429" ], + [ "Dining Out", "#61c9ea" ], + [ "General Merchandise", "#805dee" ], + [ "Clothing & Accessories", "#6ad28a" ], + [ "Electronics", "#e99537" ], + [ "Healthcare", "#4da568" ], + [ "Insurance", "#6471eb" ], + [ "Utilities", "#db5a54" ], + [ "Transportation", "#df4e92" ], + [ "Gas & Fuel", "#c44fe9" ], + [ "Education", "#eb5429" ], + [ "Charitable Donations", "#61c9ea" ], + [ "Subscriptions", "#805dee" ] + ] + end end def replace_and_destroy!(replacement) @@ -47,9 +76,14 @@ def replace_and_destroy!(replacement) end end - private + def subcategory? + parent.present? + end - def clear_internal_category - self.internal_category = nil + private + def category_level_limit + if subcategory? && parent.subcategory? + errors.add(:parent, "can't have more than 2 levels of subcategories") + end end end diff --git a/app/models/demo/generator.rb b/app/models/demo/generator.rb index 36e86200a28..d26b85c7783 100644 --- a/app/models/demo/generator.rb +++ b/app/models/demo/generator.rb @@ -90,6 +90,11 @@ def create_categories! categories.each do |category| family.categories.create!(name: category, color: COLORS.sample) end + + food = family.categories.find_by(name: "Food & Drink") + family.categories.create!(name: "Restaurants", parent_category: food) + family.categories.create!(name: "Groceries", parent_category: food) + family.categories.create!(name: "Alcohol & Bars", parent_category: food) end def create_merchants! diff --git a/app/views/account/trades/_header.html.erb b/app/views/account/trades/_header.html.erb index 7ccadfa420c..b89028afe42 100644 --- a/app/views/account/trades/_header.html.erb +++ b/app/views/account/trades/_header.html.erb @@ -34,7 +34,7 @@
<%= trade.security.ticker %>
- <% if trade.buy? %> + <% if trade.qty.positive? %>
<%= t(".purchase_qty_label") %>
<%= trade.qty.abs %>
@@ -53,7 +53,7 @@
<% end %> - <% if trade.buy? && trade.unrealized_gain_loss.present? %> + <% if trade.qty.positive? && trade.unrealized_gain_loss.present? %>
<%= t(".total_return_label") %>
diff --git a/app/views/categories/_badge.html.erb b/app/views/categories/_badge.html.erb index a9752262c9d..1b4c399b513 100644 --- a/app/views/categories/_badge.html.erb +++ b/app/views/categories/_badge.html.erb @@ -2,7 +2,7 @@ <% category ||= null_category %>
- " class="flex justify-between items-center p-4 bg-white"> +
<%= "pb-4" unless category.subcategories.any? %> bg-white">
+ <% if category.subcategory? %> + <%= lucide_icon "corner-down-right", class: "shrink-0 w-5 h-5 text-gray-400 ml-2" %> + <% end %> + <%= render partial: "categories/badge", locals: { category: category } %>
diff --git a/app/views/categories/_form.html.erb b/app/views/categories/_form.html.erb index 313e48ba417..2bca2191cf1 100644 --- a/app/views/categories/_form.html.erb +++ b/app/views/categories/_form.html.erb @@ -1,9 +1,12 @@ +<%# locals: (category:, categories:) %> +
<%= styled_form_with model: category, class: "space-y-4", data: { turbo_frame: :_top } do |f| %>
<%= render partial: "shared/color_avatar", locals: { name: category.name, color: category.color } %>
+
<% Category::COLORS.each do |color| %> <% end %>
-
- <%= f.text_field :name, placeholder: t(".placeholder"), required: true, autofocus: true, data: { color_avatar_target: "name" } %> + + <% if category.errors.any? %> + <%= render "shared/form_errors", model: category %> + <% end %> + +
+ <%= 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)" } %>
diff --git a/app/views/categories/_menu.html.erb b/app/views/categories/_menu.html.erb index 746bcb459f1..5a4627b3e41 100644 --- a/app/views/categories/_menu.html.erb +++ b/app/views/categories/_menu.html.erb @@ -5,7 +5,7 @@ <%= render partial: "categories/badge", locals: { category: transaction.category } %>