diff --git a/lib/spendable_web/components/apple_mark.ex b/lib/spendable_web/components/apple_mark.ex new file mode 100644 index 00000000..0d18f095 --- /dev/null +++ b/lib/spendable_web/components/apple_mark.ex @@ -0,0 +1,21 @@ +defmodule SpendableWeb.Components.AppleMark do + @moduledoc false + use SpendableWeb, :html + + @doc """ + Apple's own mark, standing in as the institution logo for the accounts read out of Wallet. + + The outline is the U+F8FF glyph from the system font rather than a shape library's drawing of an + apple. The phone renders that codepoint as text, which a browser on anything but an Apple device + would not, so here it is the same outline inlined. + """ + attr :class, :string, default: "size-5" + + def apple_mark(assigns) do + ~H""" + + """ + end +end diff --git a/lib/spendable_web/components/core_components.ex b/lib/spendable_web/components/core_components.ex index 4efe13e7..61948ccc 100644 --- a/lib/spendable_web/components/core_components.ex +++ b/lib/spendable_web/components/core_components.ex @@ -19,6 +19,7 @@ defmodule SpendableWeb.CoreComponents do alias Phoenix.LiveView.JS use Gettext, backend: SpendableWeb.Gettext + defdelegate apple_mark(assigns), to: SpendableWeb.Components.AppleMark defdelegate auth_backdrop(assigns), to: SpendableWeb.Components.AuthBackdrop defdelegate bulk_actions(assigns), to: SpendableWeb.Components.BulkActions diff --git a/lib/spendable_web/live/banks.ex b/lib/spendable_web/live/banks.ex index 20ece75e..2066993f 100644 --- a/lib/spendable_web/live/banks.ex +++ b/lib/spendable_web/live/banks.ex @@ -1,6 +1,7 @@ defmodule SpendableWeb.Live.Banks do use SpendableWeb, :live_view + import SpendableWeb.Utils.AccountLabel import SpendableWeb.Utils.FormOptions alias Spendable.Banks @@ -43,7 +44,14 @@ defmodule SpendableWeb.Live.Banks do
- bank logo + + <.apple_mark :if={bank_member.provider == "FinanceKit"} class="h-8 mr-2 text-white" /> + bank logo

{bank_member.name}

@@ -91,7 +99,7 @@ defmodule SpendableWeb.Live.Banks do if(bank_account.sync, do: "text-white", else: "text-gray-500"), "min-w-0 text-sm font-semibold leading-6 flex flex-col" ]}> - {bank_account.name} *{bank_account.number} + {account_label(bank_account.name, bank_account.number)} {bank_account.sub_type}
diff --git a/lib/spendable_web/live/banks_test.exs b/lib/spendable_web/live/banks_test.exs index f54da061..f062cdd4 100644 --- a/lib/spendable_web/live/banks_test.exs +++ b/lib/spendable_web/live/banks_test.exs @@ -58,6 +58,34 @@ defmodule SpendableWeb.Live.BanksTest do assert html =~ "Tartan Bank" end + # Wallet is not an institution Plaid has a logo for, so the logo endpoint has nothing to serve. + test "stands Apple's mark in for the Wallet connection", %{conn: conn, scope: scope} do + {:ok, wallet} = + Repo.insert(%BankMember{ + user_id: scope.user.id, + external_id: "finance_kit", + name: "Apple", + provider: "FinanceKit", + status: "CONNECTED" + }) + + {:ok, _view, html} = live(conn, ~p"/banks") + + assert html =~ "Apple" + refute html =~ "/banks/#{wallet.id}/logo" + end + + # An Apple Cash balance has no number to print, and dots with nothing after them say less than + # the name on its own. + test "reads an account with no number as just its name", %{conn: conn, member: member} do + {:ok, view, _html} = live(conn, ~p"/banks") + + html = render_click(view, "select_bank_member", %{"id" => member.id}) + + assert html =~ "Checking" + refute html =~ "••••" + end + test "pushes a link token when opening Plaid Link", %{conn: conn} do {:ok, view, _html} = live(conn, ~p"/banks") diff --git a/lib/spendable_web/live/budgets.ex b/lib/spendable_web/live/budgets.ex index 6503c471..8a41029f 100644 --- a/lib/spendable_web/live/budgets.ex +++ b/lib/spendable_web/live/budgets.ex @@ -100,10 +100,14 @@ defmodule SpendableWeb.Live.Budgets do

