MPT-19904: add /integration/extensions/{id}/documents service#284
Merged
albertsola merged 4 commits intomainfrom Apr 10, 2026
Merged
MPT-19904: add /integration/extensions/{id}/documents service#284albertsola merged 4 commits intomainfrom
albertsola merged 4 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds an ExtensionDocument model, config, and sync/async ExtensionDocumentsService for per-extension document CRUD, upload/download, publish/unpublish, wires documents(extension_id) accessors on ExtensionsService/AsyncExtensionsService, and introduces unit and e2e (sync & async) tests for the document lifecycle. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
albertsola
force-pushed
the
MPT-19904/integration-extension-documents
branch
4 times, most recently
from
af22a72 to
2bb3d5a
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/e2e/integration/extension_documents/test_sync_extension_documents.py (1)
44-45: Isolate the destructive delete test from the auto-cleanup fixture.Using
created_documenthere triggers a second delete during fixture teardown, which adds avoidable teardown noise.Based on learnings: In end-to-end tests, reserve isolated fixtures/resources for destructive tests that require per-test creation and cleanup.♻️ Proposed change
-def test_delete_extension_document(extension_documents_service, created_document): - extension_documents_service.delete(created_document.id) # act +def test_delete_extension_document(extension_documents_service, document_data): + document = extension_documents_service.create(document_data) + extension_documents_service.delete(document.id) # act🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/integration/extension_documents/test_sync_extension_documents.py` around lines 44 - 45, The test_delete_extension_document uses the shared created_document fixture which will be auto-deleted in teardown, causing a double-delete; change the test to create and manage its own document locally instead of using created_document: call extension_documents_service.create(...) at the start of test_delete_extension_document to obtain a new document id, call extension_documents_service.delete(id) for the assertion, and let the test-created resource be cleaned up or explicitly avoid relying on the shared auto-cleanup fixture; reference the test name test_delete_extension_document and the service extension_documents_service when making this change.
🤖 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/e2e/integration/extension_documents/test_sync_extension_documents.py`:
- Around line 44-45: The test_delete_extension_document uses the shared
created_document fixture which will be auto-deleted in teardown, causing a
double-delete; change the test to create and manage its own document locally
instead of using created_document: call extension_documents_service.create(...)
at the start of test_delete_extension_document to obtain a new document id, call
extension_documents_service.delete(id) for the assertion, and let the
test-created resource be cleaned up or explicitly avoid relying on the shared
auto-cleanup fixture; reference the test name test_delete_extension_document and
the service extension_documents_service when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 938e1901-fd35-4731-893e-066889e7b588
📒 Files selected for processing (11)
e2e_config.test.jsonmpt_api_client/resources/integration/extension_documents.pympt_api_client/resources/integration/extensions.pympt_api_client/resources/integration/mixins/__init__.pympt_api_client/resources/integration/mixins/publishable_mixin.pytests/e2e/integration/extension_documents/__init__.pytests/e2e/integration/extension_documents/conftest.pytests/e2e/integration/extension_documents/test_async_extension_documents.pytests/e2e/integration/extension_documents/test_sync_extension_documents.pytests/unit/resources/integration/mixins/test_publishable_mixin.pytests/unit/resources/integration/test_extension_documents.py
albertsola
force-pushed
the
MPT-19904/integration-extension-documents
branch
from
2bb3d5a to
7586715
Compare
svazquezco
approved these changes
Apr 10, 2026
robcsegal
approved these changes
Apr 10, 2026
…nts endpoint Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sionId}/documents Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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-19904/integration-extension-documents
branch
from
82b4ff1 to
74b9933
Compare
|
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}/documentsendpoint group.Changes
mpt_api_client/resources/integration/extension_documents.py—PublishableMixin+CreateFileMixin+ModifiableResourceMixin+CollectionMixinmpt_api_client/resources/integration/mixins/publishable_mixin.py—publish/unpublishmixin (noreview; differs from catalog variant)mpt_api_client/resources/integration/extensions.py— addeddocuments(extension_id)accessor to both sync and async services; rebased onmain(includes MPT-19906termsaccessor)tests/unit/resources/integration/test_extension_documents.pytests/unit/resources/integration/mixins/test_publishable_mixin.pytests/e2e/integration/extension_documents/QA
make check-all: all linting (ruff, flake8, mypy) + 1889 tests pass ✅publishable_mixin.py: 100% coverage ✅extension_documents.py: 100% coverage ✅Closes MPT-19904