Merge 0a50941c35 into 19ecdb5052

Co-authored-by: Baptiste Jonglez <git@bitsofnetworks.org>
Co-authored-by: Alexis Métaireau <alexis@notmyidea.org>

ihatemoney/forms.py

@@ -14,6 +14,8 @@ from wtforms.fields import (
     BooleanField,
     DateField,
     DecimalField,
+    HiddenField,
+    IntegerField,
     Label,
     PasswordField,
     SelectField,
@@ -437,6 +439,22 @@ class BillForm(FlaskForm):
             raise ValidationError(msg)
 
 
+class HiddenCommaDecimalField(HiddenField, CommaDecimalField):
+    pass
+
+
+class HiddenIntegerField(HiddenField, IntegerField):
+    pass
+
+
+class SettlementForm(FlaskForm):
+    """Used internally for validation, not directly visible to users"""
+
+    amount = HiddenCommaDecimalField("Amount", validators=[DataRequired()])
+    sender_id = HiddenIntegerField("Sender", validators=[DataRequired()])
+    receiver_id = HiddenIntegerField("Receiver", validators=[DataRequired()])
+
+
 class MemberForm(FlaskForm):
     name = StringField(_("Name"), validators=[DataRequired()], filters=[strip_filter])
 

ihatemoney/models.py

@@ -113,42 +113,57 @@ class Project(db.Model):
 
     @property
     def full_balance(self):
-        """Returns a triple of dicts:
+        """Returns a tuple of dicts:
 
-        - dict mapping each member to its balance
+        - dict mapping each member to its overall balance
 
-        - dict mapping each member to how much he/she should pay others
-          (i.e. how much he/she benefited from bills)
+        - dict mapping each member to its expenses (i.e. how much he/she
+          benefited from all bills, whoever actually paid)
 
-        - dict mapping each member to how much he/she should be paid by
-          others (i.e. how much he/she has paid for bills)
+        - dict mapping each member to how much he/she has paid for bills
+
+        - dict mapping each member to how much he/she has transferred
+          money to other members
+
+        - dict mapping each member to how much he/she has received money
+          from other members
+
+        balance, spent, paid, transferred, received
 
-        balance       spent        paid
         """
-        balances, should_pay, should_receive = (defaultdict(int) for time in (1, 2, 3))
+        balances, spent, paid, transferred, received = (
+            defaultdict(float) for _ in range(5)
+        )
         for bill in self.get_bills_unordered().all():
             total_weight = sum(ower.weight for ower in bill.owers)
 
             if bill.bill_type == BillType.EXPENSE:
-                should_receive[bill.payer.id] += bill.converted_amount
+                paid[bill.payer.id] += bill.converted_amount
                 for ower in bill.owers:
-                    should_pay[ower.id] += (
+                    spent[ower.id] += ower.weight * bill.converted_amount / total_weight
+
+            if bill.bill_type == BillType.REIMBURSEMENT:
+                transferred[bill.payer.id] += bill.converted_amount
+                for ower in bill.owers:
+                    received[ower.id] += (
                         ower.weight * bill.converted_amount / total_weight
                     )
 
-            if bill.bill_type == BillType.REIMBURSEMENT:
-                should_receive[bill.payer.id] += bill.converted_amount
-                for ower in bill.owers:
-                    should_receive[ower.id] -= bill.converted_amount
-
         for person in self.members:
-            balance = should_receive[person.id] - should_pay[person.id]
+            balance = (
+                paid[person.id]
+                - spent[person.id]
+                + transferred[person.id]
+                - received[person.id]
+            )
             balances[person.id] = balance
 
         return (
             balances,
-            should_pay,
-            should_receive,
+            spent,
+            paid,
+            transferred,
+            received,
         )
 
     @property
@@ -157,17 +172,19 @@ class Project(db.Model):
 
     @property
     def members_stats(self):
-        """Compute what each participant has paid
+        """Compute what each participant has spent, paid, transferred and received
 
         :return: one stat dict per participant
         :rtype list:
         """
-        balance, spent, paid = self.full_balance
+        balance, spent, paid, transferred, received = self.full_balance
         return [
             {
                 "member": member,
+                "spent": -1.0 * spent[member.id],
                 "paid": paid[member.id],
-                "spent": spent[member.id],
+                "transferred": transferred[member.id],
+                "received": -1.0 * received[member.id],
                 "balance": balance[member.id],
             }
             for member in self.active_members
@@ -447,6 +464,10 @@ class Project(db.Model):
             db.session.commit()
         return person
 
+    def has_member(self, member_id):
+        person = Person.query.get(member_id, self)
+        return person is not None
+
     def remove_project(self):
         # We can't import at top level without circular dependencies
         from ihatemoney.history import purge_history

ihatemoney/templates/settle_bills.html

@@ -1,25 +1,41 @@
-{% extends "sidebar_table_layout.html" %}
-
-{% block sidebar %}
-    <div id="table_overflow">
-        {{ balance_table(show_weight=False) }}
-    </div>
-{% endblock %}
-
-
-{% block content %}
+{% extends "sidebar_table_layout.html" %} {% block sidebar %}
+<div id="table_overflow">{{ balance_table(show_weight=False) }}</div>
+{% endblock %} {% block content %}
 <table id="bill_table" class="split_bills table table-striped">
-        <thead><tr><th>{{ _("Who pays?") }}</th><th>{{ _("To whom?") }}</th><th>{{ _("How much?") }}</th><th>{{ _("Settled?") }}</th></tr></thead>
+  <thead>
+    <tr>
+      <th>{{ _("Who pays?") }}</th>
+      <th>{{ _("To whom?") }}</th>
+      <th>{{ _("How much?") }}</th>
+      <th>{{ _("Settled?") }}</th>
+    </tr>
+  </thead>
   <tbody>
-    {% for bill in bills %}
-    <tr receiver={{bill.receiver.id}}>
-            <td>{{ bill.ower }}</td>
-            <td>{{ bill.receiver }}</td>
-            <td>{{ bill.amount|currency }}</td>
+    {% for transaction in transactions %}
+    <tr receiver="{{transaction.receiver.id}}">
+      <td>{{ transaction.ower }}</td>
+      <td>{{ transaction.receiver }}</td>
+      <td>{{ transaction.amount|currency }}</td>
       <td>
         <span id="settle-bill" class="ml-auto pb-2">
-                    <a href="{{ url_for('.settle', amount = bill.amount, ower_id = bill.ower.id, payer_id = bill.receiver.id) }}" class="btn btn-primary">
-                      <div data-toggle="tooltip" title='{{ _("Click here to record that the money transfer has been done") }}'>
+          <form class="" action="{{ url_for(".add_settlement_bill") }}" method="POST">
+              {{ settlement_form.csrf_token }}
+              {{ settlement_form.amount(value=transaction.amount) }}
+              {{ settlement_form.sender_id(value=transaction.ower.id) }}
+              {{ settlement_form.receiver_id(value=transaction.receiver.id) }}
+              <button class="btn btn-primary" type="submit" title="{{ _("Settle") }}">
+                <div
+                  data-toggle="tooltip"
+                  title='{{ _("Click here to record that the money transfer has been done") }}'
+                >
+                {{ _("Settle") }}
+                </div>
+              </button>
+          </form>
+          <a
+            href="{{ url_for('.add_settlement_bill', amount = transaction.amount, sender_id = transaction.ower.id, receiver_id = transaction.receiver.id) }}"
+            class="btn btn-primary"
+          >
               {{ ("Settle") }}
             </div>
           </a>

ihatemoney/templates/statistics.html

@@ -1,29 +1,39 @@
-{% extends "sidebar_table_layout.html" %}
-
-{% block sidebar %}
+{% extends "sidebar_table_layout.html" %} {% block sidebar %}
 <div id="table_overflow" class="statistics mr-md-n3">
   {{ balance_table(show_weight=False, show_header=True) }}
 </div>
-{% endblock %}
-
-
-{% block content %}
+{% endblock %} {% block content %}
 <div class="d-flex flex-column">
   <table id="bill_table" class="split_bills table table-striped ml-md-n3">
-            <thead><tr><th class="d-md-none">{{ _("Who?") }}</th><th>{{ _("Paid") }}</th><th>{{ _("Spent") }}</th></tr></thead>
+    <thead>
+      <tr>
+        <th class="d-md-none">{{ _("Who?") }}</th>
+        <th>{{ _("Paid") }}</th>
+        <th>{{ _("Expenses") }}</th>
+        <th>{{ _("Direct transfer") }}</th>
+        <th>{{ _("Received") }}</th>
+      </tr>
+    </thead>
     <tbody>
       {% for stat in members_stats|sort(attribute='member.name') %}
       <tr>
         <td class="d-md-none">{{ stat.member.name }}</td>
         <td>{{ stat.paid|currency }}</td>
         <td>{{ stat.spent|currency }}</td>
+        <td>{{ stat.transferred|currency }}</td>
+        <td>{{ stat.received|currency }}</td>
       </tr>
       {% endfor %}
     </tbody>
   </table>
-        <h2>{{ _("Expenses by Month") }}</h2>
+  <h2>{{ _("Expenses by month") }}</h2>
   <table id="monthly_stats" class="table table-striped">
-            <thead><tr><th>{{ _("Period") }}</th><th>{{ _("Spent") }}</th></tr></thead>
+    <thead>
+      <tr>
+        <th>{{ _("Period") }}</th>
+        <th>{{ _("Expenses") }}</th>
+      </tr>
+    </thead>
     <tbody>
       {% for month in months %}
       <tr>

ihatemoney/tests/api_test.py

@@ -814,7 +814,8 @@ class TestAPI(IhatemoneyTestCase):
             "/api/projects/raclette/statistics", headers=self.get_auth("raclette")
         )
         self.assertStatus(200, req)
-        assert [
+        received_stats = json.loads(req.data.decode("utf-8"))
+        assert received_stats == [
             {
                 "balance": 12.5,
                 "member": {
@@ -824,7 +825,9 @@ class TestAPI(IhatemoneyTestCase):
                     "weight": 1.0,
                 },
                 "paid": 25.0,
-                "spent": 12.5,
+                "received": 0.0,
+                "spent": -12.5,
+                "transferred": 0.0,
             },
             {
                 "balance": -12.5,
@@ -834,10 +837,12 @@ class TestAPI(IhatemoneyTestCase):
                     "name": "jeanne",
                     "weight": 1.0,
                 },
-                "paid": 0,
-                "spent": 12.5,
+                "paid": 0.0,
+                "received": 0.0,
+                "spent": -12.5,
+                "transferred": 0.0,
             },
-        ] == json.loads(req.data.decode("utf-8"))
+        ]
 
     def test_username_xss(self):
         # create a project

