[MPT-19073] Updated render options to work with all necessary http functions

[robcsegal]

This pull request introduces a new QueryOptions class to centralize and extend query option handling (such as the render() parameter) across the HTTP client, resource accessor, and query mixin layers. The change replaces the previous use of a simple render boolean with a more extensible options object, allowing for cleaner and more maintainable code as more query options are added in the future. The codebase is refactored to pass QueryOptions throughout the request lifecycle and to generate query strings accordingly.

Key changes in this pull request:

Query Options Infrastructure

• Introduced the QueryOptions class (a NamedTuple with a render flag) in mpt_api_client/http/query_options.py to encapsulate query-related options.
• Updated QueryState to accept and store a QueryOptions object instead of a raw render boolean, and refactored its usage and property accessors accordingly. [1] [2] [3] [4]

Client and Resource Accessor Refactoring

• Modified HTTP client (client.py, async_client.py) and resource accessor methods to accept and propagate the new options parameter of type QueryOptions, ensuring options flow through all request layers. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17]

Query String Generation

• Added the get_query_params utility in client_utils.py to build query strings from both parameters and QueryOptions, including logic for appending the render() function when requested. [1] [2]

Mixins and Queryable API

• Refactored all mixins (e.g., GetMixin, QueryableMixin) to use and propagate QueryOptions instead of the previous render flag, including in method signatures and calls to resource accessors. [1] [2] [3] [4] [5] [6] [7]

Closes MPT-19073

Release Notes

• Added new QueryOptions dataclass to encapsulate query rendering behavior with a configurable render boolean flag
• Refactored QueryState to accept and store QueryOptions instead of a bare render boolean parameter
• Updated HTTP client methods (HTTPClient.request() and AsyncHTTPClient.request()) to accept an optional options parameter and forward it through the request lifecycle
• Introduced get_query_params() utility function to build URL-encoded query strings from parameters and QueryOptions, conditionally appending render() when requested
• Updated resource accessor methods (ResourceAccessor.get(), AsyncResourceAccessor.get(), and related internal methods) to accept and propagate QueryOptions through the request chain
• Refactored mixins (GetMixin, QueryableMixin) to propagate QueryOptions instead of standalone render flags
• Added comprehensive unit test coverage for QueryOptions, get_query_params(), and query rendering behavior in HTTP clients and resource accessors
• Added end-to-end tests validating render and select query options with audit records in both sync and async contexts

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request introduces a new QueryOptions dataclass to encapsulate query rendering behavior and refactors the HTTP client stack to propagate options through request handling. A new get_query_params() utility function serializes query parameters and conditionally appends render() directives. The changes span from HTTP clients through resource accessors and mixins, with comprehensive unit and e2e test coverage.

Changes

Cohort / File(s)	Summary
Query Options & State Foundation mpt_api_client/http/query_options.py, mpt_api_client/http/query_state.py	New QueryOptions dataclass with render: bool field; QueryState refactored to accept options: QueryOptions | None instead of direct render parameter, storing a default instance.
Query Parameter Serialization mpt_api_client/http/client_utils.py	New get_query_params() function filters None values from query params, URL-encodes remaining pairs, and conditionally appends render() based on options.
HTTP Client Layer mpt_api_client/http/async_client.py, mpt_api_client/http/client.py	Both HTTP clients updated to accept options: QueryOptions | None parameter and use get_query_params() for query string construction instead of direct param passing.
Resource Accessor Layer mpt_api_client/http/resource_accessor.py	ResourceAccessor and AsyncResourceAccessor methods (do_request(), get(), _action()) updated with options parameter; propagates options to HTTP client layer.
Mixin Layer mpt_api_client/http/mixins/queryable_mixin.py, mpt_api_client/http/mixins/get_mixin.py	QueryableMixin wraps render argument into QueryOptions and passes to QueryState; GetMixin forwards self.query_state.options to resource accessor.
Unit Tests tests/unit/http/test_*.py, tests/unit/http/mixins/test_*.py	New tests validate query options propagation, query string serialization with URL encoding, HTTP parameter construction, and render/select option handling.
E2E Tests tests/e2e/audit/records/test_async_records.py, tests/e2e/audit/records/test_sync_records.py	New async and sync tests validate render and select options on audit record retrieval, confirming template delimiter removal and field selection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~35 minutes

Possibly related PRs

• [MPT-18938] Added render() as queryable mixin #226: Modifies QueryState and QueryableMixin to propagate render options through the query stack using a similar structural approach.
• MPT-16437 Update http/mixins file structure #200: Updates QueryableMixin and GetMixin classes to refactor how query state and options are represented and propagated through the mixin layer.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Single Commit Required	⚠️ Warning	The PR contains multiple commits rather than a single commit, violating the single commit requirement.	Use git rebase -i to squash all commits into one with a clear message summarizing all changes made.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Jira Issue Key In Title	✅ Passed	The PR title contains exactly one Jira issue key in the correct format MPT-19073.
Test Coverage Required	✅ Passed	PR modifies 8 code files in mpt_api_client/http module with comprehensive test coverage across 10 test files including unit and end-to-end tests.

