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.

Sign up for GitHub

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

Claim Limit Remaining Budget Fix #445

Merged
merged 5 commits into from
Dec 17, 2024
+51 −11
Merged

Claim Limit Remaining Budget Fix #445

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
368c03d
Fix remaining budget calculation for managed opportunities
pxwxnvermx Dec 5, 2024
7223138
Add test for managed opportunities
pxwxnvermx Dec 5, 2024
87c9cad
Add org_pay to claimed budget
pxwxnvermx Dec 12, 2024
23d7092
Fix tests for opportunity stats
pxwxnvermx Dec 12, 2024
d2138ec
Add exhausted budget test for managed opportunities
pxwxnvermx Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion commcare_connect/opportunity/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,12 @@ def minimum_budget_per_visit(self):

@property
def remaining_budget(self) -> int:
return self.total_budget - self.claimed_budget
if not self.managed:
return self.total_budget - self.claimed_budget

org_pay = self.managedopportunity.org_pay_per_visit * self.allotted_visits
total_user_budget = self.total_budget - org_pay
return total_user_budget - self.claimed_budget

@property
def claimed_budget(self):
Expand Down
23 changes: 18 additions & 5 deletions commcare_connect/opportunity/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,37 @@ def test_learn_progress(opportunity: Opportunity):


@pytest.mark.django_db
@pytest.mark.parametrize("opportunity", [{}, {"opp_options": {"managed": True}}], indirect=True)
def test_opportunity_stats(opportunity: Opportunity, user: User):
payment_unit_sub = PaymentUnitFactory(
payment_unit_sub = PaymentUnitFactory.create(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this change do

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not change anything about how these mock objects are created. Adding the create resolved some of the type errors I was getting in my editor.

opportunity=opportunity, max_total=100, max_daily=10, amount=5, parent_payment_unit=None
)
payment_unit1 = PaymentUnitFactory(
payment_unit1 = PaymentUnitFactory.create(
opportunity=opportunity,
max_total=100,
max_daily=10,
amount=3,
parent_payment_unit=payment_unit_sub,
)
payment_unit2 = PaymentUnitFactory(
payment_unit2 = PaymentUnitFactory.create(
opportunity=opportunity, max_total=100, max_daily=10, amount=5, parent_payment_unit=None
)
assert set(list(opportunity.paymentunit_set.values_list("id", flat=True))) == {
payment_unit1.id,
payment_unit2.id,
payment_unit_sub.id,
}
payment_units = [payment_unit_sub, payment_unit1, payment_unit2]
budget_per_user = sum(pu.max_total * pu.amount for pu in payment_units)
org_pay = 0
if opportunity.managed:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we know if the opportunity is managed or not in the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we have that information available, this line on the top of this test function creates 1 case with a normal opportunity and another with a managed opportunity.
@pytest.mark.parametrize("opportunity", [{}, {"opp_options": {"managed": True}}], indirect=True)

org_pay = opportunity.managedopportunity.org_pay_per_visit
budget_per_user += sum(pu.max_total * org_pay for pu in payment_units)
opportunity.total_budget = budget_per_user * 3
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we modifying the opportunity in the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to update the opportunity budget based on the Payment Units created earlier in the test and the org_pay_per_visit in case of a managed opportunity.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like there should be a nicer way to do this, but seems fine


payment_units = [payment_unit1, payment_unit2, payment_unit_sub]
assert opportunity.budget_per_user == sum([p.amount * p.max_total for p in payment_units])
assert opportunity.number_of_users == opportunity.total_budget / opportunity.budget_per_user
assert opportunity.number_of_users == 3
assert opportunity.allotted_visits == sum([pu.max_total for pu in payment_units]) * opportunity.number_of_users
assert opportunity.max_visits_per_user_new == sum([pu.max_total for pu in payment_units])
assert opportunity.daily_max_visits_per_user_new == sum([pu.max_daily for pu in payment_units])
Expand All @@ -61,7 +69,12 @@ def test_opportunity_stats(opportunity: Opportunity, user: User):
ocl1 = OpportunityClaimLimitFactory(opportunity_claim=claim, payment_unit=payment_unit1)
ocl2 = OpportunityClaimLimitFactory(opportunity_claim=claim, payment_unit=payment_unit2)

opportunity.claimed_budget == (ocl1.max_visits * payment_unit1.amount) + (ocl2.max_visits * payment_unit2.amount)
assert opportunity.claimed_budget == (ocl1.max_visits * payment_unit1.amount) + (
ocl2.max_visits * payment_unit2.amount
)
assert opportunity.remaining_budget == opportunity.total_budget - opportunity.claimed_budget - (
opportunity.allotted_visits * org_pay
)


@pytest.mark.django_db
Expand Down
Loading