diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..8897ed7 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,34 @@ +# Secrets — these must never end up in an image layer. The ** prefix is +# required: a bare ".env" would only match the context root, and the real +# file lives in expenses_manager/. +**/.env +**/db.env +!**/.env.example + +# Version control and IDE noise. +**/.git +**/.gitignore +**/.vscode +**/.idea + +# Local artifacts that would otherwise be copied in by COPY. +**/__pycache__/ +**/*.py[cod] +**/*.bak +**/venv/ +**/.venv/ +**/db.sqlite3 +**/staticfiles/ +**/.pytest_cache/ +**/.coverage +**/htmlcov/ +**/gunicorn.ctl + +# CI and test configuration, not needed at runtime. +**/Jenkinsfile +**/pytest.ini +**/tests/ + +# Documentation and planning, not needed at runtime. +**/*.md +LICENSE \ No newline at end of file diff --git a/.gitignore b/.gitignore index 1329a83..896191d 100644 --- a/.gitignore +++ b/.gitignore @@ -129,6 +129,7 @@ venv/ ENV/ env.bak/ venv.bak/ +db.env # Spyder project settings .spyderproject @@ -161,3 +162,5 @@ cython_debug/ # option (not recommended) you can uncomment the following to ignore the entire idea folder. #.idea/ +staticfiles/ +*.bak \ No newline at end of file diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000..08fd0cb --- /dev/null +++ b/Dockerfile @@ -0,0 +1,31 @@ +FROM python:3.11-slim + +# Fail fast and log straight to stdout, so `docker-compose logs` is useful. +ENV PYTHONDONTWRITEBYTECODE=1 \ + PYTHONUNBUFFERED=1 + +WORKDIR /app + +# Dependencies first: this layer is cached and only rebuilt when +# requirements.txt changes, not on every code change. +COPY expenses_manager/requirements.txt . +RUN pip install --no-cache-dir -r requirements.txt + +# Copy the Django project itself, not the repository root, so that /app is +# the directory holding manage.py. The previous version copied the repo root +# into /app/expenses_manager, which left manage.py and wsgi.py one level +# deeper than the gunicorn command expected. +COPY expenses_manager/ . + +# Collect static files at build time so the running container never needs to +# write to its own filesystem. SECRET_KEY is required by settings.py when +# DEBUG is False, but collectstatic never uses it, so a throwaway value is +# enough — the real one comes from the environment at runtime. +RUN SECRET_KEY=build-time-placeholder \ + DEBUG=False \ + ALLOWED_HOSTS=localhost \ + python manage.py collectstatic --noinput + +EXPOSE 8000 + +CMD ["gunicorn", "expenses_manager.wsgi:application", "--bind", "0.0.0.0:8000"] \ No newline at end of file diff --git a/expenses_manager/expenses/forms.py b/expenses_manager/expenses/forms.py index e2fed43..d5f6ae9 100644 --- a/expenses_manager/expenses/forms.py +++ b/expenses_manager/expenses/forms.py @@ -1,5 +1,27 @@ from django import forms -from .models import Expense, Category, Income, Tag, Account, FuelEntry, Goal +from .models import ( + Expense, + Category, + Income, + Tag, + Account, + FuelEntry, + Goal, + Transfer, +) + + +def _hide_is_transfer_when_editing(form): + """Quita la casilla de traspaso al editar un movimiento que ya existe. + + Al crear, la casilla es un atajo: lleva al formulario de traspaso, que es + quien crea las dos patas. Al editar no significa nada, porque convertir un + gasto suelto en traspaso no es editarlo (habría que inventarse la otra + cuenta), y marcarla solo dejaría el movimiento marcado pero sin pareja: justo + el estado a medias que Transfer viene a evitar. + """ + if form.instance.pk: + form.fields.pop("is_transfer", None) class ExpenseForm(forms.ModelForm): @@ -18,6 +40,8 @@ class ExpenseForm(forms.ModelForm): ) self.fields["date"].input_formats = ["%Y-%m-%d"] + _hide_is_transfer_when_editing(self) + class Meta: model = Expense fields = [ @@ -61,6 +85,37 @@ class IncomeForm(forms.ModelForm): owner=user, active=True, ) + _hide_is_transfer_when_editing(self) + + +class TransferForm(forms.ModelForm): + """Formulario del traspaso. Las patas no se editan aquí ni en ninguna parte: + las deriva Transfer.sync() de estos campos.""" + + class Meta: + model = Transfer + fields = ["date", "amount", "from_account", "to_account", "fee_amount"] + widgets = {"date": forms.DateInput(format="%Y-%m-%d", attrs={"type": "date"})} + labels = { + "from_account": "Cuenta de origen", + "to_account": "Cuenta de destino", + "fee_amount": "Comisión", + } + help_texts = { + "fee_amount": ( + "Déjalo vacío si el traspaso no tiene comisión. La comisión sí " + "cuenta como gasto real en el dashboard y en los presupuestos." + ), + } + + def __init__(self, *args, **kwargs): + user = kwargs.pop("user") + super().__init__(*args, **kwargs) + + accounts = Account.objects.filter(owner=user, active=True) + self.fields["from_account"].queryset = accounts + self.fields["to_account"].queryset = accounts + self.fields["date"].input_formats = ["%Y-%m-%d"] class FuelEntryForm(forms.ModelForm): diff --git a/expenses_manager/expenses/migrations/0014_category_is_system_transfer.py b/expenses_manager/expenses/migrations/0014_category_is_system_transfer.py new file mode 100644 index 0000000..1540b50 --- /dev/null +++ b/expenses_manager/expenses/migrations/0014_category_is_system_transfer.py @@ -0,0 +1,40 @@ +# Generated by Django 5.2.10 on 2026-09-18 08:37 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('expenses', '0013_expense_is_transfer_income_is_transfer'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddField( + model_name='category', + name='is_system', + field=models.BooleanField(default=False, help_text='Categoría creada por la app (traspasos, comisiones). No borrable.'), + ), + migrations.CreateModel( + name='Transfer', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('date', models.DateField()), + ('amount', models.DecimalField(decimal_places=2, max_digits=12)), + ('fee_amount', models.DecimalField(blank=True, decimal_places=2, max_digits=10, null=True)), + ('created_at', models.DateTimeField(auto_now_add=True)), + ('expense_leg', models.OneToOneField(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='transfer_out', to='expenses.expense')), + ('fee_expense', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='transfer_fee', to='expenses.expense')), + ('from_account', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='transfers_out', to='expenses.account')), + ('income_leg', models.OneToOneField(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='transfer_in', to='expenses.income')), + ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='transfers', to=settings.AUTH_USER_MODEL)), + ('to_account', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='transfers_in', to='expenses.account')), + ], + options={ + 'ordering': ['-date'], + }, + ), + ] diff --git a/expenses_manager/expenses/migrations/0015_link_legacy_transfers.py b/expenses_manager/expenses/migrations/0015_link_legacy_transfers.py new file mode 100644 index 0000000..f5de187 --- /dev/null +++ b/expenses_manager/expenses/migrations/0015_link_legacy_transfers.py @@ -0,0 +1,72 @@ +"""Convierte en Transfer los traspasos históricos que estaban sueltos. + +Antes de que existiera Transfer, un traspaso eran dos movimientos marcados con +is_transfer pero sin nada que los relacionara: un Expense en la cuenta de origen +y un Income en la de destino. Aquí se emparejan y se les cuelga un Transfer. + +No se pueden emparejar con certeza: nada en la base de datos dice qué gasto va +con qué ingreso. El criterio es mismo owner, mismo importe, misma fecha y cuentas +distintas, y solo cuando en ese grupo hay exactamente un gasto y un ingreso. Los +ambiguos (varios candidatos con el mismo importe y fecha) y los huérfanos (sin +pareja) se dejan tal cual, con su is_transfer=True: siguen contando en los saldos +y fuera de los análisis igual que antes, solo que sin Transfer que los orqueste. +Emparejarlos a ojo sería inventarse datos del usuario. +""" + +from collections import defaultdict + +from django.db import migrations + + +def link_legacy_transfers(apps, schema_editor): + Expense = apps.get_model("expenses", "Expense") + Income = apps.get_model("expenses", "Income") + Transfer = apps.get_model("expenses", "Transfer") + + # Solo las patas que aún no pertenecen a ningún Transfer. + expenses_by_key = defaultdict(list) + for expense in Expense.objects.filter( + is_transfer=True, transfer_out__isnull=True, transfer_fee__isnull=True + ): + expenses_by_key[(expense.owner_id, expense.amount, expense.date)].append( + expense + ) + + incomes_by_key = defaultdict(list) + for income in Income.objects.filter(is_transfer=True, transfer_in__isnull=True): + incomes_by_key[(income.owner_id, income.amount, income.date)].append(income) + + for key, expenses in expenses_by_key.items(): + incomes = incomes_by_key.get(key, []) + + # Uno y solo uno de cada lado: cualquier otra cosa es ambigua. + if len(expenses) != 1 or len(incomes) != 1: + continue + + expense, income = expenses[0], incomes[0] + + if expense.account_id == income.account_id: + continue + + Transfer.objects.create( + owner_id=expense.owner_id, + date=expense.date, + amount=expense.amount, + from_account_id=expense.account_id, + to_account_id=income.account_id, + expense_leg=expense, + income_leg=income, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ("expenses", "0014_category_is_system_transfer"), + ] + + operations = [ + # Sin marcha atrás: deshacer esto es deshacer 0014, que se lleva la tabla + # Transfer entera y deja las patas intactas, que es el estado de partida. + migrations.RunPython(link_legacy_transfers, migrations.RunPython.noop), + ] diff --git a/expenses_manager/expenses/migrations/0016_alter_transfer_expense_leg_alter_transfer_income_leg.py b/expenses_manager/expenses/migrations/0016_alter_transfer_expense_leg_alter_transfer_income_leg.py new file mode 100644 index 0000000..353a66f --- /dev/null +++ b/expenses_manager/expenses/migrations/0016_alter_transfer_expense_leg_alter_transfer_income_leg.py @@ -0,0 +1,24 @@ +# Generated by Django 5.2.10 on 2026-09-21 13:35 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('expenses', '0015_link_legacy_transfers'), + ] + + operations = [ + migrations.AlterField( + model_name='transfer', + name='expense_leg', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='transfer_out', to='expenses.expense'), + ), + migrations.AlterField( + model_name='transfer', + name='income_leg', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='transfer_in', to='expenses.income'), + ), + ] diff --git a/expenses_manager/expenses/models.py b/expenses_manager/expenses/models.py index 97e0e19..b6d763b 100644 --- a/expenses_manager/expenses/models.py +++ b/expenses_manager/expenses/models.py @@ -1,6 +1,7 @@ from datetime import date from decimal import Decimal -from django.db import models +from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django.db import models, transaction from django.conf import settings from django.db.models import Sum from functools import cached_property @@ -26,6 +27,11 @@ class Category(models.Model): related_name="children", ) + is_system = models.BooleanField( + default=False, + help_text="Categoría creada por la app (traspasos, comisiones). No borrable.", + ) + class Meta: unique_together = ("name", "parent", "owner", "slug") verbose_name_plural = "categories" @@ -246,6 +252,22 @@ class Expense(models.Model): def __str__(self): return "{} - {}".format(self.date, self.amount) + @property + def transfer(self): + """El Transfer del que este gasto es una pata, o None si es normal. + + Cubre las dos patas de gasto de un traspaso: la de salida + (transfer_out) y la comisión (transfer_fee). Un gasto que pertenece a un + traspaso no se edita ni se borra por separado: se hace desde el + traspaso, que es quien mantiene las tres patas cuadradas. + """ + for relation in ("transfer_out", "transfer_fee"): + try: + return getattr(self, relation) + except ObjectDoesNotExist: + continue + return None + class Income(models.Model): owner = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) @@ -275,6 +297,220 @@ class Income(models.Model): def __str__(self): return f"{self.name} - {self.amount}" + @property + def transfer(self): + """El Transfer del que este ingreso es la pata de entrada, o None.""" + try: + return self.transfer_in + except ObjectDoesNotExist: + return None + + +def _system_category(user, slug, name): + """Categoría de sistema del usuario (traspasos, comisiones), creándola si falta. + + Se busca por slug y no por nombre porque el nombre lo puede cambiar el + usuario; el slug es el identificador estable. + """ + cat, _ = Category.objects.get_or_create( + slug=slug, + owner=user, + defaults={"name": name, "is_system": True}, + ) + return cat + + +class Transfer(models.Model): + """Un traspaso entre dos cuentas propias. + + No sustituye a los dos movimientos que ya registraban un traspaso: los + orquesta. Sigue habiendo un Expense en la cuenta origen y un Income en la + destino, los dos con is_transfer=True, pero ahora están vinculados aquí y + es este modelo quien los crea, los edita y los borra a la vez (sync() y + delete()). + + Por eso los cuatro métodos de saldo de Account (current_balance, + monthly_balance, balance_until, monthly_net) no saben nada de Transfer y no + necesitan saberlo: el traspaso ya está dentro de sus sumas, porque sus dos + patas siguen siendo un Income y un Expense reales. + """ + + TRANSFER_CATEGORY_SLUG = "traspaso" + FEE_CATEGORY_SLUG = "comision" + + owner = models.ForeignKey( + settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="transfers" + ) + date = models.DateField() + # max_digits=12 frente a los 10 de Expense.amount: un traspaso de más de diez + # dígitos no cabría en su pata de gasto. Se deja así a propósito, porque + # igualarlo obliga a migrar Expense por un caso de 100 millones de euros. + amount = models.DecimalField(max_digits=12, decimal_places=2) + from_account = models.ForeignKey( + Account, on_delete=models.PROTECT, related_name="transfers_out" + ) + to_account = models.ForeignKey( + Account, on_delete=models.PROTECT, related_name="transfers_in" + ) + + # Las dos patas y la comisión opcional. SET_NULL para que si algo las borra + # por fuera el Transfer no arrastre, pero el flujo normal las borra en cascada + # desde el propio Transfer (ver delete()). + # + # blank=True porque un traspaso siempre acaba con sus dos patas, pero no las + # tiene en el instante en que se construye: sync() las crea a partir de él. + # Exigirlas en full_clean() describiría mal ese momento. + expense_leg = models.OneToOneField( + Expense, + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name="transfer_out", + ) + income_leg = models.OneToOneField( + Income, + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name="transfer_in", + ) + + fee_amount = models.DecimalField( + max_digits=10, decimal_places=2, null=True, blank=True + ) + fee_expense = models.OneToOneField( + Expense, + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name="transfer_fee", + ) + + created_at = models.DateTimeField(auto_now_add=True) + + class Meta: + ordering = ["-date"] + + def __str__(self): + return f"{self.date} - {self.amount} ({self.from_account} → {self.to_account})" + + def clean(self): + if self.from_account_id and self.from_account_id == self.to_account_id: + raise ValidationError( + {"to_account": "El origen y el destino tienen que ser cuentas distintas."} + ) + + @transaction.atomic + def sync(self): + """Guarda el traspaso y pone sus patas al día en la misma transacción. + + Un solo método para crear y para editar: si no hay patas las crea, y si + las hay les propaga importe, fecha y cuentas (la categoría no, que el + usuario puede haberla recolocado). La comisión aparece o desaparece + según fee_amount. + + Ojo con la comisión: va con is_transfer=False a propósito. Las dos patas + del movimiento solo mueven dinero entre cuentas propias y no son gasto + real, pero la comisión sale del patrimonio de verdad, así que es un gasto + normal y tiene que contar en el dashboard y en los presupuestos. + """ + if self.expense_leg_id: + leg = self.expense_leg + leg.date = self.date + leg.amount = self.amount + leg.account = self.from_account + leg.description = self._expense_description() + leg.save() + else: + self.expense_leg = Expense.objects.create( + owner=self.owner, + account=self.from_account, + category=_system_category( + self.owner, self.TRANSFER_CATEGORY_SLUG, "Traspaso" + ), + amount=self.amount, + date=self.date, + description=self._expense_description(), + is_transfer=True, + ) + + if self.income_leg_id: + leg = self.income_leg + leg.date = self.date + leg.amount = self.amount + leg.account = self.to_account + leg.name = self._income_name() + leg.save() + else: + self.income_leg = Income.objects.create( + owner=self.owner, + account=self.to_account, + name=self._income_name(), + amount=self.amount, + date=self.date, + is_transfer=True, + ) + + # La comisión que sobra se borra después de guardar el Transfer, para no + # dejar ni un instante el FK apuntando a una fila que ya no existe. + stale_fee = None + + if self.fee_amount: + if self.fee_expense_id: + fee = self.fee_expense + fee.date = self.date + fee.amount = self.fee_amount + fee.account = self.from_account + fee.save() + else: + self.fee_expense = Expense.objects.create( + owner=self.owner, + account=self.from_account, + category=_system_category( + self.owner, self.FEE_CATEGORY_SLUG, "Comisión" + ), + amount=self.fee_amount, + date=self.date, + description=self._fee_description(), + is_transfer=False, + ) + elif self.fee_expense_id: + stale_fee = self.fee_expense + self.fee_expense = None + + self.save() + + if stale_fee is not None: + stale_fee.delete() + + return self + + @transaction.atomic + def delete(self, *args, **kwargs): + """Borra el traspaso y sus tres patas. + + Las patas son SET_NULL, así que borrarlas no arrastraría el Transfer; + el arrastre va en este sentido, y va aquí y no en la vista para que + valga también desde el shell o los tests. + """ + legs = [self.expense_leg, self.income_leg, self.fee_expense] + result = super().delete(*args, **kwargs) + + for leg in legs: + if leg is not None: + leg.delete() + + return result + + def _expense_description(self): + return f"Traspaso a {self.to_account.name}" + + def _income_name(self): + return f"Traspaso desde {self.from_account.name}" + + def _fee_description(self): + return f"Comisión del traspaso a {self.to_account.name}" + class FuelEntry(models.Model): expense = models.OneToOneField( diff --git a/expenses_manager/expenses/static/expenses/js/transfer_redirect.js b/expenses_manager/expenses/static/expenses/js/transfer_redirect.js new file mode 100644 index 0000000..cde8ee7 --- /dev/null +++ b/expenses_manager/expenses/static/expenses/js/transfer_redirect.js @@ -0,0 +1,51 @@ +// Al marcar la casilla de traspaso en el formulario de gasto o de ingreso, salta +// al formulario de traspaso llevándose fecha, importe y cuenta. El movimiento no +// se guarda: un traspaso son dos patas y las crea Transfer.sync(). +// +// Esto es solo un atajo para no obligar a enviar el formulario: si el JS no corre, +// la vista hace la misma redirección al recibir el POST, así que el +// comportamiento no depende de este fichero. +document.addEventListener('DOMContentLoaded', function () { + var form = document.querySelector('[data-transfer-url]'); + + if (!form) { + return; + } + + var checkbox = form.querySelector('#id_is_transfer'); + + if (!checkbox) { + return; // editando no se pinta la casilla + } + + // Qué cuenta es la del formulario en el traspaso: origen si es un gasto, + // destino si es un ingreso. Lo decide la plantilla, no este fichero. + var accountParam = form.dataset.transferAccountParam; + + checkbox.addEventListener('change', function () { + if (!checkbox.checked) { + return; + } + + var params = new URLSearchParams(); + + ['date', 'amount'].forEach(function (name) { + var field = form.querySelector('[name="' + name + '"]'); + + if (field && field.value) { + params.set(name, field.value); + } + }); + + var account = form.querySelector('[name="account"]'); + + if (account && account.value) { + params.set(accountParam, account.value); + } + + var query = params.toString(); + window.location.href = query + ? form.dataset.transferUrl + '?' + query + : form.dataset.transferUrl; + }); +}); diff --git a/expenses_manager/expenses/templates/categories/confirm_delete.html b/expenses_manager/expenses/templates/categories/confirm_delete.html index 98c5bf3..59702a4 100644 --- a/expenses_manager/expenses/templates/categories/confirm_delete.html +++ b/expenses_manager/expenses/templates/categories/confirm_delete.html @@ -7,7 +7,12 @@ {% block delete_question %}¿Seguro que quieres eliminar la categoría {{ category.name }}?{% endblock %} {% block delete_warnings %} - {% if expense_count %} + {% if category.is_system %} +

