diff --git a/conftest.py b/conftest.py index 4c7883647..c0d00f764 100644 --- a/conftest.py +++ b/conftest.py @@ -1777,15 +1777,15 @@ def percent_or_amount() -> DutyExpression: @pytest.fixture -def plus_percent_or_amount() -> DutyExpression: - return factories.DutyExpressionFactory( - sid=4, - prefix="+", - description=f"+ % or amount", - duty_amount_applicability_code=ApplicabilityCode.MANDATORY, - measurement_unit_applicability_code=ApplicabilityCode.PERMITTED, - monetary_unit_applicability_code=ApplicabilityCode.PERMITTED, - ) +def plus_percent_or_amount() -> list[DutyExpression]: + kwargs = { + "prefix": "+", + "description": f"+ % or amount", + "duty_amount_applicability_code": ApplicabilityCode.MANDATORY, + "measurement_unit_applicability_code": ApplicabilityCode.PERMITTED, + "monetary_unit_applicability_code": ApplicabilityCode.PERMITTED, + } + return [factories.DutyExpressionFactory(sid=sid, **kwargs) for sid in [4, 19, 20]] @pytest.fixture @@ -1800,6 +1800,18 @@ def plus_agri_component() -> DutyExpression: ) +@pytest.fixture +def maximum() -> list[DutyExpression]: + kwargs = { + "prefix": "MAX", + "description": "Maximum", + "duty_amount_applicability_code": ApplicabilityCode.MANDATORY, + "measurement_unit_applicability_code": ApplicabilityCode.PERMITTED, + "monetary_unit_applicability_code": ApplicabilityCode.PERMITTED, + } + return [factories.DutyExpressionFactory(sid=sid, **kwargs) for sid in [17, 35]] + + @pytest.fixture def plus_amount_only() -> DutyExpression: return factories.DutyExpressionFactory( @@ -1838,39 +1850,26 @@ def supplementary_unit() -> DutyExpression: @pytest.fixture def duty_expressions( - percent_or_amount: DutyExpression, - plus_percent_or_amount: DutyExpression, - plus_agri_component: DutyExpression, - plus_amount_only: DutyExpression, - supplementary_unit: DutyExpression, - nothing: DutyExpression, + duty_expressions_list: list[DutyExpression], ) -> Dict[int, DutyExpression]: - return { - d.sid: d - for d in [ - percent_or_amount, - plus_percent_or_amount, - plus_agri_component, - plus_amount_only, - supplementary_unit, - nothing, - ] - } + return {d.sid: d for d in duty_expressions_list} @pytest.fixture def duty_expressions_list( percent_or_amount: DutyExpression, - plus_percent_or_amount: DutyExpression, + plus_percent_or_amount: list[DutyExpression], plus_agri_component: DutyExpression, + maximum: list[DutyExpression], plus_amount_only: DutyExpression, supplementary_unit: DutyExpression, nothing: DutyExpression, ) -> Sequence[DutyExpression]: return [ percent_or_amount, - plus_percent_or_amount, + *plus_percent_or_amount, plus_agri_component, + *maximum, plus_amount_only, supplementary_unit, nothing, diff --git a/measures/duty_sentence_parser.py b/measures/duty_sentence_parser.py index c5445eb34..2b1b46954 100644 --- a/measures/duty_sentence_parser.py +++ b/measures/duty_sentence_parser.py @@ -8,6 +8,7 @@ from lark import Transformer from lark import UnexpectedInput +from common.models import TrackedModel from measures import models from measures.validators import ApplicabilityCode @@ -261,8 +262,8 @@ def validate_duty_expressions(self, transformed): .exclude(prefix__isnull=True) .order_by("sid") ) - duty_expression_sids = [d.sid for d in duty_expressions] + matched_sids = set() supplementary_unit = models.DutyExpression.objects.as_at(self.date).get(sid=99) if len(transformed) == 1: @@ -287,7 +288,8 @@ def validate_duty_expressions(self, transformed): .order_by("sid") .first() ) - if match is None: + + if not self.is_valid_match(match, supplementary_unit, phrase): potential_match = ( models.DutyExpression.objects.as_at(self.date) .filter( @@ -296,25 +298,57 @@ def validate_duty_expressions(self, transformed): .order_by("sid") .first() ) + + if potential_match.sid in matched_sids: + raise ValidationError( + f"A duty expression cannot be used more than once in a duty sentence. Matching expression: {potential_match.description} ({potential_match.prefix})", + ) + raise ValidationError( - f"A duty expression cannot be used more than once in a duty sentence. Matching expression: {potential_match.description} ({potential_match.prefix})", + f"Duty expressions must be used in the duty sentence in ascending order of SID. Matching expression: {potential_match.description} ({potential_match.prefix}).", ) - # Each duty expression can only be used once in a sentence and in order of increasing - # SID so once we have a match, remove it from the list of duty expression sids - duty_expression_sids.remove(match.sid) + # Each duty expression can only be used once in a sentence and in ascending SID order + matched_sids.add(match.sid) + duty_expression_sids[:] = [ + sid for sid in duty_expression_sids if sid > match.sid + ] # Update with the matching DutyExpression we found phrase["duty_expression"] = match transformed_duty_expression_sids = [ phrase["duty_expression"].sid for phrase in transformed ] - if transformed_duty_expression_sids != sorted(transformed_duty_expression_sids): raise ValidationError( "Duty expressions must be used in the duty sentence in ascending order of SID.", ) + def is_valid_match( + self, + match: models.DutyExpression | None, + supplementary_unit: models.DutyExpression, + phrase: dict, + ) -> bool: + """ + Checks whether a match has been found and is valid, returning `True` if + so and `False` if not. + + A duty amount may be mistakenly matched by prefix as a supplementary unit if all possible duty amount expression SIDs have already been applied, + in which case `False` is returned. + """ + if match is None: + return False + + if ( + match == supplementary_unit + and "duty_amount" in phrase + and "measurement_unit" not in phrase + ): + return False + + return True + @staticmethod def validate_according_to_applicability_code( code, @@ -327,7 +361,7 @@ def validate_according_to_applicability_code( f"Duty expression {duty_expression.description} ({duty_expression.prefix}) requires a {item_name}.", ) if code == ApplicabilityCode.NOT_PERMITTED and item: - if isinstance(item, object): + if isinstance(item, TrackedModel): message = f"{item_name.capitalize()} {item.abbreviation} ({item.code}) cannot be used with duty expression {duty_expression.description} ({duty_expression.prefix})." else: message = f"{item_name.capitalize()} cannot be used with duty expression {duty_expression.description} ({duty_expression.prefix})." diff --git a/measures/tests/test_lark_parser.py b/measures/tests/test_lark_parser.py index ec662082c..c4a948252 100644 --- a/measures/tests/test_lark_parser.py +++ b/measures/tests/test_lark_parser.py @@ -13,13 +13,18 @@ from measures.duty_sentence_parser import InvalidMeasurementUnit from measures.duty_sentence_parser import InvalidMeasurementUnitQualififer from measures.duty_sentence_parser import InvalidMonetaryUnit +from measures.models import DutyExpression +from measures.validators import ApplicabilityCode pytestmark = pytest.mark.django_db PERCENT_OR_AMOUNT_FIXTURE_NAME = "percent_or_amount" -PLUS_PERCENT_OR_AMOUNT_FIXTURE_NAME = "plus_percent_or_amount" +PLUS_PERCENT_OR_AMOUNT_SID = (4, 19, 20) SUPPLEMENTARY_UNIT_FIXTURE_NAME = "supplementary_unit" PLUS_AGRI_COMPONENT_FIXTURE_NAME = "plus_agri_component" +MAXIMUM_SID = (17, 35) +NOTHING_FIXTURE_NAME = "nothing" +BRITISH_POUND_FIXTURE_NAME = "british_pound" EURO_FIXTURE_NAME = "euro" KILOGRAM_FIXTURE_NAME = "kilogram" THOUSAND_ITEMS_FIXTURE_NAME = "thousand_items" @@ -47,6 +52,26 @@ }, ], ), + ( + "9.10% MAX 1.00% + 0.90 GBP / 100 kg", + ["9.10", "%", "MAX", "1.00", "%", "+", "0.90", "GBP", "100 kg"], + [ + { + "duty_amount": 9.1, + "duty_expression": PERCENT_OR_AMOUNT_FIXTURE_NAME, + }, + { + "duty_expression_sid": MAXIMUM_SID[0], + "duty_amount": 1.0, + }, + { + "duty_expression_sid": PLUS_PERCENT_OR_AMOUNT_SID[1], + "duty_amount": 0.9, + "monetary_unit": BRITISH_POUND_FIXTURE_NAME, + "measurement_unit": HECTOKILOGRAM_FIXTURE_NAME, + }, + ], + ), ( "0.300 XEM / 100 kg / lactic.", ["0.300", "XEM", "100 kg", "lactic."], @@ -70,7 +95,7 @@ }, { "duty_amount": 20.0, - "duty_expression": PLUS_PERCENT_OR_AMOUNT_FIXTURE_NAME, + "duty_expression_sid": PLUS_PERCENT_OR_AMOUNT_SID[0], "measurement_unit": KILOGRAM_FIXTURE_NAME, "monetary_unit": EURO_FIXTURE_NAME, }, @@ -119,6 +144,7 @@ ids=[ "simple_ad_valorem", "simple_specific_duty", + "max_compound_duty", "unit_with_qualifier", "multi_component_expression", "supplementary_unit", @@ -146,7 +172,7 @@ def reversible_duty_sentence_data(request): }, { "duty_amount": 10.0, - "duty_expression": PLUS_PERCENT_OR_AMOUNT_FIXTURE_NAME, + "duty_expression_sid": PLUS_PERCENT_OR_AMOUNT_SID[0], "measurement_unit": HECTOKILOGRAM_FIXTURE_NAME, "monetary_unit": EURO_FIXTURE_NAME, }, @@ -191,6 +217,28 @@ def irreversible_duty_sentence_data(request): return duty_sentence, exp_parsed, exp_transformed +def replace_with_component_instances(expected_components: dict, request) -> None: + """ + Modifies `expected_components` so that it can be compared to a dictionary of + parsed and transformed duty components. + + Duty component fixture names and duty expression SID values are replaced + with their object instances. + """ + if "duty_expression_sid" in expected_components: + expected_components["duty_expression"] = DutyExpression.objects.get( + sid=expected_components["duty_expression_sid"], + ) + expected_components.pop("duty_expression_sid") + + with_fixtures = { + key: request.getfixturevalue(val) + for key, val in expected_components.items() + if type(val) == str + } + expected_components.update(with_fixtures) + + def duty_sentence_parser_test( lark_duty_sentence_parser: DutySentenceParser, duty_sentence_data: Tuple[str, List[str], List[Dict]], @@ -207,12 +255,7 @@ def duty_sentence_parser_test( transformed = lark_duty_sentence_parser.transform(duty_sentence) assert len(transformed) == len(exp_components) for phrase, exp in zip(transformed, exp_components): - with_fixtures = { - key: request.getfixturevalue(val) - for key, val in exp.items() - if type(val) == str - } - exp.update(with_fixtures) + replace_with_component_instances(exp, request) assert phrase == exp @@ -243,10 +286,10 @@ def test_irreversible_duty_sentence_parsing( def test_only_permitted_measurements_allowed(lark_duty_sentence_parser): with pytest.raises(ValidationError) as e: lark_duty_sentence_parser.transform("1.0 EUR / kg / lactic.") - assert ( - e.message - == "Measurement unit qualifier lactic. cannot be used with measurement unit kg." - ) + assert ( + "Measurement unit qualifier lactic. cannot be used with measurement unit kg." + in str(e.value) + ) @pytest.mark.parametrize( @@ -296,25 +339,67 @@ def test_compound_duty_not_permitted_error(sentence, simple_lark_duty_sentence_p "A duty expression cannot be used more than once in a duty sentence.", ), ( - f"+ 5.5% 10%", + f"+ AC 10%", "Duty expressions must be used in the duty sentence in ascending order of SID.", ), - ( - "+ AC 10%", - f"Duty amount cannot be used with duty expression + agricultural component (+ AC).", - ), ( "NIHIL / 100 kg", - f"Measurement unit 100 kg (KGM) cannot be used with duty expression (nothing) (NIHIL).", + f"Measurement unit 100 kg (DTN) cannot be used with duty expression (nothing) (NIHIL).", ), ], ) -def test_duty_validation_errors(sentence, exp_error_message, lark_duty_sentence_parser): - """ - Tests validation based on applicability codes. - - See conftest.py for DutyExpression fixture details. - """ - with pytest.raises(ValidationError) as e: +def test_duty_transformer_duty_expression_validation_errors( + sentence, + exp_error_message, + lark_duty_sentence_parser, +): + with pytest.raises(ValidationError) as error: lark_duty_sentence_parser.transform(sentence) - assert exp_error_message in e.message + assert exp_error_message in str(error.value) + + +@pytest.mark.parametrize( + "code, duty_expression, item, item_name, error_message", + [ + ( + ApplicabilityCode.NOT_PERMITTED, + PLUS_AGRI_COMPONENT_FIXTURE_NAME, + 10.0, + "duty amount", + "Duty amount cannot be used with duty expression + agricultural component (+ AC).", + ), + ( + ApplicabilityCode.NOT_PERMITTED, + NOTHING_FIXTURE_NAME, + BRITISH_POUND_FIXTURE_NAME, + "monetary unit", + "Monetary unit cannot be used with duty expression (nothing) (NIHIL).", + ), + ( + ApplicabilityCode.MANDATORY, + PERCENT_OR_AMOUNT_FIXTURE_NAME, + None, + "duty amount", + f"Duty expression % or amount () requires a duty amount.", + ), + ], +) +def test_duty_transformer_applicability_code_validation_errors( + code, + duty_expression, + item, + item_name, + error_message, + lark_duty_sentence_parser, + request, +): + with pytest.raises(ValidationError) as error: + transformer = lark_duty_sentence_parser.transformer + duty_expression = request.getfixturevalue(duty_expression) + transformer.validate_according_to_applicability_code( + code, + duty_expression, + item, + item_name, + ) + assert error_message in str(error.value) diff --git a/measures/tests/test_util.py b/measures/tests/test_util.py index f1eaef53c..147851ad8 100644 --- a/measures/tests/test_util.py +++ b/measures/tests/test_util.py @@ -70,7 +70,7 @@ def test_diff_components_update_multiple( component_2 = factories.MeasureComponentFactory.create( component_measure=component_1.component_measure, duty_amount=253.000, - duty_expression=plus_percent_or_amount, + duty_expression=plus_percent_or_amount[0], monetary_unit=monetary_units["GBP"], component_measurement__measurement_unit=measurement_units[1], )