Improve account internal linking and redirect behavior (#864)

* Fix transaction row link and overflow

* Allow user to access imports from account page

* Clean up accounts controller, add link to account page from settings

* Add link to accounts management from accounts summary page

* Cleanup styles

app/controllers/accounts_controller.rb

@@ -2,7 +2,7 @@ class AccountsController < ApplicationController
   layout "with_sidebar"
 
   include Filterable
-  before_action :set_account, only: %i[ show update destroy sync ]
+  before_action :set_account, only: %i[ show destroy sync update ]
 
   def index
     @accounts = Current.family.accounts
@@ -32,26 +32,9 @@ class AccountsController < ApplicationController
     @valuation_series = @account.valuations.to_series
   end
 
-  def edit
-  end
-
   def update
-    if @account.update(account_params.except(:accountable_type))
+    @account.update! account_params.except(:accountable_type)
-
+    redirect_back_or_to account_path(@account), notice: t(".success")
-      @account.sync_later if account_params[:is_active] == "1" && @account.can_sync?
-
-      respond_to do |format|
-        format.html { redirect_to accounts_path, 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("account_#{@account.id}", partial: "accounts/account", locals: { account: @account })
-          ]
-        end
-      end
-    else
-      render "show", status: :unprocessable_entity
-    end
   end
 
   def create
@@ -62,7 +45,7 @@ class AccountsController < ApplicationController
       @valuation = @account.valuations.new(date: account_params[:start_date] || Date.today, value: @account.balance, currency: @account.currency)
       @valuation.save!
 
-      redirect_to accounts_path, notice: t(".success")
+      redirect_to account_path(@account), notice: t(".success")
     else
       render "new", status: :unprocessable_entity
     end

app/controllers/imports_controller.rb

@@ -9,7 +9,8 @@ class ImportsController < ApplicationController
   end
 
   def new
-    @import = Import.new
+    account = Current.family.accounts.find_by(id: params[:account_id])
+    @import = Import.new account: account
   end
 
   def edit

app/views/accounts/_account.html.erb

@@ -4,18 +4,20 @@
       <div class="w-8 h-8 flex items-center justify-center rounded-full text-xs font-medium <%= account.is_active ? "bg-blue-500/10 text-blue-500" : "bg-gray-500/10 text-gray-500" %>">
         <%= account.name[0].upcase %>
       </div>
-      <p class="text-sm font-medium <%= account.is_active ? "text-gray-900" : "text-gray-400" %>">
+      <%= link_to account.name, account, class: [(account.is_active ? "text-gray-900" : "text-gray-400"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top" } %>
-        <%= account.name %>
-      </p>
     </div>
     <div class="flex items-center gap-8">
       <p class="text-sm font-medium <%= account.is_active ? "text-gray-900" : "text-gray-400" %>">
         <%= format_money account.balance_money %>
       </p>
-      <%= form_with model: account, method: :patch, html: { class: "flex items-center", data: { turbo_frame: "_top" } } do |form| %>
+
+      <%= form_with model: account,
+                    namespace: account.id,
+                    builder: ActionView::Helpers::FormBuilder,
+                    data: { controller: "auto-submit-form", turbo_frame: "_top" } do |form| %>
         <div class="relative inline-block select-none">
-          <%= form.check_box :is_active, class: "sr-only peer", id: "is_active_#{account.id}", onchange: "this.form.requestSubmit();" %>
+          <%= form.check_box :is_active, { class: "sr-only peer", data: { "auto-submit-form-target": "auto" } } %>
-          <label for="is_active_<%= account.id %>" class="block bg-gray-100 w-9 h-5 rounded-full cursor-pointer after:content-[''] after:block after:absolute after:top-0.5 after:left-0.5 after:bg-white after:w-4 after:h-4 after:rounded-full after:transition-transform after:duration-300 after:ease-in-out peer-checked:bg-green-600 peer-checked:after:translate-x-4"></label>
+          <%= form.label :is_active, "&nbsp;".html_safe, class: "maybe-switch" %>
         </div>
       <% end %>
     </div>

app/views/accounts/_header.html.erb

@@ -0,0 +1,21 @@
+<header class="flex justify-between items-center text-gray-900 font-medium">
+  <h1 class="text-xl"><%= t(".accounts") %></h1>
+
+  <div class="flex items-center gap-2">
+    <%= contextual_menu do %>
+      <div class="w-48 p-1 text-sm leading-6 text-gray-900 bg-white shadow-lg shrink rounded-xl ring-1 ring-gray-900/5">
+        <%= link_to accounts_path(return_to: summary_accounts_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 "settings", class: "w-5 h-5 text-gray-500" %>
+          <span class="text-black"><%= t(".manage") %></span>
+        <% end %>
+      </div>
+
+    <% end %>
+
+    <%= link_to new_account_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") %>
+      <p class="text-sm font-medium"><%= t(".new") %></p>
+    <% end %>
+  </div>
+</header>

app/views/accounts/show.html.erb

@@ -15,27 +15,30 @@
           <%= lucide_icon("chevron-down", class: "w-5 h-5 text-gray-500") %>
         </div>
       </div>
-      <div class="relative cursor-pointer" data-controller="menu">
+
-        <button data-menu-target="button" class="flex hover:bg-gray-100 p-2 rounded">
+      <%= contextual_menu do %>
-          <%= lucide_icon("more-horizontal", class: "w-5 h-5 text-gray-500") %>
+        <div class="w-48 p-1 text-sm leading-6 text-gray-900 bg-white shadow-lg shrink rounded-xl ring-1 ring-gray-900/5">
-        </button>
+          <%= link_to new_import_path(account_id: @account.id),
-        <div data-menu-target="content" class="absolute z-10 top-10 right-0 border border-alpha-black-25 bg-white rounded-lg shadow-xs hidden">
+                      class: "block w-full py-2 px-3 space-x-2 text-gray-900 hover:bg-gray-50 flex items-center rounded-lg" do %>
-          <div class="w-48 px-3 text-sm leading-6 text-gray-900 bg-white shadow-lg shrink rounded-xl ring-1 ring-gray-900/5">
+            <%= lucide_icon "download", class: "w-5 h-5 text-gray-500" %>
-            <%= button_to account_path(@account),
+
-                method: :delete,
+            <span><%= t(".import") %></span>
-                class: "block w-full py-2 text-red-600 hover:text-red-800 flex items-center",
+          <% end %>
-                data: {
+
-                  turbo_confirm: {
+          <%= button_to account_path(@account),
-                    title: t(".confirm_title"),
+                        method: :delete,
-                    body: t(".confirm_body_html"),
+                        class: "block w-full py-2 px-3 space-x-2 text-red-600 hover:bg-red-50 flex items-center rounded-lg",
-                    accept: t(".confirm_accept", name: @account.name)
+                        data: {
-                  }
+                          turbo_confirm: {
-                } do %>
+                            title: t(".confirm_title"),
-              <%= lucide_icon("trash-2", class: "w-5 h-5 mr-2") %> Delete account
+                            body: t(".confirm_body_html"),
-            <% end %>
+                            accept: t(".confirm_accept", name: @account.name)
-          </div>
+                          }
+                        } do %>
+            <%= lucide_icon("trash-2", class: "w-5 h-5 mr-2") %> Delete account
+          <% end %>
         </div>
-      </div>
+      <% end %>
     </div>
   </div>
   <%= turbo_frame_tag "sync_message" do %>

app/views/accounts/summary.html.erb

@@ -1,11 +1,6 @@
 <div class="space-y-4">
-  <div class="flex items-center justify-between">
+
-    <h1 class="text-xl font-medium text-gray-900">Accounts</h1>
+  <%= render "header" %>
-    <%= link_to new_account_path, class: "flex text-white text-sm font-medium items-center gap-1 bg-gray-900 rounded-lg p-2 pr-3", data: { turbo_frame: "modal" } do %>
-      <%= lucide_icon("plus", class: "w-5 h-5") %>
-      <span><%= t(".new") %></span>
-    <% end %>
-  </div>
 
   <% if @accounts.empty? %>
     <%= render "shared/no_account_empty_state" %>

app/views/pages/dashboard/transactions/_transaction.html.erb

@@ -1,5 +1,5 @@
 <div class="text-gray-900 flex items-center py-4 text-sm font-medium px-4">
-  <div class="grow">
+  <div class="grow max-w-72">
     <%= render "transactions/name", transaction: transaction %>
   </div>
 

app/views/settings/_nav.html.erb

@@ -1,5 +1,5 @@
 <div class="flex items-center gap-1 mb-6">
-  <%= link_to root_path, class: "flex items-center gap-1 text-gray-900 font-medium text-sm" do %>
+  <%= link_to return_to_path(params), class: "flex items-center gap-1 text-gray-900 font-medium text-sm" do %>
     <%= lucide_icon "chevron-left", class: "w-5 h-5 text-gray-500" %>
     <span>Back</span>
   <% end %>

app/views/transactions/_name.html.erb

@@ -1,9 +1,9 @@
 <%= content_tag :div, class: ["flex items-center gap-2"] do %>
-  <div class="w-8 h-8 flex items-center justify-center rounded-full bg-gray-600/5 text-gray-600">
+  <div class="flex h-8 w-8 shrink-0 items-center justify-center rounded-full bg-gray-600/5 text-gray-600">
     <%= transaction.name[0].upcase %>
   </div>
 
-  <div class="text-gray-900 truncate">
+  <div class="truncate text-gray-900">
     <% if transaction.new_record? %>
       <%= content_tag :p, transaction.name %>
     <% else %>

app/views/transactions/_transaction.html.erb

@@ -4,7 +4,9 @@
                       class: "maybe-checkbox maybe-checkbox--light",
                       data: { id: transaction.id, "bulk-select-target": "row", action: "bulk-select#toggleRowSelection" } %>
 
-    <%= render "transactions/name", transaction: transaction %>
+    <div class="max-w-full pr-10">
+      <%= render "transactions/name", transaction: transaction %>
+    </div>
   </div>
 
   <div class="col-span-3">
@@ -13,6 +15,7 @@
 
   <%= link_to transaction.account.name,
               account_path(transaction.account),
+              data: { turbo_frame: "_top" },
               class: ["col-span-3 hover:underline"] %>
 
   <div class="col-span-2 ml-auto">

config/locales/views/accounts/en.yml

@@ -12,6 +12,10 @@ en:
       success: New account created successfully
     destroy:
       success: Account deleted successfully
+    header:
+      accounts: Accounts
+      manage: Manage accounts
+      new: New account
     index:
       new_account: New account
     new:
@@ -35,6 +39,7 @@ en:
         /> <p>After deletion, there is no way you'll be able to restore the account
         information because you'll need to add it as a new account.</p>"
       confirm_title: Delete account?
+      import: Import transactions
       sync_message_missing_rates: Since exchange rates haven't been synced, balance
         graphs may not reflect accurate values.
       sync_message_unknown_error: An error has occurred during the sync.
@@ -44,4 +49,4 @@ en:
       cannot_sync: Account cannot be synced at the moment
       success: Account sync started
     update:
-      success: Account updated successfully
+      success: Account updated

test/controllers/accounts_controller_test.rb

@@ -16,10 +16,21 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     assert_response :ok
   end
 
+  test "should update account" do
+    patch account_url(@account), params: {
+      account: {
+        is_active: "0"
+      }
+    }
+
+    assert_redirected_to account_url(@account)
+    assert_equal "Account updated", flash[:notice]
+  end
+
   test "should create account" do
     assert_difference -> { Account.count }, +1 do
       post accounts_path, params: { account: { accountable_type: "Account::Credit" } }
-      assert_redirected_to accounts_url
+      assert_redirected_to account_url(Account.order(:created_at).last)
     end
   end