+ Esta categoría la usa la aplicación para clasificar los traspasos y sus + comisiones, así que no se puede eliminar. +

+ {% elif expense_count %}

Esta categoría tiene {{ expense_count }} gasto{{ expense_count|pluralize }} asociado{{ expense_count|pluralize }} y no se puede eliminar. Reasigna esos gastos a otra categoría primero. diff --git a/expenses_manager/expenses/templates/categories/list.html b/expenses_manager/expenses/templates/categories/list.html index ca0491d..cb2ba81 100644 --- a/expenses_manager/expenses/templates/categories/list.html +++ b/expenses_manager/expenses/templates/categories/list.html @@ -30,11 +30,18 @@ {% for row in category_rows %} - {{ row.category.name }} + + {{ row.category.name }} + {% if row.category.is_system %} + Del sistema + {% endif %} + {{ row.category.expense_count }} Editar - Eliminar + {% if not row.category.is_system %} + Eliminar + {% endif %} {% empty %} diff --git a/expenses_manager/expenses/templates/expenses/base.html b/expenses_manager/expenses/templates/expenses/base.html index 56aa8a9..0da53aa 100644 --- a/expenses_manager/expenses/templates/expenses/base.html +++ b/expenses_manager/expenses/templates/expenses/base.html @@ -33,6 +33,7 @@ Dashboard Gastos Ingresos + Traspasos Repostajes Cuentas

