MPT-19892: add Extension base service for /public/v1/extensibility/extensions#276
Merged
albertsola merged 1 commit intomainfrom Apr 2, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds an extensibility resource package exposing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…tensions - Add Extension model with all fields from the OpenAPI spec - Add ExtensionsService / AsyncExtensionsService with CollectionMixin, GetMixin, CreateFileMixin, UpdateFileMixin, DeleteMixin and ExtensionMixin - Add ExtensionMixin / AsyncExtensionMixin with publish, unpublish, regenerate, token and download_icon actions - Add Extensibility / AsyncExtensibility accessor class - Wire extensibility property into MPTClient and AsyncMPTClient - Add full unit test coverage (100%) for all new code Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
albertsola
force-pushed
the
MPT-19892/extension-base-service
branch
from
4f36d8b to
aae1b3b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/resources/extensibility/test_extensions.py (1)
65-80: Assert multipart keys for file upload contract in create/update tests.At the moment, Line 77/94/112/130 only validate method and response. These tests won’t catch regressions where multipart keys stop being
iconandextension(which are critical config values forCreateFileMixin/UpdateFileMixin).Suggested assertion pattern
def test_extension_create(extensions_service, tmp_path): @@ assert mock_route.call_count == 1 assert mock_route.calls[0].request.method == "POST" + body = mock_route.calls[0].request.content + assert b'name="icon"' in body + assert b'name="extension"' in body assert result.to_dict() == expected_response @@ async def test_async_extension_create(async_extensions_service, tmp_path): @@ assert mock_route.call_count == 1 assert mock_route.calls[0].request.method == "POST" + body = mock_route.calls[0].request.content + assert b'name="icon"' in body + assert b'name="extension"' in body assert result.to_dict() == expected_response @@ def test_extension_update(extensions_service, tmp_path): @@ assert mock_route.call_count == 1 assert mock_route.calls[0].request.method == "PUT" + body = mock_route.calls[0].request.content + assert b'name="icon"' in body + assert b'name="extension"' in body assert result.to_dict() == expected_response @@ async def test_async_extension_update(async_extensions_service, tmp_path): @@ assert mock_route.call_count == 1 assert mock_route.calls[0].request.method == "PUT" + body = mock_route.calls[0].request.content + assert b'name="icon"' in body + assert b'name="extension"' in body assert result.to_dict() == expected_responseAlso applies to: 82-97, 99-115, 117-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/resources/extensibility/test_extensions.py` around lines 65 - 80, Update the unit tests (e.g., test_extension_create and the related create/update tests) to assert that the outgoing multipart request uses the expected part names: the file part is named "icon" and the JSON payload part is named "extension". In the mocked respx route callbacks (or after the request is made), inspect mock_route.calls[0].request.content or .read() to parse the multipart body and assert presence of the "Content-Disposition" names "name=\"icon\"" and "name=\"extension\"" so the CreateFileMixin/UpdateFileMixin contract is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/resources/extensibility/test_extensions.py`:
- Around line 65-80: Update the unit tests (e.g., test_extension_create and the
related create/update tests) to assert that the outgoing multipart request uses
the expected part names: the file part is named "icon" and the JSON payload part
is named "extension". In the mocked respx route callbacks (or after the request
is made), inspect mock_route.calls[0].request.content or .read() to parse the
multipart body and assert presence of the "Content-Disposition" names
"name=\"icon\"" and "name=\"extension\"" so the CreateFileMixin/UpdateFileMixin
contract is enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 56d6da9e-0377-42f6-bb54-604ebe64c688
📒 Files selected for processing (14)
mpt_api_client/mpt_client.pympt_api_client/resources/__init__.pympt_api_client/resources/extensibility/__init__.pympt_api_client/resources/extensibility/extensibility.pympt_api_client/resources/extensibility/extensions.pympt_api_client/resources/extensibility/mixins/__init__.pympt_api_client/resources/extensibility/mixins/extension_mixin.pypyproject.tomltests/unit/resources/extensibility/__init__.pytests/unit/resources/extensibility/mixins/__init__.pytests/unit/resources/extensibility/mixins/test_extension_mixin.pytests/unit/resources/extensibility/test_extensibility.pytests/unit/resources/extensibility/test_extensions.pytests/unit/test_mpt_client.py
|
robcsegal
approved these changes
Apr 2, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unit/resources/extensibility/test_extensions.py (2)
23-58: Deduplicate the mixin method matrix to avoid drift.
Line 25-36andLine 47-57repeat the same list; centralizing it will reduce maintenance risk.Refactor suggestion
+MIXIN_METHODS = ( + "get", + "create", + "update", + "delete", + "publish", + "unpublish", + "regenerate", + "token", + "download_icon", + "iterate", +) + `@pytest.mark.parametrize`( "method", - [ - "get", - "create", - "update", - "delete", - "publish", - "unpublish", - "regenerate", - "token", - "download_icon", - "iterate", - ], + MIXIN_METHODS, ) def test_mixins_present(extensions_service, method): @@ `@pytest.mark.parametrize`( "method", - [ - "get", - "create", - "update", - "delete", - "publish", - "unpublish", - "regenerate", - "token", - "download_icon", - "iterate", - ], + MIXIN_METHODS, ) def test_async_mixins_present(async_extensions_service, method):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/resources/extensibility/test_extensions.py` around lines 23 - 58, The test repeats the same param list twice; extract the list of mixin names into a single constant and reuse it in both pytest.mark.parametrize decorators to avoid duplication—e.g., define a module-level tuple/variable (e.g., MIXIN_METHODS or METHODS) containing "get","create","update","delete","publish","unpublish","regenerate","token","download_icon","iterate" and update the two pytest.mark.parametrize usages around test_mixins_present and the following test to reference that variable instead of repeating the literal list.
65-133: Add multipart payload assertions for upload key contract.The create/update tests assert method and count, but they don’t verify multipart field names (
iconfile part andextensionJSON part). A direct assertion here would better protect this endpoint contract.Assertion enhancement example
assert mock_route.call_count == 1 assert mock_route.calls[0].request.method == "POST" + request_body = mock_route.calls[0].request.content + assert b'name="icon"' in request_body + assert b'name="extension"' in request_body assert result.to_dict() == expected_response🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/resources/extensibility/test_extensions.py` around lines 65 - 133, Tests for test_extension_create, test_async_extension_create, test_extension_update, and test_async_extension_update currently only assert call count and method; add assertions that the outgoing requests are multipart/form-data and include the expected field names: verify mock_route.calls[0].request.headers["content-type"] starts with "multipart/form-data" and parse the request body to assert there is an "icon" file part (with a filename) and an "extension" part containing the JSON for extension_data/update_data; update both synchronous (extensions_service.create/update) and asynchronous (async_extensions_service.create/update) tests to include these multipart field name and content assertions so the upload key contract is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/resources/extensibility/test_extensions.py`:
- Around line 23-58: The test repeats the same param list twice; extract the
list of mixin names into a single constant and reuse it in both
pytest.mark.parametrize decorators to avoid duplication—e.g., define a
module-level tuple/variable (e.g., MIXIN_METHODS or METHODS) containing
"get","create","update","delete","publish","unpublish","regenerate","token","download_icon","iterate"
and update the two pytest.mark.parametrize usages around test_mixins_present and
the following test to reference that variable instead of repeating the literal
list.
- Around line 65-133: Tests for test_extension_create,
test_async_extension_create, test_extension_update, and
test_async_extension_update currently only assert call count and method; add
assertions that the outgoing requests are multipart/form-data and include the
expected field names: verify mock_route.calls[0].request.headers["content-type"]
starts with "multipart/form-data" and parse the request body to assert there is
an "icon" file part (with a filename) and an "extension" part containing the
JSON for extension_data/update_data; update both synchronous
(extensions_service.create/update) and asynchronous
(async_extensions_service.create/update) tests to include these multipart field
name and content assertions so the upload key contract is enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 36d07b26-4128-47fb-a998-0d1322db1d3f
📒 Files selected for processing (14)
mpt_api_client/mpt_client.pympt_api_client/resources/__init__.pympt_api_client/resources/extensibility/__init__.pympt_api_client/resources/extensibility/extensibility.pympt_api_client/resources/extensibility/extensions.pympt_api_client/resources/extensibility/mixins/__init__.pympt_api_client/resources/extensibility/mixins/extension_mixin.pypyproject.tomltests/unit/resources/extensibility/__init__.pytests/unit/resources/extensibility/mixins/__init__.pytests/unit/resources/extensibility/mixins/test_extension_mixin.pytests/unit/resources/extensibility/test_extensibility.pytests/unit/resources/extensibility/test_extensions.pytests/unit/test_mpt_client.py
✅ Files skipped from review due to trivial changes (5)
- mpt_api_client/resources/extensibility/extensibility.py
- pyproject.toml
- tests/unit/test_mpt_client.py
- tests/unit/resources/extensibility/test_extensibility.py
- tests/unit/resources/extensibility/mixins/test_extension_mixin.py
🚧 Files skipped from review as they are similar to previous changes (3)
- mpt_api_client/resources/init.py
- mpt_api_client/mpt_client.py
- mpt_api_client/resources/extensibility/mixins/extension_mixin.py
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
Implements the Extension base service for the
/public/v1/extensibility/extensionsendpoint.Changes
New source files
mpt_api_client/resources/extensibility/mixins/extension_mixin.py—ExtensionMixin/AsyncExtensionMixinwithpublish,unpublish,regenerate,token,download_iconmpt_api_client/resources/extensibility/extensions.py—Extensionmodel +ExtensionsService/AsyncExtensionsServicempt_api_client/resources/extensibility/extensibility.py—Extensibility/AsyncExtensibilityaccessorModified files
mpt_api_client/resources/__init__.py— exportsExtensibility,AsyncExtensibilitympt_api_client/mpt_client.py—.extensibilityproperty on both clientspyproject.toml— per-file-ignores for new pathstests/unit/test_mpt_client.py—extensibilityadded to parametrize listsEndpoints covered
/extensibility/extensionsCollectionMixin/extensibility/extensionsCreateFileMixin/extensibility/extensions/{id}GetMixin/extensibility/extensions/{id}UpdateFileMixin/extensibility/extensions/{id}DeleteMixin/extensibility/extensions/{id}/publishExtensionMixin/extensibility/extensions/{id}/unpublishExtensionMixin/extensibility/extensions/{id}/regenerateExtensionMixin/extensibility/extensions/{id}/tokenExtensionMixin/extensibility/extensions/{id}/iconExtensionMixin.download_icon→FileModelQuality
make check-allpassesCloses MPT-19892