Added other test to the navigation

expenses_manager/expenses/templates/categories/confirm_delete.html

@@ -1,14 +1,12 @@
-{% extends "expenses/base.html" %}
+{% extends "expenses/_confirm_delete.html" %}
 
-{% block title %}
+{% block title %}Categorías{% endblock %}
-    Categorías
-{% endblock %}
 
-{% block content %}
+{% block delete_heading %}Eliminar categoría{% endblock %}
-    <h2>Eliminar categoría</h2>
 
-    <p>¿Seguro que quieres eliminar la categoría <strong>{{ category.name }}</strong>?</p>
+{% block delete_question %}¿Seguro que quieres eliminar la categoría <strong>{{ category.name }}</strong>?{% endblock %}
 
+{% block delete_warnings %}
     {% if expense_count %}
         <p class="form-errors">
             Esta categoría tiene {{ expense_count }} gasto{{ expense_count|pluralize }} asociado{{ expense_count|pluralize }}
@@ -18,7 +16,7 @@
         {% if children %}
             <p class="form-errors">
                 Atención: también se eliminarán sus subcategorías:
-                {% for child in children %}<strong>{{child.name}}</strong>{% if not forloop.last %}, {% endif %}{% endfor %}.
+                {% for child in children %}<strong>{{ child.name }}</strong>{% if not forloop.last %}, {% endif %}{% endfor %}.
             </p>
         {% endif %}
 
@@ -28,11 +26,6 @@
             </p>
         {% endif %}
     {% endif %}
+{% endblock %}
 
-    <form method="post">
+{% block cancel_url %}{% url 'category_list' %}{% endblock %}
-        {% csrf_token %}
-        {{ form.as_p }}
-        <button class="btn btn-danger">Eliminar</button>
-        <a class="btn btn-secondary" href="{% url 'category_list' %}">Cancelar</a>
-    </form>
-{% endblock %}

expenses_manager/expenses/templates/categories/form.html

@@ -10,7 +10,9 @@
     <form method="post">
         {% csrf_token %}
         {{ form.as_p }}
-        <button type="submit" class="btn btn-primary">Guardar</button>
+        <div class="form-actions">
-        <a class="btn btn-secondary" href="{% url 'category_list' %}">Cancelar</a>
+            <button type="submit" class="btn btn-primary">Guardar</button>
+            <a class="btn btn-secondary" href="{% url 'category_list' %}">Cancelar</a>
+        </div>
     </form>
 {% endblock %}

expenses_manager/expenses/templates/expenses/_confirm_delete.html

@@ -0,0 +1,20 @@
+{% extends "expenses/base.html" %}
+
+{% block title %}Eliminar{% endblock %}
+
+{% block content %}
+    <h1>{% block delete_heading %}Eliminar{% endblock %}</h1>
+
+    <p>{% block delete_question %}¿Seguro que quieres eliminar este elemento?{% endblock %}</p>
+
+    {% block delete_warnings %}{% endblock %}
+
+    <form method="post">
+        {% csrf_token %}
+        {% block delete_extra_fields %}{% endblock %}
+        <div class="form-actions">
+            <button class="btn btn-danger">{% block delete_button %}Eliminar{% endblock %}</button>
+            <a class="btn btn-secondary" href="{% block cancel_url %}{% endblock %}">Cancelar</a>
+        </div>
+    </form>
+{% endblock %}

expenses_manager/expenses/templates/expenses/account_confirm_delete.html

@@ -1,18 +1,11 @@
-{% extends "expenses/base.html" %}
+{% extends "expenses/_confirm_delete.html" %}
+
 {% block title %}Eliminar{% endblock %}
-{% block content %}
 
-    <h1>Eliminar cuenta</h1>
+{% block delete_heading %}Eliminar cuenta{% endblock %}
 
-    <p>
+{% block delete_question %}¿Seguro que quieres eliminar la cuenta <strong>{{ account.name }}</strong>?{% endblock %}
-        ¿Seguro que quieres eliminar la cuenta
-        <strong>{{ account.name }}</strong>
-    </p>
 
