Merge 23d5b73ae1 into 19ecdb5052

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

ihatemoney/currency_convertor.py

@@ -1,10 +1,17 @@
+import csv
 import traceback
 import warnings
+from abc import ABC, abstractmethod
+from decimal import Decimal
+from typing import Dict, Optional
 
 from cachetools import TTLCache, cached
 import requests
 
 
+NO_CURRENCY = "XXX"
+ExchangeRates = Dict[str, float]
+
 class Singleton(type):
     _instances = {}
 
@@ -14,205 +21,93 @@ class Singleton(type):
         return cls._instances[cls]
 
 
-class CurrencyConverter(object, metaclass=Singleton):
+class ExchangeRateGetter(ABC):
-    # Get exchange rates
+
-    no_currency = "XXX"
+    def get_rates(self) -> Optional[ExchangeRates]:
+        """Method to retrieve a list of currency conversion rates.
+
+        Returns:
+            currencies: dict - key is a three-letter currency, value is float of conversion to base currency
+        """
+        try:
+            return self._get_rates()
+        except Exception:
+            warnings.warn(
+                f"Exchange rate getter failed - {traceback.format_exc(limit=0).strip()}"
+            )
+
+    @abstractmethod
+    def _get_rates(self) -> Optional[ExchangeRates]:
+        """Actual implementation of the exchange rate getter."""
+        raise NotImplementedError
+
+
+class ApiExchangeRate(ExchangeRateGetter):
     api_url = "https://api.exchangerate.host/latest?base=USD"
 
+    def _get_rates(self) -> Optional[ExchangeRates]:
+        return requests.get(self.api_url).json()["rates"]  # TODO not working currently probably
+
+
+class UserExchangeRate(ExchangeRateGetter):
+    user_csv_file = "path/to/file.csv"
+
+    def _get_rates(self) -> Optional[ExchangeRates]:
+        """Get rates from user defined csv.
+
+        The user_csv_file should contain the currency conversions to "USD" without 1 header row
+        Example:
+        ```
+        currency_code,fx_rate_to_USD
+        CZK,25.0
+        ...
+        ```
+
+        TODO: make it work bi-directionally
+        TODO: document for the user where to place the file
+        """
+        reader = csv.reader(self.user_csv_file)
+
+        rates = {}
+        for row in reader:
+            from_currency = row[0]
+            rate = float(row[1])
+            # TODO add validation and exception handling for typos
+            rates[from_currency] = rate
+        return rates
+
+
+class HardCodedExchangeRate(ExchangeRateGetter):
+
+    def _get_rates(self) -> Optional[dict]:
+        return {"USD": 1.0}  # TODO fill in more
+
+
+class CurrencyConverter(object, metaclass=Singleton):
+    no_currency = NO_CURRENCY
+
     def __init__(self):
         pass
 
     @cached(cache=TTLCache(maxsize=1, ttl=86400))
     def get_rates(self):
-        try:
+        """Try to retrieve the exchange rate from various sources, defaulting to hard coded values."""
-            rates = requests.get(self.api_url).json()["rates"]
+        for provider in [ApiExchangeRate, UserExchangeRate, HardCodedExchangeRate]:
-        except Exception:
+            if rates:= provider.get_rates():
-            warnings.warn(
+                break
-                f"Call to {self.api_url} failed: {traceback.format_exc(limit=0).strip()}"
+        else:
-            )
-            # In case of any exception, let's have an empty value
             rates = {}
-        rates[self.no_currency] = 1.0
+        rates[NO_CURRENCY] = 1.0
         return rates
 
-    def get_currencies(self, with_no_currency=True):
+    def get_currencies(self, with_no_currency: bool=True) -> list:
-        currencies = [
+        currencies = list(self.get_rates.keys())
-            "AED",
-            "AFN",
-            "ALL",
-            "AMD",
-            "ANG",
-            "AOA",
-            "ARS",
-            "AUD",
-            "AWG",
-            "AZN",
-            "BAM",
-            "BBD",
-            "BDT",
-            "BGN",
-            "BHD",
-            "BIF",
-            "BMD",
-            "BND",
-            "BOB",
-            "BRL",
-            "BSD",
-            "BTC",
-            "BTN",
-            "BWP",
-            "BYN",
-            "BZD",
-            "CAD",
-            "CDF",
-            "CHF",
-            "CLF",
-            "CLP",
-            "CNH",
-            "CNY",
-            "COP",
-            "CRC",
-            "CUC",
-            "CUP",
-            "CVE",
-            "CZK",
-            "DJF",
-            "DKK",
-            "DOP",
-            "DZD",
-            "EGP",
-            "ERN",
-            "ETB",
-            "EUR",
-            "FJD",
-            "FKP",
-            "GBP",
-            "GEL",
-            "GGP",
-            "GHS",
-            "GIP",
-            "GMD",
-            "GNF",
-            "GTQ",
-            "GYD",
-            "HKD",
-            "HNL",
-            "HRK",
-            "HTG",
-            "HUF",
-            "IDR",
-            "ILS",
-            "IMP",
-            "INR",
-            "IQD",
-            "IRR",
-            "ISK",
-            "JEP",
-            "JMD",
-            "JOD",
-            "JPY",
-            "KES",
-            "KGS",
-            "KHR",
-            "KMF",
-            "KPW",
-            "KRW",
-            "KWD",
-            "KYD",
-            "KZT",
-            "LAK",
-            "LBP",
-            "LKR",
-            "LRD",
-            "LSL",
-            "LYD",
-            "MAD",
-            "MDL",
-            "MGA",
-            "MKD",
-            "MMK",
-            "MNT",
-            "MOP",
-            "MRU",
-            "MUR",
-            "MVR",
-            "MWK",
-            "MXN",
-            "MYR",
-            "MZN",
-            "NAD",
-            "NGN",
-            "NIO",
-            "NOK",
-            "NPR",
-            "NZD",
-            "OMR",
-            "PAB",
-            "PEN",
-            "PGK",
-            "PHP",
-            "PKR",
-            "PLN",
-            "PYG",
-            "QAR",
-            "RON",
-            "RSD",
-            "RUB",
-            "RWF",
-            "SAR",
-            "SBD",
-            "SCR",
-            "SDG",
-            "SEK",
-            "SGD",
-            "SHP",
-            "SLL",
-            "SOS",
-            "SRD",
-            "SSP",
-            "STD",
-            "STN",
-            "SVC",
-            "SYP",
-            "SZL",
-            "THB",
-            "TJS",
-            "TMT",
-            "TND",
-            "TOP",
-            "TRY",
-            "TTD",
-            "TWD",
-            "TZS",
-            "UAH",
-            "UGX",
-            "USD",
-            "UYU",
-            "UZS",
-            "VEF",
-            "VES",
-            "VND",
-            "VUV",
-            "WST",
-            "XAF",
-            "XAG",
-            "XAU",
-            "XCD",
-            "XDR",
-            "XOF",
-            "XPD",
-            "XPF",
-            "XPT",
-            "YER",
-            "ZAR",
-            "ZMW",
-            "ZWL",
-        ]
         if with_no_currency:
             currencies.append(self.no_currency)
         return currencies
 
