Enhanced Import Amount Type Selection (#506)

* Enhanced Import Amount Type Selection updated version of https://github.com/we-promise/sure/pull/179 * copilot sugestions * ai sugestions * Update import.rb * Update schema.rb * Update schema.rb * Update schema.rb --------- Signed-off-by: Juan José Mata <juanjo.mata@gmail.com> Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
2026-04-11 00:04:47 +00:00 · 2026-01-23 22:12:02 +01:00
parent 696ff0966b
commit 9b84c5bdbc
9 changed files with 94 additions and 8 deletions
--- a/app/controllers/import/configurations_controller.rb
+++ b/app/controllers/import/configurations_controller.rb
@@ -46,6 +46,7 @@ class Import::ConfigurationsController < ApplicationController
        :number_format,
        :signage_convention,
        :amount_type_strategy,
+        :amount_type_identifier_value,
        :amount_type_inflow_value,
        :rows_to_skip
      )
--- a/app/javascript/controllers/import_controller.js
+++ b/app/javascript/controllers/import_controller.js
@@ -11,6 +11,7 @@ export default class extends Controller {
    "signedAmountFieldset",
    "customColumnFieldset",
    "amountTypeValue",
+    "amountTypeInflowValue",
    "amountTypeStrategySelect",
  ];

@@ -20,6 +21,9 @@ export default class extends Controller {
      this.amountTypeColumnKeyValue
    ) {
      this.#showAmountTypeValueTargets(this.amountTypeColumnKeyValue);
+      if (this.amountTypeValueTarget.querySelector("select")?.value) {
+        this.#showAmountTypeInflowValueTargets();
+      }
    }
  }

@@ -31,6 +35,9 @@ export default class extends Controller {

      if (this.amountTypeColumnKeyValue) {
        this.#showAmountTypeValueTargets(this.amountTypeColumnKeyValue);
+        if (this.amountTypeValueTarget.querySelector("select")?.value) {
+          this.#showAmountTypeInflowValueTargets();
+        }
      }
    }

@@ -43,6 +50,11 @@ export default class extends Controller {
    const amountTypeColumnKey = event.target.value;

    this.#showAmountTypeValueTargets(amountTypeColumnKey);
+    this.#showAmountTypeInflowValueTargets();
+  }
+
+  handleAmountTypeIdentifierChange(event) {
+    this.#showAmountTypeInflowValueTargets();
  }

  refreshForm(event) {
@@ -91,6 +103,29 @@ export default class extends Controller {
    select.appendChild(fragment);
  }

+  #showAmountTypeInflowValueTargets() {
+    // Called when amount_type_identifier_value changes
+    // Updates the displayed identifier value in the UI text and shows/hides the inflow value dropdown
+    const identifierValueSelect = this.amountTypeValueTarget.querySelector("select");
+    const selectedValue = identifierValueSelect.value;
+
+    if (!selectedValue) {
+      this.amountTypeInflowValueTarget.classList.add("hidden");
+      this.amountTypeInflowValueTarget.classList.remove("flex");
+      return;
+    }
+
+    // Show the inflow value dropdown
+    this.amountTypeInflowValueTarget.classList.remove("hidden");
+    this.amountTypeInflowValueTarget.classList.add("flex");
+
+    // Update the displayed identifier value in the text
+    const identifierSpan = this.amountTypeInflowValueTarget.querySelector("span.font-medium");
+    if (identifierSpan) {
+      identifierSpan.textContent = selectedValue;
+    }
+  }
+
  #uniqueValuesForColumn(column) {
    const colIdx = this.csvValue[0].indexOf(column);
    const values = this.csvValue.slice(1).map((row) => row[colIdx]);