ihatemoney/tests/budget_test.py

@@ -790,15 +790,18 @@ class TestBudget(IhatemoneyTestCase):
         self.client.post("/rent/members/add", data={"name": "bob"})
         self.client.post("/rent/members/add", data={"name": "alice"})
 
-        members_ids = [m.id for m in self.get_project("rent").members]
-        # create a bill to test reimbursement
+        everybody = [m.id for m in self.get_project("rent").members]
+        bob = everybody[0]
+        alice = everybody[1]
+
+        # create a bill
         self.client.post(
             "/rent/add",
             data={
                 "date": "2022-12-12",
                 "what": "december rent",
-                "payer": members_ids[0],  # bob
-                "payed_for": members_ids,  # bob and alice
+                "payer": bob,
+                "payed_for": everybody,
                 "bill_type": "Expense",
                 "amount": "1000",
             },
@@ -806,32 +809,40 @@ class TestBudget(IhatemoneyTestCase):
         # check balance
         balance = self.get_project("rent").balance
         assert set(balance.values()), set([500 == -500])
-        # check paid
-        bob_paid = self.get_project("rent").full_balance[2][members_ids[0]]
-        alice_paid = self.get_project("rent").full_balance[2][members_ids[1]]
+
+        project = self.get_project("rent")
+        bob_paid = project.full_balance[2][bob]
+        alice_paid = project.full_balance[2][alice]
         assert bob_paid == 1000
         assert alice_paid == 0
 
-        # test reimbursement bill
+        # reimbursement bill
         self.client.post(
             "/rent/add",
             data={
                 "date": "2022-12-13",
                 "what": "reimbursement for rent",
-                "payer": members_ids[1],  # alice
-                "payed_for": members_ids[0],  # bob
+                "payer": alice,
+                "payed_for": bob,
                 "bill_type": "Reimbursement",
                 "amount": "500",
             },
         )
 
-        balance = self.get_project("rent").balance
+        balance = project.balance
         assert set(balance.values()), set([0 == 0])
-        # check paid
-        bob_paid = self.get_project("rent").full_balance[2][members_ids[0]]
-        alice_paid = self.get_project("rent").full_balance[2][members_ids[1]]
-        assert bob_paid == 500
-        assert alice_paid == 500
+
+        # After the reimbursement, the full balance should be populated with
+        # transfer items
+        bob_paid = project.full_balance[2][bob]
+        alice_paid = project.full_balance[2][alice]
+        assert bob_paid == 1000
+        assert alice_paid == 0
+
+        bob_received = project.full_balance[4][bob]
+        alice_transferred = project.full_balance[3][alice]
+        assert bob_received == 500
+        assert alice_transferred == 500
 
     def test_weighted_balance(self):
         self.post_project("raclette")
@@ -1069,14 +1080,25 @@ class TestBudget(IhatemoneyTestCase):
         assert len(project.active_months_range()) == 0
         assert len(project.monthly_stats) == 0
 
-        # Check that the "monthly expenses" table is empty
+        # Check that the "monthly expenses" table exists
+        # and is empty.
         response = self.client.get("/raclette/statistics")
-        regex = (
-            r"<table id=\"monthly_stats\".*>\s*<thead>\s*<tr>\s*<th>Period</th>\s*"
-            r"<th>Spent</th>\s*</tr>\s*</thead>\s*<tbody>\s*</tbody>\s*</table>"
-        )
-        assert re.search(regex, response.data.decode("utf-8"))
 
+        regex = (
+            r'<table id="monthly_stats" class="table table-striped">\n'
+            r"    <thead>\n"
+            r"      <tr>\n"
+            r"        <th>Period</th>\n"
+            r"        <th>Expenses</th>\n"
+            r"      </tr>\n"
+            r"    </thead>\n"
+            r"    <tbody>\n"
+            r"      \n"
+            r"    </tbody>\n"
+            r"  </table>"
+        )
+
+        assert re.search(regex, response.data.decode("utf-8"))
         # create bills
         self.client.post(
             "/raclette/add",
@@ -1115,22 +1137,29 @@ class TestBudget(IhatemoneyTestCase):
         )
 
         response = self.client.get("/raclette/statistics")
-        regex = r"<td class=\"d-md-none\">{}</td>\s*<td>{}</td>\s*<td>{}</td>"
-        assert re.search(
-            regex.format("zorglub", r"\$20\.00", r"\$31\.67"),
-            response.data.decode("utf-8"),
-        )
-        assert re.search(
-            regex.format("jeanne", r"\$20\.00", r"\$5\.83"),
-            response.data.decode("utf-8"),
-        )
-        assert re.search(
-            regex.format("tata", r"\$0\.00", r"\$2\.50"), response.data.decode("utf-8")
-        )
-        assert re.search(
-            regex.format("pépé", r"\$0\.00", r"\$0\.00"), response.data.decode("utf-8")
+        html = response.data.decode("utf-8")
+
+        def stat_entry(name, paid, spent, transferred=None, received=None):
+            return (
+                f'<td class="d-md-none">{name}</td>\n'
+                f"        <td>{paid}</td>\n"
+                f"        <td>{spent}</td>\n"
+                # f"        <td>${spent}</td>\n"
+                # f"        <td>${transferred}</td>"
             )
 
+        #         set_trace()
+
+        #         regex = (
+        #             r'\s*<td class="d-md-none">{}</td>\n'
+        #             r"\s*<td>{}</td>\n"
+        #             r"\s*<td>{}</td>\n"
+        #         )
+
+        assert stat_entry("zorglub", "$20.00", "-$31.67") in html
+        assert stat_entry("jeanne", "$20.00", "-$5.83") in html
+        assert stat_entry("tata", "$0.00", "-$2.50") in html
+        assert stat_entry("pépé", "$0.00", "-$0.00") in html
         # Check that the order of participants in the sidebar table is the
         # same as in the main table.
         order = ["jeanne", "pépé", "tata", "zorglub"]
@@ -1358,23 +1387,25 @@ class TestBudget(IhatemoneyTestCase):
         count = 0
         for t in transactions:
             count += 1
-            self.client.get(
-                "/raclette/settle"
-                + "/"
-                + str(t["amount"])
-                + "/"
-                + str(t["ower"].id)
-                + "/"
-                + str(t["receiver"].id)
+            self.client.post(
+                "/raclette/settle",
+                data={
+                    "amount": t["amount"],
+                    "sender_id": t["ower"].id,
+                    "receiver_id": t["receiver"].id,
+                },
             )
             temp_transactions = project.get_transactions_to_settle_bill()
             # test if the one has disappeared
             assert len(temp_transactions) == len(transactions) - count
 
-            # test if theres a new one with bill_type reimbursement
+            # test if there is a new one with bill_type reimbursement
             bill = project.get_newest_bill()
             assert bill.bill_type == models.BillType.REIMBURSEMENT
-        return
+
+        # There should be no more settlement to do at the end
+        transactions = project.get_transactions_to_settle_bill()
+        assert len(transactions) == 0
 
     def test_settle_zero(self):
         self.post_project("raclette")
@@ -1463,6 +1494,78 @@ class TestBudget(IhatemoneyTestCase):
         # Create and log in as another project
         self.post_project("tartiflette")
 
+        # Add a participant in this second project
+        self.client.post("/tartiflette/members/add", data={"name": "pirate"})
+        pirate = models.Person.query.filter(models.Person.id == 5).one()
+        assert pirate.name == "pirate"
+
+        # Try to add a new bill to another project
+        resp = self.client.post(
+            "/raclette/add",
+            data={
+                "date": "2017-01-01",
+                "what": "fromage frelaté",
+                "payer": 2,
+                "payed_for": [2, 3, 4],
+                "bill_type": "Expense",
+                "amount": "100.0",
+            },
+        )
+        # Ensure it has not been created
+        raclette = self.get_project("raclette")
+        assert raclette.get_bills().count() == 1
+
+        # Try to add a new bill in our project that references members of another project.
+        # First with invalid payed_for IDs.
+        resp = self.client.post(
+            "/tartiflette/add",
+            data={
+                "date": "2017-01-01",
+                "what": "soupe",
+                "payer": 5,
+                "payed_for": [3],
+                "bill_type": "Expense",
+                "amount": "5000.0",
+            },
+        )
+        # Ensure it has not been created
+        piratebill = models.Bill.query.filter(models.Bill.what == "soupe").one_or_none()
+        assert piratebill is None, "piratebill 1 should not exist"
+
+        # Then with invalid payer ID
+        self.client.post(
+            "/tartiflette/add",
+            data={
+                "date": "2017-02-01",
+                "what": "pain",
+                "payer": 3,
+                "payed_for": [5],
+                "bill_type": "Expense",
+                "amount": "5000.0",
+            },
+        )
+        # Ensure it has not been created
+        piratebill = models.Bill.query.filter(models.Bill.what == "pain").one_or_none()
+        assert piratebill is None, "piratebill 2 should not exist"
+
+        # Make sure we can actually create valid bills
+        self.client.post(
+            "/tartiflette/add",
+            data={
+                "date": "2017-03-01",
+                "what": "baguette",
+                "payer": 5,
+                "payed_for": [5],
+                "bill_type": "Expense",
+                "amount": "5.0",
+            },
+        )
+        # Ensure it has been created
+        okbill = models.Bill.query.filter(models.Bill.what == "baguette").one_or_none()
+        assert okbill is not None, "Bill baguette should exist"
+        assert okbill.what == "baguette"
+
+        # Now try to access and modify existing bills
         modified_bill = {
             "date": "2018-12-31",
             "what": "roblochon",
@@ -1556,6 +1659,24 @@ class TestBudget(IhatemoneyTestCase):
         member = models.Person.query.filter(models.Person.id == 1).one_or_none()
         assert member is None
 
+        # test new settle endpoint to add bills with wrong ids
+        self.client.post("/exit")
+        self.client.post(
+            "/authenticate", data={"id": "tartiflette", "password": "tartiflette"}
+        )
+        self.client.post(
+            "/tartiflette/settle",
+            data={
+                "sender_id": 4,
+                "receiver_id": 5,
+                "amount": "42.0",
+            },
+        )
+        piratebill = models.Bill.query.filter(
+            models.Bill.bill_type == models.BillType.REIMBURSEMENT
+        ).one_or_none()
+        assert piratebill is None, "piratebill 3 should not exist"
+
     @pytest.mark.skip(reason="Currency conversion is broken")
     def test_currency_switch(self):
         # A project should be editable

ihatemoney/utils.py

@@ -452,7 +452,9 @@ def format_form_errors(form, prefix):
         )
     else:
         error_list = "</li><li>".join(
-            str(error) for (field, errors) in form.errors.items() for error in errors
+            f"<strong>{field}</strong> {error}"
+            for (field, errors) in form.errors.items()
+            for error in errors
         )
         errors = f"<ul><li>{error_list}</li></ul>"
         # I18N: Form error with a list of errors

ihatemoney/web.py

@@ -56,6 +56,7 @@ from ihatemoney.forms import (
     ProjectForm,
     ProjectFormWithCaptcha,
     ResetPasswordForm,
+    SettlementForm,
     get_billform_for,
 )
 from ihatemoney.history import get_history, get_history_queries, purge_history
@@ -852,24 +853,46 @@ def change_lang(lang):
 @main.route("/<project_id>/settle_bills")
 def settle_bill():
     """Compute the sum each one have to pay to each other and display it"""
-    bills = g.project.get_transactions_to_settle_bill()
-    return render_template("settle_bills.html", bills=bills, current_view="settle_bill")
+    transactions = g.project.get_transactions_to_settle_bill()
+    settlement_form = SettlementForm()
+    return render_template(
+        "settle_bills.html",
+        transactions=transactions,
+        settlement_form=settlement_form,
+        current_view="settle_bill",
+    )
 
 
-@main.route("/<project_id>/settle/<amount>/<int:ower_id>/<int:payer_id>")
-def settle(amount, ower_id, payer_id):
-    new_reinbursement = Bill(
-        amount=float(amount),
+@main.route("/<project_id>/settle", methods=["POST"])
+def add_settlement_bill():
+    """Create a bill to register a settlement"""
+    form = SettlementForm(id=g.project.id)
+    if not form.validate():
+        flash(
+            format_form_errors(form, _("Error creating settlement bill")),
+            category="danger",
+        )
+        return redirect(url_for(".settle_bill"))
+
+    # Ensure that the sender and receiver ID are valid and part of this project
+    receiver_id = form.receiver_id.data
+    sender_id = form.sender_id.data
+
+    if not g.project.has_member(sender_id):
+        return redirect(url_for(".settle_bill"))
+
+    settlement = Bill(
+        amount=form.amount.data,
         date=datetime.datetime.today(),
-        owers=[Person.query.get(payer_id)],
-        payer_id=ower_id,
+        owers=[Person.query.get(receiver_id, g.project)],
+        payer_id=sender_id,
         project_default_currency=g.project.default_currency,
         bill_type=BillType.REIMBURSEMENT,
         what=_("Settlement"),
     )
     session.update()
 
-    db.session.add(new_reinbursement)
+    db.session.add(settlement)
     db.session.commit()
 
     flash(_("Settlement bill has been successfully added"), category="success")