_{✏️ 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.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
2.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

🧹 Nitpick comments (3)

mpt_api_client/http/query_options.py (1)

4-8: Consider making QueryOptions immutable to prevent accidental mutations.

QueryOptions is shared across query state propagation and multiple mixins. While no direct mutations are currently present, freezing the dataclass prevents future accidental side effects in shared state scenarios.

Proposed change

-@dataclass
+@dataclass(frozen=True, slots=True)
 class QueryOptions:
     """Options for query state."""

     render: bool = False

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@mpt_api_client/http/query_options.py` around lines 4 - 8, Change the
QueryOptions dataclass to be immutable by declaring it as a frozen dataclass
(e.g., `@dataclass`(frozen=True)) so instances cannot be mutated after creation;
update any code that attempted to modify QueryOptions attributes to instead
construct a new QueryOptions instance (refer to the QueryOptions class
definition and any callers that propagate or modify its fields).

tests/e2e/audit/records/test_sync_records.py (1)

81-89: Assert actor in the combined render+select test.

select=["object", "actor", "details"] is requested, but actor is not verified in this test. Adding that assertion would make the case fully aligned with its intent.

Suggested patch

 def test_get_record_with_render_and_select(mpt_vendor: MPTClient, audit_record_id) -> None:
@@
     result = service.get(audit_record_id, select=["object", "actor", "details"])

     assert result.id == audit_record_id
     assert result.object is not None
+    assert result.actor is not None
     assert not any(char in result.details for char in template_chars)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/audit/records/test_sync_records.py` around lines 81 - 89, The test
function test_get_record_with_render_and_select requests select=["object",
"actor", "details"] but never asserts actor; update the test to verify the actor
field is returned by adding an assertion (e.g., assert result.actor is not None
or an appropriate value) after the existing asserts so that the selected "actor"
field is validated when calling service.get(audit_record_id, select=[...]) with
render enabled.

tests/e2e/audit/records/test_async_records.py (1)

83-93: Assert actor in the combined render+select case.

Line 89 requests "actor" in select, but the test does not verify it. Add an explicit assertion so this case validates all requested fields.

Suggested patch

 async def test_get_record_with_render_and_select(
     async_mpt_vendor: AsyncMPTClient, audit_record_id: str
 ) -> None:
@@
     result = await service.get(audit_record_id, select=["object", "actor", "details"])

     assert result.id == audit_record_id
     assert result.object is not None
+    assert result.actor is not None
     assert not any(char in result.details for char in template_chars)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/audit/records/test_async_records.py` around lines 83 - 93, The test
test_get_record_with_render_and_select requests "actor" in the select but never
checks it; update the assertion block after calling service.get(audit_record_id,
select=["object", "actor", "details"]) to explicitly assert the actor value
(e.g., assert result.actor is not None or equals the expected actor) so the
combined render+select case validates the requested field (referencing
result.actor and the service created by
async_mpt_vendor.audit.records.options(render=True)).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@mpt_api_client/http/query_options.py`:
- Around line 4-8: Change the QueryOptions dataclass to be immutable by
declaring it as a frozen dataclass (e.g., `@dataclass`(frozen=True)) so instances
cannot be mutated after creation; update any code that attempted to modify
QueryOptions attributes to instead construct a new QueryOptions instance (refer
to the QueryOptions class definition and any callers that propagate or modify
its fields).

In `@tests/e2e/audit/records/test_async_records.py`:
- Around line 83-93: The test test_get_record_with_render_and_select requests
"actor" in the select but never checks it; update the assertion block after
calling service.get(audit_record_id, select=["object", "actor", "details"]) to
explicitly assert the actor value (e.g., assert result.actor is not None or
equals the expected actor) so the combined render+select case validates the
requested field (referencing result.actor and the service created by
async_mpt_vendor.audit.records.options(render=True)).

In `@tests/e2e/audit/records/test_sync_records.py`:
- Around line 81-89: The test function test_get_record_with_render_and_select
requests select=["object", "actor", "details"] but never asserts actor; update
the test to verify the actor field is returned by adding an assertion (e.g.,
assert result.actor is not None or an appropriate value) after the existing
asserts so that the selected "actor" field is validated when calling
service.get(audit_record_id, select=[...]) with render enabled.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 6b4da42e-3b3b-4fd1-b0fe-036fcbdbec3f

📥 Commits

Reviewing files that changed from the base of the PR and between 11651c9 and 0f44322.

📒 Files selected for processing (18)

• mpt_api_client/http/async_client.py
• mpt_api_client/http/client.py
• mpt_api_client/http/client_utils.py
• mpt_api_client/http/mixins/get_mixin.py
• mpt_api_client/http/mixins/queryable_mixin.py
• mpt_api_client/http/query_options.py
• mpt_api_client/http/query_state.py
• mpt_api_client/http/resource_accessor.py
• tests/e2e/audit/records/test_async_records.py
• tests/e2e/audit/records/test_sync_records.py
• tests/unit/http/mixins/test_get_mixin.py
• tests/unit/http/mixins/test_queryable_mixin.py
• tests/unit/http/test_async_client.py
• tests/unit/http/test_client.py
• tests/unit/http/test_client_utils.py
• tests/unit/http/test_query_options.py
• tests/unit/http/test_query_state.py
• tests/unit/http/test_resource_accessor.py