Skip to content

MPT-18065 Fix backward incompatible issues related to RQL: Support of property comparison#210

Open
jentyk wants to merge 1 commit intomainfrom
fix/MPT-18028
Open

MPT-18065 Fix backward incompatible issues related to RQL: Support of property comparison#210
jentyk wants to merge 1 commit intomainfrom
fix/MPT-18028

Conversation

@jentyk
Copy link
Contributor

@jentyk jentyk commented Feb 13, 2026

Closes MPT-18065

  • Add public helper quote_rql_value(value: str) to escape single quotes and wrap RQL literal values in single quotes
  • Quote string-encoded values in parse_kwargs for single-token lookups, non-keyword operators, and COMP operators
  • Ensure RQLQuery._bin passes encoded COMP values through quote_rql_value when building expressions
  • Update examples and extensive tests to expect quoted string literals in RQL output (e.g., eq(status,'active'), eq(id,'ID'), numeric/boolean/date literals serialized as quoted strings)

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

Adds automatic single-quote quoting (with escaping) for RQL literal values and exposes a new helper quote_rql_value(). Query-building and parsing now emit quoted string literals; tests updated to expect single-quoted literals across RQL and HTTP-related tests.

Changes

Cohort / File(s) Summary
Core RQL Implementation
mpt_api_client/rql/query_builder.py
Added public helper quote_rql_value(value: str) -> str; updated parse_kwargs() and RQLQuery._bin() to quote/escape string-encoded values for comparisons and other operators.
HTTP URL / Query Tests
tests/unit/http/mixins/test_collection_mixin.py, tests/unit/http/test_base_service.py, tests/unit/http/test_query_state.py
Updated expected URL/query-string RQL filters to use quoted string literals (e.g., eq(status,'active')).
RQL Query Builder Tests
tests/unit/rql/query_builder/test_create_rql.py, tests/unit/rql/query_builder/test_multiple_ops.py, tests/unit/rql/query_builder/test_rql.py, tests/unit/rql/query_builder/test_rql_all_any.py, tests/unit/rql/query_builder/test_rql_dot_path.py, tests/unit/rql/query_builder/test_rql_from_str.py, tests/unit/rql/query_builder/test_rql_parse_kwargs.py
Adjusted expected string representations to enclose literal operands (strings, numbers, booleans, dates) in single quotes across various operators and nested expressions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Jira Issue Key In Title ✅ Passed The PR title contains the Jira issue key MPT-18065 in the correct MPT-XXXX format.
Test Coverage Required ✅ Passed The PR modifies code file mpt_api_client/rql/query_builder.py and includes 10 test file updates validating the changes.
Single Commit Required ✅ Passed The PR contains exactly one commit (455cf38: fix(rql): enforce single-quoted values for eq/ne/gt/ge/lt/le and align tests), which satisfies the single commit requirement.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@jentyk jentyk marked this pull request as ready for review February 16, 2026 13:57
@jentyk jentyk requested a review from a team as a code owner February 16, 2026 13:57
@jentyk jentyk requested review from d3rky and svazquezco February 16, 2026 13:57
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/unit/rql/query_builder/test_rql_dot_path.py (1)

33-34: Remove unused noqa directives.

Static analysis indicates that FBT003 is not enabled in the linting configuration, making these noqa comments unnecessary.

🧹 Suggested cleanup
-    assert str(result(True)) == f"{op}(asset.id,'true')"  # noqa: FBT003
-    assert str(result(False)) == f"{op}(asset.id,'false')"  # noqa: FBT003
+    assert str(result(True)) == f"{op}(asset.id,'true')"
+    assert str(result(False)) == f"{op}(asset.id,'false')"

@jentyk jentyk changed the title MPT-18028 Fix backward incompatible issues related to RQL: Support of property comparison MPT-18065 Fix backward incompatible issues related to RQL: Support of property comparison Feb 16, 2026
@jentyk jentyk marked this pull request as draft February 16, 2026 15:08
@jentyk jentyk marked this pull request as ready for review February 17, 2026 09:54
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mpt_api_client/rql/query_builder.py`:
- Line 18: The trailing noqa comment on the parse_kwargs function signature is
invalid/unused; edit the parse_kwargs definition (function name: parse_kwargs)
to remove the "# noqa: WPS231 C901" directive so linters (ruff/flake8) can apply
configured rules correctly, then run the linting suite to confirm no remaining
rule suppressions are needed or add specific, valid noqa codes only if genuinely
required and documented.

In `@tests/unit/rql/query_builder/test_rql_dot_path.py`:
- Around line 32-34: The two assertions in test_rql_dot_path.py that read assert
str(result(True)) == f"{op}(asset.id,'true')"  # noqa: FBT003 and assert
str(result(False)) == f"{op}(asset.id,'false')"  # noqa: FBT003 should have the
unused noqa directives removed; edit the test to drop the " # noqa: FBT003"
suffixes so the lines are simply the assertions using result and op, leaving
other lines untouched.

@sonarqubecloud
Copy link

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tests/unit/rql/query_builder/test_rql_dot_path.py`:
- Around line 28-34: Remove the unused per-line noqa suppressions in the test
function test_dotted_path_comp_bool_and_str: delete the "# noqa: FBT003"
comments on the two boolean assertions so the file no longer contains
ineffective FBT003 suppressions; locate this in the test where result =
getattr(RQLQuery().asset.id, op) and the subsequent assert str(result(True)) and
assert str(result(False)) lines and simply remove the trailing noqa tokens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments