Allow user to add buy and sell trade transactions for investment accounts #1066

2024-08-07T05:24:40+08:00

zachgoll commented

2024-08-07 05:24:40 +08:00

(Migrated from github.com)

Overview

Allows users to create buy and sell trades for their portfolio:

https://github.com/user-attachments/assets/f4c8bc65-31f6-4627-b4e7-477d0687c570

Other changes

In addition to this, I ran into some areas of the codebase that needed some cleanup and reworking to make it simpler to deal with this new account entry form. I have highlighted notable changes below:

Controller per-entry type

In #923, I introduced "entryable" delegated types and organized everything in the entries view and controller. The main purpose for this was to keep everything in one spot until we had a better reason to break it into separate types.

With this PR, we introduce the final entryable type forms for Account::Trade. Each entryable has a distinct view, so I decided to break it up into 4 controllers:

Account::EntriesController - shared logic, such as show, edit, and destroy. The show/edit actions delegate to their respective entryable templates, but by keeping things consolidated in the entries controller, we can render entries in a single list and add links to the respective "show" templates via the same path helper, account_entry_path(@account, @entry)
Entry controllers - for type-specific logic such as new, index, update, and create
- Account::TransactionsController
- Account::ValuationsController
- Account::TradesController

The primary goal here is to leverage our delegated types wherever possible while keeping type-specific concerns separate from each other.

Entry groups

Prior to this PR, it has been challenging to keep the view logic for grouping entries by date DRY while still being able to pass view-specific options to the entries within each group. While a little "clever", I think this helper strikes a balance that gives us some flexibility to "specialize" the date groups based on where they're being rendered.

def entries_by_date(entries, selectable: true)
  entries.group_by(&:date).map do |date, grouped_entries|
    content = capture do
      yield grouped_entries
    end

    render partial: "account/entries/entry_group", locals: { date:, entries: grouped_entries, content:, selectable: }
  end.join.html_safe
end

For example, in the global transactions view, we render a list that also has transfers grouped in a special way:

<%= entries_by_date(@transaction_entries) do |entries| %>
  <%= render entries.reject { |e| e.transfer_id.present? }, selectable: true %>
  <%= render transfer_entries(entries), selectable: false %>
<% end %>

While account entry lists are simpler:

<%= entries_by_date(@entries) do |entries| %>
  <%= render entries %>
<% end %>

Did a consolidation and cleanup of some logic that was duplicated many times across the codebase for displaying modal forms. To add a pre-styled modal form with a title, simply use the modal_form_wrapper helper:

<%= modal_form_wrapper title: t(".new_transaction") do %>
  <%= render "form", transaction: @transaction, entry: @entry %>
<% end %>

## Overview Allows users to create buy and sell trades for their portfolio: https://github.com/user-attachments/assets/f4c8bc65-31f6-4627-b4e7-477d0687c570 ## Other changes In addition to this, I ran into some areas of the codebase that needed some cleanup and reworking to make it simpler to deal with this new account entry form. I have highlighted notable changes below: ### Controller per-entry type In #923, I introduced "entryable" delegated types and organized everything in the `entries` view and controller. The main purpose for this was to keep everything in one spot until we had a better reason to break it into separate types. With this PR, we introduce the final entryable type forms for `Account::Trade`. Each entryable has a distinct view, so I decided to break it up into 4 controllers: - `Account::EntriesController` - shared logic, such as `show`, `edit`, and `destroy`. The `show`/`edit` actions delegate to their respective entryable templates, but by keeping things consolidated in the entries controller, we can render entries in a single list and add links to the respective "show" templates via the same path helper, `account_entry_path(@account, @entry)` - Entry controllers - for type-specific logic such as `new`, `index`, `update`, and `create` - `Account::TransactionsController` - `Account::ValuationsController` - `Account::TradesController` The primary goal here is to leverage our delegated types wherever possible while keeping type-specific concerns separate from each other. ### Entry groups Prior to this PR, it has been challenging to keep the view logic for grouping entries by date DRY while still being able to pass view-specific options to the entries within each group. While a little "clever", I think this helper strikes a balance that gives us some flexibility to "specialize" the date groups based on _where_ they're being rendered. ```rb def entries_by_date(entries, selectable: true) entries.group_by(&:date).map do |date, grouped_entries| content = capture do yield grouped_entries end render partial: "account/entries/entry_group", locals: { date:, entries: grouped_entries, content:, selectable: } end.join.html_safe end ``` For example, in the global transactions view, we render a list that also has transfers grouped in a special way: ```erb <%= entries_by_date(@transaction_entries) do |entries| %> <%= render entries.reject { |e| e.transfer_id.present? }, selectable: true %> <%= render transfer_entries(entries), selectable: false %> <% end %> ``` While account entry lists are simpler: ```erb <%= entries_by_date(@entries) do |entries| %> <%= render entries %> <% end %> ``` ### Modal forms helper Did a consolidation and cleanup of some logic that was duplicated many times across the codebase for displaying modal forms. To add a pre-styled modal form with a title, simply use the `modal_form_wrapper` helper: ```erb <%= modal_form_wrapper title: t(".new_transaction") do %> <%= render "form", transaction: @transaction, entry: @entry %> <% end %> ```

zachgoll (Migrated from github.com) reviewed 2024-08-08 03:20:57 +08:00

db/migrate/20240807153618_add_currency_field_to_trade.rb

						
				@@ -0,0 +1,5 @@

				class AddCurrencyFieldToTrade < ActiveRecord::Migration[7.2]

				  def change

				    add_column :account_trades, :currency, :string

zachgoll (Migrated from github.com) commented

2024-08-08 03:20:56 +08:00

Introduces a bit of denormalization here, but the alternative was quite a bit of added complexity and DB joins to read the currency value from Account::Entry for the sake of "monetizing" the price attribute on Account::Trade

Introduces a bit of denormalization here, but the alternative was quite a bit of added complexity and DB joins to read the `currency` value from `Account::Entry` for the sake of "monetizing" the `price` attribute on `Account::Trade`

zachgoll (Migrated from github.com) reviewed 2024-08-09 23:02:04 +08:00

app/controllers/account/entries_controller.rb

zachgoll (Migrated from github.com) commented

2024-08-09 23:02:04 +08:00

For a mixed list of entries, entries = [@trade, @transaction], we want to be able to do this in our views:

<% entries.each do |entry| %>
  <%= link_to account_entry_path(entry.account, entry) %>
<% end %>

This delegation helps us avoid:

<% entries.each do |entry| %>
  <% if entry.account_transaction %>
    <%= link_to account_transaction_path(entry.account, entry) %>
  <% elsif entry.account_trade? %>
   ...
<% end %>

For a mixed list of entries, `entries = [@trade, @transaction]`, we want to be able to do this in our views: ```erb <% entries.each do |entry| %> <%= link_to account_entry_path(entry.account, entry) %> <% end %> ``` This delegation helps us avoid: ``` <% entries.each do |entry| %> <% if entry.account_transaction %> <%= link_to account_transaction_path(entry.account, entry) %> <% elsif entry.account_trade? %> ... <% end %> ```

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gavin/maybe#1066