-    <form method="post">
+{% block delete_button %}Sí, eliminar{% endblock %}
-        {% csrf_token %}
-        <button class="btn btn-danger">Sí, eliminar</button>
-        <a class="btn btn-secondary" href="{% url 'account_list' %}">Cancelar</a>
-    </form>
 
-{% endblock %}
+{% block cancel_url %}{% url 'account_list' %}{% endblock %}

expenses_manager/expenses/templates/expenses/account_form.html

@@ -14,8 +14,10 @@
         {% csrf_token %}
         {{ form.as_p }}
 
-        <button type="submit" class="btn btn-primary">Guardar</button>
+        <div class="form-actions">
-        <a class="btn btn-secondary" href="{% url 'account_list' %}">Cancelar</a>
+            <button type="submit" class="btn btn-primary">Guardar</button>
+            <a class="btn btn-secondary" href="{% url 'account_list' %}">Cancelar</a>
+        </div>
     </form>
 
 {% endblock %}

expenses_manager/expenses/templates/expenses/expense_confirm_delete.html

@@ -1,21 +1,16 @@
-{% extends "expenses/base.html" %}
+{% extends "expenses/_confirm_delete.html" %}
 
 {% block title %}Eliminar gasto{% endblock %}
 
-{% block content %}
+{% block delete_heading %}Eliminar gasto{% endblock %}
-    <h1>Eliminar gasto</h1>
-
-    <p>
-        ¿Seguro que quieres eliminar el gasto de 
-        <strong>{{ expense.amount }}€</strong>
-        del {{ expense.date }} perteneciente a 
-        <strong>{{ expense.account }}</strong>?
-    </p>
-
-    <form method="post">
-        {% csrf_token %}
-        <button class="btn btn-danger">Sí, eliminar</button>
-        <a class="btn btn-secondary" href="{% url 'expense_list' %}">Cancelar</a>
-    </form>
 
+{% block delete_question %}
+    ¿Seguro que quieres eliminar el gasto de
+    <strong>{{ expense.amount }}€</strong>
+    del {{ expense.date }} perteneciente a
+    <strong>{{ expense.account }}</strong>?
 {% endblock %}
+
+{% block delete_button %}Sí, eliminar{% endblock %}
+
+{% block cancel_url %}{% url 'expense_list' %}{% endblock %}

expenses_manager/expenses/templates/expenses/expense_form.html

@@ -46,14 +46,15 @@
             {% endif %}
         {% endfor %}
 
-        <button type="submit" class="btn btn-primary">
+        <div class="form-actions">
-            {% if form.instance.pk %}
+            <button type="submit" class="btn btn-primary">
-                Guardar gasto
+                {% if form.instance.pk %}
-            {% else %}
+                    Guardar gasto
-                Crear gasto
+                {% else %}
-            {% endif %}
+                    Crear gasto
-        </button>
+                {% endif %}
+            </button>
+            <a class="btn btn-secondary" href="{% url 'expense_list' %}">Volver</a>
+        </div>
     </form>
-
-    <a class="btn btn-secondary" href="{% url 'expense_list' %}">Volver</a>
 {% endblock %}

expenses_manager/expenses/templates/expenses/income_confirm_delete.html

@@ -1,20 +1,16 @@
-{% extends "expenses/base.html" %}
+{% extends "expenses/_confirm_delete.html" %}
+
 {% block title %}Eliminar ingreso{% endblock %}
 
-{% block content %}        
+{% block delete_heading %}Eliminar ingreso{% endblock %}
-    <h1>Eliminar ingreso</h1>
 
