Feat: Introduce dynamic property mixin and add dynamic props to native view#1366
Feat: Introduce dynamic property mixin and add dynamic props to native view#1366
Conversation
TM1py/Objects/NativeView.py
Outdated
| dynamic_props = self._filter_dynamic_properties(self._dynamic_properties) | ||
| if dynamic_props: | ||
| bottom_json += "," + json.dumps(dynamic_props, ensure_ascii=False)[1:-1] | ||
| bottom_json += "}" |
There was a problem hiding this comment.
This looks a bit archaic, how we're constructing the JSON here via string concatenation.
This PR is a good opportunity to clean up this part of the code base.
There was a problem hiding this comment.
I thought this was best practice and the project’s recommended approach for building JSON via string concatenation😉
Resolved in def6d56
There was a problem hiding this comment.
Pull request overview
This PR introduces a shared mechanism to preserve “dynamic” (extra/unknown) REST properties on view objects and extends that support to NativeView, with accompanying unit/integration tests to validate round-tripping and REST retrieval.
Changes:
- Added
DynamicPropertiesMixinto centralize storage and filtering of extra REST properties via.dynamic_properties. - Updated
MDXViewandNativeViewto use the mixin and include dynamic properties in (de)serialization. - Added tests covering dynamic properties defaults, constructor behavior, serialization, and REST create/get for native views.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/ViewService_test.py | Adds integration test that creates/retrieves/deletes a native view with dynamic properties. |
| Tests/NativeView_test.py | Adds unit tests for NativeView.dynamic_properties defaults, serialization, and from-dict behavior. |
| TM1py/Services/ViewService.py | Adjusts view retrieval to use shared dynamic property filtering for MDX/native views. |
| TM1py/Objects/NativeView.py | Adds dynamic property support and serializes filtered dynamic properties into the request body. |
| TM1py/Objects/MDXView.py | Refactors dynamic property handling to the shared mixin and shared filtering method. |
| TM1py/Objects/DynamicPropertiesMixin.py | New mixin providing .dynamic_properties and _filter_dynamic_properties(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
a5a3924 to
2811513
Compare
Replace string concatenation with dictionary-based approach using json.dumps(). This improves readability and maintainability.
seed deterministic cube data to avoid flaky update assertions
2811513 to
cbb096e
Compare
3c42784 to
746e73b
Compare
|
@MariusWirtz This PR is ready for another review. I cleaned up the commit history and separated refactoring, test fixes, and the feature. I also added explanations to the commits, so it would be great if they could be preserved. |
What changed