From bd4121183544edb6a23b27f30d16e83f79d79747 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 23 May 2024 13:53:49 -0400 Subject: [PATCH 01/31] Separate "modal" and "drawer" concepts for overlays --- app/helpers/application_helper.rb | 31 ++++++++++++++----- app/views/layouts/application.html.erb | 8 +++-- ...idebar_modal.html.erb => _drawer.html.erb} | 2 +- app/views/transactions/show.html.erb | 2 +- 4 files changed, 32 insertions(+), 11 deletions(-) rename app/views/shared/{_sidebar_modal.html.erb => _drawer.html.erb} (94%) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 288c3a78fb1..99daecc8008 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -20,23 +20,40 @@ def notification(text, **options, &block) render partial: "shared/notification", locals: { type: options[:type], content: { body: content } } end - # Wrap view with <%= modal do %> ... <% end %> to have it open in a modal - # Make sure to add data-turbo-frame="modal" to the link/button that opens the modal + ## + # Helper to open a centered and overlayed modal with custom contents + # + # @example Basic usage + # <%= modal classes: "custom-class" do %> + #
Content here
+ # <% end %> + # + # @note can also be triggered via ?modal=model&entity_id=uuid query params + # def modal(options = {}, &block) content = capture &block render partial: "shared/modal", locals: { content:, classes: options[:classes] } end + ## + # Helper to open a drawer on the right side of the screen with custom contents + # + # @example Basic usage + # <%= drawer do %> + #
Content here
+ # <% end %> + # + # @note can also be triggered via ?drawer=model&entity_id=uuid query params + def drawer(&block) + content = capture &block + render partial: "shared/drawer", locals: { content: content } + end + def account_groups(period: nil) assets, liabilities = Current.family.accounts.by_group(currency: Current.family.currency, period: period || Period.last_30_days).values_at(:assets, :liabilities) [ assets.children, liabilities.children ].flatten end - def sidebar_modal(&block) - content = capture &block - render partial: "shared/sidebar_modal", locals: { content: content } - end - def sidebar_link_to(name, path, options = {}) is_current = current_page?(path) || (request.path.start_with?(path) && path != "/") diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index ddef53e26e1..dbf9398f575 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -1,5 +1,5 @@ - + <%= content_for(:title) || "Maybe" %> @@ -13,7 +13,8 @@ <%= hotwire_livereload_tags if Rails.env.development? %> <%= turbo_refreshes_with method: :morph, scroll: :preserve %> - + @@ -30,10 +31,13 @@ <%= content_for?(:content) ? yield(:content) : yield %> <%= turbo_frame_tag "modal" %> + <%= turbo_frame_tag "drawer" %> + <%= render "shared/confirm_modal" %> <% if self_hosted? %> <%= render "shared/app_version" %> <% end %> + diff --git a/app/views/shared/_sidebar_modal.html.erb b/app/views/shared/_drawer.html.erb similarity index 94% rename from app/views/shared/_sidebar_modal.html.erb rename to app/views/shared/_drawer.html.erb index c2d7a848a4b..321667c8236 100644 --- a/app/views/shared/_sidebar_modal.html.erb +++ b/app/views/shared/_drawer.html.erb @@ -1,4 +1,4 @@ -<%= turbo_frame_tag "modal" do %> +<%= turbo_frame_tag "drawer" do %>
diff --git a/app/views/transactions/show.html.erb b/app/views/transactions/show.html.erb index 75b48a8046e..8cc963f9520 100644 --- a/app/views/transactions/show.html.erb +++ b/app/views/transactions/show.html.erb @@ -1,4 +1,4 @@ -<%= sidebar_modal do %> +<%= drawer do %>

<%= format_money @transaction.amount_money %> <%= @transaction.currency %> From 7086c5a1bb072c53240be40df6537cbd514350db Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 24 May 2024 08:01:20 -0400 Subject: [PATCH 02/31] Remove unused mobile pagination, set proper Turbo action --- app/views/transactions/_pagination.html.erb | 26 ++++----------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/app/views/transactions/_pagination.html.erb b/app/views/transactions/_pagination.html.erb index a64b3bf5256..9ba92952f46 100644 --- a/app/views/transactions/_pagination.html.erb +++ b/app/views/transactions/_pagination.html.erb @@ -1,24 +1,7 @@ - -
- <% if pagy.prev %> - <%= link_to "Previous", pagy_url_for(pagy, pagy.prev), class: "relative inline-flex items-center rounded-md border border-gray-300 bg-white px-4 py-2 text-sm font-medium text-gray-500 hover:bg-gray-50" %> - <% else %> -
Previous
- <% end %> - - <% if pagy.next %> - <%= link_to "Next", pagy_url_for(pagy, pagy.next), class: "relative ml-3 inline-flex items-center rounded-md border border-gray-300 bg-white px-4 py-2 text-sm font-medium text-gray-500 hover:bg-gray-50" %> - <% else %> -
Next
- <% end %> -
- - - - -

- +<% end %> From 7210c25d134ad93ba7fff5b594eca30005ee2db6 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 24 May 2024 10:37:39 -0400 Subject: [PATCH 03/31] Add tests for pagination and transaction filtering --- app/controllers/transactions_controller.rb | 27 ++++++----- app/models/transaction.rb | 1 + app/views/transactions/_search_form.html.erb | 5 +- app/views/transactions/_summary.html.erb | 6 +-- config/initializers/pagy.rb | 3 ++ .../transactions_controller_test.rb | 40 +++++++++++++++- test/system/transactions_test.rb | 48 +++++++++++++++++++ 7 files changed, 113 insertions(+), 17 deletions(-) create mode 100644 config/initializers/pagy.rb create mode 100644 test/system/transactions_test.rb diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 55cc84cec71..01d4fbd0ef3 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -4,16 +4,7 @@ class TransactionsController < ApplicationController before_action :set_transaction, only: %i[ show edit update destroy ] def index - search_params = session[ransack_session_key] || params[:q] - @q = Current.family.transactions.ransack(search_params) - result = @q.result.order(date: :desc) - @pagy, @transactions = pagy(result, items: 10) - @totals = { - count: result.count, - income: result.inflows.sum(&:amount_money).abs, - expense: result.outflows.sum(&:amount_money).abs - } - @filter_list = Transaction.build_filter_list(search_params, Current.family) + perform_ransack_search respond_to do |format| format.html @@ -41,7 +32,7 @@ def search session[ransack_session_key] = params[:q] end - index + perform_ransack_search respond_to do |format| format.html { render :index } @@ -131,6 +122,20 @@ def destroy private + def perform_ransack_search + search_params = session[ransack_session_key] || params[:q] + @q = Current.family.transactions.ordered.ransack(search_params) + result = @q.result + @pagy, @transactions = pagy(result, items: 10) + + @totals = { + count: result.count, + income: result.inflows.sum(&:amount_money).abs, + expense: result.outflows.sum(&:amount_money).abs + } + @filter_list = Transaction.build_filter_list(search_params, Current.family) + end + def delete_search_param(params, key, value: nil) if value params[key]&.delete(value) diff --git a/app/models/transaction.rb b/app/models/transaction.rb index c9b9508c8e5..509309f8ccd 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -12,6 +12,7 @@ class Transaction < ApplicationRecord monetize :amount + scope :ordered, -> { order(date: :desc) } scope :inflows, -> { where("amount <= 0") } scope :outflows, -> { where("amount > 0") } scope :active, -> { where(excluded: false) } diff --git a/app/views/transactions/_search_form.html.erb b/app/views/transactions/_search_form.html.erb index 7087c71263d..3b245687d8d 100644 --- a/app/views/transactions/_search_form.html.erb +++ b/app/views/transactions/_search_form.html.erb @@ -1,17 +1,18 @@ <%# locals: (q:) %>
<%= turbo_frame_tag "transactions_search_form" do %> - <%= search_form_for @q, url: search_transactions_path, html: { method: :post, data: { turbo_frame: "transactions_list" } } do |form| %> + <%= search_form_for @q, url: search_transactions_path, html: { method: :post, data: { turbo_frame: "transactions_list", controller: "auto-submit-form" } } do |form| %>
<%= render partial: "transactions/search_form/search_filter", locals: { form: form } %>
-

Total transactions

-

<%= totals[:count] %>

+

<%= totals[:count] %>

Income

-

+

<%= format_money totals[:income] %>

Expenses

-

+

<%= format_money totals[:expense] %>