{card.budget.name}

- + {card.pill}
-
- +
+

{Utils.format_currency(card.amount)} - - {card.label} +

+

{card.label}

@@ -261,7 +267,7 @@ defmodule SpendableWeb.Live.Budgets do scope = socket.assigns.current_scope selected_month = socket.assigns[:selected_month] || Date.beginning_of_month(Date.utc_today()) summary = Budgets.calculate_month_summary(scope, selected_month, search: socket.assigns[:search]) - listed = maybe_add_credit_cards(summary.budgets, scope, summary.current_month) + listed = listed_budgets(summary.budgets, scope, summary.current_month) socket |> assign(:spendable, summary.spendable) @@ -276,29 +282,49 @@ defmodule SpendableWeb.Live.Budgets do |> assign(:changeset, nil) end - # Card debt is not a budget, but it reads as one on this page: a negative balance to cover. - # It only makes sense against the current month, since it is what is owed right now. - defp maybe_add_credit_cards(budgets, _scope, false = _current_month_is_selected), do: budgets + # A past month has no Spendable figure above the list, so the budget is the only place left to + # read what came out of it. + defp listed_budgets(budgets, _scope, false = _current_month_is_selected), do: by_type(budgets) + + defp listed_budgets([], _scope, _current_month_is_selected), do: [] - defp maybe_add_credit_cards([spendable | budgets], scope, _current_month_is_selected) do + # Card debt is not a budget, but it reads as one on this page: a negative balance to cover, and + # no id because there is no row behind it. Spendable is the figure the page opens with, so + # listing it again only says the same word twice about two different numbers. + defp listed_budgets(budgets, scope, _current_month_is_selected) do credit_cards = %Budget{ name: "Credit Cards", type: :envelope, balance: scope |> Banks.calculate_credit_card_balance() |> Decimal.negate() } - [spendable, credit_cards | budgets] + [credit_cards | budgets |> Enum.reject(&(&1.name == "Spendable")) |> by_type()] end - defp maybe_add_credit_cards([], _scope, _current_month_is_selected), do: [] + # Envelopes, then what is only tracked, alphabetical inside each. Grouping them by what they are + # does the work a heading over each group would, without the headings. Goals go last: a goal is + # money going in rather than out, so it is not what the month is about. + defp by_type(budgets), do: Enum.sort_by(budgets, &{type_order(&1.type), &1.name}) + + defp type_order(:envelope), do: 0 + defp type_order(:tracking), do: 1 + defp type_order(:goal), do: 2 defp build_cards(budgets, spent, current_month_is_selected) do Enum.map(budgets, fn budget -> spent_here = spent |> Map.get(budget.id, Decimal.new(0)) |> Decimal.abs() + credit_cards? = is_nil(budget.id) + card = build_budget_card(budget, spent_here, current_month_is_selected) - budget - |> build_budget_card(spent_here, current_month_is_selected) - |> Map.merge(%{budget: budget, pill: pill(budget.type), pill_class: pill_class(budget.type)}) + # Card debt is not an envelope with something left in it, it is what is owed right now, and + # the pill calling it one is only there to satisfy the card it is built from. + Map.merge(card, %{ + budget: budget, + label: if(credit_cards?, do: "BALANCE", else: card.label), + pill: if(credit_cards?, do: nil, else: pill(budget.type)), + pill_class: pill_class(budget.type), + editable?: not credit_cards? and budget.name != "Spendable" + }) end) end diff --git a/lib/spendable_web/live/budgets_test.exs b/lib/spendable_web/live/budgets_test.exs index 31576412..45c019b0 100644 --- a/lib/spendable_web/live/budgets_test.exs +++ b/lib/spendable_web/live/budgets_test.exs @@ -226,6 +226,67 @@ defmodule SpendableWeb.Live.BudgetsTest do refute html =~ "Credit Cards" end + # Spendable is the figure the page opens with, so a card saying it again is the same word twice + # about two different numbers. + test "leaves the Spendable card off the current month", %{conn: conn, scope: scope} do + {:ok, _transaction} = + Transactions.create_transaction(scope, %{ + "amount" => "-20.00", + "date" => Date.utc_today(), + "name" => "Groceries" + }) + + {:ok, view, html} = live(conn, ~p"/budgets") + + assert html =~ "Spendable" + refute has_element?(view, "h2", "Spendable") + end + + # A past month has no Spendable figure above the list, so the budget is the only place left. + test "keeps the Spendable card on a past month", %{conn: conn, scope: scope} do + {:ok, _transaction} = + Transactions.create_transaction(scope, %{ + "amount" => "-20.00", + "date" => Date.utc_today(), + "name" => "Groceries" + }) + + last_month = Date.utc_today() |> Date.beginning_of_month() |> Date.add(-1) + + {:ok, view, _html} = live(conn, ~p"/budgets") + + render_click(view, "select_month", %{"month" => Date.to_iso8601(last_month)}) + + assert has_element?(view, "h2", "Spendable") + end + + # Card debt reads the bank accounts and Spendable is whatever is left over. Neither is a card + # anyone edits. + test "offers no edit on the credit card total", %{conn: conn, scope: scope} do + {:ok, _budget} = Budgets.create_budget(scope, %{"name" => "Groceries"}) + + {:ok, view, html} = live(conn, ~p"/budgets") + + assert html =~ "BALANCE" + assert has_element?(view, ~s(button[aria-label="Edit Groceries"])) + refute has_element?(view, ~s(button[aria-label="Edit Credit Cards"])) + end + + # Envelopes, then what is only tracked, then goals - the grouping does the work a heading would. + test "orders the cards by type, with goals last", %{conn: conn, scope: scope} do + {:ok, _goal} = Budgets.create_budget(scope, %{"name" => "Vacation", "type" => "goal"}) + {:ok, _tracking} = Budgets.create_budget(scope, %{"name" => "Amazon", "type" => "tracking"}) + {:ok, _envelope} = Budgets.create_budget(scope, %{"name" => "Rent", "type" => "envelope"}) + + {:ok, _view, html} = live(conn, ~p"/budgets") + + assert [rent, amazon, vacation] = + Enum.map(["Rent", "Amazon", "Vacation"], &(:binary.match(html, &1) |> elem(0))) + + assert rent < amazon + assert amazon < vacation + end + test "filters the list by the search box", %{conn: conn, scope: scope} do {:ok, _groceries} = Budgets.create_budget(scope, %{"name" => "Groceries"}) {:ok, _rent} = Budgets.create_budget(scope, %{"name" => "Rent"}) diff --git a/lib/spendable_web/live/transactions.ex b/lib/spendable_web/live/transactions.ex index 752167a6..22b6a49e 100644 --- a/lib/spendable_web/live/transactions.ex +++ b/lib/spendable_web/live/transactions.ex @@ -1,6 +1,7 @@ defmodule SpendableWeb.Live.Transactions do use SpendableWeb, :live_view + import SpendableWeb.Utils.AccountLabel import SpendableWeb.Utils.FormOptions alias Spendable.Budgets @@ -75,7 +76,7 @@ defmodule SpendableWeb.Live.Transactions do :for={{id, transaction} <- @streams.transactions} id={id} class={[ - if(transaction.excluded or transaction.transfer_id, do: "opacity-40"), + if(transaction.excluded, do: "opacity-40"), "group flex flex-row items-center gap-x-3 p-2" ]} > @@ -111,10 +112,7 @@ defmodule SpendableWeb.Live.Transactions do class="h-5 w-5 shrink-0 rounded-sm" /> - {bank_account(transaction).name} - - - {mask(bank_account(transaction).number)} + {account_label(bank_account(transaction).name, bank_account(transaction).number)}
@@ -589,8 +587,13 @@ defmodule SpendableWeb.Live.Transactions do (socket.assigns.show_excluded or not transaction.excluded) end + # Saying where the whole of a transaction was spent is the decision the review queue is asking + # for, so making it is what finishes the row. defp whole_amount_to(transaction, budget_id) do - %{"budget_allocations" => [%{"amount" => transaction.amount, "budget_id" => budget_id}]} + %{ + "budget_allocations" => [%{"amount" => transaction.amount, "budget_id" => budget_id}], + "reviewed" => true + } end # A transaction with one allocation splits nothing, so the whole amount is spent from that @@ -607,12 +610,8 @@ defmodule SpendableWeb.Live.Transactions do defp bank_account(%{bank_transaction: %{bank_account: account}}), do: account defp bank_account(_transaction), do: nil - # The dots stand in for the digits the bank does not give us, so the number reads as an account - # rather than as a footnote. - defp mask(number), do: "••••#{number}" - defp transfer_label(%{bank_transaction: %{bank_account: account}}) do - "#{account.name} #{mask(account.number)}" + account_label(account.name, account.number) end defp transfer_label(transfer), do: transfer.name diff --git a/lib/spendable_web/live/transactions_test.exs b/lib/spendable_web/live/transactions_test.exs index c61c1cd7..09813993 100644 --- a/lib/spendable_web/live/transactions_test.exs +++ b/lib/spendable_web/live/transactions_test.exs @@ -300,6 +300,45 @@ defmodule SpendableWeb.Live.TransactionsTest do assert Decimal.eq?(amount, "-5.00") end + # Saying where the whole of a transaction went is the decision the queue is asking for, so + # making it is what finishes the row. + test "marks a transaction reviewed once the row says what it was spent from", %{ + conn: conn, + scope: scope, + budget: budget, + attrs: attrs + } do + {:ok, transaction} = Transactions.create_transaction(scope, Map.put(attrs, "name", "Coffee")) + + refute transaction.reviewed + + {:ok, view, _html} = live(conn, ~p"/transactions") + + view + |> element("#spend-from-#{transaction.id}") + |> render_change(%{"budget_id" => budget.id}) + + assert {:ok, %{reviewed: true}} = Transactions.get_transaction(scope, id: transaction.id) + end + + # A transfer is settled rather than set aside, so it reads like any other finished row. + test "does not dim a transaction that is part of a transfer", %{conn: conn, scope: scope, attrs: attrs} do + {:ok, out} = Transactions.create_transaction(scope, Map.put(attrs, "name", "To savings")) + + {:ok, into} = + Transactions.create_transaction( + scope, + attrs |> Map.put("name", "From checking") |> Map.put("amount", "5.00") + ) + + {:ok, _pair} = Transactions.mark_as_transfer(scope, out, into) + + {:ok, view, _html} = live(conn, ~p"/transactions") + + assert has_element?(view, "#transactions-#{out.id}") + refute has_element?(view, "#transactions-#{out.id}.opacity-40") + end + # A split has no single budget to offer, so the row sends the user to the drawer instead. test "offers no select for a split transaction", %{ conn: conn, diff --git a/lib/spendable_web/utils/account_label.ex b/lib/spendable_web/utils/account_label.ex new file mode 100644 index 00000000..88500db7 --- /dev/null +++ b/lib/spendable_web/utils/account_label.ex @@ -0,0 +1,12 @@ +defmodule SpendableWeb.Utils.AccountLabel do + @moduledoc "Import this module rather than aliasing it." + + @doc """ + An account reads as its name and the last few digits of its number. + + Not every account has one - an Apple Cash balance has nothing to print - and dots with nothing + after them say less than the name on its own. Mirrors `accountLabel` in the Flutter app. + """ + def account_label(name, number) when number in [nil, ""], do: name + def account_label(name, number), do: "#{name} ••••#{number}" +end diff --git a/mobile/lib/budgets/budgets_providers.dart b/mobile/lib/budgets/budgets_providers.dart index 731affd7..bcf22764 100644 --- a/mobile/lib/budgets/budgets_providers.dart +++ b/mobile/lib/budgets/budgets_providers.dart @@ -51,8 +51,9 @@ List listedBudgets(BudgetSummary summary) { final spendable = summary.budgets.where((budget) => budget.name == spendableName).firstOrNull; - // Envelopes, then goals, then what is only tracked, alphabetical inside each. Grouping them by - // what they are does the work a heading over each group would, without the headings. + // Envelopes, then what is only tracked, alphabetical inside each. Grouping them by what they are + // does the work a heading over each group would, without the headings. Goals go last: a goal is + // money going in rather than out, so it is not what the month is about. final rest = summary.budgets.where((budget) => budget.id != spendable?.id).toList() ..sort((a, b) { final byType = _typeOrder(a.type).compareTo(_typeOrder(b.type)); @@ -79,8 +80,8 @@ List listedBudgets(BudgetSummary summary) { int _typeOrder(BudgetTypeEnum type) => switch (type) { BudgetTypeEnum.envelope => 0, - BudgetTypeEnum.goal => 1, - _ => 2, + BudgetTypeEnum.goal => 2, + _ => 1, }; /// The budget picker every other screen offers. diff --git a/mobile/test/budgets/budgets_screen_test.dart b/mobile/test/budgets/budgets_screen_test.dart index ace3f588..b20d5d6e 100644 --- a/mobile/test/budgets/budgets_screen_test.dart +++ b/mobile/test/budgets/budgets_screen_test.dart @@ -235,8 +235,8 @@ void main() { expect(find.text('No limit set'), findsNothing); }); - // Envelopes, then goals, then what is only tracked - the grouping does the work a heading would. - testWidgets('orders the budgets by type with no heading over each group', (tester) async { + // Envelopes, then what is only tracked, then goals - the grouping does the work a heading would. + testWidgets('orders the budgets by type, with goals last', (tester) async { await _pump( tester, replies: { @@ -255,7 +255,7 @@ void main() { }, ); - final rows = ['Food', 'Rent', 'Vacation', 'Amazon'].map((name) => tester.getTopLeft(find.text(name)).dy); + final rows = ['Food', 'Rent', 'Amazon', 'Vacation'].map((name) => tester.getTopLeft(find.text(name)).dy); expect(rows, orderedEquals(rows.toList()..sort())); }); diff --git a/mobile/test/design/layout_test.dart b/mobile/test/design/layout_test.dart index 4905009a..27688fc5 100644 --- a/mobile/test/design/layout_test.dart +++ b/mobile/test/design/layout_test.dart @@ -6,6 +6,7 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:spendable/api/api_client.dart'; +import 'package:spendable/budgets/budgets_screen.dart'; import 'package:spendable/design/theme.dart'; import 'package:spendable/shell.dart'; import 'package:spendable_api/spendable_api.dart'; @@ -182,8 +183,13 @@ void main() { await tester.pumpAndSettle(); // Last, because reaching a row further down the list scrolls the large title away and the - // band's own buttons go with it. - await tester.ensureVisible(find.text('Emergency fund')); + // band's own buttons go with it. Scrolled to rather than ensured visible: goals sort to the + // end, and a sliver list has not built the rows down there yet. + await tester.scrollUntilVisible( + find.text('Emergency fund'), + 200, + scrollable: find.descendant(of: find.byType(BudgetsScreen), matching: find.byType(Scrollable)).first, + ); await tester.pumpAndSettle(); await tester.tap(find.text('Emergency fund')); await tester.pumpAndSettle();