-    <p>
+{% block delete_question %}
-        ¿Seguro que quieres eliminar el ingreso de 
+    ¿Seguro que quieres eliminar el ingreso de
-        <strong>{{ income.amount }}€</strong>
+    <strong>{{ income.amount }}€</strong>
-        del {{ income.date }} perteneciente a 
+    del {{ income.date }} perteneciente a
-        <strong>{{ income.account }}</strong>?
+    <strong>{{ income.account }}</strong>?
-    </p>
+{% endblock %}
 
-    <form method="post">
+{% block delete_button %}Sí, eliminar{% endblock %}
-        {% csrf_token %}
-        <button class="btn btn-danger">Sí, eliminar</button>
-        <a class="btn btn-secondary" href="{% url 'income_list' %}">Cancelar</a>
-    </form>
 
-{% endblock %}
+{% block cancel_url %}{% url 'income_list' %}{% endblock %}

expenses_manager/expenses/templates/expenses/income_form.html

@@ -19,14 +19,15 @@
     <form method="post">
         {% csrf_token %}
         {{ form.as_p }}
-        <button type="submit" class="btn btn-primary">
+        <div class="form-actions">
-            {% if form.instance.pk %}
+            <button type="submit" class="btn btn-primary">
-                Guardar ingreso
+                {% if form.instance.pk %}
-            {% else %}
+                    Guardar ingreso
-                Crear ingreso
+                {% else %}
-            {% endif %}
+                    Crear ingreso
-        </button>            
+                {% endif %}
+            </button>
+            <a class="btn btn-secondary" href="{% url 'income_list' %}">Volver</a>
+        </div>
     </form>
-
-    <a class="btn btn-secondary" href="{% url 'income_list' %}">Volver</a>
 {% endblock %}

expenses_manager/expenses/templates/expenses/tag_confirm_delete.html

@@ -1,17 +1,9 @@
-{% extends "expenses/base.html" %}
+{% extends "expenses/_confirm_delete.html" %}
+
 {% block title %}Etiquetas{% endblock %}
-{% block content %}
-    <h1>Eliminar etiqueta</h1>
 
-    <p>
+{% block delete_heading %}Eliminar etiqueta{% endblock %}
-        ¿Seguro que quieres eliminar la etiqueta
-        <strong>{{ tag.name }}€</strong>?
-    </p>
 
-    <form method="post">
+{% block delete_question %}¿Seguro que quieres eliminar la etiqueta <strong>{{ tag.name }}</strong>?{% endblock %}
-        {% csrf_token %}
-        <button class="btn btn-danger">Eliminar</button>
-        <a class="btn btn-secondary" href="{% url 'tag_list' %}">Cancelar</a>
-    </form>
 
-{% endblock %}
+{% block cancel_url %}{% url 'tag_list' %}{% endblock %}

expenses_manager/expenses/templates/expenses/tag_form.html

@@ -6,8 +6,9 @@
     <form method="post">
         {% csrf_token %}
         {{ form.as_p }}
-        <button type="submit" class="btn btn-primary">Guardar</button>
+        <div class="form-actions">
+            <button type="submit" class="btn btn-primary">Guardar</button>
+            <a class="btn btn-secondary" href="{% url 'tag_list' %}">Volver</a>
+        </div>
     </form>
-
-    <a class="btn btn-secondary" href="{% url 'tag_list' %}">Volver</a>
 {% endblock %}

expenses_manager/expenses/templates/fuel/confirm_delete.html

@@ -1,26 +1,23 @@
-{% extends "expenses/base.html" %}
+{% extends "expenses/_confirm_delete.html" %}
 
-{% block title %}
+{% block title %}Repostajes{% endblock %}
-    Repostajes
+
+{% block delete_heading %}Eliminar repostaje.{% endblock %}
+
+{% block delete_question %}
+    ¿Seguro que quieres eliminar el repostaje del
+    <strong>{{ fuel.expense.date }}</strong>
+    ({{ fuel.liters }}L por {{ fuel.expense.amount }}€)?
 {% endblock %}
 
