MPT-17888: add first-level field annotations to Parameter and ParameterGroup models#230
Merged
albertsola merged 1 commit intomainfrom Mar 13, 2026
Merged
MPT-17888: add first-level field annotations to Parameter and ParameterGroup models#230albertsola merged 1 commit intomainfrom
albertsola merged 1 commit intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds nullable, type-annotated fields and detailed docstrings to ParameterGroup and Parameter models, introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…erGroup models Add type-annotated fields to the Parameter and ParameterGroup model classes derived from the OpenAPI spec (ParameterDefinition and ParameterGroup schemas respectively). All fields are typed as T | None since RQL select can exclude any field from the API response. Nested objects (audit, group, product, constraints, options) are typed as BaseModel | None as no dedicated sub-model classes exist for them yet. Parameter fields: name, description, scope, phase, context, type, status, external_id, display_order, group, product, constraints, audit, options. ParameterGroup fields: name, label, description, display_order, default, parameter_count, product, audit. Also extends unit tests for both models to cover field access, nested BaseModel typing, and optional field absence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
albertsola
force-pushed
the
parameters-resource
branch
from
4ea7102 to
637ccb6
Compare
|
robcsegal
approved these changes
Mar 13, 2026
d3rky
added a commit
that referenced
this pull request
Mar 13, 2026
#231) ## Summary Add typed field annotations to all 15 remaining catalog service model classes in `mpt_api_client/resources/catalog/`, following the same pattern established for `Parameter` and `ParameterGroup` in #230. All fields are typed as `T | None` since RQL `select` can exclude any field from the API response. ## Models updated - `Authorization` — 11 fields - `Item` — 11 fields - `Listing` — 10 fields - `PriceList` — 10 fields - `PriceListItem` — 20 fields (including price columns with non-standard casing) - `PricingPolicy` — 11 fields - `PricingPolicyAttachment` — 8 fields - `Term` — 6 fields - `TermVariant` — 12 fields - `Document` — 11 fields - `ItemGroup` — 10 fields - `Media` — 11 fields - `Template` — 6 fields - `Product` — 11 fields - `UnitOfMeasure` — 4 fields ## Tests Each model's test file was extended with: - Fixture with representative API response data - Round-trip test via `to_dict()` - Nested fields returning `BaseModel` instances - Absent fields verified with `not hasattr()` (model raises `AttributeError` for absent fields) ## Notes - `PriceListItem` price columns use non-standard casing (e.g. `PPx1` → `p_px1`) due to how `to_snake_case` works — round-trip via `to_dict()` is lossy for these fields, so only attribute-access tests are used for the price column group - `list[BaseModel] | None` used for array-of-object fields (`Item.parameters`, `PricingPolicy.products`) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> Closes [MPT-17888](https://softwareone.atlassian.net/browse/MPT-17888) - Added typed first-level field annotations (T | None) to 15 catalog service model classes: Authorization, Item, Listing, PriceList, PriceListItem, PricingPolicy, PricingPolicyAttachment, Term, TermVariant, Document, ItemGroup, Media, Template, Product, and UnitOfMeasure - All fields marked as optional (| None) to reflect RQL select behavior where fields can be excluded from API responses - Implemented bidirectional field name mapping via _FIELD_NAME_MAPPINGS to handle API fields with consecutive uppercase letters (e.g., PPx1 → ppx1, unitLP → unit_lp), enabling lossless round-trip serialization - Updated case conversion utilities (to_snake_case and to_camel_case) to prioritize explicit field mappings over regex-based transformations - Extended test suite with representative API fixtures for each model and comprehensive test coverage including primitive field round-trips, nested BaseModel instances, and absent field behavior - Added documentation via expanded class docstrings and a new Copilot instructions guide (.github/copilot-instructions.md) - All array-of-object fields properly typed as list[BaseModel] | None (e.g., Item.parameters, PricingPolicy.products) <!-- end of auto-generated comment: release notes by coderabbit.ai --> [MPT-17888]: https://softwareone.atlassian.net/browse/MPT-17888?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add type-annotated fields to the
ParameterandParameterGroupmodel classes derived from the OpenAPI spec (ParameterDefinitionandParameterGroupschemas respectively).Changes
Parametermodel (products_parameters.py)Fields from the
ParameterDefinitionschema:name,description,scope,phase,context,type,status,external_id,display_ordergroup,product,constraints,audit,optionsParameterGroupmodel (products_parameter_groups.py)Fields from the
ParameterGroupschema:name,label,description,display_order,default,parameter_countproduct,auditNotes
T | Nonesince RQLselectcan exclude any field from the API responseBaseModel | None(no dedicated sub-model classes needed at this level)to_dict()round-trip, nestedBaseModeltyping, and optional field absenceCloses MPT-17888