{% endblock %} + +{% block extra_js %} + +{% endblock %} diff --git a/expenses_manager/expenses/templates/expenses/expense_list.html b/expenses_manager/expenses/templates/expenses/expense_list.html index 22037b9..f26a3e2 100644 --- a/expenses_manager/expenses/templates/expenses/expense_list.html +++ b/expenses_manager/expenses/templates/expenses/expense_list.html @@ -145,7 +145,10 @@ {% endfor %} - {% if expense.fuel_data %} + {% if expense.transfer %} + {# Pata de un traspaso: se edita y se borra desde el traspaso. #} + Editar traspaso + {% elif expense.fuel_data %} Editar Eliminar {% else %} diff --git a/expenses_manager/expenses/templates/expenses/income_form.html b/expenses_manager/expenses/templates/expenses/income_form.html index 9992000..76ac36d 100644 --- a/expenses_manager/expenses/templates/expenses/income_form.html +++ b/expenses_manager/expenses/templates/expenses/income_form.html @@ -1,4 +1,5 @@ {% extends "expenses/base.html" %} +{% load static %} {% block title %} {% if form.instance.pk %} Editar ingreso @@ -16,7 +17,9 @@ {% endif %} -
+ {% csrf_token %} {% include "expenses/_form_fields.html" %}
@@ -30,4 +33,8 @@ Volver
-{% endblock %} \ No newline at end of file +{% endblock %} + +{% block extra_js %} + +{% endblock %} diff --git a/expenses_manager/expenses/templates/expenses/income_list.html b/expenses_manager/expenses/templates/expenses/income_list.html index 56820bb..a2daa74 100644 --- a/expenses_manager/expenses/templates/expenses/income_list.html +++ b/expenses_manager/expenses/templates/expenses/income_list.html @@ -93,8 +93,13 @@ {{ income.amount|floatformat:2 }} {{ income.date }} - Editar - Eliminar + {% if income.transfer %} + {# Pata de un traspaso: se edita y se borra desde el traspaso. #} + Editar traspaso + {% else %} + Editar + Eliminar + {% endif %} {% empty %} diff --git a/expenses_manager/expenses/templates/transfers/confirm_delete.html b/expenses_manager/expenses/templates/transfers/confirm_delete.html new file mode 100644 index 0000000..8bb2831 --- /dev/null +++ b/expenses_manager/expenses/templates/transfers/confirm_delete.html @@ -0,0 +1,23 @@ +{% extends "expenses/_confirm_delete.html" %} + +{% block title %}Traspasos{% endblock %} + +{% block delete_heading %}Eliminar traspaso{% endblock %} + +{% block delete_question %} + ¿Seguro que quieres eliminar el traspaso de + {{ transfer.amount|floatformat:2 }}€ + de {{ transfer.from_account.name }} + a {{ transfer.to_account.name }} + del {{ transfer.date }}? +{% endblock %} + +{% block delete_warnings %} +

+ Se eliminarán también el gasto de la cuenta de origen y el ingreso de la de + destino{% if transfer.fee_amount %}, además del gasto de la comisión{% endif %}. + Los saldos de las dos cuentas volverán a como estaban. +

+{% endblock %} + +{% block cancel_url %}{% url 'transfer_list' %}{% endblock %} diff --git a/expenses_manager/expenses/templates/transfers/form.html b/expenses_manager/expenses/templates/transfers/form.html new file mode 100644 index 0000000..31c43a1 --- /dev/null +++ b/expenses_manager/expenses/templates/transfers/form.html @@ -0,0 +1,29 @@ +{% extends "expenses/base.html" %} + +{% block title %}{{ title }}{% endblock %} + +{% block content %} +

{{ title }}

+ + {% if form.instance.pk %} +

+ Al guardar se actualizan los dos movimientos del traspaso y su comisión. + No hace falta tocarlos por separado en Gastos ni en Ingresos. +

+ {% endif %} + +
+ {% csrf_token %} + {% include "expenses/_form_fields.html" %} +
+ + Volver +
+
+{% endblock %} diff --git a/expenses_manager/expenses/templates/transfers/list.html b/expenses_manager/expenses/templates/transfers/list.html new file mode 100644 index 0000000..20614dd --- /dev/null +++ b/expenses_manager/expenses/templates/transfers/list.html @@ -0,0 +1,64 @@ +{% extends "expenses/base.html" %} + +{% block title %} + Traspasos +{% endblock %} + +{% block content %} +

Mis traspasos

+ +

+ Un traspaso mueve dinero entre dos cuentas propias: no cuenta como gasto ni + como ingreso en los análisis, pero sí cambia el saldo de las dos cuentas. + La comisión, si la hay, sí es un gasto real. +

+ +
+ âž• Nuevo traspaso +
+ +
+ +
+ + + + + + + + + + + + + {% for transfer in transfers %} + + + + + + + + + {% empty %} + + + + {% endfor %} + +
FechaOrigenDestinoImporteComisión
{{ transfer.date }}{{ transfer.from_account.name }}{{ transfer.to_account.name }}{{ transfer.amount|floatformat:2 }}€ + {% if transfer.fee_amount %} + {{ transfer.fee_amount|floatformat:2 }}€ + {% else %} + - + {% endif %} + + Editar + Eliminar +
+

No hay traspasos

+ Añade el primero +
+
+{% endblock %} diff --git a/expenses_manager/expenses/tests/test_categories.py b/expenses_manager/expenses/tests/test_categories.py index 6c9bf74..33a6329 100644 --- a/expenses_manager/expenses/tests/test_categories.py +++ b/expenses_manager/expenses/tests/test_categories.py @@ -101,6 +101,26 @@ def test_category_delete_without_expenses_succeeds(auth_client, user): assert not Category.objects.filter(pk=cat.pk).exists() +def test_system_category_cannot_be_deleted(auth_client, user): + """Las categorías de traspaso y comisión las pone la app, no el usuario.""" + cat = Category.objects.create(name="Traspaso", owner=user, is_system=True) + + response = auth_client.post(reverse('category_delete', args=[cat.pk])) + + assert response.status_code == 302 + assert Category.objects.filter(pk=cat.pk).exists() + messages = [str(m) for m in get_messages(response.wsgi_request)] + assert any("traspasos" in m for m in messages) + + +def test_system_category_has_no_delete_link_in_the_list(auth_client, user): + cat = Category.objects.create(name="Traspaso", owner=user, is_system=True) + + response = auth_client.get(reverse('category_list')) + + assert reverse('category_delete', args=[cat.pk]) not in response.content.decode() + + def test_category_list_view_returns_tree_ordered_with_depth(auth_client, user): root_a = Category.objects.create(name="A raiz", owner=user) Category.objects.create(name="B raiz", owner=user) diff --git a/expenses_manager/expenses/tests/test_transfer_model.py b/expenses_manager/expenses/tests/test_transfer_model.py new file mode 100644 index 0000000..68bc266 --- /dev/null +++ b/expenses_manager/expenses/tests/test_transfer_model.py @@ -0,0 +1,977 @@ +"""El modelo Transfer: orquesta las dos patas de un traspaso. + +Transfer no sustituye al Expense de salida y al Income de entrada: los crea, los +vincula y los mantiene sincronizados. De ahí que aquí no se compruebe ningún +método de saldo nuevo, sino que los de siempre siguen dando el número correcto +porque las patas siguen siendo un gasto y un ingreso reales. + +El punto más delicado, y el que más tests tiene: la comisión va con +is_transfer=False, porque es dinero que sale del patrimonio de verdad y sí tiene +que contar en los análisis. +""" + +import importlib +from datetime import date +from decimal import Decimal +from urllib.parse import parse_qs, urlparse + +import pytest +from django.apps import apps as django_apps +from django.core.exceptions import ValidationError +from django.urls import reverse + +from expenses.models import Account, Category, Expense, Income, Transfer + +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def savings(user): + return Account.objects.create( + owner=user, name="Ahorro", initial_balance=Decimal("0"), active=True + ) + + +def make_transfer(user, from_account, to_account, amount="400", on=None, fee=None): + transfer = Transfer( + owner=user, + date=on or date(2024, 5, 11), + amount=Decimal(amount), + from_account=from_account, + to_account=to_account, + fee_amount=Decimal(fee) if fee is not None else None, + ) + transfer.sync() + return transfer + + +# -------------------------------------------------------------------------- +# Creación: las dos patas +# -------------------------------------------------------------------------- + + +def test_creating_a_transfer_builds_and_links_both_legs(user, account, savings): + transfer = make_transfer(user, account, savings) + + assert transfer.expense_leg is not None + assert transfer.income_leg is not None + + assert transfer.expense_leg.account == account + assert transfer.expense_leg.amount == Decimal("400") + assert transfer.expense_leg.date == transfer.date + assert transfer.expense_leg.owner == user + assert transfer.expense_leg.is_transfer is True + + assert transfer.income_leg.account == savings + assert transfer.income_leg.amount == Decimal("400") + assert transfer.income_leg.date == transfer.date + assert transfer.income_leg.owner == user + assert transfer.income_leg.is_transfer is True + + # El vínculo se ve desde las dos puntas. + assert transfer.expense_leg.transfer == transfer + assert transfer.income_leg.transfer == transfer + + +def test_legs_use_the_system_transfer_category(user, account, savings): + transfer = make_transfer(user, account, savings) + + leg_category = transfer.expense_leg.category + assert leg_category.slug == "traspaso" + assert leg_category.is_system is True + assert leg_category.owner == user + + +def test_two_transfers_share_the_same_system_category(user, account, savings): + first = make_transfer(user, account, savings) + second = make_transfer(user, account, savings, on=date(2024, 6, 11)) + + assert first.expense_leg.category_id == second.expense_leg.category_id + assert Category.objects.filter(owner=user, slug="traspaso").count() == 1 + + +def test_both_balances_move(user, account, savings): + account.initial_balance = Decimal("1000") + account.save() + + make_transfer(user, account, savings) + + assert account.current_balance() == Decimal("600") + assert savings.current_balance() == Decimal("400") + + +def test_transfer_does_not_change_the_total_patrimony(user, account, savings): + account.initial_balance = Decimal("1000") + account.save() + + make_transfer(user, account, savings) + + total = account.current_balance() + savings.current_balance() + assert total == Decimal("1000") + + +# -------------------------------------------------------------------------- +# La comisión: un gasto real, sin is_transfer +# -------------------------------------------------------------------------- + + +def test_fee_creates_a_third_expense_that_is_not_a_transfer(user, account, savings): + transfer = make_transfer(user, account, savings, fee="3") + + assert transfer.fee_expense is not None + assert transfer.fee_expense.amount == Decimal("3") + assert transfer.fee_expense.account == account + assert transfer.fee_expense.date == transfer.date + # El punto donde es más fácil equivocarse: la comisión NO es un traspaso. + assert transfer.fee_expense.is_transfer is False + assert transfer.fee_expense.category.slug == "comision" + assert transfer.fee_expense.category.is_system is True + + +def test_fee_is_charged_to_the_origin_account(user, account, savings): + account.initial_balance = Decimal("1000") + account.save() + + make_transfer(user, account, savings, fee="3") + + # El origen paga el traspaso y la comisión; el destino solo recibe el traspaso. + assert account.current_balance() == Decimal("597") + assert savings.current_balance() == Decimal("400") + + +def test_dashboard_counts_the_fee_but_not_the_legs(auth_client, user, account, savings): + make_transfer(user, account, savings, fee="3", on=date(2024, 5, 11)) + + response = auth_client.get(reverse("dashboard"), {"year": 2024}) + + assert response.context["kpi_total"] == Decimal("3") + assert response.context["kpi_count"] == 1 + + rows = {row["category__name"]: row["total"] for row in response.context["by_category"]} + assert rows == {"Comisión": Decimal("3")} + + +def test_without_fee_the_dashboard_sees_nothing(auth_client, user, account, savings): + make_transfer(user, account, savings, on=date(2024, 5, 11)) + + response = auth_client.get(reverse("dashboard"), {"year": 2024}) + + assert response.context["kpi_total"] == Decimal("0") + assert response.context["kpi_count"] == 0 + + +# -------------------------------------------------------------------------- +# Edición: sincronizar las patas +# -------------------------------------------------------------------------- + + +def test_editing_the_amount_updates_both_legs(user, account, savings): + transfer = make_transfer(user, account, savings) + + transfer.amount = Decimal("250") + transfer.sync() + + transfer.expense_leg.refresh_from_db() + transfer.income_leg.refresh_from_db() + assert transfer.expense_leg.amount == Decimal("250") + assert transfer.income_leg.amount == Decimal("250") + + assert account.current_balance() == Decimal("-250") + assert savings.current_balance() == Decimal("250") + + +def test_editing_the_date_updates_both_legs(user, account, savings): + transfer = make_transfer(user, account, savings) + + transfer.date = date(2024, 8, 1) + transfer.sync() + + transfer.expense_leg.refresh_from_db() + transfer.income_leg.refresh_from_db() + assert transfer.expense_leg.date == date(2024, 8, 1) + assert transfer.income_leg.date == date(2024, 8, 1) + + +def test_editing_the_accounts_moves_the_legs(user, account, savings): + transfer = make_transfer(user, account, savings) + third = Account.objects.create( + owner=user, name="Bolsillo", initial_balance=Decimal("0"), active=True + ) + + transfer.from_account = savings + transfer.to_account = third + transfer.sync() + + transfer.expense_leg.refresh_from_db() + transfer.income_leg.refresh_from_db() + assert transfer.expense_leg.account == savings + assert transfer.income_leg.account == third + assert account.current_balance() == Decimal("0") + + +def test_editing_does_not_create_new_legs(user, account, savings): + transfer = make_transfer(user, account, savings) + expense_id, income_id = transfer.expense_leg_id, transfer.income_leg_id + + transfer.amount = Decimal("250") + transfer.sync() + + assert transfer.expense_leg_id == expense_id + assert transfer.income_leg_id == income_id + assert Expense.objects.count() == 1 + assert Income.objects.count() == 1 + + +def test_adding_a_fee_on_edit_creates_it(user, account, savings): + transfer = make_transfer(user, account, savings) + assert transfer.fee_expense is None + + transfer.fee_amount = Decimal("3") + transfer.sync() + + assert transfer.fee_expense is not None + assert transfer.fee_expense.is_transfer is False + assert Expense.objects.count() == 2 + + +def test_removing_the_fee_on_edit_deletes_it(user, account, savings): + transfer = make_transfer(user, account, savings, fee="3") + fee_id = transfer.fee_expense_id + + transfer.fee_amount = None + transfer.sync() + + assert transfer.fee_expense is None + assert not Expense.objects.filter(pk=fee_id).exists() + + transfer.refresh_from_db() + assert transfer.fee_expense_id is None + + +def test_editing_the_fee_amount_updates_the_expense(user, account, savings): + transfer = make_transfer(user, account, savings, fee="3") + + transfer.fee_amount = Decimal("5") + transfer.sync() + + transfer.fee_expense.refresh_from_db() + assert transfer.fee_expense.amount == Decimal("5") + assert Expense.objects.count() == 2 + + +# -------------------------------------------------------------------------- +# Borrado +# -------------------------------------------------------------------------- + + +def test_deleting_the_transfer_deletes_both_legs(user, account, savings): + transfer = make_transfer(user, account, savings) + + transfer.delete() + + assert Expense.objects.count() == 0 + assert Income.objects.count() == 0 + assert Transfer.objects.count() == 0 + + +def test_deleting_the_transfer_deletes_the_fee_too(user, account, savings): + transfer = make_transfer(user, account, savings, fee="3") + + transfer.delete() + + assert Expense.objects.count() == 0 + assert Income.objects.count() == 0 + assert Transfer.objects.count() == 0 + + +def test_deleting_the_transfer_restores_both_balances(user, account, savings): + account.initial_balance = Decimal("1000") + account.save() + transfer = make_transfer(user, account, savings, fee="3") + + transfer.delete() + + assert account.current_balance() == Decimal("1000") + assert savings.current_balance() == Decimal("0") + + +# -------------------------------------------------------------------------- +# Validación y atomicidad +# -------------------------------------------------------------------------- + + +def test_same_account_on_both_sides_is_rejected(user, account): + transfer = Transfer( + owner=user, + date=date(2024, 5, 11), + amount=Decimal("400"), + from_account=account, + to_account=account, + ) + + with pytest.raises(ValidationError) as error: + transfer.full_clean() + + assert "to_account" in error.value.message_dict + + +def test_different_accounts_pass_validation(user, account, savings): + transfer = Transfer( + owner=user, + date=date(2024, 5, 11), + amount=Decimal("400"), + from_account=account, + to_account=savings, + ) + + # A pelo y sin excluir nada: las patas son blank=True porque aún no existen. + transfer.full_clean() # no levanta + + +def test_a_failed_leg_leaves_nothing_behind(user, account, savings, monkeypatch): + """Atomicidad: si la segunda pata falla, la primera no queda suelta.""" + + def boom(self): + raise RuntimeError("fallo al construir la pata de entrada") + + monkeypatch.setattr(Transfer, "_income_name", boom) + + transfer = Transfer( + owner=user, + date=date(2024, 5, 11), + amount=Decimal("400"), + from_account=account, + to_account=savings, + ) + + with pytest.raises(RuntimeError): + transfer.sync() + + assert Expense.objects.count() == 0 + assert Income.objects.count() == 0 + assert Transfer.objects.count() == 0 + + +# -------------------------------------------------------------------------- +# Las patas no se editan por separado +# -------------------------------------------------------------------------- + + +def test_expense_leg_cannot_be_edited_on_its_own(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings) + + response = auth_client.get( + reverse("expense_edit", args=[transfer.expense_leg_id]), follow=True + ) + + assert response.redirect_chain[0][0] == reverse("transfer_edit", args=[transfer.pk]) + + +def test_expense_leg_cannot_be_deleted_on_its_own(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings) + + auth_client.post(reverse("expense_delete", args=[transfer.expense_leg_id])) + + assert Expense.objects.filter(pk=transfer.expense_leg_id).exists() + + +def test_income_leg_cannot_be_deleted_on_its_own(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings) + + auth_client.post(reverse("income_delete", args=[transfer.income_leg_id])) + + assert Income.objects.filter(pk=transfer.income_leg_id).exists() + + +def test_the_fee_expense_cannot_be_edited_on_its_own(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings, fee="3") + + response = auth_client.get( + reverse("expense_edit", args=[transfer.fee_expense_id]), follow=True + ) + + assert response.redirect_chain[0][0] == reverse("transfer_edit", args=[transfer.pk]) + + +def test_a_normal_expense_is_still_editable(auth_client, user, account, category): + expense = Expense.objects.create( + owner=user, + account=account, + category=category, + amount=Decimal("10"), + date=date(2024, 5, 11), + ) + + response = auth_client.get(reverse("expense_edit", args=[expense.pk])) + + assert response.status_code == 200 + assert expense.transfer is None + + +def test_the_listing_links_a_leg_to_its_transfer(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings) + + response = auth_client.get(reverse("expense_list"), {"year": 2024}) + html = response.content.decode() + + assert reverse("transfer_edit", args=[transfer.pk]) in html + assert reverse("expense_delete", args=[transfer.expense_leg_id]) not in html + + +@pytest.mark.parametrize("listing", ["expense_list", "income_list"]) +def test_no_template_comment_leaks_into_the_listing( + auth_client, user, account, savings, listing +): + """Los {# #} de Django son de una sola línea: partidos en dos se imprimen.""" + make_transfer(user, account, savings) + + response = auth_client.get(reverse(listing), {"year": 2024}) + + assert "{#" not in response.content.decode() + + +# -------------------------------------------------------------------------- +# Vistas CRUD +# -------------------------------------------------------------------------- + + +def test_create_view_builds_the_three_legs(auth_client, user, account, savings): + response = auth_client.post( + reverse("transfer_create"), + { + "date": "2024-05-11", + "amount": "400", + "from_account": account.id, + "to_account": savings.id, + "fee_amount": "3", + }, + ) + + assert response.status_code == 302 + transfer = Transfer.objects.get(owner=user) + assert transfer.expense_leg.is_transfer is True + assert transfer.income_leg.is_transfer is True + assert transfer.fee_expense.is_transfer is False + + +def test_create_view_without_fee(auth_client, user, account, savings): + auth_client.post( + reverse("transfer_create"), + { + "date": "2024-05-11", + "amount": "400", + "from_account": account.id, + "to_account": savings.id, + "fee_amount": "", + }, + ) + + transfer = Transfer.objects.get(owner=user) + assert transfer.fee_amount is None + assert transfer.fee_expense is None + assert Expense.objects.count() == 1 + + +def test_create_view_rejects_the_same_account_twice(auth_client, user, account): + response = auth_client.post( + reverse("transfer_create"), + { + "date": "2024-05-11", + "amount": "400", + "from_account": account.id, + "to_account": account.id, + }, + ) + + assert response.status_code == 200 + assert Transfer.objects.count() == 0 + assert "to_account" in response.context["form"].errors + + +def test_edit_view_syncs_the_legs(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings) + + response = auth_client.post( + reverse("transfer_edit", args=[transfer.pk]), + { + "date": "2024-05-11", + "amount": "250", + "from_account": account.id, + "to_account": savings.id, + "fee_amount": "", + }, + ) + + assert response.status_code == 302 + transfer.expense_leg.refresh_from_db() + transfer.income_leg.refresh_from_db() + assert transfer.expense_leg.amount == Decimal("250") + assert transfer.income_leg.amount == Decimal("250") + + +def test_delete_view_removes_everything(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings, fee="3") + + response = auth_client.post(reverse("transfer_delete", args=[transfer.pk])) + + assert response.status_code == 302 + assert Transfer.objects.count() == 0 + assert Expense.objects.count() == 0 + assert Income.objects.count() == 0 + + +def test_delete_view_needs_post(auth_client, user, account, savings): + transfer = make_transfer(user, account, savings) + + response = auth_client.get(reverse("transfer_delete", args=[transfer.pk])) + + assert response.status_code == 200 + assert Transfer.objects.count() == 1 + + +def test_list_view_only_shows_your_own(auth_client, user, account, savings, django_user_model): + make_transfer(user, account, savings) + + other = django_user_model.objects.create_user(username="otro", password="x") + other_account = Account.objects.create( + owner=other, name="Suya", initial_balance=Decimal("0"), active=True + ) + other_savings = Account.objects.create( + owner=other, name="Suya 2", initial_balance=Decimal("0"), active=True + ) + make_transfer(other, other_account, other_savings) + + response = auth_client.get(reverse("transfer_list")) + + assert [t.owner for t in response.context["transfers"]] == [user] + + +def test_form_only_offers_your_active_accounts(auth_client, user, account, savings): + Account.objects.create( + owner=user, name="Cerrada", initial_balance=Decimal("0"), active=False + ) + + response = auth_client.get(reverse("transfer_create")) + form = response.context["form"] + + names = {acc.name for acc in form.fields["from_account"].queryset} + assert names == {account.name, savings.name} + assert names == {acc.name for acc in form.fields["to_account"].queryset} + + +def test_transfer_edit_view_is_scoped_by_owner(auth_client, django_user_model): + other = django_user_model.objects.create_user(username="otro", password="x") + other_account = Account.objects.create( + owner=other, name="Suya", initial_balance=Decimal("0"), active=True + ) + other_savings = Account.objects.create( + owner=other, name="Suya 2", initial_balance=Decimal("0"), active=True + ) + transfer = make_transfer(other, other_account, other_savings) + + response = auth_client.get(reverse("transfer_edit", args=[transfer.pk])) + + assert response.status_code == 404 + + +# -------------------------------------------------------------------------- +# El atajo de la casilla: del gasto o del ingreso al traspaso +# -------------------------------------------------------------------------- + + +def _query(url): + return parse_qs(urlparse(url).query) + + +def test_the_expense_form_carries_its_data_to_the_transfer_form( + auth_client, user, account, category +): + """La cuenta de un gasto entra como origen: el dinero sale de ahí.""" + response = auth_client.post( + reverse("expense_create"), + { + "date": "2024-05-11", + "amount": "400", + "description": "", + "category": category.id, + "account": account.id, + "is_transfer": "on", + }, + ) + + assert response.status_code == 302 + assert urlparse(response.url).path == reverse("transfer_create") + assert _query(response.url) == { + "date": ["2024-05-11"], + "amount": ["400"], + "from_account": [str(account.id)], + } + + +def test_the_income_form_carries_its_account_as_destination(auth_client, user, savings): + response = auth_client.post( + reverse("income_create"), + { + "account": savings.id, + "name": "Traspaso", + "amount": "400", + "date": "2024-05-11", + "is_transfer": "on", + }, + ) + + assert _query(response.url) == { + "date": ["2024-05-11"], + "amount": ["400"], + "to_account": [str(savings.id)], + } + + +def test_a_half_filled_expense_still_reaches_the_transfer_form( + auth_client, user, account +): + """Sin categoría el gasto no valida, pero el salto no depende de que valide: + se lleva lo que haya y lo que falte se queda en blanco.""" + response = auth_client.post( + reverse("expense_create"), + {"date": "", "amount": "400", "account": account.id, "is_transfer": "on"}, + ) + + assert response.status_code == 302 + assert _query(response.url) == { + "amount": ["400"], + "from_account": [str(account.id)], + } + assert not Expense.objects.filter(owner=user).exists() + + +def test_the_transfer_form_prefills_from_the_url(auth_client, user, account, savings): + response = auth_client.get( + reverse("transfer_create"), + { + "date": "2024-05-11", + "amount": "400", + "from_account": account.id, + "to_account": savings.id, + }, + ) + + assert response.context["form"].initial == { + "date": date(2024, 5, 11), + "amount": Decimal("400"), + "from_account": account.id, + "to_account": savings.id, + } + + +@pytest.mark.parametrize( + "params", + [ + {"date": "11/05/2024"}, + {"date": "2024-13-45"}, + {"amount": "cuatrocientos"}, + {"amount": "NaN"}, + {"amount": "Infinity"}, + {"from_account": "primera"}, + {"to_account": "999999"}, + ], +) +def test_an_invalid_url_param_is_ignored_and_does_not_break(auth_client, user, params): + """La URL la escribe cualquiera: lo que no se entienda sale en blanco.""" + response = auth_client.get(reverse("transfer_create"), params) + + assert response.status_code == 200 + assert response.context["form"].initial == {} + + +def test_an_account_of_another_user_is_not_prefilled( + auth_client, user, django_user_model +): + intruder = django_user_model.objects.create_user(username="otro", password="x") + foreign = Account.objects.create( + owner=intruder, name="Ajena", initial_balance=Decimal("0"), active=True + ) + + response = auth_client.get(reverse("transfer_create"), {"from_account": foreign.id}) + + assert response.context["form"].initial == {} + + +def test_an_inactive_account_is_not_prefilled(auth_client, user): + """Las cuentas desactivadas no están en el desplegable: preseleccionar una + dejaría el formulario con un valor que no se puede enviar.""" + closed = Account.objects.create( + owner=user, name="Cerrada", initial_balance=Decimal("0"), active=False + ) + + response = auth_client.get(reverse("transfer_create"), {"to_account": closed.id}) + + assert response.context["form"].initial == {} + + +# -------------------------------------------------------------------------- +# La casilla de traspaso no se ofrece al editar +# -------------------------------------------------------------------------- + + +def test_the_checkbox_is_offered_when_creating_an_expense(auth_client, user): + response = auth_client.get(reverse("expense_create")) + + assert "id_is_transfer" in response.content.decode() + + +def test_the_checkbox_is_offered_when_creating_an_income(auth_client, user): + response = auth_client.get(reverse("income_create")) + + assert "id_is_transfer" in response.content.decode() + + +@pytest.mark.parametrize( + "view_name,account_param", + [("expense_create", "from_account"), ("income_create", "to_account")], +) +def test_the_create_form_tells_the_javascript_where_to_jump( + auth_client, user, view_name, account_param +): + """El JS no sabe nada de traspasos: el destino y el papel de la cuenta se los + da la plantilla en data-*. Renombrarlos rompería el salto inmediato sin que + nada se queje, porque el POST seguiría redirigiendo igual. + """ + response = auth_client.get(reverse(view_name)) + html = response.content.decode() + + assert f'data-transfer-url="{reverse("transfer_create")}"' in html + assert f'data-transfer-account-param="{account_param}"' in html + assert "transfer_redirect.js" in html + + +def test_the_checkbox_is_hidden_when_editing_an_expense( + auth_client, user, account, category +): + """Editar un gasto no es convertirlo en traspaso: la casilla no pinta nada.""" + expense = Expense.objects.create( + owner=user, + account=account, + category=category, + amount=Decimal("10"), + date=date(2024, 5, 11), + ) + + response = auth_client.get(reverse("expense_edit", args=[expense.pk])) + + assert "id_is_transfer" not in response.content.decode() + + +def test_the_checkbox_is_hidden_when_editing_an_income(auth_client, user, account): + income = Income.objects.create( + owner=user, + account=account, + name="Nómina", + amount=Decimal("10"), + date=date(2024, 5, 11), + ) + + response = auth_client.get(reverse("income_edit", args=[income.pk])) + + assert "id_is_transfer" not in response.content.decode() + + +def test_posting_is_transfer_while_editing_has_no_effect( + auth_client, user, account, category +): + """El campo no está en el formulario, así que mandarlo a mano tampoco cuela.""" + expense = Expense.objects.create( + owner=user, + account=account, + category=category, + amount=Decimal("10"), + date=date(2024, 5, 11), + ) + + auth_client.post( + reverse("expense_edit", args=[expense.pk]), + { + "date": "2024-05-11", + "amount": "10", + "description": "", + "category": category.id, + "account": account.id, + "is_transfer": "on", + }, + ) + + expense.refresh_from_db() + assert expense.is_transfer is False + + +def test_a_leg_shows_its_transfer_link_and_no_checkbox( + auth_client, user, account, savings +): + """Una pata sigue mandando al traspaso: ni casilla, ni edición suelta.""" + transfer = make_transfer(user, account, savings) + + listing = auth_client.get(reverse("expense_list"), {"year": 2024}) + html = listing.content.decode() + + assert reverse("transfer_edit", args=[transfer.pk]) in html + assert "id_is_transfer" not in html + + edit = auth_client.get( + reverse("expense_edit", args=[transfer.expense_leg_id]), follow=True + ) + + assert edit.redirect_chain[0][0] == reverse("transfer_edit", args=[transfer.pk]) + assert "id_is_transfer" not in edit.content.decode() + + +# -------------------------------------------------------------------------- +# Migración de datos: emparejar los traspasos históricos +# -------------------------------------------------------------------------- + + +def link_legacy_transfers(): + """Lanza la función de la migración 0015 contra los modelos reales.""" + migration = importlib.import_module("expenses.migrations.0015_link_legacy_transfers") + migration.link_legacy_transfers(django_apps, None) + + +def legacy_pair(user, from_account, to_account, category, amount, on): + expense = Expense.objects.create( + owner=user, + account=from_account, + category=category, + amount=Decimal(amount), + date=on, + is_transfer=True, + ) + income = Income.objects.create( + owner=user, + account=to_account, + name="Traspaso", + amount=Decimal(amount), + date=on, + is_transfer=True, + ) + return expense, income + + +def test_migration_pairs_an_unambiguous_legacy_transfer( + user, account, savings, category +): + expense, income = legacy_pair( + user, account, savings, category, "200", date(2024, 5, 11) + ) + + link_legacy_transfers() + + transfer = Transfer.objects.get() + assert transfer.expense_leg_id == expense.pk + assert transfer.income_leg_id == income.pk + assert transfer.from_account_id == account.pk + assert transfer.to_account_id == savings.pk + assert transfer.amount == Decimal("200") + assert transfer.date == date(2024, 5, 11) + assert transfer.fee_amount is None + + +def test_migration_leaves_ambiguous_candidates_alone(user, account, savings, category): + # Dos gastos y dos ingresos con el mismo importe y fecha: no hay forma de + # saber cuál va con cuál, así que no se toca ninguno. + legacy_pair(user, account, savings, category, "200", date(2024, 5, 11)) + legacy_pair(user, account, savings, category, "200", date(2024, 5, 11)) + + link_legacy_transfers() + + assert Transfer.objects.count() == 0 + assert Expense.objects.filter(is_transfer=True).count() == 2 + assert Income.objects.filter(is_transfer=True).count() == 2 + + +def test_migration_leaves_orphans_alone(user, account, category): + Expense.objects.create( + owner=user, + account=account, + category=category, + amount=Decimal("200"), + date=date(2024, 5, 11), + is_transfer=True, + ) + + link_legacy_transfers() + + assert Transfer.objects.count() == 0 + assert Expense.objects.filter(is_transfer=True).count() == 1 + + +def test_migration_ignores_normal_movements(user, account, savings, category): + Expense.objects.create( + owner=user, + account=account, + category=category, + amount=Decimal("200"), + date=date(2024, 5, 11), + ) + Income.objects.create( + owner=user, + account=savings, + name="Nómina", + amount=Decimal("200"), + date=date(2024, 5, 11), + ) + + link_legacy_transfers() + + assert Transfer.objects.count() == 0 + + +def test_migration_skips_a_pair_on_the_same_account(user, account, category): + legacy_pair(user, account, account, category, "200", date(2024, 5, 11)) + + link_legacy_transfers() + + assert Transfer.objects.count() == 0 + + +def test_migration_does_not_cross_owners(user, account, category, django_user_model): + other = django_user_model.objects.create_user(username="otro", password="x") + other_account = Account.objects.create( + owner=other, name="Suya", initial_balance=Decimal("0"), active=True + ) + + Expense.objects.create( + owner=user, + account=account, + category=category, + amount=Decimal("200"), + date=date(2024, 5, 11), + is_transfer=True, + ) + Income.objects.create( + owner=other, + account=other_account, + name="Traspaso", + amount=Decimal("200"), + date=date(2024, 5, 11), + is_transfer=True, + ) + + link_legacy_transfers() + + assert Transfer.objects.count() == 0 + + +def test_migration_is_idempotent(user, account, savings, category): + legacy_pair(user, account, savings, category, "200", date(2024, 5, 11)) + + link_legacy_transfers() + link_legacy_transfers() + + assert Transfer.objects.count() == 1 + + +def test_migration_does_not_touch_legs_of_an_existing_transfer(user, account, savings): + make_transfer(user, account, savings) + + link_legacy_transfers() + + assert Transfer.objects.count() == 1 diff --git a/expenses_manager/expenses/tests/test_transfers.py b/expenses_manager/expenses/tests/test_transfers.py index 33f42c3..86727c3 100644 --- a/expenses_manager/expenses/tests/test_transfers.py +++ b/expenses_manager/expenses/tests/test_transfers.py @@ -203,7 +203,12 @@ def test_account_list_balance_counts_the_transfer(auth_client, user, account, ca # Formularios # -------------------------------------------------------------------------- -def test_expense_form_can_mark_a_transfer(auth_client, user, account, category): +# Marcar la casilla ya no guarda un movimiento marcado: reconduce al formulario de +# traspaso, que crea las dos patas. El detalle está en test_transfer_model.py. + +def test_marking_the_expense_form_as_transfer_creates_nothing( + auth_client, user, account, category +): response = auth_client.post( reverse("expense_create"), { @@ -217,10 +222,13 @@ def test_expense_form_can_mark_a_transfer(auth_client, user, account, category): ) assert response.status_code == 302 - assert Expense.objects.get(owner=user).is_transfer is True + assert reverse("transfer_create") in response.url + assert not Expense.objects.filter(owner=user).exists() -def test_income_form_can_mark_a_transfer(auth_client, user, account): +def test_marking_the_income_form_as_transfer_creates_nothing( + auth_client, user, account +): response = auth_client.post( reverse("income_create"), { @@ -233,7 +241,8 @@ def test_income_form_can_mark_a_transfer(auth_client, user, account): ) assert response.status_code == 302 - assert Income.objects.get(owner=user).is_transfer is True + assert reverse("transfer_create") in response.url + assert not Income.objects.filter(owner=user).exists() def test_expense_defaults_to_not_a_transfer(user, account, category): diff --git a/expenses_manager/expenses/urls.py b/expenses_manager/expenses/urls.py index b3e802b..fbb07fa 100644 --- a/expenses_manager/expenses/urls.py +++ b/expenses_manager/expenses/urls.py @@ -25,6 +25,10 @@ urlpatterns = [ path('fuel/create/', views.fuel_create, name='fuel_create'), path('fuel//edit/', views.fuel_edit, name='fuel_edit'), path('fuel//delete/', views.fuel_delete, name='fuel_delete'), + path('transfers/', views.transfer_list, name='transfer_list'), + path('transfers/new/', views.transfer_create, name='transfer_create'), + path('transfers//edit/', views.transfer_edit, name='transfer_edit'), + path('transfers//delete/', views.transfer_delete, name='transfer_delete'), path('categories/', views.category_list, name='category_list'), path('categories//edit/', views.category_edit, name='category_edit'), path('categories//delete/', views.category_delete, name='category_delete'), diff --git a/expenses_manager/expenses/views.py b/expenses_manager/expenses/views.py index 3dd4a08..5e2a7c0 100644 --- a/expenses_manager/expenses/views.py +++ b/expenses_manager/expenses/views.py @@ -1,9 +1,19 @@ import logging import calendar from datetime import date, datetime, timedelta +from urllib.parse import urlencode from decimal import Decimal from django.contrib import messages -from .models import Account, Category, Expense, FuelEntry, Tag, Income, Goal +from .models import ( + Account, + Category, + Expense, + FuelEntry, + Tag, + Income, + Goal, + Transfer, +) from .forms import ( ExpenseForm, IncomeForm, @@ -12,6 +22,7 @@ from .forms import ( FuelEntryForm, CategoryForm, GoalForm, + TransferForm, ) from django.core.paginator import Paginator @@ -27,6 +38,7 @@ from django.db.models.functions import ( from django.contrib.auth.decorators import login_required from django.utils.http import url_has_allowed_host_and_scheme from django.shortcuts import get_object_or_404, render, redirect +from django.urls import reverse logger = logging.getLogger(__name__) @@ -37,11 +49,6 @@ def _get_int(value): return None -# Umbral del eje del gráfico con rango libre: dos meses es donde un gráfico -# diario deja de leerse, así que por encima se agrupa por mes. -RANGE_DAY_CHART_MAX_DAYS = 62 - - def _get_date(value): """Parsea una fecha AAAA-MM-DD de la URL. None si falta o no es válida.""" try: @@ -50,6 +57,25 @@ def _get_date(value): return None +def _get_decimal(value): + """Parsea un importe de la URL. None si falta o no es un número usable. + + Se descartan NaN e Infinity aparte: Decimal los acepta pero luego revientan + en cuanto se comparan o se guardan. + """ + try: + number = Decimal(value) + except (TypeError, ValueError, ArithmeticError): + return None + + return number if number.is_finite() else None + + +# Umbral del eje del gráfico con rango libre: dos meses es donde un gráfico +# diario deja de leerse, así que por encima se agrupa por mes. +RANGE_DAY_CHART_MAX_DAYS = 62 + + def _resolve_date_range(request): """Lee el rango de fechas libre de la URL. @@ -128,6 +154,88 @@ def _analysis_expenses(user): return Expense.objects.filter(owner=user).exclude(is_transfer=True) +def _reject_transfer_leg(request, movement): + """Corta la edición o el borrado suelto de una pata de traspaso. + + Devuelve un redirect a la edición del traspaso, o None si el movimiento es + normal y se puede tocar. Las patas las mantiene cuadradas Transfer.sync(), + así que tocar una por separado descuadraría la otra sin que nada lo avise: + es justo el problema que el modelo Transfer viene a resolver. + + Los listados ya no enseñan esos enlaces, pero la URL sigue ahí a mano. + """ + transfer = movement.transfer + + if transfer is None: + return None + + messages.error( + request, + "Este movimiento forma parte de un traspaso. Edítalo o bórralo desde el " + "traspaso para que las dos cuentas sigan cuadradas.", + ) + return redirect("transfer_edit", pk=transfer.pk) + + +def _transfer_create_url(data, account_param): + """URL de "nuevo traspaso" arrastrando lo que ya se hubiera escrito. + + `data` es el POST de un formulario de gasto o de ingreso y `account_param` el + papel que juega su cuenta en el traspaso: "from_account" si viene de un gasto + (el dinero sale de ahí) y "to_account" si viene de un ingreso. + + Lo que no se pueda interpretar se deja fuera: el formulario de traspaso saldrá + con ese campo vacío, que es mejor que un error por un dato a medio escribir. + """ + params = {} + + if (date_value := _get_date(data.get("date"))) is not None: + params["date"] = date_value.isoformat() + + if (amount := _get_decimal(data.get("amount"))) is not None: + params["amount"] = str(amount) + + if (account_id := _get_int(data.get("account"))) is not None: + params[account_param] = account_id + + url = reverse("transfer_create") + return f"{url}?{urlencode(params)}" if params else url + + +def _transfer_initial(request): + """Datos de arranque del formulario de traspaso leídos de la URL. + + Los manda el formulario de gasto o de ingreso cuando el usuario marca la + casilla de traspaso. Nada de esto es de fiar (es la URL): lo que no se pueda + interpretar se ignora y el campo sale vacío. + """ + initial = {} + + if (date_value := _get_date(request.GET.get("date"))) is not None: + initial["date"] = date_value + + if (amount := _get_decimal(request.GET.get("amount"))) is not None: + initial["amount"] = amount + + # Las cuentas se comprueban contra las del usuario: un id ajeno o de una + # cuenta desactivada no se preselecciona aunque venga en la URL. + wanted = { + field: account_id + for field in ("from_account", "to_account") + if (account_id := _get_int(request.GET.get(field))) is not None + } + + if wanted: + own = set( + Account.objects.filter( + owner=request.user, active=True, pk__in=wanted.values() + ).values_list("pk", flat=True) + ) + initial.update({f: pk for f, pk in wanted.items() if pk in own}) + + return initial + + def _account_balances(accounts): """Calcula el saldo de cada cuenta con dos queries agregadas (nada de account.current_balance() en bucle, que dispararía N+1). @@ -353,7 +461,11 @@ def home(request): @login_required def expense_list(request): - expenses = Expense.objects.filter(owner=request.user) + # Las relaciones de traspaso van en select_related porque la tabla consulta + # expense.transfer en cada fila para decidir los enlaces de acción. + expenses = Expense.objects.filter(owner=request.user).select_related( + "transfer_out", "transfer_fee" + ) categories = Category.objects.filter(owner=request.user) @@ -483,6 +595,13 @@ def expense_list(request): @login_required def expense_create(request): if request.method == "POST": + # Marcado como traspaso: esto no se guarda aquí. Un traspaso son dos patas + # que crea Transfer.sync(), así que se reconduce al formulario de traspaso + # con lo que ya hubiera escrito. Se mira el POST en bruto y antes de + # validar porque el gasto puede estar a medias y da igual: no se crea. + if request.POST.get("is_transfer"): + return redirect(_transfer_create_url(request.POST, "from_account")) + form = ExpenseForm(request.POST, user=request.user) if form.is_valid(): expense = form.save(commit=False) @@ -510,6 +629,10 @@ def expense_edit(request, pk): owner=request.user, ) + blocked = _reject_transfer_leg(request, expense) + if blocked: + return blocked + if request.method == "POST": form = ExpenseForm(request.POST or None, instance=expense, user=request.user) @@ -538,6 +661,10 @@ def expense_delete(request, pk): owner=request.user, ) + blocked = _reject_transfer_leg(request, expense) + if blocked: + return blocked + if request.method == "POST": expense.delete() messages.success(request, "Gasto eliminado correctamente.") @@ -967,6 +1094,11 @@ def account_delete(request, pk): @login_required def income_create(request): if request.method == "POST": + # Igual que en expense_create, pero la cuenta es el destino: el dinero + # entra en ella. + if request.POST.get("is_transfer"): + return redirect(_transfer_create_url(request.POST, "to_account")) + form = IncomeForm(request.POST, user=request.user) if form.is_valid(): income = form.save(commit=False) @@ -984,7 +1116,9 @@ def income_create(request): @login_required def income_list(request): - incomes = Income.objects.filter(owner=request.user).select_related("account") + incomes = Income.objects.filter(owner=request.user).select_related( + "account", "transfer_in" + ) year_list = Income.objects.filter(owner=request.user).dates("date", "year") @@ -1064,6 +1198,10 @@ def income_list(request): def income_edit(request, pk): income = get_object_or_404(Income, pk=pk, owner=request.user) + blocked = _reject_transfer_leg(request, income) + if blocked: + return blocked + if request.method == "POST": form = IncomeForm(request.POST, instance=income, user=request.user) if form.is_valid(): @@ -1082,6 +1220,10 @@ def income_edit(request, pk): def income_delete(request, pk): income = get_object_or_404(Income, pk=pk, owner=request.user) + blocked = _reject_transfer_leg(request, income) + if blocked: + return blocked + if request.method == "POST": income.delete() messages.success(request, "Ingreso eliminado.") @@ -1306,6 +1448,14 @@ def category_delete(request, pk): category = get_object_or_404(Category, pk=pk, owner=request.user) if request.method == "POST": + if category.is_system: + messages.error( + request, + f"No se puede eliminar «{category.name}»; la usa la aplicación para " + "clasificar los traspasos y sus comisiones." + ) + return redirect("category_list") + try: category.delete() except ProtectedError: @@ -1314,7 +1464,7 @@ def category_delete(request, pk): f"No se puede eliminar «{category.name}»; tiene gastos asociados. " "Reasigna esos gastos a otra categoría antes de borrarla." ) - else: + else: messages.success(request, "Categoría eliminada.") return redirect("category_list") @@ -1428,3 +1578,92 @@ def goal_delete(request, pk): @login_required def settings_index(request): return render(request, "settings/index.html", {"active_menu": "settings"}) + + +@login_required +def transfer_list(request): + transfers = Transfer.objects.filter(owner=request.user).select_related( + "from_account", "to_account" + ) + + return render( + request, + "transfers/list.html", + { + "active_menu": "transfers", + "transfers": transfers, + }, + ) + + +@login_required +def transfer_create(request): + if request.method == "POST": + form = TransferForm(request.POST, user=request.user) + + if form.is_valid(): + transfer = form.save(commit=False) + transfer.owner = request.user + # sync() guarda el traspaso y crea las patas en una sola transacción. + transfer.sync() + + messages.success(request, "Traspaso creado.") + return redirect("transfer_list") + else: + # La URL puede traer fecha, importe y cuenta del formulario de gasto o de + # ingreso del que se venga. + form = TransferForm(user=request.user, initial=_transfer_initial(request)) + + return render( + request, + "transfers/form.html", + { + "active_menu": "transfers", + "form": form, + "title": "Nuevo traspaso", + }, + ) + + +@login_required +def transfer_edit(request, pk): + transfer = get_object_or_404(Transfer, pk=pk, owner=request.user) + + if request.method == "POST": + form = TransferForm(request.POST, instance=transfer, user=request.user) + + if form.is_valid(): + transfer = form.save(commit=False) + transfer.sync() + + messages.success(request, "Traspaso actualizado.") + return redirect("transfer_list") + else: + form = TransferForm(instance=transfer, user=request.user) + + return render( + request, + "transfers/form.html", + { + "active_menu": "transfers", + "form": form, + "title": "Editar traspaso", + }, + ) + + +@login_required +def transfer_delete(request, pk): + transfer = get_object_or_404(Transfer, pk=pk, owner=request.user) + + if request.method == "POST": + # Transfer.delete() se lleva también las dos patas y la comisión. + transfer.delete() + messages.success(request, "Traspaso eliminado.") + return redirect("transfer_list") + + return render( + request, + "transfers/confirm_delete.html", + {"active_menu": "transfers", "transfer": transfer}, + ) diff --git a/expenses_manager/requirements.txt b/expenses_manager/requirements.txt index 02e2424..28af1ba 100644 Binary files a/expenses_manager/requirements.txt and b/expenses_manager/requirements.txt differ