From d6d9fd2180a280645b2da64d5b3376b0eb8f27ce Mon Sep 17 00:00:00 2001 From: chestnutFan Date: Sat, 10 Dec 2022 16:57:17 -0500 Subject: [PATCH 1/3] adding deactivated user check and according test function --- ihatemoney/tests/budget_test.py | 116 ++++++++++++++++++++++++++++++++ ihatemoney/web.py | 26 +++++++ 2 files changed, 142 insertions(+) diff --git a/ihatemoney/tests/budget_test.py b/ihatemoney/tests/budget_test.py index 093d9d17..e0e56afd 100644 --- a/ihatemoney/tests/budget_test.py +++ b/ihatemoney/tests/budget_test.py @@ -870,6 +870,122 @@ class TestBudget(IhatemoneyTestCase): balance = self.get_project("raclette").balance assert set(balance.values()) == set([6, -6]) + def test_edit_bill_with_deactivated_member(self): + """ + Bills involving deactivated members should not allowed to be edited or deleted. + """ + self.post_project("raclette") + + # add two participants + self.client.post("/raclette/members/add", data={"name": "zorglub"}) + self.client.post("/raclette/members/add", data={"name": "fred"}) + + members_ids = [m.id for m in self.get_project("raclette").members] + + # create one bill + self.client.post( + "/raclette/add", + data={ + "date": "2011-08-10", + "what": "fromage à raclette", + "payer": members_ids[0], + "payed_for": members_ids, + "amount": "25", + }, + ) + bill = models.Bill.query.one() + self.assertEqual(bill.amount, 25) + + # deactivate one user + self.client.post( + "/raclette/members/%s/delete" % self.get_project("raclette").members[-1].id + ) + self.assertEqual(len(self.get_project("raclette").members), 2) + self.assertEqual(len(self.get_project("raclette").active_members), 1) + + # editing would fail because the bill involves deactivated user + self.client.post( + f"/raclette/edit/{bill.id}", + data={ + "date": "2011-08-10", + "what": "fromage à raclette", + "payer": members_ids[0], + "payed_for": members_ids, + "amount": "10", + }, + ) + bill = models.Bill.query.one() + self.assertNotEqual(bill.amount, 10, "bill edition") + + # reactivate the user + self.client.post( + "/raclette/members/%s/reactivate" + % self.get_project("raclette").members[-1].id + ) + self.assertEqual(len(self.get_project("raclette").active_members), 2) + + # try to edit the bill again. It should succeed + self.client.post( + f"/raclette/edit/{bill.id}", + data={ + "date": "2011-08-10", + "what": "fromage à raclette", + "payer": members_ids[0], + "payed_for": members_ids, + "amount": "10", + }, + ) + bill = models.Bill.query.one() + self.assertEqual(bill.amount, 10, "bill edition") + + def test_delete_bill_with_deactivated_member(self): + """ + Bills involving deactivated members should not allowed to be edited or deleted. + """ + self.post_project("raclette") + + # add two participants + self.client.post("/raclette/members/add", data={"name": "zorglub"}) + self.client.post("/raclette/members/add", data={"name": "fred"}) + + members_ids = [m.id for m in self.get_project("raclette").members] + + # create one bill + self.client.post( + "/raclette/add", + data={ + "date": "2011-08-10", + "what": "fromage à raclette", + "payer": members_ids[0], + "payed_for": members_ids, + "amount": "25", + }, + ) + bill = models.Bill.query.one() + self.assertEqual(bill.amount, 25) + + # deactivate one user + self.client.post( + "/raclette/members/%s/delete" % self.get_project("raclette").members[-1].id + ) + self.assertEqual(len(self.get_project("raclette").active_members), 1) + + # deleting should fail because the bill involves deactivated user + response = self.client.get(f"/raclette/delete/{bill.id}") + self.assertEqual(response.status_code, 405) + self.assertEqual(1, len(models.Bill.query.all()), "bill deletion") + + # reactivate the user + self.client.post( + "/raclette/members/%s/reactivate" + % self.get_project("raclette").members[-1].id + ) + self.assertEqual(len(self.get_project("raclette").active_members), 2) + + # try to delete the bill again. It should succeed + self.client.post(f"/raclette/delete/{bill.id}") + self.assertEqual(0, len(models.Bill.query.all()), "bill deletion") + def test_trimmed_members(self): self.post_project("raclette") diff --git a/ihatemoney/web.py b/ihatemoney/web.py index 0d0bdd20..a3f44389 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -800,6 +800,19 @@ def delete_bill(bill_id): if not bill: return redirect(url_for(".list_bills")) + # Prevent deleting if the bill involves deactivated user. + active_member_id = [m.id for m in g.project.active_members] + owers_id = [int(m.id) for m in bill.owers] + deactivated_members = set([bill.payer_id] + owers_id).difference( + set(active_member_id) + ) + if len(deactivated_members): + flash( + _("Deactivated users involved. This bill cannot be deleted."), + category="warning", + ) + return redirect(url_for(".list_bills")) + db.session.delete(bill) db.session.commit() flash(_("The bill has been deleted")) @@ -816,6 +829,19 @@ def edit_bill(bill_id): form = get_billform_for(g.project, set_default=False) + # Prevent editing if the bill involves deactivated user. + active_member_id = [m.id for m in g.project.active_members] + owers_id = [int(m.id) for m in bill.owers] + deactivated_members = set([bill.payer_id] + owers_id).difference( + set(active_member_id) + ) + if len(deactivated_members): + flash( + _("Deactivated users involved. This bill cannot be edited."), + category="warning", + ) + return redirect(url_for(".list_bills")) + if request.method == "POST" and form.validate(): form.save(bill, g.project) db.session.commit() From 72320c19d7ab74040a8f1895dbfedf7598cd84a1 Mon Sep 17 00:00:00 2001 From: chestnutFan Date: Mon, 12 Dec 2022 00:11:12 -0500 Subject: [PATCH 2/3] Moved the check function to the model --- ihatemoney/models.py | 16 ++++++++++++++++ ihatemoney/web.py | 22 ++++++---------------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/ihatemoney/models.py b/ihatemoney/models.py index c591b85b..313090fc 100644 --- a/ihatemoney/models.py +++ b/ihatemoney/models.py @@ -752,6 +752,22 @@ class Bill(db.Model): else: return 0 + def involves_deactivated_members(self, project): + """Check whether the bill contains deactivated member. + Return: + True if it contains deactivated member, + False if not. + """ + owers_id = [int(m.id) for m in self.owers] + bill_member_id_list = owers_id + [self.payer_id] + deactivated_member_number = ( + Person.query.filter(Person.id.in_(bill_member_id_list)) + .filter(Person.project_id == project.id) + .filter(Person.activated == False) + .count() + ) + return deactivated_member_number != 0 + def __str__(self): return self.what diff --git a/ihatemoney/web.py b/ihatemoney/web.py index a3f44389..58b0580f 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -800,13 +800,8 @@ def delete_bill(bill_id): if not bill: return redirect(url_for(".list_bills")) - # Prevent deleting if the bill involves deactivated user. - active_member_id = [m.id for m in g.project.active_members] - owers_id = [int(m.id) for m in bill.owers] - deactivated_members = set([bill.payer_id] + owers_id).difference( - set(active_member_id) - ) - if len(deactivated_members): + # Check if the bill contains deactivated member. If yes, stop deleting. + if bill.involves_deactivated_members(g.project): flash( _("Deactivated users involved. This bill cannot be deleted."), category="warning", @@ -827,21 +822,16 @@ def edit_bill(bill_id): if not bill: raise NotFound() - form = get_billform_for(g.project, set_default=False) - - # Prevent editing if the bill involves deactivated user. - active_member_id = [m.id for m in g.project.active_members] - owers_id = [int(m.id) for m in bill.owers] - deactivated_members = set([bill.payer_id] + owers_id).difference( - set(active_member_id) - ) - if len(deactivated_members): + # Check if the bill contains deactivated member. If yes, stop editing. + if bill.involves_deactivated_members(g.project): flash( _("Deactivated users involved. This bill cannot be edited."), category="warning", ) return redirect(url_for(".list_bills")) + form = get_billform_for(g.project, set_default=False) + if request.method == "POST" and form.validate(): form.save(bill, g.project) db.session.commit() From 889576a114068b053c45a46e00b74e48e1856492 Mon Sep 17 00:00:00 2001 From: chestnutFan Date: Mon, 12 Dec 2022 21:14:28 -0500 Subject: [PATCH 3/3] -Added tooltip for bills involving deactivated members -Made involves_deactivated_members() a property of the Bill class -Removed flashed messages --- ihatemoney/models.py | 6 +++--- ihatemoney/templates/list_bills.html | 16 ++++++++++++++-- ihatemoney/web.py | 12 ++---------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/ihatemoney/models.py b/ihatemoney/models.py index 313090fc..af11da28 100644 --- a/ihatemoney/models.py +++ b/ihatemoney/models.py @@ -752,7 +752,8 @@ class Bill(db.Model): else: return 0 - def involves_deactivated_members(self, project): + @property + def involves_deactivated_members(self): """Check whether the bill contains deactivated member. Return: True if it contains deactivated member, @@ -762,8 +763,7 @@ class Bill(db.Model): bill_member_id_list = owers_id + [self.payer_id] deactivated_member_number = ( Person.query.filter(Person.id.in_(bill_member_id_list)) - .filter(Person.project_id == project.id) - .filter(Person.activated == False) + .filter(Person.activated.is_(False)) .count() ) return deactivated_member_number != 0 diff --git a/ihatemoney/templates/list_bills.html b/ihatemoney/templates/list_bills.html index 79e25262..7f4350e8 100644 --- a/ihatemoney/templates/list_bills.html +++ b/ihatemoney/templates/list_bills.html @@ -148,10 +148,22 @@ - {{ _('edit') }} + {{ _('edit') }}
{{ csrf_form.csrf_token }} - +
{% if bill.external_link %} {{ _('show') }} diff --git a/ihatemoney/web.py b/ihatemoney/web.py index 58b0580f..d3706e40 100644 --- a/ihatemoney/web.py +++ b/ihatemoney/web.py @@ -801,11 +801,7 @@ def delete_bill(bill_id): return redirect(url_for(".list_bills")) # Check if the bill contains deactivated member. If yes, stop deleting. - if bill.involves_deactivated_members(g.project): - flash( - _("Deactivated users involved. This bill cannot be deleted."), - category="warning", - ) + if bill.involves_deactivated_members: return redirect(url_for(".list_bills")) db.session.delete(bill) @@ -823,11 +819,7 @@ def edit_bill(bill_id): raise NotFound() # Check if the bill contains deactivated member. If yes, stop editing. - if bill.involves_deactivated_members(g.project): - flash( - _("Deactivated users involved. This bill cannot be edited."), - category="warning", - ) + if bill.involves_deactivated_members: return redirect(url_for(".list_bills")) form = get_billform_for(g.project, set_default=False)