diff --git a/app/apps/api/serializers/imports.py b/app/apps/api/serializers/imports.py index 93f05587..7d06635b 100644 --- a/app/apps/api/serializers/imports.py +++ b/app/apps/api/serializers/imports.py @@ -1,5 +1,7 @@ +from django.utils.translation import gettext_lazy as _ from rest_framework import serializers +from apps.accounts.models import Account from apps.import_app.models import ImportProfile, ImportRun @@ -21,6 +23,7 @@ class ImportRunSerializer(serializers.ModelSerializer): "status", "profile", "file_name", + "account", "logs", "processed_rows", "total_rows", @@ -39,8 +42,22 @@ class ImportFileSerializer(serializers.Serializer): queryset=ImportProfile.objects.all(), source="profile" ) file = serializers.FileField() + account_id = serializers.PrimaryKeyRelatedField( + queryset=Account.objects.all(), + source="account", + required=False, + allow_null=True, + ) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - # Reload the queryset to get an updated version with the requesting user + # Reload the querysets to get an updated version with the requesting user self.fields["profile_id"].queryset = ImportProfile.objects.all() + self.fields["account_id"].queryset = Account.objects.all() + + def validate(self, data): + if data.get("account") and not data["profile"].imports_transactions: + raise serializers.ValidationError( + {"account_id": _("Only transaction imports can use an account.")} + ) + return data diff --git a/app/apps/api/views/imports.py b/app/apps/api/views/imports.py index 9abb945d..4e7c73cd 100644 --- a/app/apps/api/views/imports.py +++ b/app/apps/api/views/imports.py @@ -102,6 +102,7 @@ class ImportRunViewSet(viewsets.ReadOnlyModelViewSet): "properties": { "profile_id": {"type": "integer", "description": "ID of the ImportProfile to use"}, "file": {"type": "string", "format": "binary", "description": "CSV or XLSX file to import"}, + "account_id": {"type": "integer", "description": "ID of the Account to import every transaction into. Leave empty to let the profile pick the account. Only for profiles that import transactions"}, }, "required": ["profile_id", "file"], }, @@ -137,7 +138,10 @@ class ImportViewSet(viewsets.ViewSet): # Create ImportRun record import_run = ImportRun.objects.create( - profile=profile, file_name=filename, owner=request.user + profile=profile, + file_name=filename, + account=serializer.validated_data.get("account"), + owner=request.user, ) # Queue import task diff --git a/app/apps/import_app/forms.py b/app/apps/import_app/forms.py index 0b601659..ca1e90ae 100644 --- a/app/apps/import_app/forms.py +++ b/app/apps/import_app/forms.py @@ -1,4 +1,6 @@ +from apps.accounts.models import Account from apps.common.widgets.crispy.submit import NoClassSubmit +from apps.common.widgets.tom_select import TomSelect from apps.import_app.models import ImportProfile from crispy_forms.bootstrap import FormActions from crispy_forms.helper import FormHelper @@ -42,15 +44,33 @@ class ImportProfileForm(forms.ModelForm): class ImportRunFileUploadForm(forms.Form): file = forms.FileField(label=_("Select a file")) + account = forms.ModelChoiceField( + queryset=Account.objects.filter(is_archived=False), + required=False, + label=_("Account"), + help_text=_( + "Import every transaction into this account. Leave empty to let the " + "profile pick the account." + ), + widget=TomSelect(group_by="group"), + ) - def __init__(self, *args, **kwargs): + def __init__(self, *args, profile, **kwargs): super().__init__(*args, **kwargs) self.helper = FormHelper() self.helper.form_tag = False self.helper.form_method = "post" - self.helper.layout = Layout( - "file", + self.helper.layout = Layout("file") + + if profile.imports_transactions: + self.fields["account"].queryset = Account.objects.filter(is_archived=False) + self.helper.layout.append("account") + else: + # Only transactions belong to an account + del self.fields["account"] + + self.helper.layout.append( FormActions( NoClassSubmit("submit", _("Import"), css_class="btn btn-primary"), ), diff --git a/app/apps/import_app/migrations/0004_importrun_account.py b/app/apps/import_app/migrations/0004_importrun_account.py new file mode 100644 index 00000000..5dd25175 --- /dev/null +++ b/app/apps/import_app/migrations/0004_importrun_account.py @@ -0,0 +1,20 @@ +# Generated by Django 5.2.17 on 2026-11-11 17:04 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0022_account_show_exchanged_amounts'), + ('import_app', '0003_owner_and_sharing'), + ] + + operations = [ + migrations.AddField( + model_name='importrun', + name='account', + field=models.ForeignKey(blank=True, help_text='Import every transaction into this account. Leave empty to let the profile pick the account.', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_runs', to='accounts.account', verbose_name='Account'), + ), + ] diff --git a/app/apps/import_app/models.py b/app/apps/import_app/models.py index 8209688e..26192c54 100644 --- a/app/apps/import_app/models.py +++ b/app/apps/import_app/models.py @@ -38,6 +38,15 @@ class ImportProfile(SharedObject): version_number = self.Versions(self.version).name.split("_")[1] return _("Version {number}").format(number=version_number) + @property + def imports_transactions(self): + """Whether this profile creates transactions, as opposed to accounts, tags, etc.""" + try: + config = version_1.ImportProfileSchema(**yaml.safe_load(self.yaml_config)) + except Exception: + return False + return config.settings.importing == "transactions" + def clean(self): if self.version and self.version == self.Versions.VERSION_1: try: @@ -70,6 +79,18 @@ class ImportRun(OwnedObject): max_length=10000, help_text=_("File name"), ) + account = models.ForeignKey( + "accounts.Account", + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name="import_runs", + verbose_name=_("Account"), + help_text=_( + "Import every transaction into this account. Leave empty to let the " + "profile pick the account." + ), + ) transactions = models.ManyToManyField( "transactions.Transaction", related_name="import_runs" ) diff --git a/app/apps/import_app/services/v1.py b/app/apps/import_app/services/v1.py index 2bc16981..ba2a6d48 100644 --- a/app/apps/import_app/services/v1.py +++ b/app/apps/import_app/services/v1.py @@ -289,7 +289,8 @@ class ImportService: # Ignore if category doesn't exist and create is False or not set data["category"] = None - if "account" in data: + # Look the account up, unless the run already picked one + if "account" in data and not isinstance(data["account"], Account): account_id = data.pop("account") account_mapping = next( ( @@ -710,6 +711,10 @@ class ImportService: if mapped_data: # Handle different import types if self.settings.importing == "transactions": + if self.import_run.account: + # The run's account wins over the profile's account mapping + mapped_data["account"] = self.import_run.account + if self.deduplication and self._check_duplicate_transaction( mapped_data ): @@ -920,7 +925,9 @@ class ImportService: current_transaction = {} raw_lines_buffer = [] - account = Account.objects.filter(name=account_name).first() + account = ( + self.import_run.account or Account.objects.filter(name=account_name).first() + ) if not account: raise ValueError(f"Account '{account_name}' not found.") @@ -1149,6 +1156,11 @@ class ImportService: self.import_run.save(update_fields=["started_at"]) self._log("info", "Starting import process") + if self.import_run.account: + self._log( + "info", + f"Importing transactions into account '{self.import_run.account.name}'", + ) try: if isinstance(self.settings, version_1.CSVImportSettings): diff --git a/app/apps/import_app/tests/test_run_account.py b/app/apps/import_app/tests/test_run_account.py new file mode 100644 index 00000000..053ee932 --- /dev/null +++ b/app/apps/import_app/tests/test_run_account.py @@ -0,0 +1,370 @@ +"""Tests for importing a file into an account picked for the import run.""" + +import shutil +import tempfile +from decimal import Decimal +from pathlib import Path +from unittest.mock import patch + +from django.contrib.auth import get_user_model +from django.core.files.uploadedfile import SimpleUploadedFile +from django.test import TestCase, override_settings +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from apps.accounts.models import Account +from apps.common.middleware.thread_local import delete_current_user, write_current_user +from apps.currencies.models import Currency +from apps.import_app.models import ImportProfile, ImportRun +from apps.import_app.services.v1 import ImportService +from apps.import_app.tests.test_view_permissions import ( + HTMX, + ImportObjectPermissionTestCase, +) +from apps.transactions.models import Transaction + +CSV_SETTINGS = """ +settings: + file_type: csv + importing: transactions + trigger_transaction_rules: false +""" + +CSV_MAPPING = """ +mapping: + date: + target: date + source: date + format: "%Y-%m-%d" + amount: + target: amount + source: amount + type: + target: type + source: amount + detection_method: sign + description: + target: description + source: description +""" + +CSV_ACCOUNT_MAPPING = """ + account: + target: account + default: Checking +""" + +CSV_ACCOUNT_DEDUPLICATION = """ +deduplication: + - type: compare + fields: + - date + - amount + - account + match_type: strict +""" + +CSV_CONTENT = """date,amount,description +2026-01-05,-50.00,Groceries +2026-01-06,1000.00,Salary +""" + +TAGS_YAML_CONFIG = """ +settings: + file_type: csv + importing: tags +mapping: + name: + target: tag_name + source: name +""" + + +class ImportRunAccountServiceTests(TestCase): + def setUp(self): + self.temp_root = Path(tempfile.mkdtemp()).resolve() + self.addCleanup(shutil.rmtree, self.temp_root, ignore_errors=True) + temp_root_override = override_settings(IMPORT_TEMP_ROOT=self.temp_root) + temp_root_override.enable() + self.addCleanup(temp_root_override.disable) + + self.user = get_user_model().objects.create_user( + email="test@example.com", password="password" + ) + write_current_user(self.user) + self.addCleanup(delete_current_user) + + currency = Currency.objects.create( + code="EUR", name="Euro", decimal_places=2, prefix="€ " + ) + self.checking = Account.objects.create( + name="Checking", currency=currency, owner=self.user + ) + self.savings = Account.objects.create( + name="Savings", currency=currency, owner=self.user + ) + + def _import(self, yaml_config, file_name, content, account=None): + profile = ImportProfile.objects.create( + name="Profile", yaml_config=yaml_config, owner=self.user + ) + run = ImportRun.objects.create( + profile=profile, file_name=file_name, account=account, owner=self.user + ) + file_path = self.temp_root / file_name + file_path.write_text(content, encoding="utf-8") + + ImportService(run).process_file(str(file_path)) + + run.refresh_from_db() + return run + + def test_run_account_overrides_the_profile_account(self): + run = self._import( + CSV_SETTINGS + CSV_MAPPING + CSV_ACCOUNT_MAPPING, + "statement.csv", + CSV_CONTENT, + account=self.savings, + ) + + self.assertEqual(run.status, ImportRun.Status.FINISHED) + self.assertEqual(run.successful_rows, 2) + self.assertEqual(Transaction.objects.filter(account=self.savings).count(), 2) + self.assertFalse(Transaction.objects.filter(account=self.checking).exists()) + self.assertIn("Importing transactions into account 'Savings'", run.logs) + + def test_profile_account_is_used_without_a_run_account(self): + run = self._import( + CSV_SETTINGS + CSV_MAPPING + CSV_ACCOUNT_MAPPING, + "statement.csv", + CSV_CONTENT, + ) + + self.assertEqual(run.successful_rows, 2) + self.assertEqual(Transaction.objects.filter(account=self.checking).count(), 2) + + def test_run_account_imports_with_a_profile_without_account_mapping(self): + run = self._import( + CSV_SETTINGS + CSV_MAPPING, + "statement.csv", + CSV_CONTENT, + account=self.checking, + ) + + self.assertEqual(run.successful_rows, 2) + expense = Transaction.objects.get(description="Groceries") + self.assertEqual(expense.account, self.checking) + self.assertEqual(expense.type, Transaction.Type.EXPENSE) + self.assertEqual(expense.amount, Decimal("50.00")) + + def test_deduplication_by_account_uses_the_run_account(self): + yaml_config = CSV_SETTINGS + CSV_MAPPING + CSV_ACCOUNT_DEDUPLICATION + + self._import(yaml_config, "first.csv", CSV_CONTENT, account=self.checking) + # Same rows, other account: not duplicates + into_savings = self._import( + yaml_config, "second.csv", CSV_CONTENT, account=self.savings + ) + # Same rows, same account: duplicates + into_savings_again = self._import( + yaml_config, "third.csv", CSV_CONTENT, account=self.savings + ) + + self.assertEqual(into_savings.successful_rows, 2) + self.assertEqual(into_savings.skipped_rows, 0) + self.assertEqual(into_savings_again.successful_rows, 0) + self.assertEqual(into_savings_again.skipped_rows, 2) + self.assertEqual(Transaction.objects.filter(account=self.savings).count(), 2) + + def test_qif_run_account_is_used_instead_of_the_file_name(self): + yaml_config = """ +settings: + file_type: qif + importing: transactions + date_format: "%d/%m/%Y" +mapping: {} +""" + content = """!Type:Bank +D05/01/2026 +T-100.00 +PSupermarket +^ +""" + run = self._import( + yaml_config, "export-2026.qif", content, account=self.savings + ) + + self.assertEqual(run.status, ImportRun.Status.FINISHED) + transaction = Transaction.objects.get() + self.assertEqual(transaction.account, self.savings) + self.assertEqual(transaction.amount, Decimal("100.00")) + + +class ImportRunAccountTestCase(ImportObjectPermissionTestCase): + def setUp(self): + super().setUp() + self.temp_root = Path(tempfile.mkdtemp()).resolve() + self.addCleanup(shutil.rmtree, self.temp_root, ignore_errors=True) + temp_root_override = override_settings(IMPORT_TEMP_ROOT=self.temp_root) + temp_root_override.enable() + self.addCleanup(temp_root_override.disable) + + currency = Currency.objects.create( + code="EUR", name="Euro", decimal_places=2, prefix="€ " + ) + self.account = Account.all_objects.create( + name="Checking", currency=currency, owner=self.owner + ) + self.archived_account = Account.all_objects.create( + name="Old checking", currency=currency, owner=self.owner, is_archived=True + ) + self.strangers_account = Account.all_objects.create( + name="Stranger's checking", currency=currency, owner=self.stranger + ) + self.tags_profile = ImportProfile.all_objects.create( + name="Tags profile", yaml_config=TAGS_YAML_CONFIG, owner=self.owner + ) + + +class ImportRunAccountViewTests(ImportRunAccountTestCase): + def _upload_form(self, profile): + self.login(self.owner) + response = self.client.get( + reverse("import_run_add", kwargs={"profile_id": profile.id}), **HTMX + ) + self.assertEqual(response.status_code, 200) + return response.context["form"] + + def test_upload_form_offers_active_accounts_for_transaction_profiles(self): + form = self._upload_form(self.private_profile) + + self.assertIn("account", form.fields) + self.assertFalse(form.fields["account"].required) + self.assertEqual(list(form.fields["account"].queryset), [self.account]) + + def test_upload_form_has_no_account_for_other_profiles(self): + form = self._upload_form(self.tags_profile) + + self.assertNotIn("account", form.fields) + + @patch("apps.import_app.views.process_import.defer") + def test_web_upload_stores_the_account_on_the_run(self, mock_defer): + self.login(self.owner) + response = self.client.post( + reverse("import_run_add", kwargs={"profile_id": self.private_profile.id}), + { + "file": SimpleUploadedFile("web.csv", b"date,amount\n"), + "account": self.account.id, + }, + **HTMX, + ) + + self.assertEqual(response.status_code, 204) + run = ImportRun.all_objects.get(file_name="web.csv") + self.assertEqual(run.account, self.account) + mock_defer.assert_called_once() + + @patch("apps.import_app.views.process_import.defer") + def test_web_upload_without_account_keeps_the_profile_account(self, mock_defer): + self.login(self.owner) + response = self.client.post( + reverse("import_run_add", kwargs={"profile_id": self.private_profile.id}), + {"file": SimpleUploadedFile("web.csv", b"date,amount\n")}, + **HTMX, + ) + + self.assertEqual(response.status_code, 204) + self.assertIsNone(ImportRun.all_objects.get(file_name="web.csv").account) + + @patch("apps.import_app.views.process_import.defer") + def test_web_upload_rejects_another_users_account(self, mock_defer): + self.login(self.owner) + response = self.client.post( + reverse("import_run_add", kwargs={"profile_id": self.private_profile.id}), + { + "file": SimpleUploadedFile("web.csv", b"date,amount\n"), + "account": self.strangers_account.id, + }, + **HTMX, + ) + + self.assertEqual(response.status_code, 200) + self.assertIn("account", response.context["form"].errors) + self.assertFalse(ImportRun.all_objects.filter(file_name="web.csv").exists()) + mock_defer.assert_not_called() + + def test_runs_list_shows_the_run_account(self): + ImportRun.all_objects.create( + profile=self.private_profile, + file_name="statement.csv", + account=self.account, + owner=self.owner, + ) + self.login(self.owner) + response = self.client.get( + reverse( + "import_profile_runs_list", + kwargs={"profile_id": self.private_profile.id}, + ), + **HTMX, + ) + + self.assertContains(response, "Checking") + + +class ImportRunAccountAPITests(ImportRunAccountTestCase): + def setUp(self): + super().setUp() + self.api = APIClient() + self.api.force_authenticate(user=self.owner) + + def _post(self, profile, **data): + return self.api.post( + "/api/import/import/", + { + "profile_id": profile.id, + "file": SimpleUploadedFile("api.csv", b"date,amount\n"), + **data, + }, + format="multipart", + ) + + @patch("apps.api.views.imports.process_import.defer") + def test_api_upload_stores_the_account_on_the_run(self, mock_defer): + response = self._post(self.private_profile, account_id=self.account.id) + + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) + run = ImportRun.all_objects.get(id=response.data["import_run_id"]) + self.assertEqual(run.account, self.account) + + response = self.api.get(f"/api/import/runs/{run.id}/") + self.assertEqual(response.data["account"], self.account.id) + + @patch("apps.api.views.imports.process_import.defer") + def test_api_upload_without_account_keeps_the_profile_account(self, mock_defer): + response = self._post(self.private_profile) + + self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED) + run = ImportRun.all_objects.get(id=response.data["import_run_id"]) + self.assertIsNone(run.account) + + @patch("apps.api.views.imports.process_import.defer") + def test_api_rejects_another_users_account(self, mock_defer): + response = self._post( + self.private_profile, account_id=self.strangers_account.id + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("account_id", response.data) + mock_defer.assert_not_called() + + @patch("apps.api.views.imports.process_import.defer") + def test_api_rejects_an_account_for_profiles_not_importing_transactions( + self, mock_defer + ): + response = self._post(self.tags_profile, account_id=self.account.id) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn("account_id", response.data) + mock_defer.assert_not_called() diff --git a/app/apps/import_app/views.py b/app/apps/import_app/views.py index 45dc61ab..f6e82e69 100644 --- a/app/apps/import_app/views.py +++ b/app/apps/import_app/views.py @@ -168,7 +168,11 @@ def import_runs_list(request, profile_id): # ImportRun.objects only returns the requesting user's runs, so a shared # profile does not expose the runs other users made with it. - runs = ImportRun.objects.filter(profile=profile).order_by("-id") + runs = ( + ImportRun.objects.filter(profile=profile) + .select_related("account") + .order_by("-id") + ) return render( request, @@ -201,7 +205,7 @@ def import_run_add(request, profile_id): ) if request.method == "POST": - form = ImportRunFileUploadForm(request.POST, request.FILES) + form = ImportRunFileUploadForm(request.POST, request.FILES, profile=profile) if form.is_valid(): uploaded_file = request.FILES["file"] @@ -210,7 +214,10 @@ def import_run_add(request, profile_id): file_path = fs.path(filename) import_run = ImportRun.objects.create( - profile=profile, file_name=filename, owner=request.user + profile=profile, + file_name=filename, + account=form.cleaned_data.get("account"), + owner=request.user, ) # Defer the procrastinate task @@ -229,7 +236,7 @@ def import_run_add(request, profile_id): }, ) else: - form = ImportRunFileUploadForm() + form = ImportRunFileUploadForm(profile=profile) return render( request, diff --git a/app/templates/import_app/fragments/runs/list.html b/app/templates/import_app/fragments/runs/list.html index 681f5bdf..a414a504 100644 --- a/app/templates/import_app/fragments/runs/list.html +++ b/app/templates/import_app/fragments/runs/list.html @@ -87,6 +87,21 @@ + {% if run.account %} +
+
+
+
+ {% trans 'Account' %} +
+
+ {{ run.account.name }} +
+
+
+
+ {% endif %} +