Skip to content

Commit c73121b

Browse files
fix(workflows): unwrap groups in the remediation parser too
Addressing review feedback. The evaluator now unwraps a parenthesised group, but _unresolvable_term did not, so a grouped operand reached the path check as literal text. A condition the evaluator resolves was therefore reported unresolvable and format_condition_remediation withheld the wrap correction from it: inputs.a or inputs.b -> Wrap the expression: ... (inputs.a or inputs.b) and inputs.c -> No correction is offered because '(inputs.a or inputs.b)' is not a name ... Mirror the unwrap branch, and cover grouped bare conditions both ways: a valid group keeps the correction, and an unresolvable name inside a group still refuses it.
1 parent d376a22 commit c73121b

2 files changed

Lines changed: 37 additions & 0 deletions

File tree

‎src/specify_cli/workflows/expressions.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,6 +1179,13 @@ def _unresolvable_term(text: str) -> str | None:
11791179
if not stripped:
11801180
return "an operand is empty"
11811181

1182+
# Mirror the evaluator's group unwrapping. Without this a grouped operand
1183+
# reached the path check as literal text, so `(inputs.a or inputs.b) and
1184+
# inputs.c` -- which the evaluator resolves -- was reported unresolvable and
1185+
# the wrap correction was withheld from a condition that would have worked.
1186+
if _is_wrapped_in_parens(stripped):
1187+
return _unresolvable_term(stripped[1:-1])
1188+
11821189
if _find_top_level(stripped, "|") != -1:
11831190
segments = _split_top_level(stripped, "|")
11841191
reason = _unresolvable_term(segments[0])

‎tests/unit/test_condition_expression_block.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,36 @@ def test_resolvable_filter_arguments_keep_the_correction(condition):
804804
assert _wrapped_evaluates(condition)
805805

806806

807+
@pytest.mark.parametrize(
808+
"condition",
809+
[
810+
"(inputs.a or inputs.b) and inputs.c",
811+
"(inputs.a)",
812+
"((inputs.a))",
813+
"(inputs.a) and (inputs.c)",
814+
],
815+
)
816+
def test_a_grouped_bare_condition_keeps_the_correction(condition):
817+
"""Remediation tracks the evaluator, which now unwraps a parenthesised group.
818+
819+
Before the evaluator learned to unwrap, a grouped operand reached the path
820+
check as literal text, so a condition that wrapping would have fixed was
821+
reported unresolvable and the correction was withheld.
822+
"""
823+
ctx = StepContext(inputs={"a": True, "b": False, "c": True})
824+
assert CORRECTION_OFFERED in format_condition_remediation(condition)
825+
assert evaluate_condition("{{ " + condition + " }}", ctx) is True
826+
827+
828+
@pytest.mark.parametrize(
829+
"condition",
830+
["(bogus or inputs.b) and inputs.c", "(inputs.a or bogus)"],
831+
)
832+
def test_an_unresolvable_name_inside_a_group_still_refuses(condition):
833+
"""The unwrap must not become a blanket pass for anything parenthesised."""
834+
assert CORRECTION_OFFERED not in format_condition_remediation(condition)
835+
836+
807837
@pytest.mark.parametrize("condition", ["item[0] == 'x'", "item[1] == 'y'"])
808838
def test_an_indexed_item_root_keeps_the_correction(condition):
809839
"""`item` is the only root that is not always a mapping.

0 commit comments

Comments
 (0)