diff --git a/config/initializers/pagy.rb b/config/initializers/pagy.rb new file mode 100644 index 00000000000..1a41c5c3523 --- /dev/null +++ b/config/initializers/pagy.rb @@ -0,0 +1,3 @@ +require "pagy/extras/overflow" + +Pagy::DEFAULT[:overflow] = :last_page diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb index 11326f7feef..54dbf836a53 100644 --- a/test/controllers/transactions_controller_test.rb +++ b/test/controllers/transactions_controller_test.rb @@ -5,11 +5,49 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest sign_in @user = users(:family_admin) @transaction = transactions(:checking_one) @account = @transaction.account + @recent_transactions = @user.family.transactions.ordered.limit(20).to_a end - test "should get index" do + test "should get paginated index with most recent transactions first" do get transactions_url assert_response :success + + @recent_transactions.first(10).each do |transaction| + assert_dom "#" + dom_id(transaction), count: 1 + end + end + + test "transaction count represents filtered total" do + get transactions_url + assert_dom "#total-transactions", count: 1, text: @user.family.transactions.count.to_s + + new_transaction = @user.family.accounts.first.transactions.create! \ + name: "Ransack transaction to search for", + date: Date.current, + amount: 0 + + get transactions_url(q: { category_name_or_merchant_name_or_account_name_or_name_cont: new_transaction.name }) + + # Only finds 1 transaction that matches filter + assert_dom "#" + dom_id(new_transaction), count: 1 + assert_dom "#total-transactions", count: 1, text: "1" + end + + test "can navigate to paginated result" do + get transactions_url(page: 2) + assert_response :success + + @recent_transactions[10, 10].each do |transaction| + assert_dom "#" + dom_id(transaction), count: 1 + end + end + + test "loads last page when page is out of range" do + user_oldest_transaction = @user.family.transactions.ordered.last + get transactions_url(page: 9999999999) + + assert_response :success + assert_dom "#" + dom_id(user_oldest_transaction), count: 1 end test "should get new" do diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb new file mode 100644 index 00000000000..26ef12c2dd2 --- /dev/null +++ b/test/system/transactions_test.rb @@ -0,0 +1,48 @@ +require "application_system_test_case" + +class TransactionsTest < ApplicationSystemTestCase + setup do + sign_in @user = users(:family_admin) + + @test_category = @user.family.transaction_categories.create! name: "System Test Category" + @target_txn = @user.family.accounts.first.transactions.create! \ + name: "Oldest transaction", + date: 10.years.ago.to_date, + category: @test_category, + amount: 100 + + visit transactions_url + end + + test "can search for a transaction" do + assert_selector "h1", text: "Transactions" + + within "form#transaction_search" do + fill_in "Search transaction by name, merchant, category or amount", with: @target_txn.name + end + + assert_selector "#" + dom_id(@target_txn), count: 1 + + within "#transactions_filters" do + assert_text @target_txn.name + end + end + + test "can open filters and apply one or more" do + find("#transaction-filters-button").click + + within "#transaction-filters-menu" do + check(@target_txn.account.name) + click_button "Category" + check(@test_category.name) + click_button "Apply" + end + + assert_selector "#" + dom_id(@target_txn), count: 1 + + within "#transactions_filters" do + assert_text @target_txn.account.name + assert_text @target_txn.category.name + end + end +end From 9cc34e3f817d0ba1b5ebf6033a132377b9fd8cb1 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 27 May 2024 07:23:57 -0400 Subject: [PATCH 04/31] Searchable concern --- app/controllers/concerns/searchable.rb | 35 +++++++++++++++++++ .../searches/filters_controller.rb | 6 ++++ .../transactions/searches_controller.rb | 17 +++++++++ app/controllers/transactions_controller.rb | 6 ++++ app/models/transaction/search/filter.rb | 2 ++ config/routes.rb | 6 ++-- 6 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 app/controllers/concerns/searchable.rb create mode 100644 app/controllers/transactions/searches/filters_controller.rb create mode 100644 app/controllers/transactions/searches_controller.rb create mode 100644 app/models/transaction/search/filter.rb diff --git a/app/controllers/concerns/searchable.rb b/app/controllers/concerns/searchable.rb new file mode 100644 index 00000000000..5960b20a84b --- /dev/null +++ b/app/controllers/concerns/searchable.rb @@ -0,0 +1,35 @@ +module Searchable + extend ActiveSupport::Concern + + included do + before_action :set_search_session_key + end + + private + + def set_search_session_key + @search_session_key = "#{get_session_key_prefix}_search_params" + end + + def search_params + session[@search_session_key] || {} + end + + def update_search_params(new_params) + session[@search_session_key] = search_params.merge(new_params) + end + + def clear_search + session[@search_session_key] = nil + end + + def remove_search_param(param_key) + current_params = search_params + current_params.delete(param_key) + session[@search_session_key] = current_params + end + + def get_session_key_prefix + controller_path.split("/").first + end +end diff --git a/app/controllers/transactions/searches/filters_controller.rb b/app/controllers/transactions/searches/filters_controller.rb new file mode 100644 index 00000000000..467d24e26f8 --- /dev/null +++ b/app/controllers/transactions/searches/filters_controller.rb @@ -0,0 +1,6 @@ +class Transactions::Searches::FiltersController < ApplicationController + include Searchable + + def destroy + end +end diff --git a/app/controllers/transactions/searches_controller.rb b/app/controllers/transactions/searches_controller.rb new file mode 100644 index 00000000000..98da9092a05 --- /dev/null +++ b/app/controllers/transactions/searches_controller.rb @@ -0,0 +1,17 @@ +class Transactions::SearchesController < ApplicationController + include Searchable + + def update + update_search_params(search_params) + end + + def destroy + clear_search + end + + private + + def search_params + params.require(:q) + end +end diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 01d4fbd0ef3..241130beb9e 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -1,4 +1,6 @@ class TransactionsController < ApplicationController + include Searchable + layout "with_sidebar" before_action :set_transaction, only: %i[ show edit update destroy ] @@ -122,6 +124,10 @@ def destroy private + def search_session_key + Transaction.search_session_key + end + def perform_ransack_search search_params = session[ransack_session_key] || params[:q] @q = Current.family.transactions.ordered.ransack(search_params) diff --git a/app/models/transaction/search/filter.rb b/app/models/transaction/search/filter.rb new file mode 100644 index 00000000000..8edc4ceb949 --- /dev/null +++ b/app/models/transaction/search/filter.rb @@ -0,0 +1,2 @@ +class Transaction::Search::Filter +end diff --git a/config/routes.rb b/config/routes.rb index 9ff307d20cb..2143c4e4074 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -43,9 +43,11 @@ resources :transactions do collection do - match "search" => "transactions#search", via: %i[ get post ] - scope module: :transactions, as: :transaction do + resource :search, only: %i[ update destroy ] do + resources :filters, only: :destroy, module: :searches + end + resources :categories do resources :deletions, only: %i[ new create ], module: :categories collection do From 5b03bfaf558a06dcc8c13296da2ff058ed82e831 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 27 May 2024 07:25:02 -0400 Subject: [PATCH 05/31] Revert "Searchable concern" This reverts commit 9cc34e3f817d0ba1b5ebf6033a132377b9fd8cb1. --- app/controllers/concerns/searchable.rb | 35 ------------------- .../searches/filters_controller.rb | 6 ---- .../transactions/searches_controller.rb | 17 --------- app/controllers/transactions_controller.rb | 6 ---- app/models/transaction/search/filter.rb | 2 -- config/routes.rb | 6 ++-- 6 files changed, 2 insertions(+), 70 deletions(-) delete mode 100644 app/controllers/concerns/searchable.rb delete mode 100644 app/controllers/transactions/searches/filters_controller.rb delete mode 100644 app/controllers/transactions/searches_controller.rb delete mode 100644 app/models/transaction/search/filter.rb diff --git a/app/controllers/concerns/searchable.rb b/app/controllers/concerns/searchable.rb deleted file mode 100644 index 5960b20a84b..00000000000 --- a/app/controllers/concerns/searchable.rb +++ /dev/null @@ -1,35 +0,0 @@ -module Searchable - extend ActiveSupport::Concern - - included do - before_action :set_search_session_key - end - - private - - def set_search_session_key - @search_session_key = "#{get_session_key_prefix}_search_params" - end - - def search_params - session[@search_session_key] || {} - end - - def update_search_params(new_params) - session[@search_session_key] = search_params.merge(new_params) - end - - def clear_search - session[@search_session_key] = nil - end - - def remove_search_param(param_key) - current_params = search_params - current_params.delete(param_key) - session[@search_session_key] = current_params - end - - def get_session_key_prefix - controller_path.split("/").first - end -end diff --git a/app/controllers/transactions/searches/filters_controller.rb b/app/controllers/transactions/searches/filters_controller.rb deleted file mode 100644 index 467d24e26f8..00000000000 --- a/app/controllers/transactions/searches/filters_controller.rb +++ /dev/null @@ -1,6 +0,0 @@ -class Transactions::Searches::FiltersController < ApplicationController - include Searchable - - def destroy - end -end diff --git a/app/controllers/transactions/searches_controller.rb b/app/controllers/transactions/searches_controller.rb deleted file mode 100644 index 98da9092a05..00000000000 --- a/app/controllers/transactions/searches_controller.rb +++ /dev/null @@ -1,17 +0,0 @@ -class Transactions::SearchesController < ApplicationController - include Searchable - - def update - update_search_params(search_params) - end - - def destroy - clear_search - end - - private - - def search_params - params.require(:q) - end -end diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 241130beb9e..01d4fbd0ef3 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -1,6 +1,4 @@ class TransactionsController < ApplicationController - include Searchable - layout "with_sidebar" before_action :set_transaction, only: %i[ show edit update destroy ] @@ -124,10 +122,6 @@ def destroy private - def search_session_key - Transaction.search_session_key - end - def perform_ransack_search search_params = session[ransack_session_key] || params[:q] @q = Current.family.transactions.ordered.ransack(search_params) diff --git a/app/models/transaction/search/filter.rb b/app/models/transaction/search/filter.rb deleted file mode 100644 index 8edc4ceb949..00000000000 --- a/app/models/transaction/search/filter.rb +++ /dev/null @@ -1,2 +0,0 @@ -class Transaction::Search::Filter -end diff --git a/config/routes.rb b/config/routes.rb index 2143c4e4074..9ff307d20cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -43,11 +43,9 @@ resources :transactions do collection do - scope module: :transactions, as: :transaction do - resource :search, only: %i[ update destroy ] do - resources :filters, only: :destroy, module: :searches - end + match "search" => "transactions#search", via: %i[ get post ] + scope module: :transactions, as: :transaction do resources :categories do resources :deletions, only: %i[ new create ], module: :categories collection do From 565e6cbf97018cc5a6f792aeea6ba0d8000087c5 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 27 May 2024 11:30:55 -0400 Subject: [PATCH 06/31] Reorganize transaction partials part 1 --- app/controllers/transactions_controller.rb | 3 +- app/helpers/transactions/searches_helper.rb | 20 +++++++ app/helpers/transactions_helper.rb | 19 ------- app/views/transactions/_filters.html.erb | 10 ---- app/views/transactions/_search_form.html.erb | 56 ------------------- app/views/transactions/index.html.erb | 7 ++- .../search_form/_search_filter.html.erb | 8 --- .../transactions/searches/_form.html.erb | 26 +++++++++ .../transactions/searches/_menu.html.erb | 40 +++++++++++++ .../transactions/searches/_search.html.erb | 7 +++ .../filters}/_account_filter.html.erb | 0 .../filters}/_amount_filter.html.erb | 0 .../filters/_badge.html.erb} | 0 .../filters}/_category_filter.html.erb | 0 .../filters}/_date_filter.html.erb | 0 .../filters}/_merchant_filter.html.erb | 0 .../filters}/_type_filter.html.erb | 0 test/system/transactions_test.rb | 4 +- 18 files changed, 101 insertions(+), 99 deletions(-) create mode 100644 app/helpers/transactions/searches_helper.rb delete mode 100644 app/views/transactions/_filters.html.erb delete mode 100644 app/views/transactions/_search_form.html.erb delete mode 100644 app/views/transactions/search_form/_search_filter.html.erb create mode 100644 app/views/transactions/searches/_form.html.erb create mode 100644 app/views/transactions/searches/_menu.html.erb create mode 100644 app/views/transactions/searches/_search.html.erb rename app/views/transactions/{search_form => searches/filters}/_account_filter.html.erb (100%) rename app/views/transactions/{search_form => searches/filters}/_amount_filter.html.erb (100%) rename app/views/transactions/{_filter.html.erb => searches/filters/_badge.html.erb} (100%) rename app/views/transactions/{search_form => searches/filters}/_category_filter.html.erb (100%) rename app/views/transactions/{search_form => searches/filters}/_date_filter.html.erb (100%) rename app/views/transactions/{search_form => searches/filters}/_merchant_filter.html.erb (100%) rename app/views/transactions/{search_form => searches/filters}/_type_filter.html.erb (100%) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 01d4fbd0ef3..2987298efef 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -39,8 +39,7 @@ def search format.turbo_stream do render turbo_stream: [ turbo_stream.replace("transactions_summary", partial: "transactions/summary", locals: { totals: @totals }), - turbo_stream.replace("transactions_search_form", partial: "transactions/search_form", locals: { q: @q }), - turbo_stream.replace("transactions_filters", partial: "transactions/filters", locals: { filters: @filter_list }), + turbo_stream.replace("transactions-search", partial: "transactions/searches/search", locals: { q: @q, filter_list: @filter_list }), turbo_stream.replace("transactions_list", partial: "transactions/list", locals: { transactions: @transactions, pagy: @pagy }) ] end diff --git a/app/helpers/transactions/searches_helper.rb b/app/helpers/transactions/searches_helper.rb new file mode 100644 index 00000000000..af1a1cb1e4a --- /dev/null +++ b/app/helpers/transactions/searches_helper.rb @@ -0,0 +1,20 @@ +module Transactions::SearchesHelper + def transaction_search_filters + [ + { key: "account_filter", name: "Account", icon: "layers" }, + { key: "date_filter", name: "Date", icon: "calendar" }, + { key: "type_filter", name: "Type", icon: "shapes" }, + { key: "amount_filter", name: "Amount", icon: "hash" }, + { key: "category_filter", name: "Category", icon: "tag" }, + { key: "merchant_filter", name: "Merchant", icon: "store" } + ] + end + + def get_transaction_search_filter_partial_path(filter) + "transactions/searches/filters/#{filter[:key]}" + end + + def get_default_transaction_search_filter + transaction_search_filters[0] + end +end diff --git a/app/helpers/transactions_helper.rb b/app/helpers/transactions_helper.rb index 7f270a1f8da..0872260a62c 100644 --- a/app/helpers/transactions_helper.rb +++ b/app/helpers/transactions_helper.rb @@ -1,23 +1,4 @@ module TransactionsHelper - def transaction_filters - [ - { name: "Account", partial: "account_filter", icon: "layers" }, - { name: "Date", partial: "date_filter", icon: "calendar" }, - { name: "Type", partial: "type_filter", icon: "shapes" }, - { name: "Amount", partial: "amount_filter", icon: "hash" }, - { name: "Category", partial: "category_filter", icon: "tag" }, - { name: "Merchant", partial: "merchant_filter", icon: "store" } - ] - end - - def transaction_filter_id(filter) - "txn-#{filter[:name].downcase}-filter" - end - - def transaction_filter_by_name(name) - transaction_filters.find { |filter| filter[:name] == name } - end - def full_width_transaction_row?(route) route != "/" end diff --git a/app/views/transactions/_filters.html.erb b/app/views/transactions/_filters.html.erb deleted file mode 100644 index 5349f2b05b5..00000000000 --- a/app/views/transactions/_filters.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<%# locals: (filters:) %> -
- <%= turbo_frame_tag "transactions_filters" do %> -
- <% filters.each do |filter| %> - <%= render partial: "transactions/filter", locals: { filter: filter } %> - <% end %> -
- <% end %> -
diff --git a/app/views/transactions/_search_form.html.erb b/app/views/transactions/_search_form.html.erb deleted file mode 100644 index 3b245687d8d..00000000000 --- a/app/views/transactions/_search_form.html.erb +++ /dev/null @@ -1,56 +0,0 @@ -<%# locals: (q:) %> -
- <%= turbo_frame_tag "transactions_search_form" do %> - <%= search_form_for @q, url: search_transactions_path, html: { method: :post, data: { turbo_frame: "transactions_list", controller: "auto-submit-form" } } do |form| %> -
-
- <%= render partial: "transactions/search_form/search_filter", locals: { form: form } %> -
-
- -
" - class="hidden absolute flex z-10 h-80 w-[540px] top-12 right-0 border border-alpha-black-25 bg-white rounded-lg shadow-xs"> -
- <% transaction_filters.each do |filter| %> - - <% end %> -
-
-
- <% transaction_filters.each do |filter| %> -
- <%= render partial: "transactions/search_form/#{filter[:partial]}", locals: { form: form } %> -
- <% end %> -
-
- <%= button_tag type: "reset", data: { action: "menu#close" }, class: "py-2 px-3 bg-gray-50 rounded-lg text-sm text-gray-900 font-medium" do %> - Cancel - <% end %> - <%= button_tag type: "submit", class: "py-2 px-3 bg-gray-900 rounded-lg text-sm text-white font-medium" do %> - Apply - <% end %> -
-
-
-
-
- <% end %> - <% end %> -
diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index d7151e73dc1..ea1f65a442a 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -36,8 +36,11 @@ <%= render partial: "transactions/summary", locals: { totals: @totals } %>
- <%= render partial: "transactions/search_form", locals: { q: @q } %> - <%= render partial: "transactions/filters", locals: { filters: @filter_list } %> + + + <%= render partial: "transactions/list", locals: { transactions: @transactions, pagy: @pagy } %>
diff --git a/app/views/transactions/search_form/_search_filter.html.erb b/app/views/transactions/search_form/_search_filter.html.erb deleted file mode 100644 index 4273c689305..00000000000 --- a/app/views/transactions/search_form/_search_filter.html.erb +++ /dev/null @@ -1,8 +0,0 @@ -<%# locals: (form:) %> -
- <%= form.search_field :category_name_or_merchant_name_or_account_name_or_name_cont, - placeholder: "Search transaction by name, merchant, category or amount", - class: "placeholder:text-sm placeholder:text-gray-500 relative pl-10 w-full border-none rounded-lg", - "data-auto-submit-form-target": "auto" %> - <%= lucide_icon("search", class: "w-5 h-5 text-gray-500 ml-2 absolute inset-0 transform top-1/2 -translate-y-1/2") %> -
diff --git a/app/views/transactions/searches/_form.html.erb b/app/views/transactions/searches/_form.html.erb new file mode 100644 index 00000000000..c71c0a04d7d --- /dev/null +++ b/app/views/transactions/searches/_form.html.erb @@ -0,0 +1,26 @@ +<%# locals: (q:) %> +
+ <%= turbo_frame_tag "transactions_search_form" do %> + <%= search_form_for @q, url: search_transactions_path, html: { method: :post, data: { turbo_frame: "transactions_list", controller: "auto-submit-form" } } do |form| %> +
+
+
+ <%= form.search_field :category_name_or_merchant_name_or_account_name_or_name_cont, + placeholder: "Search transaction by name, merchant, category or amount", + class: "placeholder:text-sm placeholder:text-gray-500 relative pl-10 w-full border-none rounded-lg", + "data-auto-submit-form-target": "auto" %> + <%= lucide_icon("search", class: "w-5 h-5 text-gray-500 ml-2 absolute inset-0 transform top-1/2 -translate-y-1/2") %> +
+
+
+ + + <%= render "transactions/searches/menu", form: form %> +
+
+ <% end %> + <% end %> +
diff --git a/app/views/transactions/searches/_menu.html.erb b/app/views/transactions/searches/_menu.html.erb new file mode 100644 index 00000000000..9275bdd06fb --- /dev/null +++ b/app/views/transactions/searches/_menu.html.erb @@ -0,0 +1,40 @@ + \ No newline at end of file diff --git a/app/views/transactions/searches/_search.html.erb b/app/views/transactions/searches/_search.html.erb new file mode 100644 index 00000000000..2d842e5fd1f --- /dev/null +++ b/app/views/transactions/searches/_search.html.erb @@ -0,0 +1,7 @@ +<%= render partial: "transactions/searches/form", locals: { q: q } %> + +
+ <% filter_list.each do |filter| %> + <%= render partial: "transactions/searches/filters/badge", locals: { filter: filter } %> + <% end %> +
diff --git a/app/views/transactions/search_form/_account_filter.html.erb b/app/views/transactions/searches/filters/_account_filter.html.erb similarity index 100% rename from app/views/transactions/search_form/_account_filter.html.erb rename to app/views/transactions/searches/filters/_account_filter.html.erb diff --git a/app/views/transactions/search_form/_amount_filter.html.erb b/app/views/transactions/searches/filters/_amount_filter.html.erb similarity index 100% rename from app/views/transactions/search_form/_amount_filter.html.erb rename to app/views/transactions/searches/filters/_amount_filter.html.erb diff --git a/app/views/transactions/_filter.html.erb b/app/views/transactions/searches/filters/_badge.html.erb similarity index 100% rename from app/views/transactions/_filter.html.erb rename to app/views/transactions/searches/filters/_badge.html.erb diff --git a/app/views/transactions/search_form/_category_filter.html.erb b/app/views/transactions/searches/filters/_category_filter.html.erb similarity index 100% rename from app/views/transactions/search_form/_category_filter.html.erb rename to app/views/transactions/searches/filters/_category_filter.html.erb diff --git a/app/views/transactions/search_form/_date_filter.html.erb b/app/views/transactions/searches/filters/_date_filter.html.erb similarity index 100% rename from app/views/transactions/search_form/_date_filter.html.erb rename to app/views/transactions/searches/filters/_date_filter.html.erb diff --git a/app/views/transactions/search_form/_merchant_filter.html.erb b/app/views/transactions/searches/filters/_merchant_filter.html.erb similarity index 100% rename from app/views/transactions/search_form/_merchant_filter.html.erb rename to app/views/transactions/searches/filters/_merchant_filter.html.erb diff --git a/app/views/transactions/search_form/_type_filter.html.erb b/app/views/transactions/searches/filters/_type_filter.html.erb similarity index 100% rename from app/views/transactions/search_form/_type_filter.html.erb rename to app/views/transactions/searches/filters/_type_filter.html.erb diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index 26ef12c2dd2..a5e0fafe306 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -23,7 +23,7 @@ class TransactionsTest < ApplicationSystemTestCase assert_selector "#" + dom_id(@target_txn), count: 1 - within "#transactions_filters" do + within "#transaction-search-filters" do assert_text @target_txn.name end end @@ -40,7 +40,7 @@ class TransactionsTest < ApplicationSystemTestCase assert_selector "#" + dom_id(@target_txn), count: 1 - within "#transactions_filters" do + within "#transaction-search-filters" do assert_text @target_txn.account.name assert_text @target_txn.category.name end From aab19785da328a043eb8891b53c55b4ca15f6d7a Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 27 May 2024 12:55:54 -0400 Subject: [PATCH 07/31] Simplify search filters, temporarily remove Turbo --- app/controllers/transactions_controller.rb | 79 +++---------------- app/helpers/transactions/searches_helper.rb | 17 ++++ app/models/transaction.rb | 39 --------- app/views/transactions/index.html.erb | 2 +- .../transactions/searches/_form.html.erb | 2 +- .../transactions/searches/_search.html.erb | 6 +- .../searches/filters/_badge.html.erb | 49 +----------- test/system/transactions_test.rb | 8 +- 8 files changed, 43 insertions(+), 159 deletions(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 2987298efef..95eca26da91 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -4,46 +4,16 @@ class TransactionsController < ApplicationController before_action :set_transaction, only: %i[ show edit update destroy ] def index - perform_ransack_search - - respond_to do |format| - format.html - format.turbo_stream - end - end - - def search - if params[:clear] - session.delete(ransack_session_key) - elsif params[:remove_param] - current_params = session[ransack_session_key] || {} - if params[:remove_param] == "date_range" - updated_params = current_params.except("date_gteq", "date_lteq") - elsif params[:remove_param_value] - key_to_remove = params[:remove_param] - value_to_remove = params[:remove_param_value] - updated_params = current_params.deep_dup - updated_params[key_to_remove] = updated_params[key_to_remove] - [ value_to_remove ] - else - updated_params = current_params.except(params[:remove_param]) - end - session[ransack_session_key] = updated_params - elsif params[:q] - session[ransack_session_key] = params[:q] - end - - perform_ransack_search - - respond_to do |format| - format.html { render :index } - format.turbo_stream do - render turbo_stream: [ - turbo_stream.replace("transactions_summary", partial: "transactions/summary", locals: { totals: @totals }), - turbo_stream.replace("transactions-search", partial: "transactions/searches/search", locals: { q: @q, filter_list: @filter_list }), - turbo_stream.replace("transactions_list", partial: "transactions/list", locals: { transactions: @transactions, pagy: @pagy }) - ] - end - end + search_params = params[:q] + @q = Current.family.transactions.ordered.ransack(search_params) + result = @q.result + @pagy, @transactions = pagy(result, items: 10) + + @totals = { + count: result.count, + income: result.inflows.sum(&:amount_money).abs, + expense: result.outflows.sum(&:amount_money).abs + } end def show @@ -121,35 +91,6 @@ def destroy private - def perform_ransack_search - search_params = session[ransack_session_key] || params[:q] - @q = Current.family.transactions.ordered.ransack(search_params) - result = @q.result - @pagy, @transactions = pagy(result, items: 10) - - @totals = { - count: result.count, - income: result.inflows.sum(&:amount_money).abs, - expense: result.outflows.sum(&:amount_money).abs - } - @filter_list = Transaction.build_filter_list(search_params, Current.family) - end - - def delete_search_param(params, key, value: nil) - if value - params[key]&.delete(value) - params.delete(key) if params[key].empty? # Remove key if it's empty after deleting value - else - params.delete(key) - end - - params - end - - def ransack_session_key - :ransack_transactions_q - end - # Use callbacks to share common setup or constraints between actions. def set_transaction @transaction = Transaction.find(params[:id]) diff --git a/app/helpers/transactions/searches_helper.rb b/app/helpers/transactions/searches_helper.rb index af1a1cb1e4a..09bd882aafa 100644 --- a/app/helpers/transactions/searches_helper.rb +++ b/app/helpers/transactions/searches_helper.rb @@ -17,4 +17,21 @@ def get_transaction_search_filter_partial_path(filter) def get_default_transaction_search_filter transaction_search_filters[0] end + + def transactions_path_without_param(param_key, param_value) + updated_params = request.query_parameters.deep_dup + + q_params = updated_params[:q] || {} + + current_value = q_params[param_key] + if current_value.is_a?(Array) + q_params[param_key] = current_value - [ param_value ] + else + q_params.delete(param_key) + end + + updated_params[:q] = q_params + + transactions_path(updated_params) + end end diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 509309f8ccd..1b5c776d8df 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -63,43 +63,4 @@ def self.ransackable_attributes(auth_object = nil) def self.ransackable_associations(auth_object = nil) %w[category merchant account] end - - def self.build_filter_list(params, family) - filters = [] - - date_filters = { gteq: nil, lteq: nil } - - if params - params.each do |key, value| - next if value.blank? - - case key - when "account_id_in" - value.each do |account_id| - filters << { type: "account", value: family.accounts.find(account_id), original: { key: key, value: account_id } } - end - when "category_id_in" - value.each do |category_id| - filters << { type: "category", value: family.transaction_categories.find(category_id), original: { key: key, value: category_id } } - end - when "merchant_id_in" - value.each do |merchant_id| - filters << { type: "merchant", value: family.transaction_merchants.find(merchant_id), original: { key: key, value: merchant_id } } - end - when "category_name_or_merchant_name_or_account_name_or_name_cont" - filters << { type: "search", value: value, original: { key: key, value: nil } } - when "date_gteq" - date_filters[:gteq] = value - when "date_lteq" - date_filters[:lteq] = value - end - end - - unless date_filters.values.compact.empty? - filters << { type: "date_range", value: date_filters, original: { key: "date_range", value: nil } } - end - end - - filters - end end diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index ea1f65a442a..e3998e0e09d 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -38,7 +38,7 @@
<%= render partial: "transactions/list", locals: { transactions: @transactions, pagy: @pagy } %> diff --git a/app/views/transactions/searches/_form.html.erb b/app/views/transactions/searches/_form.html.erb index c71c0a04d7d..c2e82aa2bed 100644 --- a/app/views/transactions/searches/_form.html.erb +++ b/app/views/transactions/searches/_form.html.erb @@ -1,7 +1,7 @@ <%# locals: (q:) %>
<%= turbo_frame_tag "transactions_search_form" do %> - <%= search_form_for @q, url: search_transactions_path, html: { method: :post, data: { turbo_frame: "transactions_list", controller: "auto-submit-form" } } do |form| %> + <%= search_form_for @q, url: transactions_path, data: { turbo: false } do |form| %>
diff --git a/app/views/transactions/searches/_search.html.erb b/app/views/transactions/searches/_search.html.erb index 2d842e5fd1f..c10d1b9c085 100644 --- a/app/views/transactions/searches/_search.html.erb +++ b/app/views/transactions/searches/_search.html.erb @@ -1,7 +1,9 @@ <%= render partial: "transactions/searches/form", locals: { q: q } %>
- <% filter_list.each do |filter| %> - <%= render partial: "transactions/searches/filters/badge", locals: { filter: filter } %> + <% q.conditions.each do |condition| %> + <% condition.values.each do |value| %> + <%= render partial: "transactions/searches/filters/badge", locals: { filter_key: condition.key, filter_value: value.value } %> + <% end %> <% end %>
diff --git a/app/views/transactions/searches/filters/_badge.html.erb b/app/views/transactions/searches/filters/_badge.html.erb index f40246111ea..813cb28a9c7 100644 --- a/app/views/transactions/searches/filters/_badge.html.erb +++ b/app/views/transactions/searches/filters/_badge.html.erb @@ -1,48 +1,7 @@ -<%# locals: (filter:) %> +<%# locals: (filter_key:,filter_value:) %>
- <% case filter[:type] %> - <% when "account" %> -
-
<%= filter[:value].name[0].upcase %>
-

<%= filter[:value].name %>

-
- <% when "category" %> -
-
-

<%= filter[:value].name %>

-
- <% when "merchant" %> -
-
-

<%= filter[:value].name %>

-
- <% when "search" %> -
- <%= lucide_icon "text", class: "w-5 h-5 text-gray-500" %> -

<%= "\"#{filter[:value]}\"".truncate(20) %>

-
- <% when "date_range" %> -
- <%= lucide_icon "calendar", class: "w-5 h-5 text-gray-500" %> -

- <% if filter[:value][:gteq] && filter[:value][:lteq] %> - <%= filter[:value][:gteq] %> → <%= filter[:value][:lteq] %> - <% elsif filter[:value][:gteq] %> - on or after <%= filter[:value][:gteq] %> - <% elsif filter[:value][:lteq] %> - on or before <%= filter[:value][:lteq] %> - <% end %> -

-
- <% end %> - <%= form_with url: search_transactions_path, html: { class: "flex items-center" } do |form| %> - <%= form.hidden_field :remove_param, value: filter[:original][:key] %> - <% if filter[:original][:value] %> - <%= form.hidden_field :remove_param_value, value: filter[:original][:value] %> - <% else %> - <% end %> - <%= form.button type: "submit", class: "hover:text-gray-900" do %> - <%= lucide_icon "x", class: "w-4 h-4 text-gray-500" %> - <% end %> + <%= filter_value %> + <%= link_to transactions_path_without_param(filter_key, filter_value), data: { turbo: false }, class: "flex items-center" do %> + <%= lucide_icon "x", class: "w-4 h-4 text-gray-500" %> <% end %>
diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index a5e0fafe306..a213cb4b222 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -19,6 +19,9 @@ class TransactionsTest < ApplicationSystemTestCase within "form#transaction_search" do fill_in "Search transaction by name, merchant, category or amount", with: @target_txn.name + + # TODO: Add back Turbo + send_keys :enter end assert_selector "#" + dom_id(@target_txn), count: 1 @@ -41,8 +44,9 @@ class TransactionsTest < ApplicationSystemTestCase assert_selector "#" + dom_id(@target_txn), count: 1 within "#transaction-search-filters" do - assert_text @target_txn.account.name - assert_text @target_txn.category.name + # TODO: Add back account and category names (rather than IDs) + assert_text @target_txn.account.id + assert_text @target_txn.category.id end end end From e4f44cf394cb4de15bcd726680402f547664af79 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 27 May 2024 18:43:08 -0400 Subject: [PATCH 08/31] Simplify transaction searching, clean up controller --- Gemfile | 3 -- Gemfile.lock | 11 ----- app/controllers/transactions_controller.rb | 9 ++-- app/models/account.rb | 4 -- app/models/transaction.rb | 21 ++++++--- app/models/transaction/category.rb | 8 ---- app/models/transaction/merchant.rb | 8 ---- app/views/transactions/index.html.erb | 2 +- .../transactions/searches/_form.html.erb | 47 +++++++++---------- .../transactions/searches/_menu.html.erb | 9 ++-- .../transactions/searches/_search.html.erb | 10 ++-- .../searches/filters/_account_filter.html.erb | 11 ++++- .../searches/filters/_badge.html.erb | 6 +-- .../filters/_category_filter.html.erb | 11 ++++- .../searches/filters/_date_filter.html.erb | 10 +++- .../filters/_merchant_filter.html.erb | 11 ++++- .../transactions_controller_test.rb | 4 +- test/system/transactions_test.rb | 9 ++-- 18 files changed, 97 insertions(+), 97 deletions(-) diff --git a/Gemfile b/Gemfile index 1adb7b690e3..48f6b8e050d 100644 --- a/Gemfile +++ b/Gemfile @@ -25,9 +25,6 @@ gem "turbo-rails" # Background Jobs gem "good_job" -# Search -gem "ransack", github: "maybe-finance/ransack", branch: "main" - # Error logging gem "stackprof" gem "sentry-ruby" diff --git a/Gemfile.lock b/Gemfile.lock index 4929055d7ea..17f8854ad08 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -5,16 +5,6 @@ GIT lucide-rails (0.2.0) railties (>= 4.1.0) -GIT - remote: https://github.com/maybe-finance/ransack.git - revision: dec20edc9ccccac77f5b4b8a1c1a9f20dc58fa04 - branch: main - specs: - ransack (4.1.1) - activerecord (>= 6.1.5) - activesupport (>= 6.1.5) - i18n - GIT remote: https://github.com/rails/rails.git revision: ed50b93ebcf3c1a92ac3481297c07c33d9f7c161 @@ -494,7 +484,6 @@ DEPENDENCIES puma (>= 5.0) rails! rails-settings-cached - ransack! rubocop-rails-omakase ruby-lsp-rails selenium-webdriver diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 95eca26da91..acfb8eb9562 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -4,9 +4,8 @@ class TransactionsController < ApplicationController before_action :set_transaction, only: %i[ show edit update destroy ] def index - search_params = params[:q] - @q = Current.family.transactions.ordered.ransack(search_params) - result = @q.result + @q = search_params + result = Current.family.transactions.search(@q).ordered @pagy, @transactions = pagy(result, items: 10) @totals = { @@ -108,6 +107,10 @@ def nature params[:transaction][:nature].to_s.inquiry end + def search_params + params.fetch(:q, {}).permit(:start_date, :end_date, :search, accounts: [], categories: [], merchants: []) + end + def transaction_params params.require(:transaction).permit(:name, :date, :amount, :currency, :notes, :excluded, :category_id, :merchant_id, :tag_id, :remove_tag_id).except(:tag_id, :remove_tag_id) end diff --git a/app/models/account.rb b/app/models/account.rb index 2550acef542..c77f498f53e 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -22,10 +22,6 @@ class Account < ApplicationRecord delegated_type :accountable, types: Accountable::TYPES, dependent: :destroy - def self.ransackable_attributes(auth_object = nil) - %w[name id] - end - def balance_on(date) balances.where("date <= ?", date).order(date: :desc).first&.balance end diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 1b5c776d8df..8ec70866ae3 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -26,6 +26,12 @@ class Transaction < ApplicationRecord .joins(sanitize_sql_array([ "LEFT JOIN exchange_rates er ON transactions.date = er.date AND transactions.currency = er.base_currency AND er.converted_currency = ?", currency ])) .where("er.rate IS NOT NULL OR transactions.currency = ?", currency) } + scope :by_name, ->(name) { where("transactions.name ILIKE ?", "%#{name}%") if name.present? } + scope :with_categories, ->(categories) { joins(:category).where(transaction_categories: { name: categories }) if categories.present? } + scope :with_accounts, ->(accounts) { joins(:account).where(accounts: { name: accounts }) if accounts.present? } + scope :with_merchants, ->(merchants) { joins(:merchant).where(transaction_merchants: { name: merchants }) if merchants.present? } + scope :on_or_after_date, ->(date) { where("transactions.date >= ?", date) if date.present? } + scope :on_or_before_date, ->(date) { where("transactions.date <= ?", date) if date.present? } def self.daily_totals(transactions, period: Period.last_30_days, currency: Current.family.currency) # Sum spending and income for each day in the period with the given currency @@ -56,11 +62,14 @@ def self.daily_rolling_totals(transactions, period: Period.last_30_days, currenc select("*").from(rolling_totals).where("date >= ?", period.date_range.first) end - def self.ransackable_attributes(auth_object = nil) - %w[name amount date] - end - - def self.ransackable_associations(auth_object = nil) - %w[category merchant account] + def self.search(params) + query = all + query = query.by_name(params[:search]) + .with_categories(params[:categories]) + .with_accounts(params[:accounts]) + .with_merchants(params[:merchants]) + .on_or_after_date(params[:start_date]) + .on_or_before_date(params[:end_date]) + query end end diff --git a/app/models/transaction/category.rb b/app/models/transaction/category.rb index d884a3bdfb0..58955720f99 100644 --- a/app/models/transaction/category.rb +++ b/app/models/transaction/category.rb @@ -23,14 +23,6 @@ class Transaction::Category < ApplicationRecord { internal_category: "home_improvement", color: COLORS[7] } ] - def self.ransackable_attributes(auth_object = nil) - %w[name id] - end - - def self.ransackable_associations(auth_object = nil) - %w[] - end - def self.create_default_categories(family) if family.transaction_categories.size > 0 raise ArgumentError, "Family already has some categories" diff --git a/app/models/transaction/merchant.rb b/app/models/transaction/merchant.rb index e0395bb86e2..369ab916588 100644 --- a/app/models/transaction/merchant.rb +++ b/app/models/transaction/merchant.rb @@ -7,12 +7,4 @@ class Transaction::Merchant < ApplicationRecord scope :alphabetically, -> { order(:name) } COLORS = %w[#e99537 #4da568 #6471eb #db5a54 #df4e92 #c44fe9 #eb5429 #61c9ea #805dee #6ad28a] - - def self.ransackable_attributes(auth_object = nil) - %w[name id] - end - - def self.ransackable_associations(auth_object = nil) - %w[] - end end diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index e3998e0e09d..d17f9f2dcf6 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -38,7 +38,7 @@
<%= render partial: "transactions/list", locals: { transactions: @transactions, pagy: @pagy } %> diff --git a/app/views/transactions/searches/_form.html.erb b/app/views/transactions/searches/_form.html.erb index c2e82aa2bed..b66a77743d7 100644 --- a/app/views/transactions/searches/_form.html.erb +++ b/app/views/transactions/searches/_form.html.erb @@ -1,26 +1,23 @@ -<%# locals: (q:) %> -
- <%= turbo_frame_tag "transactions_search_form" do %> - <%= search_form_for @q, url: transactions_path, data: { turbo: false } do |form| %> -
-
-
- <%= form.search_field :category_name_or_merchant_name_or_account_name_or_name_cont, - placeholder: "Search transaction by name, merchant, category or amount", - class: "placeholder:text-sm placeholder:text-gray-500 relative pl-10 w-full border-none rounded-lg", - "data-auto-submit-form-target": "auto" %> - <%= lucide_icon("search", class: "w-5 h-5 text-gray-500 ml-2 absolute inset-0 transform top-1/2 -translate-y-1/2") %> -
-
-
- - - <%= render "transactions/searches/menu", form: form %> -
+<%# locals: (transactions:) %> +<%= form_with url: transactions_path, scope: :q, method: :get, id: "transactions-search", data: { turbo: false } do |form| %> +
+
+
+ <%= form.text_field :search, + placeholder: "Search transactions by name", + value: @q[:search], + class: "placeholder:text-sm placeholder:text-gray-500 relative pl-10 w-full border-none rounded-lg", + "data-auto-submit-form-target": "auto" %> + <%= lucide_icon("search", class: "w-5 h-5 text-gray-500 ml-2 absolute inset-0 transform top-1/2 -translate-y-1/2") %>
- <% end %> - <% end %> -
+
+
+ + + <%= render "transactions/searches/menu", form: form %> +
+
+<% end %> diff --git a/app/views/transactions/searches/_menu.html.erb b/app/views/transactions/searches/_menu.html.erb index 9275bdd06fb..48950c19ce9 100644 --- a/app/views/transactions/searches/_menu.html.erb +++ b/app/views/transactions/searches/_menu.html.erb @@ -4,8 +4,7 @@ data-controller="tabs" data-tabs-active-class="bg-gray-25 text-gray-900" data-tabs-default-tab-value="<%= get_default_transaction_search_filter[:key] %>" - class="hidden absolute flex z-10 h-80 w-[540px] top-12 right-0 border border-alpha-black-25 bg-white rounded-lg shadow-xs" -> + class="hidden absolute flex z-10 h-80 w-[540px] top-12 right-0 border border-alpha-black-25 bg-white rounded-lg shadow-xs">
<% transaction_search_filters.each do |filter| %>
-
\ No newline at end of file +
diff --git a/app/views/transactions/searches/_search.html.erb b/app/views/transactions/searches/_search.html.erb index c10d1b9c085..07e1f88b173 100644 --- a/app/views/transactions/searches/_search.html.erb +++ b/app/views/transactions/searches/_search.html.erb @@ -1,9 +1,11 @@ -<%= render partial: "transactions/searches/form", locals: { q: q } %> +<%= render partial: "transactions/searches/form", locals: { transactions: transactions } %>
- <% q.conditions.each do |condition| %> - <% condition.values.each do |value| %> - <%= render partial: "transactions/searches/filters/badge", locals: { filter_key: condition.key, filter_value: value.value } %> + <% @q.each do |param_key, param_value| %> + <% unless param_value.blank? %> + <% Array(param_value).each do |value| %> + <%= render partial: "transactions/searches/filters/badge", locals: { param_key: param_key, param_value: value } %> + <% end %> <% end %> <% end %>
diff --git a/app/views/transactions/searches/filters/_account_filter.html.erb b/app/views/transactions/searches/filters/_account_filter.html.erb index 38fbe7c15b3..5a0de6d2bcb 100644 --- a/app/views/transactions/searches/filters/_account_filter.html.erb +++ b/app/views/transactions/searches/filters/_account_filter.html.erb @@ -7,8 +7,15 @@
<% Current.family.accounts.each do |account| %>
- <%= form.check_box :account_id_in, { multiple: true, class: "rounded-sm border-gray-300 text-indigo-600 shadow-xs focus:border-indigo-300 focus:ring focus:ring-indigo-200 focus:ring-opacity-50" }, account.id, nil %> - <%= form.label :account_id_in, account.name, value: account.id, class: "text-sm text-gray-900" %> + <%= form.check_box :accounts, + { + multiple: true, + checked: @q[:accounts]&.include?(account.name), + class: "rounded-sm border-gray-300 text-indigo-600 shadow-xs focus:border-indigo-300 focus:ring focus:ring-indigo-200 focus:ring-opacity-50" + }, + account.name, + nil %> + <%= form.label :accounts, account.name, value: account.name, class: "text-sm text-gray-900" %>
<% end %>
diff --git a/app/views/transactions/searches/filters/_badge.html.erb b/app/views/transactions/searches/filters/_badge.html.erb index 813cb28a9c7..e082bc0d219 100644 --- a/app/views/transactions/searches/filters/_badge.html.erb +++ b/app/views/transactions/searches/filters/_badge.html.erb @@ -1,7 +1,7 @@ -<%# locals: (filter_key:,filter_value:) %> +<%# locals: (param_key:, param_value:) %>
- <%= filter_value %> - <%= link_to transactions_path_without_param(filter_key, filter_value), data: { turbo: false }, class: "flex items-center" do %> + <%= param_value %> + <%= link_to transactions_path_without_param(param_key, param_value), data: { turbo: false }, class: "flex items-center" do %> <%= lucide_icon "x", class: "w-4 h-4 text-gray-500" %> <% end %>
diff --git a/app/views/transactions/searches/filters/_category_filter.html.erb b/app/views/transactions/searches/filters/_category_filter.html.erb index a90abf86dad..1bcf0b8ca79 100644 --- a/app/views/transactions/searches/filters/_category_filter.html.erb +++ b/app/views/transactions/searches/filters/_category_filter.html.erb @@ -7,8 +7,15 @@
<% Current.family.transaction_categories.each do |transaction_category| %>
- <%= form.check_box :category_id_in, { "data-auto-submit-form-target": "auto", multiple: true, class: "rounded-sm border-gray-300 text-indigo-600 shadow-xs focus:border-indigo-300 focus:ring focus:ring-indigo-200 focus:ring-opacity-50" }, transaction_category.id, nil %> - <%= form.label :category_id_in, transaction_category.name, value: transaction_category.id, class: "text-sm text-gray-900 cursor-pointer" do %> + <%= form.check_box :categories, + { + multiple: true, + checked: @q[:categories]&.include?(transaction_category.name), + class: "rounded-sm border-gray-300 text-indigo-600 shadow-xs focus:border-indigo-300 focus:ring focus:ring-indigo-200 focus:ring-opacity-50" + }, + transaction_category.name, + nil %> + <%= form.label :categories, transaction_category.name, value: transaction_category.name, class: "text-sm text-gray-900 cursor-pointer" do %> <%= render partial: "transactions/categories/badge", locals: { category: transaction_category } %> <% end %>
diff --git a/app/views/transactions/searches/filters/_date_filter.html.erb b/app/views/transactions/searches/filters/_date_filter.html.erb index 265a8980702..a77b4934d51 100644 --- a/app/views/transactions/searches/filters/_date_filter.html.erb +++ b/app/views/transactions/searches/filters/_date_filter.html.erb @@ -1,5 +1,11 @@ <%# locals: (form:) %>
- <%= form.date_field :date_gteq, placeholder: "Start date", class: "block w-full border border-gray-200 rounded-md py-2 pl-3 pr-3 focus:border-gray-500 focus:ring-gray-500 sm:text-sm" %> - <%= form.date_field :date_lteq, placeholder: "End date", class: "block w-full border border-gray-200 rounded-md py-2 pl-3 pr-3 focus:border-gray-500 focus:ring-gray-500 sm:text-sm mt-2" %> + <%= form.date_field :start_date, + placeholder: "Start date", + value: @q[:start_date], + class: "block w-full border border-gray-200 rounded-md py-2 pl-3 pr-3 focus:border-gray-500 focus:ring-gray-500 sm:text-sm" %> + <%= form.date_field :end_date, + placeholder: "End date", + value: @q[:end_date], + class: "block w-full border border-gray-200 rounded-md py-2 pl-3 pr-3 focus:border-gray-500 focus:ring-gray-500 sm:text-sm mt-2" %>
diff --git a/app/views/transactions/searches/filters/_merchant_filter.html.erb b/app/views/transactions/searches/filters/_merchant_filter.html.erb index 37d55d70765..0db867cbb20 100644 --- a/app/views/transactions/searches/filters/_merchant_filter.html.erb +++ b/app/views/transactions/searches/filters/_merchant_filter.html.erb @@ -7,8 +7,15 @@
<% Current.family.transaction_merchants.each do |merchant| %>
- <%= form.check_box :merchant_id_in, { multiple: true, class: "rounded-sm border-gray-300 text-indigo-600 shadow-xs focus:border-indigo-300 focus:ring focus:ring-indigo-200 focus:ring-opacity-50" }, merchant.id, nil %> - <%= form.label :merchant_id_in, merchant.name, value: merchant.id, class: "text-sm text-gray-900" %> + <%= form.check_box :merchants, + { + multiple: true, + checked: @q[:merchants]&.include?(merchant.name), + class: "rounded-sm border-gray-300 text-indigo-600 shadow-xs focus:border-indigo-300 focus:ring focus:ring-indigo-200 focus:ring-opacity-50" + }, + merchant.name, + nil %> + <%= form.label :merchants, merchant.name, class: "text-sm text-gray-900" %>
<% end %>
diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb index 54dbf836a53..eed4aca7b38 100644 --- a/test/controllers/transactions_controller_test.rb +++ b/test/controllers/transactions_controller_test.rb @@ -22,11 +22,11 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest assert_dom "#total-transactions", count: 1, text: @user.family.transactions.count.to_s new_transaction = @user.family.accounts.first.transactions.create! \ - name: "Ransack transaction to search for", + name: "Transaction to search for", date: Date.current, amount: 0 - get transactions_url(q: { category_name_or_merchant_name_or_account_name_or_name_cont: new_transaction.name }) + get transactions_url(q: { search: new_transaction.name }) # Only finds 1 transaction that matches filter assert_dom "#" + dom_id(new_transaction), count: 1 diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index a213cb4b222..113b04c7abd 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -17,8 +17,8 @@ class TransactionsTest < ApplicationSystemTestCase test "can search for a transaction" do assert_selector "h1", text: "Transactions" - within "form#transaction_search" do - fill_in "Search transaction by name, merchant, category or amount", with: @target_txn.name + within "form#transactions-search" do + fill_in "Search transactions by name", with: @target_txn.name # TODO: Add back Turbo send_keys :enter @@ -44,9 +44,8 @@ class TransactionsTest < ApplicationSystemTestCase assert_selector "#" + dom_id(@target_txn), count: 1 within "#transaction-search-filters" do - # TODO: Add back account and category names (rather than IDs) - assert_text @target_txn.account.id - assert_text @target_txn.category.id + assert_text @target_txn.account.name + assert_text @target_txn.category.name end end end From 76795941b6f5c0ec71f4501c22cd8fce95214f85 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 28 May 2024 08:22:01 -0400 Subject: [PATCH 09/31] Complete filter badges and add test --- .../transactions/searches/_search.html.erb | 4 +- .../searches/filters/_badge.html.erb | 35 ++++++++++-- .../filters/_merchant_filter.html.erb | 2 +- test/system/transactions_test.rb | 54 +++++++++++++++++++ 4 files changed, 88 insertions(+), 7 deletions(-) diff --git a/app/views/transactions/searches/_search.html.erb b/app/views/transactions/searches/_search.html.erb index 07e1f88b173..fcd93fc458f 100644 --- a/app/views/transactions/searches/_search.html.erb +++ b/app/views/transactions/searches/_search.html.erb @@ -1,6 +1,6 @@ <%= render partial: "transactions/searches/form", locals: { transactions: transactions } %> -
+
    <% @q.each do |param_key, param_value| %> <% unless param_value.blank? %> <% Array(param_value).each do |value| %> @@ -8,4 +8,4 @@ <% end %> <% end %> <% end %> -
+ diff --git a/app/views/transactions/searches/filters/_badge.html.erb b/app/views/transactions/searches/filters/_badge.html.erb index e082bc0d219..14f1bf64dc5 100644 --- a/app/views/transactions/searches/filters/_badge.html.erb +++ b/app/views/transactions/searches/filters/_badge.html.erb @@ -1,7 +1,34 @@ <%# locals: (param_key:, param_value:) %> -
- <%= param_value %> - <%= link_to transactions_path_without_param(param_key, param_value), data: { turbo: false }, class: "flex items-center" do %> +
  • + + <% if param_key == "start_date" || param_key == "end_date" %> +
    + <%= lucide_icon "calendar", class: "w-5 h-5 text-gray-500" %> +

    + <% if param_key == "start_date" %> + on or after <%= param_value %> + <% else %> + on or before <%= param_value %> + <% end %> +

    +
    + <% elsif param_key == "search" %> +
    + <%= lucide_icon "text", class: "w-5 h-5 text-gray-500" %> +

    <%= "\"#{param_value}\"".truncate(20) %>

    +
    + <% elsif param_key == "accounts" %> +
    +
    <%= param_value[0].upcase %>
    +

    <%= param_value %>

    +
    + <% else %> +
    +

    <%= param_value %>

    +
    + <% end %> + + <%= link_to transactions_path_without_param(param_key, param_value), data: { id: "clear-param-btn", turbo: false }, class: "flex items-center" do %> <%= lucide_icon "x", class: "w-4 h-4 text-gray-500" %> <% end %> -
    • + diff --git a/app/views/transactions/searches/filters/_merchant_filter.html.erb b/app/views/transactions/searches/filters/_merchant_filter.html.erb index 0db867cbb20..1bdf74e8e0e 100644 --- a/app/views/transactions/searches/filters/_merchant_filter.html.erb +++ b/app/views/transactions/searches/filters/_merchant_filter.html.erb @@ -15,7 +15,7 @@ }, merchant.name, nil %> - <%= form.label :merchants, merchant.name, class: "text-sm text-gray-900" %> + <%= form.label :merchants, merchant.name, value: merchant.name, class: "text-sm text-gray-900" %>
    <% end %>
    diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index 113b04c7abd..c9ef93bd09f 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -5,10 +5,12 @@ class TransactionsTest < ApplicationSystemTestCase sign_in @user = users(:family_admin) @test_category = @user.family.transaction_categories.create! name: "System Test Category" + @test_merchant = @user.family.transaction_merchants.create! name: "System Test Merchant" @target_txn = @user.family.accounts.first.transactions.create! \ name: "Oldest transaction", date: 10.years.ago.to_date, category: @test_category, + merchant: @test_merchant, amount: 100 visit transactions_url @@ -48,4 +50,56 @@ class TransactionsTest < ApplicationSystemTestCase assert_text @target_txn.category.name end end + + test "all filters work and empty state shows if no match" do + within "form#transactions-search" do + fill_in "Search transactions by name", with: @target_txn.name + + # TODO: Add back Turbo + send_keys :enter + end + + find("#transaction-filters-button").click + + within "#transaction-filters-menu" do + click_button "Account" + check(@target_txn.account.name) + + click_button "Date" + fill_in "q_start_date", with: 10.days.ago.to_date + fill_in "q_end_date", with: Date.current + + click_button "Type" + assert_text "Filter by type coming soon..." + + click_button "Amount" + assert_text "Filter by amount coming soon..." + + click_button "Category" + check(@test_category.name) + + click_button "Merchant" + check(@test_merchant.name) + + click_button "Apply" + end + + assert_text "No transactions found" + + # Page reload doesn't affect results + visit current_url + + assert_text "No transactions found" + + within "ul#transaction-search-filters" do + find("li", text: @target_txn.name).first("a").click + find("li", text: @target_txn.account.name).first("a").click + find("li", text: "on or after #{10.days.ago.to_date}").first("a").click + find("li", text: "on or before #{Date.current}").first("a").click + find("li", text: @target_txn.category.name).first("a").click + find("li", text: @target_txn.merchant.name).first("a").click + end + + assert_selector "#" + dom_id(@user.family.transactions.ordered.first), count: 1 + end end From bc1b04ac673461a864f4966a5ad60af1223d4178 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 28 May 2024 12:15:24 -0400 Subject: [PATCH 10/31] Reorganize transaction partials part 2 --- app/controllers/transactions_controller.rb | 2 +- app/views/accounts/_transactions.html.erb | 4 +- .../transactions/_date_group.html.erb} | 7 +-- .../transactions/_transaction.html.erb | 13 +++++ .../transactions/_transaction.html.erb | 2 +- app/views/pages/dashboard.html.erb | 9 ++- .../transactions/_date_group.html.erb | 10 ++++ .../transactions/_transaction.html.erb | 10 ++++ app/views/transactions/_date_group.html.erb | 10 ++++ app/views/transactions/_empty.html.erb | 4 ++ app/views/transactions/_header.html.erb | 33 +++++++++++ app/views/transactions/_list.html.erb | 32 ----------- app/views/transactions/_name.html.erb | 13 +++++ app/views/transactions/_pagination.html.erb | 24 ++++---- app/views/transactions/_summary.html.erb | 38 ++++++------- app/views/transactions/_transaction.html.erb | 35 +++++------- .../transactions/_transaction_name.html.erb | 10 ---- app/views/transactions/index.html.erb | 56 +++++++------------ .../{transaction => transactions}/en.yml | 5 +- 19 files changed, 176 insertions(+), 141 deletions(-) rename app/views/{transactions/_transaction_group.html.erb => accounts/transactions/_date_group.html.erb} (68%) create mode 100644 app/views/accounts/transactions/_transaction.html.erb create mode 100644 app/views/pages/dashboard/transactions/_date_group.html.erb create mode 100644 app/views/pages/dashboard/transactions/_transaction.html.erb create mode 100644 app/views/transactions/_date_group.html.erb create mode 100644 app/views/transactions/_empty.html.erb create mode 100644 app/views/transactions/_header.html.erb delete mode 100644 app/views/transactions/_list.html.erb create mode 100644 app/views/transactions/_name.html.erb delete mode 100644 app/views/transactions/_transaction_name.html.erb rename config/locales/views/{transaction => transactions}/en.yml (95%) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index acfb8eb9562..980721dfc74 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -6,7 +6,7 @@ class TransactionsController < ApplicationController def index @q = search_params result = Current.family.transactions.search(@q).ordered - @pagy, @transactions = pagy(result, items: 10) + @pagy, @transactions = pagy(result, items: 100) @totals = { count: result.count, diff --git a/app/views/accounts/_transactions.html.erb b/app/views/accounts/_transactions.html.erb index ab72aa206f1..5c4f63131b3 100644 --- a/app/views/accounts/_transactions.html.erb +++ b/app/views/accounts/_transactions.html.erb @@ -11,7 +11,9 @@

    No transactions for this account yet.

    <% else %>
    - <%= render partial: "transactions/transaction_group", collection: transactions.group_by(&:date), as: :transaction_group %> + <% transactions.group_by(&:date).each do |date, transactions| %> + <%= render partial: "accounts/transactions/date_group", locals: { date: date, transactions: transactions } %> + <% end %>
    <% end %>
    diff --git a/app/views/transactions/_transaction_group.html.erb b/app/views/accounts/transactions/_date_group.html.erb similarity index 68% rename from app/views/transactions/_transaction_group.html.erb rename to app/views/accounts/transactions/_date_group.html.erb index 5bfd9c0457c..f489fd1c213 100644 --- a/app/views/transactions/_transaction_group.html.erb +++ b/app/views/accounts/transactions/_date_group.html.erb @@ -1,13 +1,10 @@ -<%# locals: (transaction_group:) %> -<% date = transaction_group[0] %> -<% transactions = transaction_group[1] %> - +<%# locals: (date:, transactions:) %>

    <%= date.strftime("%b %d, %Y") %> · <%= transactions.size %>

    <%= format_money -transactions.sum(&:amount_money) %>
    - <%= render partial: "transactions/transaction", collection: transactions %> + <%= render partial: "accounts/transactions/transaction", collection: transactions %>
    diff --git a/app/views/accounts/transactions/_transaction.html.erb b/app/views/accounts/transactions/_transaction.html.erb new file mode 100644 index 00000000000..4d9d070ba32 --- /dev/null +++ b/app/views/accounts/transactions/_transaction.html.erb @@ -0,0 +1,13 @@ +
    +
    + <%= render "transactions/name", transaction: transaction %> +
    + +
    + <%= render "transactions/categories/menu", transaction: transaction %> +
    + + <%= content_tag :p, + format_money(transaction.amount_money), + class: ["col-span-1 ml-auto", "text-green-600": transaction.amount.negative?] %> +
    diff --git a/app/views/imports/transactions/_transaction.html.erb b/app/views/imports/transactions/_transaction.html.erb index 9fbcdef3b01..4ff851ef632 100644 --- a/app/views/imports/transactions/_transaction.html.erb +++ b/app/views/imports/transactions/_transaction.html.erb @@ -1,6 +1,6 @@ <%# locals: (transaction:) %>
    - <%= render partial: "transactions/transaction_name", locals: { name: transaction.name } %> + <%= render "transactions/name", transaction: transaction %>
    <%= render partial: "transactions/categories/badge", locals: { category: transaction.category } %> diff --git a/app/views/pages/dashboard.html.erb b/app/views/pages/dashboard.html.erb index 8c527050d95..2ccd4d51f9f 100644 --- a/app/views/pages/dashboard.html.erb +++ b/app/views/pages/dashboard.html.erb @@ -161,9 +161,12 @@

    <%= t(".no_transactions") %>

    <% else %> -
    - <%= render partial: "transactions/transaction_group", collection: @transactions.group_by(&:date), as: :transaction_group %> -

    <%= link_to t(".view_all"), transactions_path %>

    +
    + <% @transactions.group_by(&:date).each do |date, transactions| %> + <%= render partial: "pages/dashboard/transactions/date_group", locals: { date: date, transactions: transactions } %> + <% end %> + +

    <%= link_to t(".view_all"), transactions_path %>

    <% end %>
    diff --git a/app/views/pages/dashboard/transactions/_date_group.html.erb b/app/views/pages/dashboard/transactions/_date_group.html.erb new file mode 100644 index 00000000000..961d97e45a8 --- /dev/null +++ b/app/views/pages/dashboard/transactions/_date_group.html.erb @@ -0,0 +1,10 @@ +<%# locals: (date:, transactions:) %> +
    +
    +

    <%= date.strftime("%b %d, %Y") %> · <%= transactions.size %>

    + <%= format_money -transactions.sum(&:amount_money) %> +
    +
    + <%= render partial: "pages/dashboard/transactions/transaction", collection: transactions %> +
    +
    diff --git a/app/views/pages/dashboard/transactions/_transaction.html.erb b/app/views/pages/dashboard/transactions/_transaction.html.erb new file mode 100644 index 00000000000..06bc7afe187 --- /dev/null +++ b/app/views/pages/dashboard/transactions/_transaction.html.erb @@ -0,0 +1,10 @@ +
    + +
    + <%= render "transactions/name", transaction: transaction %> +
    + + <%= content_tag :p, + format_money(transaction.amount_money), + class: ["ml-auto", "text-green-600": transaction.amount.negative?] %> +
    diff --git a/app/views/transactions/_date_group.html.erb b/app/views/transactions/_date_group.html.erb new file mode 100644 index 00000000000..146a5760b7a --- /dev/null +++ b/app/views/transactions/_date_group.html.erb @@ -0,0 +1,10 @@ +<%# locals: (date:, transactions:) %> +
    +
    +

    <%= date.strftime("%b %d, %Y") %> · <%= transactions.size %>

    + <%= format_money -transactions.sum(&:amount_money) %> +
    +
    + <%= render transactions %> +
    +
    diff --git a/app/views/transactions/_empty.html.erb b/app/views/transactions/_empty.html.erb new file mode 100644 index 00000000000..ced5ee3ff25 --- /dev/null +++ b/app/views/transactions/_empty.html.erb @@ -0,0 +1,4 @@ +
    +

    <%= t(".title") %>

    +

    <%= t(".description") %>

    +
    \ No newline at end of file diff --git a/app/views/transactions/_header.html.erb b/app/views/transactions/_header.html.erb new file mode 100644 index 00000000000..e89db329975 --- /dev/null +++ b/app/views/transactions/_header.html.erb @@ -0,0 +1,33 @@ +
    +

    Transactions

    +
    +
    + <%= contextual_menu do %> +
    + <%= link_to transaction_categories_path, + class: "block w-full py-2 px-3 space-x-2 text-gray-900 hover:bg-gray-50 flex items-center rounded-lg font-normal" do %> + <%= lucide_icon "tags", class: "w-5 h-5 text-gray-500" %> + <%= t(".edit_categories") %> + <% end %> + + <%= link_to imports_path, + class: "block w-full py-2 px-3 space-x-2 text-gray-900 hover:bg-gray-50 flex items-center rounded-lg font-normal" do %> + <%= lucide_icon "hard-drive-upload", class: "w-5 h-5 text-gray-500" %> + <%= t(".edit_imports") %> + <% end %> +
    + + <% end %> + + <%= link_to new_import_path(enable_type_selector: true), class: "rounded-lg bg-gray-50 border border-gray-200 flex items-center gap-1 justify-center px-3 py-2", data: { turbo_frame: "modal" } do %> + <%= lucide_icon("download", class: "text-gray-500 w-4 h-4") %> +

    <%= t(".import") %>

    + <% end %> + + <%= link_to new_transaction_path, class: "rounded-lg bg-gray-900 text-white flex items-center gap-1 justify-center hover:bg-gray-700 px-3 py-2", data: { turbo_frame: :modal } do %> + <%= lucide_icon("plus", class: "w-5 h-5") %> +

    New transaction

    + <% end %> +
    +
    +
    \ No newline at end of file diff --git a/app/views/transactions/_list.html.erb b/app/views/transactions/_list.html.erb deleted file mode 100644 index 775f677b898..00000000000 --- a/app/views/transactions/_list.html.erb +++ /dev/null @@ -1,32 +0,0 @@ -<%# locals: (transactions:, pagy:) %> -
    - <%= turbo_frame_tag "transactions_list" do %> - <% if transactions.empty? %> -
    -

    No transactions found

    -

    Try adding a transaction, editing filters or refining your search

    -
    - <% else %> -
    -
    -

    transaction

    -
    -
    -

    category

    -
    -
    -

    account

    -

    amount

    -
    -
    -
    - <%= render partial: "transactions/transaction_group", collection: transactions.group_by(&:date), as: :transaction_group %> -
    - <% end %> - <% if pagy.pages > 1 %> - - <% end %> - <% end %> -
    diff --git a/app/views/transactions/_name.html.erb b/app/views/transactions/_name.html.erb new file mode 100644 index 00000000000..f48af50d166 --- /dev/null +++ b/app/views/transactions/_name.html.erb @@ -0,0 +1,13 @@ +<%= content_tag :div, class: ["flex items-center gap-2"] do %> +
    + <%= transaction.name[0].upcase %> +
    + + <% if transaction.new_record? %> + <%= content_tag :p, transaction.name, class: "text-gray-900 hover:underline hover:text-gray-800 truncate" %> + <% else %> + <%= link_to transaction.name, + transaction_path(transaction), + class: "text-gray-900 hover:underline hover:text-gray-800 truncate" %> + <% end %> +<% end %> \ No newline at end of file diff --git a/app/views/transactions/_pagination.html.erb b/app/views/transactions/_pagination.html.erb index 9ba92952f46..f7251fc804d 100644 --- a/app/views/transactions/_pagination.html.erb +++ b/app/views/transactions/_pagination.html.erb @@ -1,4 +1,4 @@ -<%= turbo_frame_tag "pagination-controls", target: "_top", action: "advance", class: "flex -mt-px" do %> + \ No newline at end of file diff --git a/app/views/transactions/_summary.html.erb b/app/views/transactions/_summary.html.erb index 04ec0cd2803..468f5a73480 100644 --- a/app/views/transactions/_summary.html.erb +++ b/app/views/transactions/_summary.html.erb @@ -1,21 +1,19 @@ - <%# locals: (totals:) %> -<%= turbo_frame_tag "transactions_summary" do %> -
    -
    -

    Total transactions

    -

    <%= totals[:count] %>

    -
    -
    -

    Income

    -

    - <%= format_money totals[:income] %> -

    -
    -
    -

    Expenses

    -

    - <%= format_money totals[:expense] %> -

    -
    +<%# locals: (totals:) %> +
    +
    +

    Total transactions

    +

    <%= totals[:count] %>

    -<% end %> +
    +

    Income

    +

    + <%= format_money totals[:income] %> +

    +
    +
    +

    Expenses

    +

    + <%= format_money totals[:expense] %> +

    +
    +
    diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index ad2c4b6bc59..c2c61013caf 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -1,22 +1,17 @@ -<%# locals: (transaction:) %> -<%= turbo_frame_tag dom_id(transaction), class:"text-gray-900 flex items-center gap-6 py-4 text-sm font-medium px-4" do %> - <% if full_width_transaction_row?(request.path) %> - <%= link_to transaction_path(transaction), data: { turbo_frame: "modal" }, class: "group" do %> - <%= render partial: "transactions/transaction_name", locals: { name: transaction.name } %> - <% end %> -
    - <%= render partial: "transactions/categories/menu", locals: { transaction: } %> -
    -
    -

    <%= transaction.account.name %>

    -
    - <% else %> - <%= render partial: "transactions/transaction_name", locals: { name: transaction.name } %> -
    - <%= render partial: "transactions/categories/badge", locals: { category: transaction.category } %> -
    - <% end %> -
    - <%= content_tag :p, format_money(-transaction.amount_money), class: ["whitespace-nowrap", { "text-green-600": transaction.amount.negative? }] %> +<%= content_tag :div, id: dom_id(transaction), class: "grid grid-cols-12 items-center text-gray-900 py-4 text-sm font-medium px-4" do %> +
    + <%= render "name", transaction: transaction %>
    + +
    + <%= render "transactions/categories/menu", transaction: transaction %> +
    + + <%= link_to transaction.account.name, + account_path(transaction.account), + class: ["col-span-3 hover:underline"] %> + + <%= content_tag :p, + format_money(transaction.amount_money), + class: ["col-span-2 ml-auto", "text-green-600": transaction.amount.negative?] %> <% end %> diff --git a/app/views/transactions/_transaction_name.html.erb b/app/views/transactions/_transaction_name.html.erb deleted file mode 100644 index 06fea69d3b1..00000000000 --- a/app/views/transactions/_transaction_name.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<%# locals: (name:) %> - -<%= content_tag :div, class: ["flex items-center gap-2", { "w-40": !full_width_transaction_row?(request.path), "w-96": full_width_transaction_row?(request.path) }] do %> -
    - <%= name[0].upcase %> -
    -

    - <%= name %> -

    -<% end %> diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index d17f9f2dcf6..6bee2a3511a 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -1,46 +1,32 @@
    -
    -

    Transactions

    -
    -
    - <%= contextual_menu do %> -
    - <%= link_to transaction_categories_path, - class: "block w-full py-2 px-3 space-x-2 text-gray-900 hover:bg-gray-50 flex items-center rounded-lg font-normal" do %> - <%= lucide_icon "tags", class: "w-5 h-5 text-gray-500" %> - <%= t(".edit_categories") %> - <% end %> - <%= link_to imports_path, - class: "block w-full py-2 px-3 space-x-2 text-gray-900 hover:bg-gray-50 flex items-center rounded-lg font-normal" do %> - <%= lucide_icon "hard-drive-upload", class: "w-5 h-5 text-gray-500" %> - <%= t(".edit_imports") %> - <% end %> -
    + <%= render "header" %> - <% end %> + <%= render partial: "transactions/summary", locals: { totals: @totals } %> - <%= link_to new_import_path(enable_type_selector: true), class: "rounded-lg bg-gray-50 border border-gray-200 flex items-center gap-1 justify-center px-3 py-2", data: { turbo_frame: "modal" } do %> - <%= lucide_icon("download", class: "text-gray-500 w-4 h-4") %> -

    <%= t(".import") %>

    - <% end %> +
    + + <%= render partial: "transactions/searches/search", locals: { transactions: @transactions } %> - <%= link_to new_transaction_path, class: "rounded-lg bg-gray-900 text-white flex items-center gap-1 justify-center hover:bg-gray-700 px-3 py-2", data: { turbo_frame: :modal } do %> - <%= lucide_icon("plus", class: "w-5 h-5") %> -

    New transaction

    + <% if @transactions.present? %> +
    +

    transaction

    +

    category

    +

    account

    +

    amount

    +
    +
    + <% @transactions.group_by(&:date).each do |date, transactions| %> + <%= render "date_group", date: date, transactions: transactions %> <% end %>
    -
    -
    -
    - <%= render partial: "transactions/summary", locals: { totals: @totals } %> -
    -
    + <% else %> + <%= render "empty" %> + <% end %> - + <% if @pagy.pages > 1 %> + <%= render "pagination", pagy: @pagy %> + <% end %> - <%= render partial: "transactions/list", locals: { transactions: @transactions, pagy: @pagy } %>
    diff --git a/config/locales/views/transaction/en.yml b/config/locales/views/transactions/en.yml similarity index 95% rename from config/locales/views/transaction/en.yml rename to config/locales/views/transactions/en.yml index 5b09edb42b6..de022daa250 100644 --- a/config/locales/views/transaction/en.yml +++ b/config/locales/views/transactions/en.yml @@ -1,6 +1,9 @@ --- en: transactions: + empty: + title: No transactions found + description: Try adding a transaction, editing filters or refining your search categories: create: success: New transaction category created successfully @@ -59,7 +62,7 @@ en: income: Income submit: Add transaction transfer: Transfer - index: + header: edit_categories: Edit categories edit_imports: Edit imports import: Import From fdaf044c80500bd957379b83915f8d73a633e7fc Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 28 May 2024 12:35:51 -0400 Subject: [PATCH 11/31] Fix tests --- .github/workflows/ci.yml | 1 + app/views/transactions/_empty.html.erb | 2 +- app/views/transactions/_header.html.erb | 2 +- app/views/transactions/_name.html.erb | 2 +- app/views/transactions/_pagination.html.erb | 2 +- config/locales/views/transactions/en.yml | 6 +++--- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d8275946d68..7c150dad9c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,6 +90,7 @@ jobs: bin/rails db:create bin/rails db:schema:load bin/rails test + bin/rails test:system bin/rails db:seed - name: Keep screenshots from failed system tests diff --git a/app/views/transactions/_empty.html.erb b/app/views/transactions/_empty.html.erb index ced5ee3ff25..9cb5e3fa7a3 100644 --- a/app/views/transactions/_empty.html.erb +++ b/app/views/transactions/_empty.html.erb @@ -1,4 +1,4 @@

    <%= t(".title") %>

    <%= t(".description") %>

    -
    \ No newline at end of file +
    diff --git a/app/views/transactions/_header.html.erb b/app/views/transactions/_header.html.erb index e89db329975..f34690b895a 100644 --- a/app/views/transactions/_header.html.erb +++ b/app/views/transactions/_header.html.erb @@ -30,4 +30,4 @@ <% end %>
    - \ No newline at end of file + diff --git a/app/views/transactions/_name.html.erb b/app/views/transactions/_name.html.erb index f48af50d166..d9d181c70d6 100644 --- a/app/views/transactions/_name.html.erb +++ b/app/views/transactions/_name.html.erb @@ -10,4 +10,4 @@ transaction_path(transaction), class: "text-gray-900 hover:underline hover:text-gray-800 truncate" %> <% end %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/transactions/_pagination.html.erb b/app/views/transactions/_pagination.html.erb index f7251fc804d..4f92cd2d758 100644 --- a/app/views/transactions/_pagination.html.erb +++ b/app/views/transactions/_pagination.html.erb @@ -36,4 +36,4 @@
    <% end %>
    - \ No newline at end of file + diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index de022daa250..b8c66d41f97 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -1,9 +1,6 @@ --- en: transactions: - empty: - title: No transactions found - description: Try adding a transaction, editing filters or refining your search categories: create: success: New transaction category created successfully @@ -49,6 +46,9 @@ en: success: New transaction created successfully destroy: success: Transaction deleted successfully + empty: + description: Try adding a transaction, editing filters or refining your search + title: No transactions found form: account: Account account_prompt: Select an Account From 8f57916e6507722ca60167080f0a3eacc02dddd8 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 28 May 2024 15:06:51 -0400 Subject: [PATCH 12/31] Run system tests serially in CI --- .github/workflows/ci.yml | 2 +- test/test_helper.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7c150dad9c9..d873af11638 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,7 +90,7 @@ jobs: bin/rails db:create bin/rails db:schema:load bin/rails test - bin/rails test:system + DISABLE_PARALLELIZATION=true bin/rails test:system bin/rails db:seed - name: Keep screenshots from failed system tests diff --git a/test/test_helper.rb b/test/test_helper.rb index c37d06a4e4e..74c587f51a8 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -33,7 +33,7 @@ module ActiveSupport class TestCase # Run tests in parallel with specified workers - parallelize(workers: :number_of_processors) + parallelize(workers: :number_of_processors) unless ENV["DISABLE_PARALLELIZATION"] # https://github.com/simplecov-ruby/simplecov/issues/718#issuecomment-538201587 if ENV["COVERAGE"] From c800ffb0ae4599ce3edc6bb7f99dba95568d3488 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 06:30:38 -0400 Subject: [PATCH 13/31] Remove unused edit action --- app/controllers/transactions_controller.rb | 2 +- app/views/transactions/edit.html.erb | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) delete mode 100644 app/views/transactions/edit.html.erb diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 980721dfc74..b998e8f0fce 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -72,7 +72,7 @@ def update ] end else - format.html { render :edit, status: :unprocessable_entity } + format.html { render :show, status: :unprocessable_entity } end end end diff --git a/app/views/transactions/edit.html.erb b/app/views/transactions/edit.html.erb deleted file mode 100644 index 80feff59732..00000000000 --- a/app/views/transactions/edit.html.erb +++ /dev/null @@ -1,8 +0,0 @@ -
    -

    Editing transaction

    - - <%= render "form", transaction: @transaction %> - - <%= link_to "Show this transaction", @transaction, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %> - <%= link_to "Back to transactions", transactions_path, class: "ml-2 rounded-lg py-3 px-5 bg-gray-100 inline-block font-medium" %> -
    From 2e7fdc70cb9cf80e1775648f505350556d91a8fc Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 07:36:42 -0400 Subject: [PATCH 14/31] Add back turbo --- app/views/transactions/_pagination.html.erb | 2 +- app/views/transactions/searches/_form.html.erb | 6 +++++- test/controllers/transactions_controller_test.rb | 5 ----- test/system/transactions_test.rb | 6 ------ 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/app/views/transactions/_pagination.html.erb b/app/views/transactions/_pagination.html.erb index 4f92cd2d758..2ad3149ab56 100644 --- a/app/views/transactions/_pagination.html.erb +++ b/app/views/transactions/_pagination.html.erb @@ -13,7 +13,7 @@
    <% pagy.series.each do |series_item| %> <% if series_item.is_a?(Integer) %> - <%= link_to pagy_url_for(pagy, series_item), class: "inline-flex items-center px-3 py-3 text-sm font-medium text-gray-500 hover:border-gray-300 hover:text-gray-700" do %> + <%= link_to pagy_url_for(pagy, series_item), class: "inline-flex items-center px-3 py-3 text-sm font-medium text-gray-500 hover:border-gray-300 hover:text-gray-700" do %> <%= series_item %> <% end %> <% elsif series_item.is_a?(String) %> diff --git a/app/views/transactions/searches/_form.html.erb b/app/views/transactions/searches/_form.html.erb index b66a77743d7..22f3414c430 100644 --- a/app/views/transactions/searches/_form.html.erb +++ b/app/views/transactions/searches/_form.html.erb @@ -1,5 +1,9 @@ <%# locals: (transactions:) %> -<%= form_with url: transactions_path, scope: :q, method: :get, id: "transactions-search", data: { turbo: false } do |form| %> +<%= form_with url: transactions_path, + id: "transactions-search", + scope: :q, + method: :get, + data: { controller: "auto-submit-form" } do |form| %>
    diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb index eed4aca7b38..7769f00c3d8 100644 --- a/test/controllers/transactions_controller_test.rb +++ b/test/controllers/transactions_controller_test.rb @@ -126,11 +126,6 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest assert_response :success end - test "should get edit" do - get edit_transaction_url(@transaction) - assert_response :success - end - test "should update transaction" do patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: @transaction.date, name: @transaction.name } } assert_redirected_to transaction_url(@transaction) diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index c9ef93bd09f..845dd267add 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -21,9 +21,6 @@ class TransactionsTest < ApplicationSystemTestCase within "form#transactions-search" do fill_in "Search transactions by name", with: @target_txn.name - - # TODO: Add back Turbo - send_keys :enter end assert_selector "#" + dom_id(@target_txn), count: 1 @@ -54,9 +51,6 @@ class TransactionsTest < ApplicationSystemTestCase test "all filters work and empty state shows if no match" do within "form#transactions-search" do fill_in "Search transactions by name", with: @target_txn.name - - # TODO: Add back Turbo - send_keys :enter end find("#transaction-filters-button").click From 91e9fcda935922a907c83d70a70e443b14a92235 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 07:36:51 -0400 Subject: [PATCH 15/31] Clean up transaction model --- app/controllers/transactions_controller.rb | 11 ++- app/models/transaction.rb | 94 ++++++++++++---------- test/models/transaction_test.rb | 5 +- 3 files changed, 58 insertions(+), 52 deletions(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index b998e8f0fce..d4fc61a207e 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -6,12 +6,12 @@ class TransactionsController < ApplicationController def index @q = search_params result = Current.family.transactions.search(@q).ordered - @pagy, @transactions = pagy(result, items: 100) + @pagy, @transactions = pagy(result, items: 50) @totals = { - count: result.count, - income: result.inflows.sum(&:amount_money).abs, - expense: result.outflows.sum(&:amount_money).abs + count: @transactions.count, + income: @transactions.select { |t| t.inflow? }.sum(&:amount_money).abs, + expense: @transactions.select { |t| t.outflow? }.sum(&:amount_money).abs } end @@ -90,9 +90,8 @@ def destroy private - # Use callbacks to share common setup or constraints between actions. def set_transaction - @transaction = Transaction.find(params[:id]) + @transaction = Current.family.transactions.find(params[:id]) end def amount diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 8ec70866ae3..d534f41908f 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -1,21 +1,24 @@ class Transaction < ApplicationRecord include Monetizable + monetize :amount + belongs_to :account belongs_to :category, optional: true belongs_to :merchant, optional: true - - has_many :taggings, as: :taggable, dependent: :destroy has_many :tags, through: :taggings + has_many :taggings, as: :taggable, dependent: :destroy validates :name, :date, :amount, :account, presence: true - monetize :amount - scope :ordered, -> { order(date: :desc) } - scope :inflows, -> { where("amount <= 0") } - scope :outflows, -> { where("amount > 0") } scope :active, -> { where(excluded: false) } + scope :by_name, ->(name) { where("transactions.name ILIKE ?", "%#{name}%") if name.present? } + scope :with_categories, ->(categories) { joins(:category).where(transaction_categories: { name: categories }) if categories.present? } + scope :with_accounts, ->(accounts) { joins(:account).where(accounts: { name: accounts }) if accounts.present? } + scope :with_merchants, ->(merchants) { joins(:merchant).where(transaction_merchants: { name: merchants }) if merchants.present? } + scope :on_or_after_date, ->(date) { where("transactions.date >= ?", date) if date.present? } + scope :on_or_before_date, ->(date) { where("transactions.date <= ?", date) if date.present? } scope :with_converted_amount, ->(currency = Current.family.currency) { # Join with exchange rates to convert the amount to the given currency # If no rate is available, exclude the transaction from the results @@ -26,50 +29,53 @@ class Transaction < ApplicationRecord .joins(sanitize_sql_array([ "LEFT JOIN exchange_rates er ON transactions.date = er.date AND transactions.currency = er.base_currency AND er.converted_currency = ?", currency ])) .where("er.rate IS NOT NULL OR transactions.currency = ?", currency) } - scope :by_name, ->(name) { where("transactions.name ILIKE ?", "%#{name}%") if name.present? } - scope :with_categories, ->(categories) { joins(:category).where(transaction_categories: { name: categories }) if categories.present? } - scope :with_accounts, ->(accounts) { joins(:account).where(accounts: { name: accounts }) if accounts.present? } - scope :with_merchants, ->(merchants) { joins(:merchant).where(transaction_merchants: { name: merchants }) if merchants.present? } - scope :on_or_after_date, ->(date) { where("transactions.date >= ?", date) if date.present? } - scope :on_or_before_date, ->(date) { where("transactions.date <= ?", date) if date.present? } - def self.daily_totals(transactions, period: Period.last_30_days, currency: Current.family.currency) - # Sum spending and income for each day in the period with the given currency - select( - "gs.date", - "COALESCE(SUM(converted_amount) FILTER (WHERE converted_amount > 0), 0) AS spending", - "COALESCE(SUM(-converted_amount) FILTER (WHERE converted_amount < 0), 0) AS income" - ) - .from(transactions.with_converted_amount(currency), :t) - .joins(sanitize_sql([ "RIGHT JOIN generate_series(?, ?, interval '1 day') AS gs(date) ON t.date = gs.date", period.date_range.first, period.date_range.last ])) - .group("gs.date") + def inflow? + amount <= 0 end - def self.daily_rolling_totals(transactions, period: Period.last_30_days, currency: Current.family.currency) - # Extend the period to include the rolling window - period_with_rolling = period.extend_backward(period.date_range.count.days) + def outflow? + amount > 0 + end - # Aggregate the rolling sum of spending and income based on daily totals - rolling_totals = from(daily_totals(transactions, period: period_with_rolling, currency: currency)) - .select( - "*", - sanitize_sql_array([ "SUM(spending) OVER (ORDER BY date RANGE BETWEEN INTERVAL ? PRECEDING AND CURRENT ROW) as rolling_spend", "#{period.date_range.count} days" ]), - sanitize_sql_array([ "SUM(income) OVER (ORDER BY date RANGE BETWEEN INTERVAL ? PRECEDING AND CURRENT ROW) as rolling_income", "#{period.date_range.count} days" ]) + class << self + def daily_totals(transactions, period: Period.last_30_days, currency: Current.family.currency) + # Sum spending and income for each day in the period with the given currency + select( + "gs.date", + "COALESCE(SUM(converted_amount) FILTER (WHERE converted_amount > 0), 0) AS spending", + "COALESCE(SUM(-converted_amount) FILTER (WHERE converted_amount < 0), 0) AS income" ) - .order("date") + .from(transactions.with_converted_amount(currency), :t) + .joins(sanitize_sql([ "RIGHT JOIN generate_series(?, ?, interval '1 day') AS gs(date) ON t.date = gs.date", period.date_range.first, period.date_range.last ])) + .group("gs.date") + end - # Trim the results to the original period - select("*").from(rolling_totals).where("date >= ?", period.date_range.first) - end + def daily_rolling_totals(transactions, period: Period.last_30_days, currency: Current.family.currency) + # Extend the period to include the rolling window + period_with_rolling = period.extend_backward(period.date_range.count.days) + + # Aggregate the rolling sum of spending and income based on daily totals + rolling_totals = from(daily_totals(transactions, period: period_with_rolling, currency: currency)) + .select( + "*", + sanitize_sql_array([ "SUM(spending) OVER (ORDER BY date RANGE BETWEEN INTERVAL ? PRECEDING AND CURRENT ROW) as rolling_spend", "#{period.date_range.count} days" ]), + sanitize_sql_array([ "SUM(income) OVER (ORDER BY date RANGE BETWEEN INTERVAL ? PRECEDING AND CURRENT ROW) as rolling_income", "#{period.date_range.count} days" ]) + ) + .order("date") + + # Trim the results to the original period + select("*").from(rolling_totals).where("date >= ?", period.date_range.first) + end - def self.search(params) - query = all - query = query.by_name(params[:search]) - .with_categories(params[:categories]) - .with_accounts(params[:accounts]) - .with_merchants(params[:merchants]) - .on_or_after_date(params[:start_date]) - .on_or_before_date(params[:end_date]) - query + def search(params) + all + .by_name(params[:search]) + .with_categories(params[:categories]) + .with_accounts(params[:accounts]) + .with_merchants(params[:merchants]) + .on_or_after_date(params[:start_date]) + .on_or_before_date(params[:end_date]) + end end end diff --git a/test/models/transaction_test.rb b/test/models/transaction_test.rb index c5a47fde11b..0f3319e99bb 100644 --- a/test/models/transaction_test.rb +++ b/test/models/transaction_test.rb @@ -7,8 +7,9 @@ class TransactionTest < ActiveSupport::TestCase outflow_transaction = transactions(:checking_five) assert inflow_transaction.amount < 0 + assert inflow_transaction.inflow? + assert outflow_transaction.amount >= 0 - assert Transaction.inflows.include? inflow_transaction - assert Transaction.outflows.include? outflow_transaction + assert outflow_transaction.outflow? end end From 7d0ac15f4a1d1f96324239a994f76ffaee10afa9 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 07:48:01 -0400 Subject: [PATCH 16/31] Fix association ordering --- app/models/transaction.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/transaction.rb b/app/models/transaction.rb index d534f41908f..8be0768bc3d 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -6,8 +6,8 @@ class Transaction < ApplicationRecord belongs_to :account belongs_to :category, optional: true belongs_to :merchant, optional: true - has_many :tags, through: :taggings has_many :taggings, as: :taggable, dependent: :destroy + has_many :tags, through: :taggings validates :name, :date, :amount, :account, presence: true From 6d1a67c92f5744dc0b5a514cacceb556f6660793 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 12:52:22 -0400 Subject: [PATCH 17/31] Transaction controller cleanup part 3 --- app/controllers/transactions_controller.rb | 58 ++++----------- app/models/account.rb | 4 ++ app/models/transaction.rb | 16 +++++ .../transactions_controller_test.rb | 72 +++++++------------ test/models/account/syncable_test.rb | 12 ++++ test/models/account_test.rb | 7 ++ test/models/transaction_test.rb | 29 ++++++++ 7 files changed, 107 insertions(+), 91 deletions(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index d4fc61a207e..98e05e5b226 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -34,58 +34,30 @@ def create .find(params[:transaction][:account_id]) .transactions.build(transaction_params.merge(amount: amount)) - respond_to do |format| - if @transaction.save - @transaction.account.sync_later(@transaction.date) - format.html { redirect_to transactions_url, notice: t(".success") } - else - format.html { render :new, status: :unprocessable_entity } - end - end + @transaction.save! + @transaction.sync_account_later + redirect_to transactions_url, notice: t(".success") end def update - respond_to do |format| - sync_start_date = if transaction_params[:date] - [ @transaction.date, Date.parse(transaction_params[:date]) ].compact.min - else - @transaction.date - end - - if params[:transaction][:tag_id].present? - tag = Current.family.tags.find(params[:transaction][:tag_id]) - @transaction.tags << tag unless @transaction.tags.include?(tag) - end - - if params[:transaction][:remove_tag_id].present? - @transaction.tags.delete(params[:transaction][:remove_tag_id]) - end - - if @transaction.update(transaction_params) - @transaction.account.sync_later(sync_start_date) + if params[:transaction][:tag_id].present? + tag = Current.family.tags.find(params[:transaction][:tag_id]) + @transaction.tags << tag unless @transaction.tags.include?(tag) + end - format.html { redirect_to transaction_url(@transaction), notice: t(".success") } - format.turbo_stream do - render turbo_stream: [ - turbo_stream.append("notification-tray", partial: "shared/notification", locals: { type: "success", content: { body: t(".success") } }), - turbo_stream.replace("transaction_#{@transaction.id}", partial: "transactions/transaction", locals: { transaction: @transaction }) - ] - end - else - format.html { render :show, status: :unprocessable_entity } - end + if params[:transaction][:remove_tag_id].present? + @transaction.tags.delete(params[:transaction][:remove_tag_id]) end + + @transaction.update! transaction_params + @transaction.sync_account_later + redirect_to transaction_url(@transaction), notice: t(".success") end def destroy - @account = @transaction.account - sync_start_date = @account.transactions.where("date < ?", @transaction.date).order(date: :desc).first&.date @transaction.destroy! - @account.sync_later(sync_start_date) - - respond_to do |format| - format.html { redirect_to transactions_url, notice: t(".success") } - end + @transaction.sync_account_later + redirect_to transactions_url, notice: t(".success") end private diff --git a/app/models/account.rb b/app/models/account.rb index c77f498f53e..c57efe7b22d 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -26,6 +26,10 @@ def balance_on(date) balances.where("date <= ?", date).order(date: :desc).first&.balance end + def find_prior_transaction(date) + transactions.where("date < ?", date).order(date: :desc).first + end + # e.g. Wise, Revolut accounts that have transactions in multiple currencies def multi_currency? currencies = [ valuations.pluck(:currency), transactions.pluck(:currency) ].flatten.uniq diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 8be0768bc3d..0ff3a8dd4a4 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -38,6 +38,16 @@ def outflow? amount > 0 end + def sync_account_later + if destroyed? + sync_start_date = prior_transaction&.date + else + sync_start_date = [ date_previously_was, date ].compact.min + end + + account.sync_later(sync_start_date) + end + class << self def daily_totals(transactions, period: Period.last_30_days, currency: Current.family.currency) # Sum spending and income for each day in the period with the given currency @@ -78,4 +88,10 @@ def search(params) .on_or_before_date(params[:end_date]) end end + + private + + def prior_transaction + account.find_prior_transaction(self.date) + end end diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb index 7769f00c3d8..839e38c6a96 100644 --- a/test/controllers/transactions_controller_test.rb +++ b/test/controllers/transactions_controller_test.rb @@ -4,7 +4,6 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest setup do sign_in @user = users(:family_admin) @transaction = transactions(:checking_one) - @account = @transaction.account @recent_transactions = @user.family.transactions.ordered.limit(20).to_a end @@ -62,33 +61,23 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest end test "should create transaction" do - name = "transaction_name" - assert_difference("Transaction.count") do - post transactions_url, params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: @transaction.date, name: } } - end - - assert_redirected_to transactions_url - end - - test "create should sync account with correct start date" do - assert_enqueued_with(job: AccountSyncJob, args: [ @account, @transaction.date ]) do - post transactions_url, params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: @transaction.date, name: @transaction.name } } - end - end + account = @user.family.accounts.first + transaction_params = { + account_id: account.id, + amount: 100.45, + currency: "USD", + date: Date.current, + name: "Test transaction" + } - test "creation preserves decimals" do assert_difference("Transaction.count") do - post transactions_url, params: { transaction: { - nature: "expense", - account_id: @transaction.account_id, - amount: 123.45, - currency: @transaction.currency, - date: @transaction.date, - name: @transaction.name } } + post transactions_url, params: { transaction: transaction_params } end + assert_equal transaction_params[:amount].to_d, Transaction.order(created_at: :desc).first.amount + assert_equal flash[:notice], "New transaction created successfully" + assert_enqueued_with(job: AccountSyncJob) assert_redirected_to transactions_url - assert_equal 123.45.to_d, Transaction.order(created_at: :desc).first.amount end test "expenses are positive" do @@ -127,20 +116,18 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest end test "should update transaction" do - patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: @transaction.date, name: @transaction.name } } - assert_redirected_to transaction_url(@transaction) - end - - test "update should sync account with correct start date" do - new_date = @transaction.date - 1.day - assert_enqueued_with(job: AccountSyncJob, args: [ @account, new_date ]) do - patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: new_date, name: @transaction.name } } - end + patch transaction_url(@transaction), params: { + transaction: { + account_id: @transaction.account_id, + amount: @transaction.amount, + currency: @transaction.currency, + date: @transaction.date, + name: @transaction.name + } + } - new_date = @transaction.reload.date + 1.day - assert_enqueued_with(job: AccountSyncJob, args: [ @account, @transaction.date ]) do - patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: new_date, name: @transaction.name } } - end + assert_redirected_to transaction_url(@transaction) + assert_enqueued_with(job: AccountSyncJob) end test "should destroy transaction" do @@ -149,17 +136,6 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest end assert_redirected_to transactions_url - end - - test "destroy should sync account with correct start date" do - first, second = @transaction.account.transactions.order(:date).all - - assert_enqueued_with(job: AccountSyncJob, args: [ @account, first.date ]) do - delete transaction_url(second) - end - - assert_enqueued_with(job: AccountSyncJob, args: [ @account, nil ]) do - delete transaction_url(first) - end + assert_enqueued_with(job: AccountSyncJob) end end diff --git a/test/models/account/syncable_test.rb b/test/models/account/syncable_test.rb index 786083e83bb..cb9d2655a0d 100644 --- a/test/models/account/syncable_test.rb +++ b/test/models/account/syncable_test.rb @@ -1,6 +1,18 @@ require "test_helper" class Account::SyncableTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + setup do + @account = accounts(:savings_with_valuation_overrides) + end + + test "triggers sync job" do + assert_enqueued_with(job: AccountSyncJob, args: [ @account, Date.current ]) do + @account.sync_later(Date.current) + end + end + test "account has no balances until synced" do account = accounts(:savings_with_valuation_overrides) diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 949335c663d..6a4383d66da 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -17,6 +17,13 @@ def setup end end + test "finds prior transaction" do + transactions = @account.transactions.order(:date) + first_txn = transactions.first + second_txn = transactions.second + assert_equal first_txn, @account.find_prior_transaction(second_txn.date) + end + test "new account should be valid" do assert @account.valid? assert_not_nil @account.accountable_id diff --git a/test/models/transaction_test.rb b/test/models/transaction_test.rb index 0f3319e99bb..5e21f065df0 100644 --- a/test/models/transaction_test.rb +++ b/test/models/transaction_test.rb @@ -1,6 +1,10 @@ require "test_helper" class TransactionTest < ActiveSupport::TestCase + setup do + @transaction = transactions(:checking_one) + end + # See: https://github.com/maybe-finance/maybe/wiki/vision#signage-of-money test "negative amounts are inflows, positive amounts are outflows to an account" do inflow_transaction = transactions(:checking_four) @@ -12,4 +16,29 @@ class TransactionTest < ActiveSupport::TestCase assert outflow_transaction.amount >= 0 assert outflow_transaction.outflow? end + + test "triggers sync with correct start date when transaction is set to prior date" do + prior_date = @transaction.date - 1 + @transaction.update! date: prior_date + + @transaction.account.expects(:sync_later).with(prior_date) + @transaction.sync_account_later + end + + test "triggers sync with correct start date when transaction is set to future date" do + prior_date = @transaction.date + @transaction.update! date: @transaction.date + 1 + + @transaction.account.expects(:sync_later).with(prior_date) + @transaction.sync_account_later + end + + test "triggers sync with correct start date when transaction deleted" do + prior_transaction = transactions(:checking_two) # 12 days ago + current_transaction = transactions(:checking_one) # 5 days ago + current_transaction.destroy! + + current_transaction.account.expects(:sync_later).with(prior_transaction.date) + current_transaction.sync_account_later + end end From 7f7a289a7649464a5d8cfca41f81341812645fd3 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 15:39:30 -0400 Subject: [PATCH 18/31] Rework sync --- app/controllers/transactions_controller.rb | 6 ++--- app/models/account.rb | 5 ---- app/models/account/syncable.rb | 11 +++++++++ app/models/transaction.rb | 4 ++++ test/models/account/syncable_test.rb | 28 ++++++++++++++++++++++ test/models/account_test.rb | 7 ------ test/models/transaction_test.rb | 27 ++++----------------- 7 files changed, 50 insertions(+), 38 deletions(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 98e05e5b226..ee51b9f2176 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -35,7 +35,7 @@ def create .transactions.build(transaction_params.merge(amount: amount)) @transaction.save! - @transaction.sync_account_later + @transaction.account.sync_associated_record_change_later(@transaction) redirect_to transactions_url, notice: t(".success") end @@ -50,13 +50,13 @@ def update end @transaction.update! transaction_params - @transaction.sync_account_later + @transaction.account.sync_associated_record_change_later(@transaction) redirect_to transaction_url(@transaction), notice: t(".success") end def destroy @transaction.destroy! - @transaction.sync_account_later + @transaction.account.sync_associated_record_change_later(@transaction) redirect_to transactions_url, notice: t(".success") end diff --git a/app/models/account.rb b/app/models/account.rb index c57efe7b22d..0ab95bfc0f2 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -26,10 +26,6 @@ def balance_on(date) balances.where("date <= ?", date).order(date: :desc).first&.balance end - def find_prior_transaction(date) - transactions.where("date < ?", date).order(date: :desc).first - end - # e.g. Wise, Revolut accounts that have transactions in multiple currencies def multi_currency? currencies = [ valuations.pluck(:currency), transactions.pluck(:currency) ].flatten.uniq @@ -50,7 +46,6 @@ def self.some_syncing? exists?(status: "syncing") end - def series(period: Period.all, currency: self.currency) balance_series = balances.in_period(period).where(currency: Money::Currency.new(currency).iso_code) diff --git a/app/models/account/syncable.rb b/app/models/account/syncable.rb index 1b47caebe02..db02e0271d3 100644 --- a/app/models/account/syncable.rb +++ b/app/models/account/syncable.rb @@ -5,6 +5,17 @@ def sync_later(start_date = nil) AccountSyncJob.perform_later(self, start_date) end + def sync_associated_record_change_later(record) + if record.destroyed? + prior_record = record.previous_series_value + sync_start_date = prior_record&.date + else + sync_start_date = [ record.date_previously_was, record.date ].compact.min + end + + sync_later(sync_start_date) + end + def sync(start_date = nil) update!(status: "syncing") diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 0ff3a8dd4a4..69735915dc4 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -38,6 +38,10 @@ def outflow? amount > 0 end + def previous_series_value + self.account.transactions.where("date < ?", date).order(date: :desc).first + end + def sync_account_later if destroyed? sync_start_date = prior_transaction&.date diff --git a/test/models/account/syncable_test.rb b/test/models/account/syncable_test.rb index cb9d2655a0d..948c34cf067 100644 --- a/test/models/account/syncable_test.rb +++ b/test/models/account/syncable_test.rb @@ -98,4 +98,32 @@ class Account::SyncableTest < ActiveSupport::TestCase account.sync end end + + test "triggers sync with correct start date when transaction is set to prior date" do + transaction = @account.transactions.order(:date).last + prior_date = transaction.date - 1 + transaction.update! date: prior_date + + @account.expects(:sync_later).with(prior_date) + @account.sync_associated_record_change_later(transaction) + end + + test "triggers sync with correct start date when transaction is set to future date" do + transaction = @account.transactions.order(:date).last + prior_date = transaction.date + transaction.update! date: transaction.date + 1 + + @account.expects(:sync_later).with(prior_date) + @account.sync_associated_record_change_later(transaction) + end + + test "triggers sync with correct start date when transaction deleted" do + prior_transaction = @account.transactions.order(:date)[-2] + current_transaction = @account.transactions.order(:date)[-1] + + current_transaction.destroy! + + @account.expects(:sync_later).with(prior_transaction.date) + @account.sync_associated_record_change_later(current_transaction) + end end diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 6a4383d66da..949335c663d 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -17,13 +17,6 @@ def setup end end - test "finds prior transaction" do - transactions = @account.transactions.order(:date) - first_txn = transactions.first - second_txn = transactions.second - assert_equal first_txn, @account.find_prior_transaction(second_txn.date) - end - test "new account should be valid" do assert @account.valid? assert_not_nil @account.accountable_id diff --git a/test/models/transaction_test.rb b/test/models/transaction_test.rb index 5e21f065df0..79b10168df1 100644 --- a/test/models/transaction_test.rb +++ b/test/models/transaction_test.rb @@ -17,28 +17,9 @@ class TransactionTest < ActiveSupport::TestCase assert outflow_transaction.outflow? end - test "triggers sync with correct start date when transaction is set to prior date" do - prior_date = @transaction.date - 1 - @transaction.update! date: prior_date - - @transaction.account.expects(:sync_later).with(prior_date) - @transaction.sync_account_later - end - - test "triggers sync with correct start date when transaction is set to future date" do - prior_date = @transaction.date - @transaction.update! date: @transaction.date + 1 - - @transaction.account.expects(:sync_later).with(prior_date) - @transaction.sync_account_later - end - - test "triggers sync with correct start date when transaction deleted" do - prior_transaction = transactions(:checking_two) # 12 days ago - current_transaction = transactions(:checking_one) # 5 days ago - current_transaction.destroy! - - current_transaction.account.expects(:sync_later).with(prior_transaction.date) - current_transaction.sync_account_later + test "finds prior series value" do + latest_txn = transactions(:checking_one) + prior_txn = transactions(:checking_two) + assert_equal prior_txn, latest_txn.previous_series_value end end From 2209fa1e0ad1c7c71f68b265e00575a3a8441766 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 15:39:52 -0400 Subject: [PATCH 19/31] Revert "Rework sync" This reverts commit 7f7a289a7649464a5d8cfca41f81341812645fd3. --- app/controllers/transactions_controller.rb | 6 ++--- app/models/account.rb | 5 ++++ app/models/account/syncable.rb | 11 --------- app/models/transaction.rb | 4 ---- test/models/account/syncable_test.rb | 28 ---------------------- test/models/account_test.rb | 7 ++++++ test/models/transaction_test.rb | 27 +++++++++++++++++---- 7 files changed, 38 insertions(+), 50 deletions(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index ee51b9f2176..98e05e5b226 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -35,7 +35,7 @@ def create .transactions.build(transaction_params.merge(amount: amount)) @transaction.save! - @transaction.account.sync_associated_record_change_later(@transaction) + @transaction.sync_account_later redirect_to transactions_url, notice: t(".success") end @@ -50,13 +50,13 @@ def update end @transaction.update! transaction_params - @transaction.account.sync_associated_record_change_later(@transaction) + @transaction.sync_account_later redirect_to transaction_url(@transaction), notice: t(".success") end def destroy @transaction.destroy! - @transaction.account.sync_associated_record_change_later(@transaction) + @transaction.sync_account_later redirect_to transactions_url, notice: t(".success") end diff --git a/app/models/account.rb b/app/models/account.rb index 0ab95bfc0f2..c57efe7b22d 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -26,6 +26,10 @@ def balance_on(date) balances.where("date <= ?", date).order(date: :desc).first&.balance end + def find_prior_transaction(date) + transactions.where("date < ?", date).order(date: :desc).first + end + # e.g. Wise, Revolut accounts that have transactions in multiple currencies def multi_currency? currencies = [ valuations.pluck(:currency), transactions.pluck(:currency) ].flatten.uniq @@ -46,6 +50,7 @@ def self.some_syncing? exists?(status: "syncing") end + def series(period: Period.all, currency: self.currency) balance_series = balances.in_period(period).where(currency: Money::Currency.new(currency).iso_code) diff --git a/app/models/account/syncable.rb b/app/models/account/syncable.rb index db02e0271d3..1b47caebe02 100644 --- a/app/models/account/syncable.rb +++ b/app/models/account/syncable.rb @@ -5,17 +5,6 @@ def sync_later(start_date = nil) AccountSyncJob.perform_later(self, start_date) end - def sync_associated_record_change_later(record) - if record.destroyed? - prior_record = record.previous_series_value - sync_start_date = prior_record&.date - else - sync_start_date = [ record.date_previously_was, record.date ].compact.min - end - - sync_later(sync_start_date) - end - def sync(start_date = nil) update!(status: "syncing") diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 69735915dc4..0ff3a8dd4a4 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -38,10 +38,6 @@ def outflow? amount > 0 end - def previous_series_value - self.account.transactions.where("date < ?", date).order(date: :desc).first - end - def sync_account_later if destroyed? sync_start_date = prior_transaction&.date diff --git a/test/models/account/syncable_test.rb b/test/models/account/syncable_test.rb index 948c34cf067..cb9d2655a0d 100644 --- a/test/models/account/syncable_test.rb +++ b/test/models/account/syncable_test.rb @@ -98,32 +98,4 @@ class Account::SyncableTest < ActiveSupport::TestCase account.sync end end - - test "triggers sync with correct start date when transaction is set to prior date" do - transaction = @account.transactions.order(:date).last - prior_date = transaction.date - 1 - transaction.update! date: prior_date - - @account.expects(:sync_later).with(prior_date) - @account.sync_associated_record_change_later(transaction) - end - - test "triggers sync with correct start date when transaction is set to future date" do - transaction = @account.transactions.order(:date).last - prior_date = transaction.date - transaction.update! date: transaction.date + 1 - - @account.expects(:sync_later).with(prior_date) - @account.sync_associated_record_change_later(transaction) - end - - test "triggers sync with correct start date when transaction deleted" do - prior_transaction = @account.transactions.order(:date)[-2] - current_transaction = @account.transactions.order(:date)[-1] - - current_transaction.destroy! - - @account.expects(:sync_later).with(prior_transaction.date) - @account.sync_associated_record_change_later(current_transaction) - end end diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 949335c663d..6a4383d66da 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -17,6 +17,13 @@ def setup end end + test "finds prior transaction" do + transactions = @account.transactions.order(:date) + first_txn = transactions.first + second_txn = transactions.second + assert_equal first_txn, @account.find_prior_transaction(second_txn.date) + end + test "new account should be valid" do assert @account.valid? assert_not_nil @account.accountable_id diff --git a/test/models/transaction_test.rb b/test/models/transaction_test.rb index 79b10168df1..5e21f065df0 100644 --- a/test/models/transaction_test.rb +++ b/test/models/transaction_test.rb @@ -17,9 +17,28 @@ class TransactionTest < ActiveSupport::TestCase assert outflow_transaction.outflow? end - test "finds prior series value" do - latest_txn = transactions(:checking_one) - prior_txn = transactions(:checking_two) - assert_equal prior_txn, latest_txn.previous_series_value + test "triggers sync with correct start date when transaction is set to prior date" do + prior_date = @transaction.date - 1 + @transaction.update! date: prior_date + + @transaction.account.expects(:sync_later).with(prior_date) + @transaction.sync_account_later + end + + test "triggers sync with correct start date when transaction is set to future date" do + prior_date = @transaction.date + @transaction.update! date: @transaction.date + 1 + + @transaction.account.expects(:sync_later).with(prior_date) + @transaction.sync_account_later + end + + test "triggers sync with correct start date when transaction deleted" do + prior_transaction = transactions(:checking_two) # 12 days ago + current_transaction = transactions(:checking_one) # 5 days ago + current_transaction.destroy! + + current_transaction.account.expects(:sync_later).with(prior_transaction.date) + current_transaction.sync_account_later end end From e01057fe6efe3dac6d601489c8a3cf92292e88f9 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 16:22:09 -0400 Subject: [PATCH 20/31] Tweaks to transaction model --- app/models/account.rb | 4 ---- app/models/transaction.rb | 10 +++++++--- test/models/account_test.rb | 7 ------- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index c57efe7b22d..c77f498f53e 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -26,10 +26,6 @@ def balance_on(date) balances.where("date <= ?", date).order(date: :desc).first&.balance end - def find_prior_transaction(date) - transactions.where("date < ?", date).order(date: :desc).first - end - # e.g. Wise, Revolut accounts that have transactions in multiple currencies def multi_currency? currencies = [ valuations.pluck(:currency), transactions.pluck(:currency) ].flatten.uniq diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 0ff3a8dd4a4..58958e79bcd 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -40,7 +40,7 @@ def outflow? def sync_account_later if destroyed? - sync_start_date = prior_transaction&.date + sync_start_date = previous_transaction_date else sync_start_date = [ date_previously_was, date ].compact.min end @@ -91,7 +91,11 @@ def search(params) private - def prior_transaction - account.find_prior_transaction(self.date) + def previous_transaction_date + self.account + .transactions + .where("date < ?", date) + .order(date: :desc) + .first&.date end end diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 6a4383d66da..949335c663d 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -17,13 +17,6 @@ def setup end end - test "finds prior transaction" do - transactions = @account.transactions.order(:date) - first_txn = transactions.first - second_txn = transactions.second - assert_equal first_txn, @account.find_prior_transaction(second_txn.date) - end - test "new account should be valid" do assert @account.valid? assert_not_nil @account.accountable_id From 3080f213cefdd35cc87ad9299566c2556f2608df Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 17:42:00 -0400 Subject: [PATCH 21/31] Render transaction amount sign correctly --- app/views/accounts/transactions/_transaction.html.erb | 6 +++--- .../pages/dashboard/transactions/_transaction.html.erb | 7 +++---- app/views/transactions/_amount.html.erb | 3 +++ app/views/transactions/_transaction.html.erb | 6 +++--- 4 files changed, 12 insertions(+), 10 deletions(-) create mode 100644 app/views/transactions/_amount.html.erb diff --git a/app/views/accounts/transactions/_transaction.html.erb b/app/views/accounts/transactions/_transaction.html.erb index 4d9d070ba32..b69d6b327fc 100644 --- a/app/views/accounts/transactions/_transaction.html.erb +++ b/app/views/accounts/transactions/_transaction.html.erb @@ -7,7 +7,7 @@ <%= render "transactions/categories/menu", transaction: transaction %>
    - <%= content_tag :p, - format_money(transaction.amount_money), - class: ["col-span-1 ml-auto", "text-green-600": transaction.amount.negative?] %> +
    + <%= render "transactions/amount", transaction: transaction %> +
    diff --git a/app/views/pages/dashboard/transactions/_transaction.html.erb b/app/views/pages/dashboard/transactions/_transaction.html.erb index 06bc7afe187..b8dbc57b3dc 100644 --- a/app/views/pages/dashboard/transactions/_transaction.html.erb +++ b/app/views/pages/dashboard/transactions/_transaction.html.erb @@ -1,10 +1,9 @@
    -
    <%= render "transactions/name", transaction: transaction %>
    - <%= content_tag :p, - format_money(transaction.amount_money), - class: ["ml-auto", "text-green-600": transaction.amount.negative?] %> +
    + <%= render "transactions/amount", transaction: transaction %> +
    diff --git a/app/views/transactions/_amount.html.erb b/app/views/transactions/_amount.html.erb new file mode 100644 index 00000000000..6c618400c78 --- /dev/null +++ b/app/views/transactions/_amount.html.erb @@ -0,0 +1,3 @@ +<%= content_tag :p, + format_money(-transaction.amount_money), + class: ["text-green-600": transaction.amount.negative?] %> \ No newline at end of file diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index c2c61013caf..b6451b8e9e9 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -11,7 +11,7 @@ account_path(transaction.account), class: ["col-span-3 hover:underline"] %> - <%= content_tag :p, - format_money(transaction.amount_money), - class: ["col-span-2 ml-auto", "text-green-600": transaction.amount.negative?] %> +
    + <%= render "amount", transaction: transaction %> +
    <% end %> From 9aeb10ffa159df5b8f3844824547c802dae8efab Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Wed, 29 May 2024 17:43:00 -0400 Subject: [PATCH 22/31] Add back simplified transaction turbo stream --- app/controllers/transactions_controller.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 98e05e5b226..0d43b098500 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -51,7 +51,11 @@ def update @transaction.update! transaction_params @transaction.sync_account_later - redirect_to transaction_url(@transaction), notice: t(".success") + + respond_to do |format| + format.turbo_stream { render turbo_stream: turbo_stream.replace(@transaction) } + format.html { redirect_to transaction_url(@transaction), notice: t(".success") } + end end def destroy From c41af4157f9e8a111175bc2019bbbfdaa99347d1 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 13:00:28 -0400 Subject: [PATCH 23/31] Consolidate transaction partials and fix form update frame redirects --- .../transactions/rows_controller.rb | 22 ++++++ app/controllers/transactions_controller.rb | 24 ++---- app/helpers/transactions_helper.rb | 19 ++++- app/models/transaction.rb | 32 ++++---- app/views/accounts/_transactions.html.erb | 3 +- .../transactions/_date_group.html.erb | 10 --- .../transactions/_transaction.html.erb | 13 ---- app/views/imports/confirm.html.erb | 4 +- .../transactions/_transaction.html.erb | 14 ++-- .../transactions/_transaction_group.html.erb | 13 ---- app/views/pages/dashboard.html.erb | 2 +- .../transactions/_date_group.html.erb | 10 --- app/views/shared/_list_group.html.erb | 9 +++ app/views/transactions/_amount.html.erb | 2 +- app/views/transactions/_name.html.erb | 17 +++-- app/views/transactions/_transaction.html.erb | 6 +- .../categories/dropdowns/_row.html.erb | 2 +- .../categories/dropdowns/show.html.erb | 3 +- app/views/transactions/index.html.erb | 2 +- app/views/transactions/rows/show.html.erb | 1 + app/views/transactions/show.html.erb | 25 +++--- config/locales/views/transactions/en.yml | 2 + config/routes.rb | 2 + lib/tasks/demo_data.rake | 76 +++++++++++++------ 24 files changed, 170 insertions(+), 143 deletions(-) create mode 100644 app/controllers/transactions/rows_controller.rb delete mode 100644 app/views/accounts/transactions/_date_group.html.erb delete mode 100644 app/views/accounts/transactions/_transaction.html.erb delete mode 100644 app/views/imports/transactions/_transaction_group.html.erb delete mode 100644 app/views/pages/dashboard/transactions/_date_group.html.erb create mode 100644 app/views/shared/_list_group.html.erb create mode 100644 app/views/transactions/rows/show.html.erb diff --git a/app/controllers/transactions/rows_controller.rb b/app/controllers/transactions/rows_controller.rb new file mode 100644 index 00000000000..159e75a9c78 --- /dev/null +++ b/app/controllers/transactions/rows_controller.rb @@ -0,0 +1,22 @@ +class Transactions::RowsController < ApplicationController + before_action :set_transaction, only: %i[ show update ] + + def show + end + + def update + @transaction.update! transaction_params + + redirect_to transaction_row_path(@transaction) + end + + private + + def transaction_params + params.require(:transaction).permit(:category_id) + end + + def set_transaction + @transaction = Current.family.transactions.find(params[:id]) + end +end diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 0d43b098500..bc12c6772d5 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -9,9 +9,9 @@ def index @pagy, @transactions = pagy(result, items: 50) @totals = { - count: @transactions.count, - income: @transactions.select { |t| t.inflow? }.sum(&:amount_money).abs, - expense: @transactions.select { |t| t.outflow? }.sum(&:amount_money).abs + count: result.count, + income: result.inflows.sum(&:amount_money).abs, + expense: result.outflows.sum(&:amount_money).abs } end @@ -40,22 +40,10 @@ def create end def update - if params[:transaction][:tag_id].present? - tag = Current.family.tags.find(params[:transaction][:tag_id]) - @transaction.tags << tag unless @transaction.tags.include?(tag) - end - - if params[:transaction][:remove_tag_id].present? - @transaction.tags.delete(params[:transaction][:remove_tag_id]) - end - @transaction.update! transaction_params @transaction.sync_account_later - respond_to do |format| - format.turbo_stream { render turbo_stream: turbo_stream.replace(@transaction) } - format.html { redirect_to transaction_url(@transaction), notice: t(".success") } - end + redirect_to transaction_url(@transaction), notice: t(".success") end def destroy @@ -83,10 +71,10 @@ def nature end def search_params - params.fetch(:q, {}).permit(:start_date, :end_date, :search, accounts: [], categories: [], merchants: []) + params.fetch(:q, {}).permit(:start_date, :end_date, :search, accounts: [], account_ids: [], categories: [], merchants: []) end def transaction_params - params.require(:transaction).permit(:name, :date, :amount, :currency, :notes, :excluded, :category_id, :merchant_id, :tag_id, :remove_tag_id).except(:tag_id, :remove_tag_id) + params.require(:transaction).permit(:name, :date, :amount, :currency, :notes, :excluded, :category_id, :merchant_id, tag_ids: [], taggings_attributes: [ :id, :tag_id, :_destroy ]) end end diff --git a/app/helpers/transactions_helper.rb b/app/helpers/transactions_helper.rb index 0872260a62c..f5f4e9c4e22 100644 --- a/app/helpers/transactions_helper.rb +++ b/app/helpers/transactions_helper.rb @@ -1,5 +1,20 @@ module TransactionsHelper - def full_width_transaction_row?(route) - route != "/" + def transactions_group(date, transactions, transaction_partial_path = "transactions/transaction") + header_left = content_tag :span do + "#{date.strftime('%b %d, %Y')} ยท #{transactions.size}".html_safe + end + + header_right = content_tag :span do + format_money(-transactions.sum(&:amount_money)) + end + + header = header_left.concat(header_right) + + content = render partial: transaction_partial_path, collection: transactions + + render partial: "shared/list_group", locals: { + header: header, + content: content + } end end diff --git a/app/models/transaction.rb b/app/models/transaction.rb index 58958e79bcd..4ffdc137bca 100644 --- a/app/models/transaction.rb +++ b/app/models/transaction.rb @@ -8,17 +8,21 @@ class Transaction < ApplicationRecord belongs_to :merchant, optional: true has_many :taggings, as: :taggable, dependent: :destroy has_many :tags, through: :taggings + accepts_nested_attributes_for :taggings, allow_destroy: true validates :name, :date, :amount, :account, presence: true scope :ordered, -> { order(date: :desc) } scope :active, -> { where(excluded: false) } - scope :by_name, ->(name) { where("transactions.name ILIKE ?", "%#{name}%") if name.present? } - scope :with_categories, ->(categories) { joins(:category).where(transaction_categories: { name: categories }) if categories.present? } - scope :with_accounts, ->(accounts) { joins(:account).where(accounts: { name: accounts }) if accounts.present? } - scope :with_merchants, ->(merchants) { joins(:merchant).where(transaction_merchants: { name: merchants }) if merchants.present? } - scope :on_or_after_date, ->(date) { where("transactions.date >= ?", date) if date.present? } - scope :on_or_before_date, ->(date) { where("transactions.date <= ?", date) if date.present? } + scope :inflows, -> { where("amount <= 0") } + scope :outflows, -> { where("amount > 0") } + scope :by_name, ->(name) { where("transactions.name ILIKE ?", "%#{name}%") } + scope :with_categories, ->(categories) { joins(:category).where(transaction_categories: { name: categories }) } + scope :with_accounts, ->(accounts) { joins(:account).where(accounts: { name: accounts }) } + scope :with_account_ids, ->(account_ids) { joins(:account).where(accounts: { id: account_ids }) } + scope :with_merchants, ->(merchants) { joins(:merchant).where(transaction_merchants: { name: merchants }) } + scope :on_or_after_date, ->(date) { where("transactions.date >= ?", date) } + scope :on_or_before_date, ->(date) { where("transactions.date <= ?", date) } scope :with_converted_amount, ->(currency = Current.family.currency) { # Join with exchange rates to convert the amount to the given currency # If no rate is available, exclude the transaction from the results @@ -79,13 +83,15 @@ def daily_rolling_totals(transactions, period: Period.last_30_days, currency: Cu end def search(params) - all - .by_name(params[:search]) - .with_categories(params[:categories]) - .with_accounts(params[:accounts]) - .with_merchants(params[:merchants]) - .on_or_after_date(params[:start_date]) - .on_or_before_date(params[:end_date]) + query = all + query = query.by_name(params[:search]) if params[:search].present? + query = query.with_categories(params[:categories]) if params[:categories].present? + query = query.with_accounts(params[:accounts]) if params[:accounts].present? + query = query.with_account_ids(params[:account_ids]) if params[:account_ids].present? + query = query.with_merchants(params[:merchants]) if params[:merchants].present? + query = query.on_or_after_date(params[:start_date]) if params[:start_date].present? + query = query.on_or_before_date(params[:end_date]) if params[:end_date].present? + query end end diff --git a/app/views/accounts/_transactions.html.erb b/app/views/accounts/_transactions.html.erb index 5c4f63131b3..c60eef767d7 100644 --- a/app/views/accounts/_transactions.html.erb +++ b/app/views/accounts/_transactions.html.erb @@ -7,12 +7,13 @@ New transaction <% end %>
    + <% if transactions.empty? %>

    No transactions for this account yet.

    <% else %>
    <% transactions.group_by(&:date).each do |date, transactions| %> - <%= render partial: "accounts/transactions/date_group", locals: { date: date, transactions: transactions } %> + <%= transactions_group(date, transactions) %> <% end %>
    <% end %> diff --git a/app/views/accounts/transactions/_date_group.html.erb b/app/views/accounts/transactions/_date_group.html.erb deleted file mode 100644 index f489fd1c213..00000000000 --- a/app/views/accounts/transactions/_date_group.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<%# locals: (date:, transactions:) %> -
    -
    -

    <%= date.strftime("%b %d, %Y") %> · <%= transactions.size %>

    - <%= format_money -transactions.sum(&:amount_money) %> -
    -
    - <%= render partial: "accounts/transactions/transaction", collection: transactions %> -
    -
    diff --git a/app/views/accounts/transactions/_transaction.html.erb b/app/views/accounts/transactions/_transaction.html.erb deleted file mode 100644 index b69d6b327fc..00000000000 --- a/app/views/accounts/transactions/_transaction.html.erb +++ /dev/null @@ -1,13 +0,0 @@ -
    -
    - <%= render "transactions/name", transaction: transaction %> -
    - -
    - <%= render "transactions/categories/menu", transaction: transaction %> -
    - -
    - <%= render "transactions/amount", transaction: transaction %> -
    -
    diff --git a/app/views/imports/confirm.html.erb b/app/views/imports/confirm.html.erb index 56ac1975428..32694dff232 100644 --- a/app/views/imports/confirm.html.erb +++ b/app/views/imports/confirm.html.erb @@ -9,7 +9,9 @@
    - <%= render partial: "imports/transactions/transaction_group", collection: @import.dry_run.group_by(&:date) %> + <% @import.dry_run.group_by(&:date).each do |date, draft_transactions| %> + <%= transactions_group(date, draft_transactions, "imports/transactions/transaction") %> + <% end %>
    <%= button_to "Import " + @import.csv.table.size.to_s + " transactions", confirm_import_path(@import), method: :patch, class: "px-4 py-2 block w-60 text-center mx-auto rounded-lg bg-gray-900 text-white text-sm font-medium", data: { turbo: false } %> diff --git a/app/views/imports/transactions/_transaction.html.erb b/app/views/imports/transactions/_transaction.html.erb index 4ff851ef632..a467c3c30e4 100644 --- a/app/views/imports/transactions/_transaction.html.erb +++ b/app/views/imports/transactions/_transaction.html.erb @@ -1,18 +1,20 @@ <%# locals: (transaction:) %> -
    - <%= render "transactions/name", transaction: transaction %> +
    +
    + <%= render "transactions/name", transaction: transaction %> +
    -
    +
    <%= render partial: "transactions/categories/badge", locals: { category: transaction.category } %>
    -
    +
    <% transaction.tags.each do |tag| %> <%= render partial: "tags/badge", locals: { tag: tag } %> <% end %>
    -
    - <%= content_tag :p, format_money(Money.new(-transaction.amount, @import.account.currency)), class: ["whitespace-nowrap", BigDecimal(transaction.amount).negative? ? "text-green-600" : "text-red-600"] %> +
    + <%= render "transactions/amount", transaction: transaction %>
    diff --git a/app/views/imports/transactions/_transaction_group.html.erb b/app/views/imports/transactions/_transaction_group.html.erb deleted file mode 100644 index fae987924ba..00000000000 --- a/app/views/imports/transactions/_transaction_group.html.erb +++ /dev/null @@ -1,13 +0,0 @@ -<%# locals: (transaction_group:) %> -<% date = transaction_group[0] %> -<% transactions = transaction_group[1] %> - -
    -
    -

    <%= date.strftime("%b %d, %Y") %> · <%= transactions.size %>

    - <%= format_money Money.new(-transactions.sum { |t| t.amount }, @import.account.currency) %> -
    -
    - <%= render partial: "imports/transactions/transaction", collection: transactions %> -
    -
    diff --git a/app/views/pages/dashboard.html.erb b/app/views/pages/dashboard.html.erb index 2ccd4d51f9f..a240cc97aa3 100644 --- a/app/views/pages/dashboard.html.erb +++ b/app/views/pages/dashboard.html.erb @@ -163,7 +163,7 @@ <% else %>
    <% @transactions.group_by(&:date).each do |date, transactions| %> - <%= render partial: "pages/dashboard/transactions/date_group", locals: { date: date, transactions: transactions } %> + <%= transactions_group(date, transactions, "pages/dashboard/transactions/transaction") %> <% end %>

    <%= link_to t(".view_all"), transactions_path %>

    diff --git a/app/views/pages/dashboard/transactions/_date_group.html.erb b/app/views/pages/dashboard/transactions/_date_group.html.erb deleted file mode 100644 index 961d97e45a8..00000000000 --- a/app/views/pages/dashboard/transactions/_date_group.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<%# locals: (date:, transactions:) %> -
    -
    -

    <%= date.strftime("%b %d, %Y") %> · <%= transactions.size %>

    - <%= format_money -transactions.sum(&:amount_money) %> -
    -
    - <%= render partial: "pages/dashboard/transactions/transaction", collection: transactions %> -
    -
    diff --git a/app/views/shared/_list_group.html.erb b/app/views/shared/_list_group.html.erb new file mode 100644 index 00000000000..526bb503d7d --- /dev/null +++ b/app/views/shared/_list_group.html.erb @@ -0,0 +1,9 @@ +<%# locals: (header:, content:) %> +
    +
    + <%= header %> +
    +
    + <%= content %> +
    +
    diff --git a/app/views/transactions/_amount.html.erb b/app/views/transactions/_amount.html.erb index 6c618400c78..1ebf81ff726 100644 --- a/app/views/transactions/_amount.html.erb +++ b/app/views/transactions/_amount.html.erb @@ -1,3 +1,3 @@ <%= content_tag :p, format_money(-transaction.amount_money), - class: ["text-green-600": transaction.amount.negative?] %> \ No newline at end of file + class: ["text-green-600": transaction.inflow?] %> diff --git a/app/views/transactions/_name.html.erb b/app/views/transactions/_name.html.erb index d9d181c70d6..6f27f324ec3 100644 --- a/app/views/transactions/_name.html.erb +++ b/app/views/transactions/_name.html.erb @@ -3,11 +3,14 @@ <%= transaction.name[0].upcase %>
    - <% if transaction.new_record? %> - <%= content_tag :p, transaction.name, class: "text-gray-900 hover:underline hover:text-gray-800 truncate" %> - <% else %> - <%= link_to transaction.name, - transaction_path(transaction), - class: "text-gray-900 hover:underline hover:text-gray-800 truncate" %> - <% end %> +
    + <% if transaction.new_record? %> + <%= content_tag :p, transaction.name %> + <% else %> + <%= link_to transaction.name, + transaction_path(transaction), + data: { turbo_frame: "drawer" }, + class: "hover:underline hover:text-gray-800" %> + <% end %> +
    <% end %> diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index b6451b8e9e9..667ba49dbe0 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -1,6 +1,6 @@ -<%= content_tag :div, id: dom_id(transaction), class: "grid grid-cols-12 items-center text-gray-900 py-4 text-sm font-medium px-4" do %> +<%= turbo_frame_tag dom_id(transaction), class: "grid grid-cols-12 items-center text-gray-900 py-4 text-sm font-medium px-4" do %>
    - <%= render "name", transaction: transaction %> + <%= render "transactions/name", transaction: transaction %>
    @@ -12,6 +12,6 @@ class: ["col-span-3 hover:underline"] %>
    - <%= render "amount", transaction: transaction %> + <%= render "transactions/amount", transaction: transaction %>
    <% end %> diff --git a/app/views/transactions/categories/dropdowns/_row.html.erb b/app/views/transactions/categories/dropdowns/_row.html.erb index 3a088b3f565..8092ce394a6 100644 --- a/app/views/transactions/categories/dropdowns/_row.html.erb +++ b/app/views/transactions/categories/dropdowns/_row.html.erb @@ -2,7 +2,7 @@ <% is_selected = category.id === @selected_category&.id %> <%= content_tag :div, class: ["filterable-item flex justify-between items-center border-none rounded-lg px-2 py-1 group w-full", { "bg-gray-25": is_selected }], data: { filter_name: category.name } do %> - <%= button_to transaction_path(@transaction, transaction: { category_id: category.id }), method: :patch, class: "flex w-full items-center gap-1.5 cursor-pointer" do %> + <%= button_to transaction_row_path(@transaction, transaction: { category_id: category.id }), method: :patch, data: { turbo_frame: dom_id(@transaction) }, class: "flex w-full items-center gap-1.5 cursor-pointer" do %> <%= lucide_icon("check", class: "w-5 h-5 text-gray-500") if is_selected %> diff --git a/app/views/transactions/categories/dropdowns/show.html.erb b/app/views/transactions/categories/dropdowns/show.html.erb index b88c066c536..b36926d6699 100644 --- a/app/views/transactions/categories/dropdowns/show.html.erb +++ b/app/views/transactions/categories/dropdowns/show.html.erb @@ -25,8 +25,9 @@ <% end %> <% if @transaction.category %> - <%= button_to transaction_path(@transaction), + <%= button_to transaction_row_path(@transaction), method: :patch, + data: { turbo_frame: dom_id(@transaction) }, params: { transaction: { category_id: nil } }, class: "flex text-sm font-medium items-center gap-2 text-gray-500 w-full rounded-lg p-2 hover:bg-gray-100" do %> <%= lucide_icon "minus", class: "w-5 h-5" %> diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb index 6bee2a3511a..b624e2d5a5f 100644 --- a/app/views/transactions/index.html.erb +++ b/app/views/transactions/index.html.erb @@ -17,7 +17,7 @@
    <% @transactions.group_by(&:date).each do |date, transactions| %> - <%= render "date_group", date: date, transactions: transactions %> + <%= transactions_group(date, transactions) %> <% end %>
    <% else %> diff --git a/app/views/transactions/rows/show.html.erb b/app/views/transactions/rows/show.html.erb new file mode 100644 index 00000000000..e40ab43eed0 --- /dev/null +++ b/app/views/transactions/rows/show.html.erb @@ -0,0 +1 @@ +<%= render "transactions/transaction", transaction: @transaction %> diff --git a/app/views/transactions/show.html.erb b/app/views/transactions/show.html.erb index 8cc963f9520..a4649eb460a 100644 --- a/app/views/transactions/show.html.erb +++ b/app/views/transactions/show.html.erb @@ -62,22 +62,15 @@
    - - <% if @transaction.tags.any? %> -
    - <% @transaction.tags.each do |tag| %> -
    - <%= render partial: "tags/badge", locals: { tag: tag } %> - <%= button_to transaction_path(@transaction, transaction: { remove_tag_id: tag.id }), method: :patch, "data-turbo": false, class: "absolute -top-2 -right-1 px-0.5 py rounded-full hover:bg-alpha-black-200 border border-alpha-black-100" do %> - <%= lucide_icon("x", class: "w-3 h-3") %> - <% end %> -
    - <% end %> -
    - <% end %> - - <%= form_with model: @transaction, html: { data: { controller: "auto-submit-form", turbo: false } } do |f| %> - <%= f.collection_select :tag_id, Current.family.tags.alphabetically.excluding(@transaction.tags), :id, :name, { prompt: "Select a tag", label: "Select a tag", class: "placeholder:text-gray-500" }, "data-auto-submit-form-target": "auto", "data-turbo": false %> + <%= form_with model: @transaction, html: { data: { controller: "auto-submit-form" } } do |f| %> + <%= f.select :tag_ids, + options_for_select(Current.family.tags.alphabetically.pluck(:name, :id), @transaction.tag_ids), + { + multiple: true, + label: t(".select_tags"), + class: "placeholder:text-gray-500" + }, + "data-auto-submit-form-target": "auto" %> <% end %>
    diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml index b8c66d41f97..f61664f1899 100644 --- a/config/locales/views/transactions/en.yml +++ b/config/locales/views/transactions/en.yml @@ -93,5 +93,7 @@ en: title: New merchant update: success: Merchant updated successfully + show: + select_tags: Select one or more tags update: success: Transaction updated successfully diff --git a/config/routes.rb b/config/routes.rb index 9ff307d20cb..4afc82669a4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -46,6 +46,8 @@ match "search" => "transactions#search", via: %i[ get post ] scope module: :transactions, as: :transaction do + resources :rows, only: %i[ show update ] + resources :categories do resources :deletions, only: %i[ new create ], module: :categories collection do diff --git a/lib/tasks/demo_data.rake b/lib/tasks/demo_data.rake index 2c3cad6bb82..e5d97cf5b81 100644 --- a/lib/tasks/demo_data.rake +++ b/lib/tasks/demo_data.rake @@ -6,6 +6,8 @@ namespace :demo_data do family.accounts.delete_all ExchangeRate.delete_all family.transaction_categories.delete_all + Tagging.delete_all + family.tags.delete_all Transaction::Category.create_default_categories(family) user = User.find_or_create_by(email: "user@maybe.local") do |u| @@ -17,6 +19,15 @@ namespace :demo_data do puts "Reset user: #{user.email} with family: #{family.name}" + # Tags + tags = [ + { name: "Hawaii Trip", color: "#e99537" }, + { name: "Trips", color: "#4da568" }, + { name: "Emergency Fund", color: "#db5a54" } + ] + + family.tags.insert_all(tags) + # Mock exchange rates for last 60 days (these rates are reasonable for EUR:USD, but not exact) exchange_rates = (0..60).map do |days_ago| { @@ -69,16 +80,16 @@ namespace :demo_data do # ========== Transactions ================ multi_currency_checking_transactions = [ - { date: Date.today - 45, amount: 3000, name: "Paycheck", currency: "USD" }, - { date: Date.today - 41, amount: -1500, name: "Rent Payment", currency: "EUR" }, - { date: Date.today - 39, amount: -200, name: "Groceries", currency: "EUR" }, - { date: Date.today - 34, amount: 3000, name: "Paycheck", currency: "USD" }, - { date: Date.today - 31, amount: -1500, name: "Rent Payment", currency: "EUR" }, - { date: Date.today - 28, amount: -100, name: "Utilities", currency: "EUR" }, - { date: Date.today - 28, amount: 3000, name: "Paycheck", currency: "USD" }, - { date: Date.today - 28, amount: -1500, name: "Rent Payment", currency: "EUR" }, - { date: Date.today - 28, amount: -50, name: "Internet Bill", currency: "EUR" }, - { date: Date.today - 14, amount: 3000, name: "Paycheck", currency: "USD" } + { date: Date.today - 45, amount: -3000, name: "Paycheck", currency: "USD" }, + { date: Date.today - 41, amount: 1500, name: "Rent Payment", currency: "EUR" }, + { date: Date.today - 39, amount: 200, name: "Groceries", currency: "EUR" }, + { date: Date.today - 34, amount: -3000, name: "Paycheck", currency: "USD" }, + { date: Date.today - 31, amount: 1500, name: "Rent Payment", currency: "EUR" }, + { date: Date.today - 28, amount: 100, name: "Utilities", currency: "EUR" }, + { date: Date.today - 28, amount: -3000, name: "Paycheck", currency: "USD" }, + { date: Date.today - 28, amount: 1500, name: "Rent Payment", currency: "EUR" }, + { date: Date.today - 28, amount: 50, name: "Internet Bill", currency: "EUR" }, + { date: Date.today - 14, amount: -3000, name: "Paycheck", currency: "USD" } ] checking_transactions = [ @@ -171,24 +182,24 @@ namespace :demo_data do ] mortgage_transactions = [ - { date: Date.today - 90, amount: -1500, name: "Mortgage Payment" }, - { date: Date.today - 60, amount: -1500, name: "Mortgage Payment" }, - { date: Date.today - 30, amount: -1500, name: "Mortgage Payment" } + { date: Date.today - 90, amount: 1500, name: "Mortgage Payment" }, + { date: Date.today - 60, amount: 1500, name: "Mortgage Payment" }, + { date: Date.today - 30, amount: 1500, name: "Mortgage Payment" } ] car_loan_transactions = [ - { date: 12.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 11.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 10.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 9.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 8.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 7.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 6.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 5.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 4.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 3.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 2.months.ago.to_date, amount: -1250, name: "Car Loan Payment" }, - { date: 1.month.ago.to_date, amount: -1250, name: "Car Loan Payment" } + { date: 12.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 11.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 10.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 9.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 8.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 7.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 6.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 5.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 4.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 3.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 2.months.ago.to_date, amount: 1250, name: "Car Loan Payment" }, + { date: 1.month.ago.to_date, amount: 1250, name: "Car Loan Payment" } ] # ========== Valuations ================ @@ -261,6 +272,21 @@ namespace :demo_data do mortgage.transactions.insert_all(mortgage_transactions) car_loan.transactions.insert_all(car_loan_transactions) + # Tag a few transactions + emergency_fund_tag = Tag.find_by(name: "Emergency Fund") + trips_tag = Tag.find_by(name: "Trips") + hawaii_trip_tag = Tag.find_by(name: "Hawaii Trip") + + savings.transactions.order(date: :desc).limit(5).each do |txn| + txn.tags << emergency_fund_tag + txn.save! + end + + checking.transactions.order(date: :desc).limit(5).each do |txn| + txn.tags = [ trips_tag, hawaii_trip_tag ] + txn.save! + end + puts "Created demo accounts, transactions, and valuations for family: #{family.name}" puts "Syncing accounts... This may take a few seconds." From 5c29d9539e7a189d0a5ed815410668cdd0730962 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 13:16:11 -0400 Subject: [PATCH 24/31] Bump capybara wait time --- test/application_system_test_case.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 4713e4b6b29..39205984966 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -1,6 +1,10 @@ require "test_helper" class ApplicationSystemTestCase < ActionDispatch::SystemTestCase + setup do + Capybara.default_max_wait_time = 8 + end + driven_by :selenium, using: ENV["CI"].present? ? :headless_chrome : :chrome, screen_size: [ 1400, 1400 ] private From c96e32d912c63b46c572b2e26dc1e675068a5813 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 13:25:36 -0400 Subject: [PATCH 25/31] System test stabilization --- app/helpers/application_helper.rb | 3 --- test/application_system_test_case.rb | 2 +- test/system/transactions_test.rb | 2 ++ 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 99daecc8008..67082b18f71 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -28,8 +28,6 @@ def notification(text, **options, &block) #
    Content here
    # <% end %> # - # @note can also be triggered via ?modal=model&entity_id=uuid query params - # def modal(options = {}, &block) content = capture &block render partial: "shared/modal", locals: { content:, classes: options[:classes] } @@ -43,7 +41,6 @@ def modal(options = {}, &block) #
    Content here
    # <% end %> # - # @note can also be triggered via ?drawer=model&entity_id=uuid query params def drawer(&block) content = capture &block render partial: "shared/drawer", locals: { content: content } diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 39205984966..83b45e4f7d3 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -2,7 +2,7 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase setup do - Capybara.default_max_wait_time = 8 + Capybara.default_max_wait_time = 5 end driven_by :selenium, using: ENV["CI"].present? ? :headless_chrome : :chrome, screen_size: [ 1400, 1400 ] diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index 845dd267add..d59355c013c 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -53,6 +53,8 @@ class TransactionsTest < ApplicationSystemTestCase fill_in "Search transactions by name", with: @target_txn.name end + assert_selector "#" + dom_id(@target_txn), count: 1 + find("#transaction-filters-button").click within "#transaction-filters-menu" do From e5c102e36af770650c5933351217c4a3687f637c Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 13:37:18 -0400 Subject: [PATCH 26/31] Remove redundant test --- test/system/transactions_test.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/system/transactions_test.rb b/test/system/transactions_test.rb index d59355c013c..6eb3d975834 100644 --- a/test/system/transactions_test.rb +++ b/test/system/transactions_test.rb @@ -49,12 +49,6 @@ class TransactionsTest < ApplicationSystemTestCase end test "all filters work and empty state shows if no match" do - within "form#transactions-search" do - fill_in "Search transactions by name", with: @target_txn.name - end - - assert_selector "#" + dom_id(@target_txn), count: 1 - find("#transaction-filters-button").click within "#transaction-filters-menu" do @@ -88,7 +82,6 @@ class TransactionsTest < ApplicationSystemTestCase assert_text "No transactions found" within "ul#transaction-search-filters" do - find("li", text: @target_txn.name).first("a").click find("li", text: @target_txn.account.name).first("a").click find("li", text: "on or after #{10.days.ago.to_date}").first("a").click find("li", text: "on or before #{Date.current}").first("a").click From 6bca270aaad0bea8a1be1995bf26de7849652271 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 13:58:39 -0400 Subject: [PATCH 27/31] Don't enforce system tests yet for PRs --- .github/workflows/ci.yml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d873af11638..7e527e2cb31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -82,17 +82,19 @@ jobs: ruby-version: .ruby-version bundler-cache: true - - name: Run tests and smoke test seed - env: - RAILS_ENV: test - DATABASE_URL: postgres://postgres:postgres@localhost:5432 + - name: DB Smoke test run: | bin/rails db:create bin/rails db:schema:load - bin/rails test - DISABLE_PARALLELIZATION=true bin/rails test:system bin/rails db:seed + - name: Unit and integration tests + run: bin/rails test + + - name: System tests + run: DISABLE_PARALLELIZATION=true bin/rails test:system + continue-on-error: true # TODO: Eventually we'll enforce for PRs + - name: Keep screenshots from failed system tests uses: actions/upload-artifact@v4 if: failure() From 14ac8a856239e94e2861d9b517bde792e0c9133c Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 14:01:22 -0400 Subject: [PATCH 28/31] Add back test DB url --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e527e2cb31..4d9c34c58da 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,6 +83,9 @@ jobs: bundler-cache: true - name: DB Smoke test + env: + RAILS_ENV: test + DATABASE_URL: postgres://postgres:postgres@localhost:5432 run: | bin/rails db:create bin/rails db:schema:load From ad7c28000d78632df2a6e116a08c78622fbee0d4 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 14:05:27 -0400 Subject: [PATCH 29/31] Job env --- .github/workflows/ci.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4d9c34c58da..2e1359d321a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,8 @@ jobs: test: runs-on: ubuntu-latest timeout-minutes: 10 + env: + DATABASE_URL: postgres://postgres:postgres@localhost:5432 services: postgres: @@ -83,9 +85,6 @@ jobs: bundler-cache: true - name: DB Smoke test - env: - RAILS_ENV: test - DATABASE_URL: postgres://postgres:postgres@localhost:5432 run: | bin/rails db:create bin/rails db:schema:load From ddeaa0a81612b08a134d13820f886d32f57463f1 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 14:11:13 -0400 Subject: [PATCH 30/31] Rails env for action --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2e1359d321a..54f992671a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,6 +60,7 @@ jobs: timeout-minutes: 10 env: DATABASE_URL: postgres://postgres:postgres@localhost:5432 + RAILS_ENV: test services: postgres: From 75a654cfa17abab1202f6ce44d24019491ad28a4 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Thu, 30 May 2024 14:58:27 -0400 Subject: [PATCH 31/31] Remove unused partial --- app/views/transactions/_date_group.html.erb | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 app/views/transactions/_date_group.html.erb diff --git a/app/views/transactions/_date_group.html.erb b/app/views/transactions/_date_group.html.erb deleted file mode 100644 index 146a5760b7a..00000000000 --- a/app/views/transactions/_date_group.html.erb +++ /dev/null @@ -1,10 +0,0 @@ -<%# locals: (date:, transactions:) %> -
    -
    -

    <%= date.strftime("%b %d, %Y") %> · <%= transactions.size %>

    - <%= format_money -transactions.sum(&:amount_money) %> -
    -
    - <%= render transactions %> -
    -