From e856691c8693303c9bee86fd0b0f1d65c960f728 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 23 Aug 2024 08:47:08 -0400 Subject: [PATCH] Add Property Details View (#1116) * Add backend for property account details * Rubocop updates * Add property form with details * Revert "Rubocop updates" This reverts commit 05b0b8f3a4eb8f444997b9bef18ffc972a8be69e. * Bump brakeman to latest version * Add overview section to property view * Lint fixes --- Gemfile.lock | 2 +- app/controllers/accounts_controller.rb | 2 + app/controllers/properties_controller.rb | 41 ++++++++++ app/helpers/accounts_helper.rb | 34 ++++++-- app/helpers/properties_helper.rb | 2 + app/models/account.rb | 35 ++++---- app/models/address.rb | 24 ++++++ app/models/measurement.rb | 20 +++++ app/models/property.rb | 27 +++++++ app/views/accounts/_form.html.erb | 21 +++++ app/views/accounts/_overview.html.erb | 3 + .../accounts/accountables/_property.html.erb | 34 ++++++++ .../accountables/property/_overview.html.erb | 40 ++++++++++ app/views/accounts/edit.html.erb | 14 +--- app/views/accounts/new.html.erb | 22 +----- app/views/accounts/show.html.erb | 25 ++++-- config/brakeman.ignore | 38 ++++++++- config/i18n-tasks.yml | 1 + config/locales/models/address/en.yml | 15 ++++ config/locales/views/accounts/en.yml | 40 +++++++--- config/locales/views/properties/en.yml | 7 ++ config/routes.rb | 2 + db/migrate/20240822174006_create_addresses.rb | 16 ++++ .../20240822180845_add_property_attributes.rb | 7 ++ db/schema.rb | 20 ++++- .../controllers/properties_controller_test.rb | 79 +++++++++++++++++++ test/fixtures/addresses.yml | 9 +++ test/fixtures/properties.yml | 5 +- test/models/address_test.rb | 15 ++++ test/system/accounts_test.rb | 28 ++++++- 30 files changed, 547 insertions(+), 81 deletions(-) create mode 100644 app/controllers/properties_controller.rb create mode 100644 app/helpers/properties_helper.rb create mode 100644 app/models/address.rb create mode 100644 app/models/measurement.rb create mode 100644 app/views/accounts/_form.html.erb create mode 100644 app/views/accounts/_overview.html.erb create mode 100644 app/views/accounts/accountables/property/_overview.html.erb create mode 100644 config/locales/models/address/en.yml create mode 100644 config/locales/views/properties/en.yml create mode 100644 db/migrate/20240822174006_create_addresses.rb create mode 100644 db/migrate/20240822180845_add_property_attributes.rb create mode 100644 test/controllers/properties_controller_test.rb create mode 100644 test/fixtures/addresses.yml create mode 100644 test/models/address_test.rb diff --git a/Gemfile.lock b/Gemfile.lock index 300ed3ad..005ec39a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -110,7 +110,7 @@ GEM bindex (0.8.1) bootsnap (1.18.4) msgpack (~> 1.2) - brakeman (6.1.2) + brakeman (6.2.1) racc builder (3.3.0) capybara (3.40.0) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 30a0e0d7..d2455152 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -25,6 +25,8 @@ class AccountsController < ApplicationController def new @account = Account.new(accountable: Accountable.from_type(params[:type])&.new) + @account.accountable.address = Address.new if @account.accountable.is_a?(Property) + if params[:institution_id] @account.institution = Current.family.institutions.find_by(id: params[:institution_id]) end diff --git a/app/controllers/properties_controller.rb b/app/controllers/properties_controller.rb new file mode 100644 index 00000000..b663b6ae --- /dev/null +++ b/app/controllers/properties_controller.rb @@ -0,0 +1,41 @@ +class PropertiesController < ApplicationController + before_action :set_account, only: :update + + def create + account = Current.family + .accounts + .create_with_optional_start_balance! \ + attributes: account_params.except(:start_date, :start_balance), + start_date: account_params[:start_date], + start_balance: account_params[:start_balance] + + account.sync_later + redirect_to account, notice: t(".success") + end + + def update + @account.update!(account_params) + @account.sync_later + redirect_to @account, notice: t(".success") + end + + private + + def set_account + @account = Current.family.accounts.find(params[:id]) + end + + def account_params + params.require(:account) + .permit( + :name, :balance, :start_date, :start_balance, :currency, :accountable_type, + accountable_attributes: [ + :id, + :year_built, + :area_unit, + :area_value, + address_attributes: [ :line1, :line2, :locality, :region, :country, :postal_code ] + ] + ) + end +end diff --git a/app/helpers/accounts_helper.rb b/app/helpers/accounts_helper.rb index 7436de02..c26fde98 100644 --- a/app/helpers/accounts_helper.rb +++ b/app/helpers/accounts_helper.rb @@ -23,13 +23,35 @@ module AccountsHelper class_mapping(accountable_type)[:hex] end - def account_tabs(account) - holdings_tab = { key: "holdings", label: t("accounts.show.holdings"), path: account_path(account, tab: "holdings"), content_path: account_holdings_path(account) } - cash_tab = { key: "cash", label: t("accounts.show.cash"), path: account_path(account, tab: "cash"), content_path: account_cashes_path(account) } - value_tab = { key: "valuations", label: t("accounts.show.value"), path: account_path(account, tab: "valuations"), content_path: account_valuations_path(account) } - transactions_tab = { key: "transactions", label: t("accounts.show.transactions"), path: account_path(account, tab: "transactions"), content_path: account_transactions_path(account) } - trades_tab = { key: "trades", label: t("accounts.show.trades"), path: account_path(account, tab: "trades"), content_path: account_trades_path(account) } + # Eventually, we'll have an accountable form for each type of accountable, so + # this helper is a convenience for now to reuse common logic in the accounts controller + def new_account_form_url(account) + case account.accountable_type + when "Property" + properties_path + else + accounts_path + end + end + def edit_account_form_url(account) + case account.accountable_type + when "Property" + property_path(account) + else + account_path(account) + end + end + + def account_tabs(account) + overview_tab = { key: "overview", label: t("accounts.show.overview"), path: account_path(account, tab: "overview"), partial_path: "accounts/overview" } + holdings_tab = { key: "holdings", label: t("accounts.show.holdings"), path: account_path(account, tab: "holdings"), route: account_holdings_path(account) } + cash_tab = { key: "cash", label: t("accounts.show.cash"), path: account_path(account, tab: "cash"), route: account_cashes_path(account) } + value_tab = { key: "valuations", label: t("accounts.show.value"), path: account_path(account, tab: "valuations"), route: account_valuations_path(account) } + transactions_tab = { key: "transactions", label: t("accounts.show.transactions"), path: account_path(account, tab: "transactions"), route: account_transactions_path(account) } + trades_tab = { key: "trades", label: t("accounts.show.trades"), path: account_path(account, tab: "trades"), route: account_trades_path(account) } + + return [ overview_tab, value_tab ] if account.property? return [ holdings_tab, cash_tab, trades_tab ] if account.investment? [ value_tab, transactions_tab ] diff --git a/app/helpers/properties_helper.rb b/app/helpers/properties_helper.rb new file mode 100644 index 00000000..e9841903 --- /dev/null +++ b/app/helpers/properties_helper.rb @@ -0,0 +1,2 @@ +module PropertiesHelper +end diff --git a/app/models/account.rb b/app/models/account.rb index 9f17f756..d29f5479 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -28,6 +28,8 @@ class Account < ApplicationRecord delegated_type :accountable, types: Accountable::TYPES, dependent: :destroy + accepts_nested_attributes_for :accountable + delegate :value, :series, to: :accountable class << self @@ -51,27 +53,28 @@ class Account < ApplicationRecord end def create_with_optional_start_balance!(attributes:, start_date: nil, start_balance: nil) - account = self.new(attributes.except(:accountable_type)) - account.accountable = Accountable.from_type(attributes[:accountable_type])&.new + transaction do + attributes[:accountable_attributes] ||= {} # Ensure accountable is created + account = new(attributes) - # Always build the initial valuation - account.entries.build \ - date: Date.current, - amount: attributes[:balance], - currency: account.currency, - entryable: Account::Valuation.new - - # Conditionally build the optional start valuation - if start_date.present? && start_balance.present? + # Always initialize an account with a valuation entry to begin tracking value history account.entries.build \ - date: start_date, - amount: start_balance, + date: Date.current, + amount: account.balance, currency: account.currency, entryable: Account::Valuation.new - end - account.save! - account + if start_date.present? && start_balance.present? + account.entries.build \ + date: start_date, + amount: start_balance, + currency: account.currency, + entryable: Account::Valuation.new + end + + account.save! + account + end end end diff --git a/app/models/address.rb b/app/models/address.rb new file mode 100644 index 00000000..6ad09276 --- /dev/null +++ b/app/models/address.rb @@ -0,0 +1,24 @@ +class Address < ApplicationRecord + belongs_to :addressable, polymorphic: true + + validates :line1, :locality, presence: true + validates :postal_code, presence: true, if: :postal_code_required? + + def to_s + I18n.t("address.format", + line1: line1, + line2: line2, + county: county, + locality: locality, + region: region, + country: country, + postal_code: postal_code + ) + end + + private + + def postal_code_required? + country.in?(%w[US CA GB]) + end +end diff --git a/app/models/measurement.rb b/app/models/measurement.rb new file mode 100644 index 00000000..56b3ec43 --- /dev/null +++ b/app/models/measurement.rb @@ -0,0 +1,20 @@ +class Measurement + include ActiveModel::Validations + + attr_reader :value, :unit + + VALID_UNITS = %w[sqft sqm] + + validates :unit, inclusion: { in: VALID_UNITS } + validates :value, presence: true + + def initialize(value, unit) + @value = value.to_f + @unit = unit.to_s.downcase.strip + validate! + end + + def to_s + "#{@value.to_i} #{@unit}" + end +end diff --git a/app/models/property.rb b/app/models/property.rb index cae953c9..d5af127b 100644 --- a/app/models/property.rb +++ b/app/models/property.rb @@ -1,3 +1,30 @@ class Property < ApplicationRecord include Accountable + + has_one :address, as: :addressable, dependent: :destroy + + accepts_nested_attributes_for :address + + attribute :area_unit, :string, default: "sqft" + + def area + Measurement.new(area_value, area_unit) if area_value.present? + end + + def purchase_price + first_valuation_amount + end + + def equity_value + account.balance_money + end + + def trend + TimeSeries::Trend.new(current: account.balance_money, previous: first_valuation_amount) + end + + private + def first_valuation_amount + account.entries.account_valuations.order(:date).first&.amount_money || account.balance_money + end end diff --git a/app/views/accounts/_form.html.erb b/app/views/accounts/_form.html.erb new file mode 100644 index 00000000..86ee96bd --- /dev/null +++ b/app/views/accounts/_form.html.erb @@ -0,0 +1,21 @@ +<%# locals: (account:, url:) %> + +<%= styled_form_with model: account, url: url, scope: :account, class: "flex flex-col gap-4 justify-between grow", data: { turbo: false } do |f| %> +
+ <%= f.hidden_field :accountable_type %> + <%= f.text_field :name, placeholder: t(".name_placeholder"), required: "required", label: t(".name_label"), autofocus: true %> + <%= f.collection_select :institution_id, Current.family.institutions.alphabetically, :id, :name, { include_blank: t(".ungrouped"), label: t(".institution") } %> + <%= money_with_currency_field f, :balance_money, label: t(".balance"), required: "required", default_currency: Current.family.currency %> + + <% if account.new_record? %> +
+
<%= f.date_field :start_date, label: t(".start_date"), max: Date.yesterday, min: Account::Entry.min_supported_date %>
+
<%= f.number_field :start_balance, label: t(".start_balance"), placeholder: 90 %>
+
+ <% end %> + + <%= render "accounts/accountables/#{permitted_accountable_partial(account.accountable_type)}", f: f %> +
+ + <%= f.submit "#{account.new_record? ? "Add" : "Update"} #{account.accountable.model_name.human.downcase}" %> +<% end %> diff --git a/app/views/accounts/_overview.html.erb b/app/views/accounts/_overview.html.erb new file mode 100644 index 00000000..51569c02 --- /dev/null +++ b/app/views/accounts/_overview.html.erb @@ -0,0 +1,3 @@ +<%# locals: (account:) %> + +<%= render partial: "accounts/accountables/#{account.accountable_type.downcase}/overview", locals: { account: account } %> diff --git a/app/views/accounts/accountables/_property.html.erb b/app/views/accounts/accountables/_property.html.erb index e69de29b..47f84a0f 100644 --- a/app/views/accounts/accountables/_property.html.erb +++ b/app/views/accounts/accountables/_property.html.erb @@ -0,0 +1,34 @@ +<%# locals: (f:) %> + +
+
+ +
+ <%= f.fields_for :accountable do |af| %> +
+ <%= af.number_field :year_built, label: t(".year_built"), placeholder: 2005 %> + <%= af.number_field :area_value, label: t(".area_value"), placeholder: 2000 %> + <%= af.select :area_unit, + [["Square feet", "sqft"], ["Square meters", "sqm"]], + { label: t(".area_unit") } %> +
+ + <%= af.fields_for :address do |address_form| %> +
+ <%= address_form.text_field :line1, label: t(".line1"), placeholder: "123 Main St", required: true %> + <%= address_form.text_field :line2, label: t(".line2"), placeholder: "Apt 1" %> +
+ +
+ <%= address_form.text_field :locality, label: t(".city"), placeholder: "Sacramento", required: true %> + <%= address_form.text_field :region, label: t(".state"), placeholder: "CA", required: true %> +
+ +
+ <%= address_form.text_field :postal_code, label: t(".postal_code"), placeholder: "95814" %> + <%= address_form.text_field :country, label: t(".country"), placeholder: "USA", required: true %> +
+ <% end %> + <% end %> +
+
diff --git a/app/views/accounts/accountables/property/_overview.html.erb b/app/views/accounts/accountables/property/_overview.html.erb new file mode 100644 index 00000000..5e11257c --- /dev/null +++ b/app/views/accounts/accountables/property/_overview.html.erb @@ -0,0 +1,40 @@ +<%# locals: (account:) %> + +
+
+

<%= t(".market_value") %>

+

<%= format_money(account.balance_money) %>

+
+ +
+

<%= t(".purchase_price") %>

+

+ <%= account.property.purchase_price ? format_money(account.property.purchase_price) : t(".unknown") %> +

+
+ +
+

<%= t(".trend") %>

+
+

+ <%= account.property.trend.value %> +

+ +

(<%= account.property.trend.percent %>%)

+
+
+ +
+

<%= t(".year_built") %>

+

+ <%= account.property.year_built || t(".unknown") %> +

+
+ +
+

<%= t(".living_area") %>

+

+ <%= account.property.area || t(".unknown") %> +

+
+
diff --git a/app/views/accounts/edit.html.erb b/app/views/accounts/edit.html.erb index 8767c4ad..26eff37d 100644 --- a/app/views/accounts/edit.html.erb +++ b/app/views/accounts/edit.html.erb @@ -1,15 +1,3 @@ <%= modal_form_wrapper title: t(".edit", account: @account.name) do %> - <%= styled_form_with model: @account, class: "space-y-4", data: { turbo_frame: "_top" } do |f| %> - <%= f.text_field :name, label: t(".name") %> - <%= money_with_currency_field f, :balance_money, label: t(".balance"), default_currency: @account.currency, disable_currency: true %> - -
- <%= f.collection_select :institution_id, Current.family.institutions.alphabetically, :id, :name, { include_blank: t(".ungrouped"), label: t(".institution") } %> - <%= link_to new_institution_path do %> - <%= lucide_icon "plus", class: "text-gray-700 hover:text-gray-500 w-4 h-4 absolute right-3 top-2" %> - <% end %> -
- - <%= f.submit %> - <% end %> + <%= render "form", account: @account, url: edit_account_form_url(@account) %> <% end %> diff --git a/app/views/accounts/new.html.erb b/app/views/accounts/new.html.erb index cdcf59bf..c7ca188a 100644 --- a/app/views/accounts/new.html.erb +++ b/app/views/accounts/new.html.erb @@ -73,26 +73,10 @@ <% end %> Add <%= @account.accountable.model_name.human.downcase %> - <%= styled_form_with model: @account, url: accounts_path, scope: :account, class: "m-5 mt-1 flex flex-col justify-between grow", data: { turbo: false } do |f| %> -
- <%= f.hidden_field :accountable_type %> - <%= f.text_field :name, placeholder: t(".name.placeholder"), required: "required", label: t(".name.label"), autofocus: true %> - <%= f.collection_select :institution_id, Current.family.institutions.alphabetically, :id, :name, { include_blank: t(".ungrouped"), label: t(".institution") } %> - <%= render "accounts/accountables/#{permitted_accountable_partial(@account.accountable_type)}", f: f %> - <%= money_with_currency_field f, :balance_money, label: t(".balance"), required: "required", default_currency: Current.family.currency %> -
- <%= check_box_tag :add_start_values, class: "maybe-checkbox maybe-checkbox--light peer mb-1" %> - <%= label_tag :add_start_values, t(".optional_start_balance_message"), class: "pl-1 text-sm text-gray-500" %> - - -
-
- <%= f.submit "Add #{@account.accountable.model_name.human.downcase}" %> - <% end %> +
+ <%= render "form", account: @account, url: new_account_form_url(@account) %> +
<% end %> <% end %> diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index 6da5ab9b..b1cead95 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -1,10 +1,16 @@ <%= turbo_stream_from @account %> <%= tag.div id: dom_id(@account), class: "space-y-4" do %> -
+
<%= image_tag account_logo_url(@account), class: "w-8 h-8" %> -

<%= @account.name %>

+
+

<%= @account.name %>

+ + <% if @account.property? && @account.property.address %> +

<%= @account.property.address %>

+ <% end %> +
<%= button_to sync_account_path(@account), method: :post, class: "flex items-center gap-2", title: "Sync Account" do %> @@ -43,7 +49,7 @@
<% end %>
- + <% if @account.highest_priority_issue %> <%= render partial: "issues/issue", locals: { issue: @account.highest_priority_issue } %> @@ -79,7 +85,10 @@ - <% selected_tab_key, selected_tab_content_path = selected_account_tab(@account).values_at(:key, :content_path) %> + <% selected_tab = selected_account_tab(@account) %> + <% selected_tab_key = selected_tab[:key] %> + <% selected_tab_partial_path = selected_tab[:partial_path] %> + <% selected_tab_route = selected_tab[:route] %>
<% account_tabs(@account).each do |tab| %> @@ -88,8 +97,12 @@
- <%= turbo_frame_tag dom_id(@account, selected_tab_key), src: selected_tab_content_path do %> - <%= render "account/entries/loading" %> + <% if selected_tab_route.present? %> + <%= turbo_frame_tag dom_id(@account, selected_tab_key), src: selected_tab_route do %> + <%= render "account/entries/loading" %> + <% end %> + <% else %> + <%= render selected_tab_partial_path, account: @account %> <% end %>
<% end %> diff --git a/config/brakeman.ignore b/config/brakeman.ignore index b0e688cd..d4a3640e 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -22,8 +22,42 @@ 22 ], "note": "" + }, + { + "warning_type": "Dynamic Render Path", + "warning_code": 15, + "fingerprint": "b7a59d6dd91f4d30873b271659636c7975e25b47f436b4f03900a08809af2e92", + "check_name": "Render", + "message": "Render path contains parameter value", + "file": "app/views/accounts/show.html.erb", + "line": 105, + "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", + "code": "render(action => selected_account_tab(Current.family.accounts.find(params[:id]))[:partial_path], { :account => Current.family.accounts.find(params[:id]) })", + "render_path": [ + { + "type": "controller", + "class": "AccountsController", + "method": "show", + "line": 38, + "file": "app/controllers/accounts_controller.rb", + "rendered": { + "name": "accounts/show", + "file": "app/views/accounts/show.html.erb" + } + } + ], + "location": { + "type": "template", + "template": "accounts/show" + }, + "user_input": "params[:id]", + "confidence": "Weak", + "cwe_id": [ + 22 + ], + "note": "" } ], - "updated": "2024-08-16 10:19:50 -0400", - "brakeman_version": "6.1.2" + "updated": "2024-08-23 08:29:05 -0400", + "brakeman_version": "6.2.1" } diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml index 7e58f1f3..028f9607 100644 --- a/config/i18n-tasks.yml +++ b/config/i18n-tasks.yml @@ -29,3 +29,4 @@ ignore_unused: - 'helpers.submit.*' # i18n-tasks does not detect used at forms - 'helpers.label.*' # i18n-tasks does not detect used at forms - 'accounts.show.sync_message_*' # messages generated in the sync ActiveJob + - 'address.attributes.*' diff --git a/config/locales/models/address/en.yml b/config/locales/models/address/en.yml new file mode 100644 index 00000000..eb0db551 --- /dev/null +++ b/config/locales/models/address/en.yml @@ -0,0 +1,15 @@ +--- +en: + address: + attributes: + country: Country + line1: Address Line 1 + line2: Address Line 2 + locality: Locality + postal_code: Postal Code + region: Region + format: |- + %{line1} + %{line2} + %{locality}, %{region} %{postal_code} + %{country} diff --git a/config/locales/views/accounts/en.yml b/config/locales/views/accounts/en.yml index 45bc7fce..eb4bdba1 100644 --- a/config/locales/views/accounts/en.yml +++ b/config/locales/views/accounts/en.yml @@ -1,20 +1,42 @@ --- en: accounts: + accountables: + property: + area_unit: Area unit + area_value: Area value (optional) + city: City + country: Country + line1: Address line 1 + line2: Address line 2 (optional) + overview: + living_area: Living Area + market_value: Market Value + purchase_price: Purchase Price + trend: Trend + unknown: Unknown + year_built: Year Built + postal_code: Postal code (optional) + state: State + year_built: Year built (optional) create: success: New account created successfully destroy: success: Account deleted successfully edit: - balance: Balance edit: Edit %{account} - institution: Financial institution - name: Name - ungrouped: "(none)" empty: empty_message: Add an account either via connection, importing or entering manually. new_account: New account no_accounts: No accounts yet + form: + balance: Current balance + institution: Financial institution + name_label: Account name + name_placeholder: Example account name + start_balance: Start balance (optional) + start_date: Start date (optional) + ungrouped: "(none)" header: accounts: Accounts manage: Manage accounts @@ -36,17 +58,8 @@ en: institutionless_accounts: other_accounts: Other accounts new: - balance: Current balance - institution: Financial institution - name: - label: Account name - placeholder: Example account name - optional_start_balance_message: Add a start balance for this account select_accountable_type: What would you like to add? - start_balance: Start balance (optional) - start_date: Start date (optional) title: Add an account - ungrouped: "(none)" show: cash: Cash confirm_accept: Delete "%{name}" @@ -60,6 +73,7 @@ en: holdings: Holdings import: Import transactions no_change: No change + overview: Overview 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. diff --git a/config/locales/views/properties/en.yml b/config/locales/views/properties/en.yml new file mode 100644 index 00000000..a59f2f24 --- /dev/null +++ b/config/locales/views/properties/en.yml @@ -0,0 +1,7 @@ +--- +en: + properties: + create: + success: Property created successfully + update: + success: Property updated successfully diff --git a/config/routes.rb b/config/routes.rb index bbf8c8ac..25b92740 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -89,6 +89,8 @@ Rails.application.routes.draw do end end + resources :properties, only: %i[ create update ] + resources :transactions, only: %i[ index new create ] do collection do post "bulk_delete" diff --git a/db/migrate/20240822174006_create_addresses.rb b/db/migrate/20240822174006_create_addresses.rb new file mode 100644 index 00000000..a4e7e1bd --- /dev/null +++ b/db/migrate/20240822174006_create_addresses.rb @@ -0,0 +1,16 @@ +class CreateAddresses < ActiveRecord::Migration[7.2] + def change + create_table :addresses, id: :uuid do |t| + t.references :addressable, type: :uuid, polymorphic: true + t.string :line1 + t.string :line2 + t.string :county + t.string :locality + t.string :region + t.string :country + t.integer :postal_code + + t.timestamps + end + end +end diff --git a/db/migrate/20240822180845_add_property_attributes.rb b/db/migrate/20240822180845_add_property_attributes.rb new file mode 100644 index 00000000..793a0fa8 --- /dev/null +++ b/db/migrate/20240822180845_add_property_attributes.rb @@ -0,0 +1,7 @@ +class AddPropertyAttributes < ActiveRecord::Migration[7.2] + def change + add_column :properties, :year_built, :integer + add_column :properties, :area_value, :integer + add_column :properties, :area_unit, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 3a423928..cb2fe1ff 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_08_17_144454) do +ActiveRecord::Schema[7.2].define(version: 2024_08_22_180845) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -152,6 +152,21 @@ ActiveRecord::Schema[7.2].define(version: 2024_08_17_144454) do t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true end + create_table "addresses", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.string "addressable_type" + t.uuid "addressable_id" + t.string "line1" + t.string "line2" + t.string "county" + t.string "locality" + t.string "region" + t.string "country" + t.integer "postal_code" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["addressable_type", "addressable_id"], name: "index_addresses_on_addressable" + end + create_table "categories", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.string "name", null: false t.string "color", default: "#6172F3", null: false @@ -358,6 +373,9 @@ ActiveRecord::Schema[7.2].define(version: 2024_08_17_144454) do create_table "properties", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "year_built" + t.integer "area_value" + t.string "area_unit" end create_table "securities", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| diff --git a/test/controllers/properties_controller_test.rb b/test/controllers/properties_controller_test.rb new file mode 100644 index 00000000..c2debe3e --- /dev/null +++ b/test/controllers/properties_controller_test.rb @@ -0,0 +1,79 @@ +require "test_helper" + +class PropertiesControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in @user = users(:family_admin) + @account = accounts(:property) + end + + test "creates property" do + assert_difference -> { Account.count } => 1, + -> { Property.count } => 1, + -> { Account::Valuation.count } => 2, + -> { Account::Entry.count } => 2 do + post properties_path, params: { + account: { + name: "Property", + balance: 500000, + currency: "USD", + accountable_type: "Property", + start_date: 3.years.ago.to_date, + start_balance: 450000, + accountable_attributes: { + year_built: 2002, + area_value: 1000, + area_unit: "sqft", + address_attributes: { + line1: "123 Main St", + line2: "Apt 1", + locality: "Los Angeles", + region: "CA", # ISO3166-2 code + country: "US", # ISO3166-1 Alpha-2 code + postal_code: "90001" + } + } + } + } + end + + created_account = Account.order(:created_at).last + + assert created_account.property.year_built.present? + assert created_account.property.address.line1.present? + + assert_redirected_to account_path(created_account) + assert_equal "Property created successfully", flash[:notice] + assert_enqueued_with(job: AccountSyncJob) + end + + test "updates property" do + assert_no_difference [ "Account.count", "Property.count", "Account::Valuation.count", "Account::Entry.count" ] do + patch property_path(@account), params: { + account: { + name: "Updated Property", + balance: 500000, + currency: "USD", + accountable_type: "Property", + accountable_attributes: { + id: @account.accountable_id, + year_built: 2002, + area_value: 1000, + area_unit: "sqft", + address_attributes: { + line1: "123 Main St", + line2: "Apt 1", + locality: "Los Angeles", + region: "CA", # ISO3166-2 code + country: "US", # ISO3166-1 Alpha-2 code + postal_code: "90001" + } + } + } + } + end + + assert_redirected_to account_path(@account) + assert_equal "Property updated successfully", flash[:notice] + assert_enqueued_with(job: AccountSyncJob) + end +end diff --git a/test/fixtures/addresses.yml b/test/fixtures/addresses.yml new file mode 100644 index 00000000..0124887e --- /dev/null +++ b/test/fixtures/addresses.yml @@ -0,0 +1,9 @@ +one: + line1: 123 Main Street + line2: Apt 4B + locality: Los Angeles + region: CA + country: US + postal_code: 90001 + addressable: one + addressable_type: Property diff --git a/test/fixtures/properties.yml b/test/fixtures/properties.yml index e0553ab0..c330c7f3 100644 --- a/test/fixtures/properties.yml +++ b/test/fixtures/properties.yml @@ -1 +1,4 @@ -one: { } \ No newline at end of file +one: + year_built: 2002 + area_value: 1000 + area_unit: "sqft" \ No newline at end of file diff --git a/test/models/address_test.rb b/test/models/address_test.rb new file mode 100644 index 00000000..e6d50548 --- /dev/null +++ b/test/models/address_test.rb @@ -0,0 +1,15 @@ +require "test_helper" + +class AddressTest < ActiveSupport::TestCase + test "can print a formatted address" do + address = Address.new( + line1: "123 Main St", + locality: "San Francisco", + region: "CA", + country: "US", + postal_code: "94101" + ) + + assert_equal "123 Main St\n\nSan Francisco, CA 94101\nUS", address.to_s + end +end diff --git a/test/system/accounts_test.rb b/test/system/accounts_test.rb index f4504ab3..3873e784 100644 --- a/test/system/accounts_test.rb +++ b/test/system/accounts_test.rb @@ -21,7 +21,16 @@ class AccountsTest < ApplicationSystemTestCase end test "can create property account" do - assert_account_created("Property") + assert_account_created "Property" do + fill_in "Year built (optional)", with: 2005 + fill_in "Area value (optional)", with: 2250 + fill_in "Address line 1", with: "123 Main St" + fill_in "Address line 2", with: "Apt 4B" + fill_in "City", with: "San Francisco" + fill_in "State", with: "CA" + fill_in "Postal code (optional)", with: "94101" + fill_in "Country", with: "US" + end end test "can create vehicle account" do @@ -50,7 +59,7 @@ class AccountsTest < ApplicationSystemTestCase click_link "sidebar-new-account" end - def assert_account_created(accountable_type) + def assert_account_created(accountable_type, &block) click_link humanized_accountable(accountable_type) click_link "Enter account balance manually" @@ -59,9 +68,11 @@ class AccountsTest < ApplicationSystemTestCase fill_in "Account name", with: account_name select "Chase", from: "Financial institution" fill_in "account[balance]", with: 100.99 - check "Add a start balance for this account" fill_in "Start date (optional)", with: 10.days.ago.to_date fill_in "Start balance (optional)", with: 95 + + yield if block_given? + click_button "Add #{humanized_accountable(accountable_type).downcase}" find("details", text: humanized_accountable(accountable_type)).click @@ -69,6 +80,17 @@ class AccountsTest < ApplicationSystemTestCase visit accounts_url assert_text account_name + + visit account_url(Account.order(:created_at).last) + + within "header" do + find('button[data-menu-target="button"]').click + click_on "Edit" + end + + fill_in "Account name", with: "Updated account name" + click_button "Update #{humanized_accountable(accountable_type).downcase}" + assert_selector "h2", text: "Updated account name" end def humanized_accountable(accountable_type)