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 <[email protected]> * 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] <[email protected]> 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](freerange/mocha@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] <[email protected]> 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](octokit/octokit.rb@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] <[email protected]> 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](rails/rails@f9c847f...5d34172) --- updated-dependencies: - dependency-name: rails dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Zach Gollwitzer <[email protected]> * 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 <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Karan Kiri <[email protected]> Co-authored-by: Mattia <[email protected]> Co-authored-by: Zach Gollwitzer <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Zach Gollwitzer <[email protected]> Co-authored-by: Jakub Kottnauer <[email protected]> Co-authored-by: ziraq young <[email protected]>
maybe-finance · Jun 21, 2024 · e13c3d9 · e13c3d9
1 parent 1e0635b
commit e13c3d9
Show file tree

Hide file tree

Showing 7 changed files with 82 additions and 56 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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