MPT-19905: add /integration/extensions/{id}/media service#285
Merged
albertsola merged 3 commits intomainfrom Apr 10, 2026
Merged
MPT-19905: add /integration/extensions/{id}/media service#285albertsola merged 3 commits intomainfrom
albertsola merged 3 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughIntroduces a new extension media integration resource with synchronous and asynchronous service classes, including a model for media metadata and file properties. Adds media accessor methods to extension services and implements a media mixin providing image upload functionality. Includes comprehensive unit and end-to-end tests for media operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
albertsola
force-pushed
the
MPT-19905/integration-extension-media
branch
3 times, most recently
from
b520317 to
52db84c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
mpt_api_client/resources/integration/extension_media.py (1)
74-87: Align docstring placeholder naming with the configured endpoint.At Line 74 and Line 87, docstrings use
{extensionId}while the actual endpoint template at Line 57 uses{extension_id}. Matching these improves clarity for maintainers.📝 Proposed docstring alignment
- """Sync service for the /public/v1/integration/extensions/{extensionId}/media endpoint.""" + """Sync service for the /public/v1/integration/extensions/{extension_id}/media endpoint.""" @@ - """Async service for the /public/v1/integration/extensions/{extensionId}/media endpoint.""" + """Async service for the /public/v1/integration/extensions/{extension_id}/media endpoint."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mpt_api_client/resources/integration/extension_media.py` around lines 74 - 87, Update the two docstrings that currently reference the endpoint placeholder "{extensionId}" to use the configured template "{extension_id}" so they match the endpoint definition; specifically, edit the module-level docstring and the AsyncExtensionMediaService class docstring (class name: AsyncExtensionMediaService) to replace "{extensionId}" with "{extension_id}" for consistency with the endpoint template.tests/unit/resources/integration/test_extension_media.py (1)
93-107: Strengthen multipart contract assertions in the create test.At Line 105-Line 107, the test proves path/method/response, but it can still pass if multipart composition regresses (e.g., missing
mediaorfilepart). Adding request-content assertions would better protect_upload_data_key/_upload_file_keybehavior.♻️ Proposed test hardening
assert mock_route.call_count == 1 - assert mock_route.calls[0].request.method == "POST" + request = mock_route.calls[0].request + assert request.method == "POST" + assert b'name="media"' in request.content + assert b'name="file"' in request.content assert result.to_dict() == expected_responseAs per coding guidelines,
**/test_*.py: Follow the testing standards defined instandards/testing-standard.mdfrom the mpt-extension-skills repository.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/resources/integration/test_extension_media.py` around lines 93 - 107, The test_extension_media_create currently only asserts path/method/response but should also validate the multipart request body; update the test (in test_extension_media_create) to inspect mock_route.calls[0].request (e.g., request.content or request.read() and headers) and assert the multipart payload contains a part named with the expected data key (match your _upload_data_key, e.g., "media") containing the JSON payload, and a part named with the expected file key (match your _upload_file_key, e.g., "file") containing the image bytes and correct filename/Content-Type; ensure Content-Type is multipart/form-data and boundary is present so regressions in media/file part names or composition fail the test.
🤖 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/resources/integration/extension_media.py`:
- Around line 74-87: Update the two docstrings that currently reference the
endpoint placeholder "{extensionId}" to use the configured template
"{extension_id}" so they match the endpoint definition; specifically, edit the
module-level docstring and the AsyncExtensionMediaService class docstring (class
name: AsyncExtensionMediaService) to replace "{extensionId}" with
"{extension_id}" for consistency with the endpoint template.
In `@tests/unit/resources/integration/test_extension_media.py`:
- Around line 93-107: The test_extension_media_create currently only asserts
path/method/response but should also validate the multipart request body; update
the test (in test_extension_media_create) to inspect mock_route.calls[0].request
(e.g., request.content or request.read() and headers) and assert the multipart
payload contains a part named with the expected data key (match your
_upload_data_key, e.g., "media") containing the JSON payload, and a part named
with the expected file key (match your _upload_file_key, e.g., "file")
containing the image bytes and correct filename/Content-Type; ensure
Content-Type is multipart/form-data and boundary is present so regressions in
media/file part names or composition fail the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 3569fbd6-9442-434e-ba65-607bb078be6e
📒 Files selected for processing (12)
e2e_config.test.jsonmpt_api_client/resources/integration/extension_media.pympt_api_client/resources/integration/extensions.pympt_api_client/resources/integration/mixins/__init__.pympt_api_client/resources/integration/mixins/media_mixin.pympt_api_client/resources/integration/mixins/publishable_mixin.pytests/e2e/integration/extension_media/__init__.pytests/e2e/integration/extension_media/conftest.pytests/e2e/integration/extension_media/test_async_extension_media.pytests/e2e/integration/extension_media/test_sync_extension_media.pytests/unit/resources/integration/mixins/test_media_mixin.pytests/unit/resources/integration/test_extension_media.py
…endpoint Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sionId}/media Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
albertsola
force-pushed
the
MPT-19905/integration-extension-media
branch
from
52db84c to
79174cd
Compare
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
mpt_api_client/resources/integration/extension_media.py (1)
66-90: Remove redundant delete mixins from service inheritance.Both
DeleteMixin(line 72) andAsyncDeleteMixin(line 86) are redundant becauseModifiableResourceMixinandAsyncModifiableResourceMixinalready inherit delete behavior respectively.♻️ Suggested cleanup
class ExtensionMediaService( MediaMixin[ExtensionMedia], PublishableMixin[ExtensionMedia], DownloadFileMixin[ExtensionMedia], CreateFileMixin[ExtensionMedia], ModifiableResourceMixin[ExtensionMedia], - DeleteMixin, CollectionMixin[ExtensionMedia], Service[ExtensionMedia], ExtensionMediaServiceConfig, ): @@ class AsyncExtensionMediaService( AsyncMediaMixin[ExtensionMedia], AsyncPublishableMixin[ExtensionMedia], AsyncDownloadFileMixin[ExtensionMedia], AsyncCreateFileMixin[ExtensionMedia], AsyncModifiableResourceMixin[ExtensionMedia], - AsyncDeleteMixin, AsyncCollectionMixin[ExtensionMedia], AsyncService[ExtensionMedia], ExtensionMediaServiceConfig, ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mpt_api_client/resources/integration/extension_media.py` around lines 66 - 90, Remove the redundant DeleteMixin and AsyncDeleteMixin entries from the ExtensionMediaService and AsyncExtensionMediaService class inheritance lists respectively; ModifiableResourceMixin and AsyncModifiableResourceMixin already provide the delete behavior, so update the classes (ExtensionMediaService and AsyncExtensionMediaService) to exclude DeleteMixin and AsyncDeleteMixin from their bases.
🤖 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/resources/integration/extension_media.py`:
- Around line 66-90: Remove the redundant DeleteMixin and AsyncDeleteMixin
entries from the ExtensionMediaService and AsyncExtensionMediaService class
inheritance lists respectively; ModifiableResourceMixin and
AsyncModifiableResourceMixin already provide the delete behavior, so update the
classes (ExtensionMediaService and AsyncExtensionMediaService) to exclude
DeleteMixin and AsyncDeleteMixin from their bases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 2a87d78f-0cd3-420a-a2a6-d0636f952253
📒 Files selected for processing (10)
mpt_api_client/resources/integration/extension_media.pympt_api_client/resources/integration/extensions.pympt_api_client/resources/integration/mixins/__init__.pympt_api_client/resources/integration/mixins/media_mixin.pytests/e2e/integration/extension_media/__init__.pytests/e2e/integration/extension_media/conftest.pytests/e2e/integration/extension_media/test_async_extension_media.pytests/e2e/integration/extension_media/test_sync_extension_media.pytests/unit/resources/integration/mixins/test_media_mixin.pytests/unit/resources/integration/test_extension_media.py
🚧 Files skipped from review as they are similar to previous changes (3)
- mpt_api_client/resources/integration/extensions.py
- tests/e2e/integration/extension_media/test_sync_extension_media.py
- tests/e2e/integration/extension_media/test_async_extension_media.py
svazquezco
approved these changes
Apr 10, 2026
robcsegal
approved these changes
Apr 10, 2026
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
3 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
/public/v1/integration/extensions/{extensionId}/mediaendpoint group as part of MPT-13310.Changes
mpt_api_client/resources/integration/extension_media.py— MediaMixin + PublishableMixin + CreateFileMixin + ModifiableResourceMixin + CollectionMixinmpt_api_client/resources/integration/mixins/media_mixin.py—upload_image()forPUT /{id}/imageextensions.pyto exposemedia(extension_id)accessortests/unit/resources/integration/test_extension_media.py+test_media_mixin.pytests/e2e/integration/extension_media/Depends on
#277 (MPT-19892 — rename extensibility → integration)
Closes MPT-19905
/public/v1/integration/extensions/{extensionId}/mediaendpoint group withExtensionMediamodel supporting metadata, file attributes, and related entitiesExtensionMediaServiceandAsyncExtensionMediaServiceclasses providing full-featured media management including CRUD operations, publish/unpublish, file download/upload, and collection iterationMediaMixinandAsyncMediaMixinmixins withupload_image()method to support image uploads viaPUT /{id}/imageendpointExtensionsServiceandAsyncExtensionsServicewithmedia(extension_id)accessor methods to instantiate media service clients scoped to a specific extension