-
Notifications
You must be signed in to change notification settings - Fork 6
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
WIP // Adjust PaymentPlan model to contain informations coming from TP and HS #4372
base: long-term/remove-tp
Are you sure you want to change the base?
Conversation
updates = [] | ||
with transaction.atomic(): | ||
payment: Payment | ||
for payment in payment_plan.eligible_payments: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for payment in payment_plan.payment_items.all()
with transaction.atomic(): | ||
payment: Payment | ||
for payment in payment_plan.eligible_payments: | ||
result = rule.execute( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rules should be also refactored to handle payment_plan instead of targeting right?
@@ -564,17 +653,43 @@ class Action(models.TextChoices): | |||
"self", null=True, blank=True, on_delete=models.CASCADE, related_name="follow_ups" | |||
) | |||
is_follow_up = models.BooleanField(default=False) | |||
excluded_ids = models.TextField(blank=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add comment that this is a 'targeting level' exclusion
@@ -610,6 +725,29 @@ def excluded_beneficiaries_ids(self) -> List[str]: | |||
) | |||
return beneficiaries_ids | |||
|
|||
@property | |||
def excluded_household_ids(self) -> List: | |||
# TODO: moved from TP, maybe remove because we have excluded_beneficiaries_ids() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have 2 exclusions, on targeting level and on payment plan level, let's rename these properties to avoid confusion
"program_cycle": "program_cycle", | ||
"targeting_criteria": "targeting_criteria", | ||
"sent_to_datahub": "internal_data__sent_to_datahub", | ||
"steficon_rule": "steficon_rule", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need 2 separate fields for steficon rules. One for targeting steficon rule (calculate vulnerabilities), second for payment plan rule (calculate entitlement)
AB#215785: Adjust PaymentPlan model to contain informations coming from TP and HS