@@ -120,6 +155,11 @@ export default class extends Controller {
    this.customColumnFieldsetTarget.classList.add("hidden");
    this.signedAmountFieldsetTarget.classList.remove("hidden");

+    // Hide the inflow value targets when using signed amount strategy
+    this.amountTypeValueTarget.classList.add("hidden");
+    this.amountTypeValueTarget.classList.remove("flex");
+    this.amountTypeInflowValueTarget.classList.add("hidden");
+    this.amountTypeInflowValueTarget.classList.remove("flex");
    // Remove required from custom column fields
    this.customColumnFieldsetTarget
      .querySelectorAll("select, input")
--- a/app/models/import.rb
+++ b/app/models/import.rb
@@ -40,6 +40,7 @@ class Import < ApplicationRecord
  validates :col_sep, inclusion: { in: SEPARATORS.map(&:last) }
  validates :signage_convention, inclusion: { in: SIGNAGE_CONVENTIONS }, allow_nil: true
  validates :number_format, presence: true, inclusion: { in: NUMBER_FORMATS.keys }
+  validate :custom_column_import_requires_identifier
  validates :rows_to_skip, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
  validate :account_belongs_to_family
  validate :rows_to_skip_within_file_bounds
@@ -305,6 +306,14 @@ class Import < ApplicationRecord
      self.number_format ||= "1,234.56" # Default to US/UK format
    end

+    def custom_column_import_requires_identifier
+      return unless amount_type_strategy == "custom_column"
+
+      if amount_type_inflow_value.blank?
+        errors.add(:base, I18n.t("imports.errors.custom_column_requires_inflow"))
+      end
+    end
+
    # Common encodings to try when UTF-8 detection fails
    # Windows-1250 is prioritized for Central/Eastern European languages
    COMMON_ENCODINGS = [ "Windows-1250", "Windows-1252", "ISO-8859-1", "ISO-8859-2" ].freeze
--- a/app/models/import/row.rb
+++ b/app/models/import/row.rb
@@ -47,12 +47,27 @@ class Import::Row < ApplicationRecord
      if import.amount_type_strategy == "signed_amount"
        value * (import.signage_convention == "inflows_positive" ? -1 : 1)
      elsif import.amount_type_strategy == "custom_column"
-        inflow_value = import.amount_type_inflow_value
+        legacy_identifier = import.amount_type_inflow_value
+        selected_identifier =
+          if import.amount_type_identifier_value.present?
+            import.amount_type_identifier_value
+          else
+            legacy_identifier
+          end

-        if entity_type == inflow_value
-          value * -1
+        inflow_treatment =
+          if import.amount_type_inflow_value.in?(%w[inflows_positive inflows_negative])
+            import.amount_type_inflow_value
+          elsif import.signage_convention.in?(%w[inflows_positive inflows_negative])
+            import.signage_convention
+          else
+            "inflows_positive"
+          end
+
+        if entity_type == selected_identifier
+          value * (inflow_treatment == "inflows_positive" ? -1 : 1)
        else
-          value
+          value * (inflow_treatment == "inflows_positive" ? 1 : -1)
        end
      else
        raise "Unknown amount type strategy for import: #{import.amount_type_strategy}"
--- a/app/views/import/configurations/_transaction_import.html.erb
+++ b/app/views/import/configurations/_transaction_import.html.erb
@@ -82,11 +82,21 @@
      <div class="items-center gap-2 text-sm <%= @import.entity_type_col_label.nil? ? "hidden" : "flex" %>" data-import-target="amountTypeValue">
        <span class="shrink-0 text-secondary">↪</span>
        <span class="text-secondary">Set</span>
-        <%= form.select :amount_type_inflow_value,
+        <%= form.select :amount_type_identifier_value,
                        @import.selectable_amount_type_values,
-                        { prompt: "Select column", container_class: "w-48 px-3 py-1.5 border border-secondary rounded-md" },
+                        { prompt: "Select value", container_class: "w-48 px-3 py-1.5 border border-secondary rounded-md" },
+                        required: @import.amount_type_strategy == "custom_column",
+                        data: { action: "import#handleAmountTypeIdentifierChange" } %>
+        <span class="text-secondary">as identifier value</span>
+      </div>
+
+      <div class="items-center gap-2 text-sm <%= @import.amount_type_identifier_value.nil? ? "hidden" : "flex" %>" data-import-target="amountTypeInflowValue">
+        <span class="shrink-0 text-secondary">↪</span>
+        <span class="text-secondary">Treat "<span class="font-medium"><%= @import.amount_type_identifier_value %></span>" as</span>
+        <%= form.select :amount_type_inflow_value,
+                        [["Income (inflow)", "inflows_positive"], ["Expense (outflow)", "inflows_negative"]],
+                        { prompt: "Select type", container_class: "w-48 px-3 py-1.5 border border-secondary rounded-md" },
                        required: @import.amount_type_strategy == "custom_column" %>
-        <span class="text-secondary">as "income" (inflow) value</span>
      </div>
    </div>
  <% end %>
--- a/config/locales/views/imports/en.yml
+++ b/config/locales/views/imports/en.yml
@@ -109,3 +109,5 @@ en:
      description: Here's a summary of the new items that will be added to your account
        once you publish this import.
      title: Confirm your import data
+    errors:
+      custom_column_requires_inflow: "Custom column imports require an inflow column to be selected"
--- a/db/migrate/20251226091500_add_amount_type_identifier_value_to_imports.rb
+++ b/db/migrate/20251226091500_add_amount_type_identifier_value_to_imports.rb
@@ -0,0 +1,7 @@
+class AddAmountTypeIdentifierValueToImports < ActiveRecord::Migration[7.2]
+  def change
+    unless column_exists?(:imports, :amount_type_identifier_value)
+      add_column :imports, :amount_type_identifier_value, :string
+    end
+  end
+end
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -659,6 +659,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_01_23_000000) do
    t.string "amount_type_inflow_value"
    t.integer "rows_to_skip", default: 0, null: false
    t.integer "rows_count", default: 0, null: false
+    t.string "amount_type_identifier_value"
    t.index ["family_id"], name: "index_imports_on_family_id"
  end

--- a/test/models/transaction_import_test.rb
+++ b/test/models/transaction_import_test.rb
@@ -85,7 +85,8 @@ class TransactionImportTest < ActiveSupport::TestCase
      date_format: "%m/%d/%Y",
      amount_col_label: "amount",
      entity_type_col_label: "amount_type",
-      amount_type_inflow_value: "debit",
+      amount_type_identifier_value: "debit",
+      amount_type_inflow_value: "inflows_positive",
      amount_type_strategy: "custom_column",
      signage_convention: nil # Explicitly set to nil to prove this is not needed
    )