-{% block content %}
+{% block delete_warnings %}
-    <h2>Eliminar repostaje.</h2>
-
-    <p>
-        ¿Seguro que quieres eliminar el repostaje del 
-        <strong>{{ fuel.expense.date}}</strong>
-        ({{ fuel.liters }}L por {{fuel.expense.amount}}€)?
-    </p>
-
     <p class="form-errors">
         Se eliminará también el gasto asociado en el listado de gastos.
     </p>
+{% endblock %}
 
-    <form method="post">
+{% block delete_extra_fields %}
-        {% csrf_token %}
+    {% if next %}<input type="hidden" name="next" value="{{ next }}">{% endif %}
-        {% if next %}<input type="hidden" name="next" value="{{ next }}">{% endif %}
+{% endblock %}
-        <button class="btn btn-danger">Eliminar</button>
+
-        <a class="btn btn-secondary" href="{% url 'fuel_list' %}">Cancelar</a>
+{% block cancel_url %}{% if next %}{{ next }}{% else %}{% url 'fuel_list' %}{% endif %}{% endblock %}
-    </form>
-{% endblock %}

expenses_manager/expenses/templates/fuel/create.html

@@ -21,20 +21,22 @@
         {% csrf_token %}
         {% if next %}<input type="hidden" name="next" value="{{ next }}">{% endif %}
         {{ form.as_p }}
-        <button type="submit" class="btn btn-primary">
+        <div class="form-actions">
-            {% if editing %}
+            <button type="submit" class="btn btn-primary">
-                Guardar repostaje
+                {% if editing %}
+                    Guardar repostaje
+                {% else %}
+                    Crear repostaje
+                {% endif %}
+            </button>
+
+            {% if next %}
+                <a class="btn btn-secondary" href="{{ next }}">Volver</a>
+            {% elif editing %}
+                <a class="btn btn-secondary" href="{% url 'expense_list' %}">Volver</a>
             {% else %}
-                Crear repostaje
+                <a class="btn btn-secondary" href="{% url 'fuel_list' %}">Volver</a>
             {% endif %}
-        </button>            
+        </div>
     </form>
-    
-    {% if next %}
-        <a class="btn btn-secondary" href="{{ next }}">Volver</a>
-    {% elif editing %}
-        <a class="btn btn-secondary" href="{% url 'expense_list' %}">Volver</a>
-    {% else %}
-        <a class="btn btn-secondary" href="{% url 'fuel_list' %}">Volver</a>
-    {% endif %}
 {% endblock %}

expenses_manager/expenses/templates/goals/confirm_delete.html

@@ -1,18 +1,9 @@
-{% extends "expenses/base.html" %}
+{% extends "expenses/_confirm_delete.html" %}
+
 {% block title %}Objetivos{% endblock %}
-{% block content %}
 
-<h1>Eliminar objetivo</h1>
+{% block delete_heading %}Eliminar objetivo{% endblock %}
 
-<p>
+{% block delete_question %}¿Seguro que quieres eliminar el objetivo <strong>{{ goal.name }}</strong>?{% endblock %}
-    ¿Seguro que quieres eliminar el objetivo
-    <strong>{{ goal.name }}</strong>?
-</p>
 
-<form method="post">
+{% block cancel_url %}{% url 'goal_list' %}{% endblock %}
-    {% csrf_token %}
-    <button class="btn btn-danger">Eliminar</button>
-    <a class="btn btn-secondary" href="{% url 'goal_list' %}">Cancelar</a>
-</form>
-
-{% endblock %}

expenses_manager/expenses/templates/goals/form.html

@@ -11,10 +11,11 @@
     <form method="post">
         {% csrf_token %}
         {{ form.as_p }}
-        <button type="submit" class="btn btn-primary">
+        <div class="form-actions">
-            Guardar
+            <button type="submit" class="btn btn-primary">
-        </button>
+                Guardar
+            </button>
+            <a class="btn btn-secondary" href="{% url 'goal_list' %}">Volver</a>
+        </div>
     </form>