-    def exchange_currency(self, amount, source_currency, dest_currency):
+    def exchange_currency(self, amount: float, source_currency: str, dest_currency: str) -> float:
+        """Return the money amount converted from source_currency to dest_currency."""
         if (
             source_currency == dest_currency
             or source_currency == self.no_currency
@@ -223,6 +118,8 @@ class CurrencyConverter(object, metaclass=Singleton):
         rates = self.get_rates()
         source_rate = rates[source_currency]
         dest_rate = rates[dest_currency]
-        new_amount = (float(amount) / source_rate) * dest_rate
+        # Using Decimal to not introduce floating-point operation absolute errors
-        # round to two digits because we are dealing with money
+        new_amount = (Decimal(amount) / Decimal(source_rate)) * Decimal(dest_rate)
-        return round(new_amount, 2)
+        # dealing with money - only round the shown amount before showing it to user
+        # - think about 10 * 0.0003 == 0?
+        return float(new_amount)

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

@@ -447,6 +447,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" %}
+{% extends "sidebar_table_layout.html" %} {% block sidebar %}
-
+<div id="table_overflow">{{ balance_table(show_weight=False) }}</div>
-{% block sidebar %}
+{% endblock %} {% block content %}
-    <div id="table_overflow">
+<table id="bill_table" class="split_bills table table-striped">
-        {{ balance_table(show_weight=False) }}
+  <thead>
-    </div>
+    <tr>
-{% endblock %}
+      <th>{{ _("Who pays?") }}</th>
-
+      <th>{{ _("To whom?") }}</th>
-
+      <th>{{ _("How much?") }}</th>
-{% block content %}
+      <th>{{ _("Settled?") }}</th>
-    <table id="bill_table" class="split_bills table table-striped">
+    </tr>
-        <thead><tr><th>{{ _("Who pays?") }}</th><th>{{ _("To whom?") }}</th><th>{{ _("How much?") }}</th><th>{{ _("Settled?") }}</th></tr></thead>
+  </thead>
   <tbody>
-    {% for bill in bills %}
+    {% for transaction in transactions %}
-    <tr receiver={{bill.receiver.id}}>
+    <tr receiver="{{transaction.receiver.id}}">
-            <td>{{ bill.ower }}</td>
+      <td>{{ transaction.ower }}</td>
-            <td>{{ bill.receiver }}</td>
+      <td>{{ transaction.receiver }}</td>
-            <td>{{ bill.amount|currency }}</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">
+          <form class="" action="{{ url_for(".add_settlement_bill") }}" method="POST">
-                      <div data-toggle="tooltip" title='{{ _("Click here to record that the money transfer has been done") }}'>
+              {{ 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>
@@ -28,6 +44,6 @@
     </tr>
     {% endfor %}
   </tbody>
-    </table>
+</table>
 
 {% endblock %}

ihatemoney/tests/budget_test.py

@@ -1358,23 +1358,25 @@ class TestBudget(IhatemoneyTestCase):
         count = 0
         for t in transactions:
             count += 1
-            self.client.get(
+            self.client.post(
-                "/raclette/settle"
+                "/raclette/settle",
-                + "/"
+                data={
-                + str(t["amount"])
+                    "amount": t["amount"],
-                + "/"
+                    "sender_id": t["ower"].id,
-                + str(t["ower"].id)
+                    "receiver_id": t["receiver"].id,
-                + "/"
+                },
-                + str(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 +1465,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 +1630,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()
+    transactions = g.project.get_transactions_to_settle_bill()
-    return render_template("settle_bills.html", bills=bills, current_view="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>")
+@main.route("/<project_id>/settle", methods=["POST"])
-def settle(amount, ower_id, payer_id):
+def add_settlement_bill():
-    new_reinbursement = Bill(
+    """Create a bill to register a settlement"""
-        amount=float(amount),
+    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)],
+        owers=[Person.query.get(receiver_id, g.project)],
-        payer_id=ower_id,
+        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")