Skip to content

Commit

Permalink
Correct payment_per_time field definition - see HEA-572
Browse files Browse the repository at this point in the history
payment_per_time was defined as a PositiveSmallIntegerField,
but some data contains values to large for a small integer,
and it is possible that for OtherCashIncome the payment_per_time
could be a non-integer. Therefore, change to a FloatField
with a validator to ensure the value is positive.
  • Loading branch information
rhunwicks committed Nov 22, 2024
1 parent c934b0a commit 67a8fca
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Generated by Django 5.1.1 on 2024-11-22 21:21

from django.db import migrations, models

import common.models


class Migration(migrations.Migration):

dependencies = [
("baseline", "0017_alter_livelihoodactivity_percentage_kcals"),
]

operations = [
migrations.AlterField(
model_name="livelihoodactivity",
name="price",
field=models.FloatField(
blank=True,
help_text="Price per unit",
null=True,
validators=[common.models.validate_positive],
verbose_name="Price",
),
),
migrations.AlterField(
model_name="othercashincome",
name="payment_per_time",
field=models.FloatField(
blank=True,
help_text="Amount of money received each time the labor is performed",
null=True,
validators=[common.models.validate_positive],
verbose_name="Payment per time",
),
),
migrations.AlterField(
model_name="paymentinkind",
name="payment_per_time",
field=models.FloatField(
blank=True,
help_text="Amount of item received each time the labor is performed",
null=True,
validators=[common.models.validate_positive],
verbose_name="Payment per time",
),
),
]
40 changes: 35 additions & 5 deletions apps/baseline/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,13 @@ class LivelihoodActivity(common_models.Model):
# but there are exceptions, such as MilkProduction, where there is also an amount used for ButterProduction
quantity_consumed = models.PositiveIntegerField(blank=True, null=True, verbose_name=_("Quantity Consumed"))

price = models.FloatField(blank=True, null=True, verbose_name=_("Price"), help_text=_("Price per unit"))
price = models.FloatField(
blank=True,
null=True,
validators=[common_models.validate_positive],
verbose_name=_("Price"),
help_text=_("Price per unit"),
)
# Can be calculated / validated as `quantity_sold * price` for livelihood strategies that involve the sale of
# a proportion of the household's own production.
income = models.FloatField(blank=True, null=True, help_text=_("Income"))
Expand Down Expand Up @@ -1584,8 +1590,12 @@ class PaymentInKind(LivelihoodActivity):
"""

# Production calculation/validation is `people_per_household * times_per_month * months_per_year`
payment_per_time = models.PositiveSmallIntegerField(
verbose_name=_("Payment per time"), help_text=_("Amount of item received each time the labor is performed")
payment_per_time = models.FloatField(
blank=True,
null=True, # Not required if people_per_household or times_per_month is null
validators=[common_models.validate_positive],
verbose_name=_("Payment per time"),
help_text=_("Amount of item received each time the labor is performed"),
)
people_per_household = models.PositiveSmallIntegerField(
verbose_name=_("People per household"), help_text=_("Number of household members who perform the labor")
Expand All @@ -1601,6 +1611,14 @@ class PaymentInKind(LivelihoodActivity):
help_text=_("Number of times in a year that the labor is performed"),
)

def clean(self):
# payment_per_time is only required if people_per_household and times_per_month are provided
if self.people_per_household and self.times_per_month and not self.payment_per_time:
raise ValidationError(
_("Payment per time is required if people per household and times per month are provided")
)
super().clean()

def validate_quantity_produced(self):
if (
self.quantity_produced
Expand Down Expand Up @@ -1711,8 +1729,12 @@ class OtherCashIncome(LivelihoodActivity):
# However, some other income (e.g. Remittances) just has a number of times per year and is not calculated from
# people_per_household, etc. Therefore those fields must be nullable, and we must store the total number of times
# per year as a separate field
payment_per_time = models.PositiveSmallIntegerField(
verbose_name=_("Payment per time"), help_text=_("Amount of money received each time the labor is performed")
payment_per_time = models.FloatField(
blank=True,
null=True, # Not required if people_per_household or times_per_month is null
validators=[common_models.validate_positive],
verbose_name=_("Payment per time"),
help_text=_("Amount of money received each time the labor is performed"),
)
people_per_household = models.PositiveSmallIntegerField(
verbose_name=_("People per household"),
Expand All @@ -1734,6 +1756,14 @@ class OtherCashIncome(LivelihoodActivity):
help_text=_("Number of times in a year that the income is received"),
)

def clean(self):
# payment_per_time is only required if people_per_household and times_per_month are provided
if self.people_per_household and self.times_per_month and not self.payment_per_time:
raise ValidationError(
_("Payment per time is required if people per household and times per month are provided")
)
super().clean()

def validate_income(self):
if (
self.people_per_household
Expand Down
11 changes: 11 additions & 0 deletions apps/common/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@
logger = logging.getLogger(__name__)


def validate_positive(value):
"""
Validator to ensure that a value is positive.
The normal models.validators.MinValueValidator(0) isn't suitable because it allows zero,
which isn't appropriate for some attributes, such as price.
"""
if value < 0:
raise ValidationError(_("Value must be greater than 0."))


class ShowQueryVariablesMixin(object):
"""
Mixin for models.Manager classes that shows the query arguments when a get() query fails
Expand Down
3 changes: 2 additions & 1 deletion pipelines/assets/livelihood_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,8 @@ def get_instances_from_dataframe(
# that is embedded in the ButterProduction calculations in current BSSs
livelihood_activity["type_of_milk_consumed"] = MilkProduction.MilkType.WHOLE

# Add the `times_per_year` to FoodPurchase, because it is not present in any current BSS
# Add the `times_per_year` to FoodPurchase, PaymentInKind and OtherCashIncome,
# because it is not in the current BSSs
if (
"times_per_month" in livelihood_strategy["attribute_rows"]
and "months_per_year" in livelihood_strategy["attribute_rows"]
Expand Down

0 comments on commit 67a8fca

Please sign in to comment.