-
-    <a class="btn btn-secondary" href="{% url 'goal_list' %}">Volver</a>
 {% endblock %}

expenses_manager/expenses/tests/test_fuel.py

@@ -67,3 +67,165 @@ def test_fuel_delete_post_cascades_to_fuelentry(auth_client, user, account, cate
 
     assert not Expense.objects.filter(pk=expense.pk).exists()
     assert not FuelEntry.objects.filter(pk=entry.pk).exists()
+
+
+def _edit_payload(account, edit_date, amount, odometer, liters, next_url=None):
+    payload = {
+        "date": edit_date.strftime("%Y-%m-%d"),
+        "amount": str(amount),
+        "account": account.pk,
+        "odometer": str(odometer),
+        "liters": str(liters),
+    }
+    if next_url is not None:
+        payload["next"] = next_url
+    return payload
+
+
+def test_fuel_edit_post_updates_expense_and_fuelentry(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"), amount=Decimal("50"))
+    expense = entry.expense
+    new_date = date(2024, 3, 1)
+
+    response = auth_client.post(
+        reverse('fuel_edit', args=[expense.pk]),
+        _edit_payload(account, new_date, Decimal("45.50"), 1200, Decimal("22.5")),
+    )
+
+    assert response.status_code == 302
+
+    expense.refresh_from_db()
+    entry.refresh_from_db()
+
+    assert expense.amount == Decimal("45.50")
+    assert expense.date == new_date
+    assert expense.account_id == account.pk
+    assert entry.odometer == 1200
+    assert entry.liters == Decimal("22.5")
+
+
+def test_fuel_edit_invalid_post_returns_200_and_does_not_modify(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"), amount=Decimal("50"))
+    expense = entry.expense
+    original_date = expense.date
+
+    payload = _edit_payload(account, date(2024, 3, 1), Decimal("45.50"), 1200, Decimal("22.5"))
+    payload["liters"] = ""
+
+    response = auth_client.post(reverse('fuel_edit', args=[expense.pk]), payload)
+
+    assert response.status_code == 200
+
+    expense.refresh_from_db()
+    entry.refresh_from_db()
+
+    assert expense.amount == Decimal("50")
+    assert expense.date == original_date
+    assert entry.odometer == 1000
+    assert entry.liters == Decimal("20")
+
+
+def test_fuel_edit_redirects_to_next_when_relative(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+    next_url = reverse('expense_list')
+
+    response = auth_client.post(
+        reverse('fuel_edit', args=[expense.pk]),
+        _edit_payload(account, date(2024, 3, 1), Decimal("45.50"), 1200, Decimal("22.5"), next_url=next_url),
+    )
+
+    assert response.status_code == 302
+    assert response.url == next_url
+
+
+def test_fuel_edit_ignores_external_next(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+
+    response = auth_client.post(
+        reverse('fuel_edit', args=[expense.pk]),
+        _edit_payload(
+            account, date(2024, 3, 1), Decimal("45.50"), 1200, Decimal("22.5"),
+            next_url="https://evil.com",
+        ),
+    )
+
+    assert response.status_code == 302
+    assert response.url == reverse('expense_list')
+
+
+def test_fuel_delete_get_does_not_delete(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+
+    response = auth_client.get(reverse('fuel_delete', args=[expense.pk]))
+
+    assert response.status_code == 200
+    assert Expense.objects.filter(pk=expense.pk).exists()
+    assert FuelEntry.objects.filter(pk=entry.pk).exists()
+
+
+def test_fuel_delete_get_cancel_link_uses_next_when_provided(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+    next_url = reverse('expense_list')
+
+    response = auth_client.get(reverse('fuel_delete', args=[expense.pk]), {"next": next_url})
+
+    assert response.status_code == 200
+    assert f'<a class="btn btn-secondary" href="{next_url}">Cancelar</a>'.encode() in response.content
+
+
+def test_fuel_delete_get_cancel_link_falls_back_to_fuel_list_without_next(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+    fuel_list_url = reverse('fuel_list')
+
+    response = auth_client.get(reverse('fuel_delete', args=[expense.pk]))
+
+    assert response.status_code == 200
+    assert f'<a class="btn btn-secondary" href="{fuel_list_url}">Cancelar</a>'.encode() in response.content
+
+
+def test_fuel_delete_post_redirects_to_next_when_relative(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+    next_url = reverse('expense_list')
+
+    response = auth_client.post(reverse('fuel_delete', args=[expense.pk]), {"next": next_url})
+
+    assert response.status_code == 302
+    assert response.url == next_url
+
+
+def test_fuel_delete_post_ignores_external_next(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+
+    response = auth_client.post(reverse('fuel_delete', args=[expense.pk]), {"next": "https://evil.com"})
+
+    assert response.status_code == 302
+    assert response.url == reverse('fuel_list')
+
+
+def test_fuel_delete_get_ignores_unsafe_next_in_cancel_link(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+    fuel_list_url = reverse('fuel_list')
+
+    response = auth_client.get(reverse('fuel_delete', args=[expense.pk]), {"next": "javascript:alert(1)"})
+
+    assert response.status_code == 200
+    assert b"javascript:" not in response.content
+    assert f'<a class="btn btn-secondary" href="{fuel_list_url}">Cancelar</a>'.encode() in response.content
+
+
+def test_fuel_edit_get_ignores_unsafe_next_in_cancel_link(auth_client, user, account, category):
+    entry = _make_fuel_entry(user, account, category, odometer=1000, liters=Decimal("20"))
+    expense = entry.expense
+
+    response = auth_client.get(reverse('fuel_edit', args=[expense.pk]), {"next": "javascript:alert(1)"})
+
+    assert response.status_code == 200
+    assert b"javascript:" not in response.content

expenses_manager/expenses/views.py

@@ -30,17 +30,22 @@ def _get_int(value):
         return None
 
 
-def _redirect_back(request, fallback):
+def _safe_next(request):
     next_url = request.POST.get("next") or request.GET.get("next")
-    
+
     if next_url and url_has_allowed_host_and_scheme(
         next_url,
         allowed_hosts={request.get_host()},
         require_https=request.is_secure(),
     ):
-        return redirect(next_url)
+        return next_url
-    
+
-    return redirect(fallback)
+    return ""
+
+
+def _redirect_back(request, fallback):
+    next_url = _safe_next(request)
+    return redirect(next_url or fallback)
 
 
 def sub_months(year, month, n):
@@ -804,20 +809,16 @@ def fuel_list(request):
 
 @login_required
 def fuel_edit(request, pk):
-    expense = get_object_or_404(
+    expense = get_object_or_404(Expense, pk=pk, owner=request.user)
-        Expense,
-        pk=pk,
-        owner=request.user,
-    )
 
     fuel = get_object_or_404(FuelEntry, expense=expense)
 
     if request.method == "POST":
-        form = FuelEntryForm(request.POST, user=request.user)
+        form = FuelEntryForm(request.POST, instance=expense, user=request.user)
 
         if form.is_valid():
             # Update expense
-            expense.date = form.save(commit=False)
+            expense = form.save(commit=False)
             expense.description = "Repostaje"
             expense.save()
 
@@ -830,8 +831,8 @@ def fuel_edit(request, pk):
     else:
         form = FuelEntryForm(instance=expense, user=request.user)
 
-    next_url = request.POST.get("next") or request.GET.get("next", "")
+    next_url = _safe_next(request)
-    
+
     return render(
         request,
         "fuel/create.html",
@@ -854,8 +855,8 @@ def fuel_delete(request, pk):
         messages.success(request, "Repostaje eliminado.")
         return _redirect_back(request, "fuel_list")
 
-    next_url = request.POST.get("next") or request.GET.get("next", "")
+    next_url = _safe_next(request)
-    
+
     return render(
         request,
         "fuel/confirm_delete.html",