From e13c3d92711704368920a05478864f9d5f307cdb Mon Sep 17 00:00:00 2001
From: Karan Kiri <karankiri.96@gmail.com>
Date: Fri, 21 Jun 2024 21:34:40 +0530
Subject: [PATCH] feat: Transaction pagination Improvements (#873)

* feat: make transaction container fixed height

* feat: pagination per page query

* fix: linting errors

* Changelog page that pulls from Github Release notes (#867)

* Changelog page that pulls from Github Release notes

* Review changelog page styles

* Move changelog page title to i18n translations

* Bump to 0.1.0-alpha.6

Signed-off-by: Zach Gollwitzer <zach@maybe.co>

* Bump aws-sdk-s3 from 1.152.0 to 1.152.3 (#880)

Bumps [aws-sdk-s3](https://github.com/aws/aws-sdk-ruby) from 1.152.0 to 1.152.3.
- [Release notes](https://github.com/aws/aws-sdk-ruby/releases)
- [Changelog](https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-sdk-s3/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-ruby/commits)

---
updated-dependencies:
- dependency-name: aws-sdk-s3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump mocha from 2.3.0 to 2.4.0 (#878)

Bumps [mocha](https://github.com/freerange/mocha) from 2.3.0 to 2.4.0.
- [Changelog](https://github.com/freerange/mocha/blob/main/RELEASE.md)
- [Commits](https://github.com/freerange/mocha/compare/v2.3.0...v2.4.0)

---
updated-dependencies:
- dependency-name: mocha
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump octokit from 8.1.0 to 9.1.0 (#877)

Bumps [octokit](https://github.com/octokit/octokit.rb) from 8.1.0 to 9.1.0.
- [Release notes](https://github.com/octokit/octokit.rb/releases)
- [Changelog](https://github.com/octokit/octokit.rb/blob/main/RELEASE.md)
- [Commits](https://github.com/octokit/octokit.rb/compare/v8.1.0...v9.1.0)

---
updated-dependencies:
- dependency-name: octokit
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump rails from `f9c847f` to `5d34172` (#879)

Bumps [rails](https://github.com/rails/rails) from `f9c847f` to `5d34172`.
- [Release notes](https://github.com/rails/rails/releases)
- [Commits](https://github.com/rails/rails/compare/f9c847fac102039d9174106f44b59144da267751...5d34172ff44ec0c88ac03a979679b31e1ed78745)

---
updated-dependencies:
- dependency-name: rails
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Zach Gollwitzer <zach@maybe.co>

* Update issue templates

* Add merchant select when editing transaction (#885)

* Transaction transfers, payments, and matching (#883)

* Add transfer model and clean up family snapshot fixtures

* Ignore transfers in income and expense snapshots

* Add transfer validations

* Implement basic transfer matching UI

* Fix merge conflicts

* Add missing translations

* Tweak selection states for transfer types

* Add missing i18n translation

* Ensure correct form's hidden input for selectedIds (#891)

* feat: make transaction container fixed height

* feat: pagination per page query

* fix: linting errors

* Transaction transfers, payments, and matching (#883)

* Add transfer model and clean up family snapshot fixtures

* Ignore transfers in income and expense snapshots

* Add transfer validations

* Implement basic transfer matching UI

* Fix merge conflicts

* Add missing translations

* Tweak selection states for transfer types

* Add missing i18n translation

* feat: make transaction container fixed height

* feat: pagination per page query

* fix: linting errors

* revert unnecessary changes

* revert unnecessary changes

* code review changes

* code review changes

* code review changes

* remove unused imports

* fix: unit tests

* remove border

* fix: transaction padding

* fix: transaction container height

---------

Signed-off-by: Zach Gollwitzer <zach@maybe.co>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Karan Kiri <karankiri.96@gmail.com>
Co-authored-by: Mattia <malnis.mattia@gmail.com>
Co-authored-by: Zach Gollwitzer <zach@maybe.co>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Co-authored-by: Jakub Kottnauer <jk@jakubkottnauer.com>
Co-authored-by: ziraq young <ziraqyoung@outlook.com>
---
 app/controllers/transactions_controller.rb    |  2 +-
 app/views/layouts/with_sidebar.html.erb       |  2 +-
 app/views/transactions/_pagination.html.erb   | 81 +++++++++++--------
 app/views/transactions/index.html.erb         | 45 ++++++-----
 .../transactions/searches/_search.html.erb    |  2 +
 config/locales/views/transactions/en.yml      |  2 +
 .../transactions_controller_test.rb           |  4 +-
 7 files changed, 82 insertions(+), 56 deletions(-)

diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb
index 0c569d9654b..98be1c396ca 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: 50)
+    @pagy, @transactions = pagy(result, items: params[:per_page] || "10")
 
     @totals = {
       count: result.select { |t| t.currency == Current.family.currency }.count,
diff --git a/app/views/layouts/with_sidebar.html.erb b/app/views/layouts/with_sidebar.html.erb
index f014dd666d4..ca824d45f5c 100644
--- a/app/views/layouts/with_sidebar.html.erb
+++ b/app/views/layouts/with_sidebar.html.erb
@@ -7,7 +7,7 @@
         <%= render "layouts/sidebar" %>
       <% end %>
     </div>
-    <main class="grow px-20 pt-6 pb-32 h-full overflow-y-auto">
+    <main class="grow px-20 py-6 h-full overflow-y-auto">
       <%= yield %>
     </main>
   </div>
diff --git a/app/views/transactions/_pagination.html.erb b/app/views/transactions/_pagination.html.erb
index 2ad3149ab56..a485ce7111e 100644
--- a/app/views/transactions/_pagination.html.erb
+++ b/app/views/transactions/_pagination.html.erb
@@ -1,39 +1,56 @@
-<nav class="flex items-center justify-center px-4 mt-4 sm:px-0">
-  <div>
-    <% if pagy.prev %>
-      <%= link_to pagy_url_for(pagy, pagy.prev), 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 %>
-        <%= lucide_icon("chevron-left", class: "w-5 h-5 text-gray-500") %>
+<nav class="flex items-center justify-between px-4 mt-4 sm:px-0 w-full">
+  <div class="flex">
+    <div>
+      <% if pagy.prev %>
+        <%= link_to pagy_url_for(pagy, pagy.prev), 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 %>
+          <%= lucide_icon("chevron-left", class: "w-5 h-5 text-gray-500") %>
+        <% end %>
+      <% else %>
+        <div class="inline-flex items-center px-3 py-3 text-sm font-medium hover:border-gray-300">
+          <%= lucide_icon("chevron-left", class: "w-5 h-5 text-gray-200") %>
+        </div>
       <% end %>
-    <% else %>
-      <div class="inline-flex items-center px-3 py-3 text-sm font-medium hover:border-gray-300">
-        <%= lucide_icon("chevron-left", class: "w-5 h-5 text-gray-200") %>
-      </div>
-    <% end %>
-  </div>
-  <div class="bg-gray-25 rounded-xl">
-    <% 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 %>
-          <%= series_item %>
+    </div>
+    <div class="bg-gray-25 rounded-xl">
+      <% 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 %>
+            <%= series_item %>
+          <% end %>
+        <% elsif series_item.is_a?(String) %>
+          <%= link_to pagy_url_for(pagy, series_item), class: "shadow-lg ring-2 ring-inset ring-gray-200 rounded-xl bg-white inline-flex items-center m-1 px-4 py-2 text-sm font-medium text-gray-900" do %>
+            <%= series_item %>
+          <% end %>
+        <% elsif series_item == :gap %>
+          <span class="inline-flex items-center  px-3 py-3 text-sm font-medium text-gray-500">...</span>
         <% end %>
-      <% elsif series_item.is_a?(String) %>
-        <%= link_to pagy_url_for(pagy, series_item), class: "shadow-lg ring-2 ring-inset ring-gray-200 rounded-xl bg-white inline-flex items-center m-1 px-4 py-2 text-sm font-medium text-gray-900" do %>
-          <%= series_item %>
+      <% end %>
+    </div>
+    <div>
+      <% if pagy.next %>
+        <%= link_to pagy_url_for(pagy, pagy.next), 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 %>
+          <%= lucide_icon("chevron-right", class: "w-5 h-5 text-gray-500") %>
         <% end %>
-      <% elsif series_item == :gap %>
-        <span class="inline-flex items-center  px-3 py-3 text-sm font-medium text-gray-500">...</span>
+      <% else %>
+        <div class="inline-flex items-center px-3 py-3 text-sm font-medium hover:border-gray-300">
+          <%= lucide_icon("chevron-right", class: "w-5 h-5 text-gray-200") %>
+        </div>
       <% end %>
-    <% end %>
+    </div>
   </div>
-  <div>
-    <% if pagy.next %>
-      <%= link_to pagy_url_for(pagy, pagy.next), 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 %>
-        <%= lucide_icon("chevron-right", class: "w-5 h-5 text-gray-500") %>
-      <% end %>
-    <% else %>
-      <div class="inline-flex items-center px-3 py-3 text-sm font-medium hover:border-gray-300">
-        <%= lucide_icon("chevron-right", class: "w-5 h-5 text-gray-200") %>
-      </div>
-    <% end %>
+  <% per_page = params[:per_page] || "50" %>
+  <div class="flex items-center gap-4">
+    <%= form_with url: transactions_path,
+                builder: ActionView::Helpers::FormBuilder,
+                method: :get,
+                class: "flex items-center gap-4",
+                data: { controller: "auto-submit-form" } do |f| %>
+    <%= f.label :per_page, t(".rows_per_page"), class: "text-sm text-gray-500" %>
+    <%= f.select :per_page,
+                 options_for_select(["10", "20", "30", "50"], params[:per_page]),
+                 {},
+                 class: "py-1.5 pr-8 text-sm text-gray-900 font-medium border border-gray-200 rounded-lg focus:border-gray-900 focus:ring-gray-900 focus-visible:ring-gray-900",
+                 data: { "auto-submit-form-target": "auto" } %>
+  <% end %>
   </div>
 </nav>
diff --git a/app/views/transactions/index.html.erb b/app/views/transactions/index.html.erb
index 23fff412204..faf0f1109c1 100644
--- a/app/views/transactions/index.html.erb
+++ b/app/views/transactions/index.html.erb
@@ -1,41 +1,46 @@
-<div class="space-y-4">
+<div class="space-y-4 h-full flex flex-col overflow-y-auto">
   <%= render "header" %>
 
   <%= render partial: "transactions/summary", locals: { totals: @totals } %>
 
-  <div id="transactions" data-controller="bulk-select" data-bulk-select-resource-value="<%= t(".transaction") %>" class="bg-white rounded-xl border border-alpha-black-25 shadow-xs p-4 space-y-4">
+  <div id="transactions" data-controller="bulk-select" data-bulk-select-resource-value="<%= t(".transaction") %>" class="overflow-y-auto flex flex-col bg-white rounded-xl border border-alpha-black-25 shadow-xs pb-4">
 
-    <%= render partial: "transactions/searches/search", locals: { transactions: @transactions } %>
+    <div class="p-4 pb-0">
+      <%= render partial: "transactions/searches/search", locals: { transactions: @transactions } %>
+    </div>
 
     <% if @transactions.present? %>
       <div hidden id="transaction-selection-bar" data-bulk-select-target="selectionBar">
         <%= render "selection_bar" %>
       </div>
-
-      <div class="grid grid-cols-12 bg-gray-25 rounded-xl px-5 py-3 text-xs uppercase font-medium text-gray-500 items-center mb-4">
-        <div class="pl-0.5 col-span-4 flex items-center gap-4">
-          <%= check_box_tag "selection_transaction",
-                            class: "maybe-checkbox maybe-checkbox--light",
-                            data: { action: "bulk-select#togglePageSelection" } %>
-          <p class="col-span-4">transaction</p>
+      <div class="grow overflow-y-auto px-4">
+        <div class="grid grid-cols-12 bg-gray-25 rounded-xl px-5 py-3 text-xs uppercase font-medium text-gray-500 items-center mb-4">
+          <div class="pl-0.5 col-span-4 flex items-center gap-4">
+            <%= check_box_tag "selection_transaction",
+                              class: "maybe-checkbox maybe-checkbox--light",
+                              data: { action: "bulk-select#togglePageSelection" } %>
+            <p class="col-span-4">transaction</p>
+          </div>
+
+          <p class="col-span-3 pl-4">category</p>
+          <p class="col-span-3">account</p>
+          <p class="col-span-2 justify-self-end">amount</p>
         </div>
-
-        <p class="col-span-3 pl-4">category</p>
-        <p class="col-span-3">account</p>
-        <p class="col-span-2 justify-self-end">amount</p>
-      </div>
-      <div class="space-y-6">
-        <% group_transactions_by_date(@transactions).each do |date, group| %>
+        <div class="space-y-6">
+         <% group_transactions_by_date(@transactions).each do |date, group| %>
           <%= render partial: "date_group", locals: { date:, group: } %>
         <% end %>
+        </div>
       </div>
     <% else %>
       <%= render "empty" %>
     <% end %>
 
-    <% if @pagy.pages > 1 %>
-      <%= render "pagination", pagy: @pagy %>
-    <% end %>
+    <div class="px-4">
+      <% if @pagy.pages > 1 %>
+        <%= render "pagination", pagy: @pagy %>
+      <% end %>
+    </div>
 
   </div>
 </div>
diff --git a/app/views/transactions/searches/_search.html.erb b/app/views/transactions/searches/_search.html.erb
index fcd93fc458f..22491ddebf1 100644
--- a/app/views/transactions/searches/_search.html.erb
+++ b/app/views/transactions/searches/_search.html.erb
@@ -3,9 +3,11 @@
 <ul id="transaction-search-filters" class="flex items-center flex-wrap gap-2">
   <% @q.each do |param_key, param_value| %>
     <% unless param_value.blank? %>
+    <div class="pb-4">
       <% Array(param_value).each do |value| %>
         <%= render partial: "transactions/searches/filters/badge", locals: { param_key: param_key, param_value: value } %>
       <% end %>
+    </div>
     <% end %>
   <% end %>
 </ul>
diff --git a/config/locales/views/transactions/en.yml b/config/locales/views/transactions/en.yml
index 78120fe9c2d..6d485f484b7 100644
--- a/config/locales/views/transactions/en.yml
+++ b/config/locales/views/transactions/en.yml
@@ -50,6 +50,8 @@ en:
       transaction: transaction
     mark_transfers:
       success: Marked as transfer
+    pagination:
+      rows_per_page: Rows per page
     selection_bar:
       mark_transfers: Mark as transfers?
       mark_transfers_confirm: Mark as transfers
diff --git a/test/controllers/transactions_controller_test.rb b/test/controllers/transactions_controller_test.rb
index 7dbcde8d2db..9e3cc9d9fe0 100644
--- a/test/controllers/transactions_controller_test.rb
+++ b/test/controllers/transactions_controller_test.rb
@@ -4,7 +4,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
     @transaction = transactions(:checking_one)
-    @recent_transactions = @user.family.transactions.ordered.where(transfer_id: nil).limit(20).to_a
+    @recent_transactions = @user.family.transactions.ordered.limit(20).to_a
   end
 
   test "should get paginated index with most recent transactions first" do
@@ -36,7 +36,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
     get transactions_url(page: 2)
     assert_response :success
 
-    @recent_transactions[10, 10].each do |transaction|
+    @recent_transactions[10, 10].select { |t| t.transfer_id == nil }.each do |transaction|
       assert_dom "#" + dom_id(transaction), count: 1
     end
   end