Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix #1080: "Deactivated users reappeared" #1104

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions ihatemoney/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,22 @@ def pay_each_default(self, amount):
else:
return 0

@property
def involves_deactivated_members(self):
"""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.activated.is_(False))
.count()
)
return deactivated_member_number != 0

def __str__(self):
return self.what

Expand Down
16 changes: 14 additions & 2 deletions ihatemoney/templates/list_bills.html
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,22 @@ <h3 class="modal-title">{{ _('Add a bill') }}</h3>
</span>
</td>
<td class="bill-actions">
<a class="edit" href="{{ url_for(".edit_bill", bill_id=bill.id) }}" title="{{ _("edit") }}">{{ _('edit') }}</a>
<a class="edit" href="{{ url_for(".edit_bill", bill_id=bill.id) }}" data-toggle="tooltip"
{% if bill.involves_deactivated_members %}
title="Cannot be edited as deactivated members involved"
{% else %}
title="Click to edit this bill"
{% endif %}
>{{ _('edit') }}</a>
<form action="{{ url_for(".delete_bill", bill_id=bill.id) }}" method="POST">
{{ csrf_form.csrf_token }}
<button class="action delete" type="submit" title="{{ _("delete") }}"></button>
<button class="action delete" type="submit" data-toggle="tooltip"
{% if bill.involves_deactivated_members %}
title="Cannot be deleted as deactivated members involved"
{% else %}
title="Click to delete this bill"
{% endif %}
></button>
</form>
{% if bill.external_link %}
<a class="show" href="{{ bill.external_link }}" ref="noopener" target="_blank" title="{{ _("show") }}">{{ _('show') }} </a>
Expand Down
116 changes: 116 additions & 0 deletions ihatemoney/tests/budget_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,122 @@ def test_weighted_balance(self):
balance = self.get_project("raclette").balance
self.assertEqual(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")

Expand Down
8 changes: 8 additions & 0 deletions ihatemoney/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,10 @@ def delete_bill(bill_id):
if not bill:
return redirect(url_for(".list_bills"))

# Check if the bill contains deactivated member. If yes, stop deleting.
if bill.involves_deactivated_members:
return redirect(url_for(".list_bills"))

db.session.delete(bill)
db.session.commit()
flash(_("The bill has been deleted"))
Expand All @@ -774,6 +778,10 @@ def edit_bill(bill_id):
if not bill:
raise NotFound()

# Check if the bill contains deactivated member. If yes, stop editing.
if bill.involves_deactivated_members:
return redirect(url_for(".list_bills"))

form = get_billform_for(g.project, set_default=False)

if request.method == "POST" and form.validate():
Expand Down