Skip to content

Preserve nested right-hand expressions in grouped filters - #327

Merged
phanak-sap merged 1 commit into
SAP:masterfrom
UmutAlisoglu:codex/nested-filter-regressions
Oct 29, 2026
Merged

phanak-sap merged 1 commit into
SAP:masterfrom
UmutAlisoglu:codex/nested-filter-regressions

Conversation

@UmutAlisoglu

Copy link
Copy Markdown
Contributor

Nested right-hand Q expressions lose conditions even when group=True is enabled. For example, Q(NameLast="Smith") & (Q(ID=1) | (Q(ID=3) & Q(NickName="Tim"))) currently keeps only NameLast and ID=1, dropping the second matching branch.

Follow-up to the additional AND/OR/parenthesis coverage suggested in #324. Recursively render right-hand subtrees in the opt-in grouped mode; keep the default legacy rendering unchanged. No change to the mutable Q operator API or its existing restriction on reusing an initialized left operand.

Four new parameterized cases cover alternating operators across multiple levels, nested OR/AND tails and three grouped filter calls. They check exact filter text, independently parsed row selections, full encoded URLs and executed mocked requests. Three nested regressions fail before the fix; all 18 grouping tests pass afterward.

Validation on Windows / Python 3.11.9:

  • Full suite: 318 passed, 93% coverage.
  • Pylint 10/10, Flake8 and diff checks pass.
  • No live OData service calls.

Signed off. AI assistance was used for implementation and regression tests.

@phanak-sap phanak-sap added the bug Something isn't working label Oct 29, 2026
@phanak-sap phanak-sap self-assigned this Oct 29, 2026
@phanak-sap

Copy link
Copy Markdown
Contributor

Hi @UmutAlisoglu thx for quick follow-up.

Yeah, exactly something like this I had in mind, to either resurface possible bug or at least have more stronger regression suite.

Comment thread tests/test_filter_grouping.py Resolved
Comment thread CHANGELOG.md Resolved

@phanak-sap phanak-sap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thx for PR, pls just small notes, overly it is OK to merge and will go in after requested changes.

Signed-off-by: Umut Alisoglu <umutalisoglu@gmail.com>
@UmutAlisoglu
UmutAlisoglu force-pushed the codex/nested-filter-regressions branch from 0377a49 to 24f752e Compare October 29, 2026 17:53

@phanak-sap phanak-sap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good, thx

@phanak-sap
phanak-sap merged commit 8e84a3e into SAP:master Oct 29, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants