Skip to content

Conversation

@albertsola
Copy link
Contributor

@albertsola albertsola commented Feb 2, 2026

Closes MPT-16437

  • Reorganise catalog mixins into separate modules for clearer structure and maintainability.
  • Add ActivatableMixin and AsyncActivatableMixin (mpt_api_client.resources.catalog.mixins.activatable_mixin) with activate() and deactivate() helpers.
  • Add DocumentMixin and AsyncDocumentMixin (mpt_api_client.resources.catalog.mixins.document_mixin) composing create-file, download-file and publishable capabilities.
  • Add MediaMixin and AsyncMediaMixin (mpt_api_client.resources.catalog.mixins.media_mixin) composing create-file, download-file and publishable capabilities.
  • Refactor publishable_mixin.py to retain only PublishableMixin/AsyncPublishableMixin (removed composite mixin definitions).
  • Centralise public exports in mpt_api_client.resources.catalog.mixins.init.py to re-export all mixins.
  • Add focused unit tests for Activatable, Document and Publishable mixins (sync + async) in dedicated test modules.
  • Remove legacy tests/unit/resources/catalog/test_mixins.py (tests migrated to module-specific files).
  • Remove several per-file lint suppressions from pyproject.toml related to the reorganised files.

@albertsola albertsola requested a review from a team as a code owner February 2, 2026 16:32
@albertsola albertsola requested review from d3rky and jentyk February 2, 2026 16:32
@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

📝 Walkthrough

Walkthrough

Splits composite catalog mixins into separate modules (activatable, document, media), re-exports all mixins from mpt_api_client.resources.catalog.mixins.__init__, removes duplicate composite classes from publishable_mixin.py, updates pyproject lint ignores, and moves tests from a consolidated module into focused mixin test modules.

Changes

Cohort / File(s) Summary
Re-exports
mpt_api_client/resources/catalog/mixins/__init__.py
Adds explicit imports and updates __all__ to re-export Activatable, Document, Media, and Publishable mixins and their async variants.
Activatable mixin
mpt_api_client/resources/catalog/mixins/activatable_mixin.py
Introduces ActivatableMixin and AsyncActivatableMixin with activate() / deactivate() delegating to _resource_action.
Document composition
mpt_api_client/resources/catalog/mixins/document_mixin.py
Adds DocumentMixin and AsyncDocumentMixin composing CreateFile/DownloadFile/Publishable mixins (sync and async variants).
Media composition
mpt_api_client/resources/catalog/mixins/media_mixin.py
Adds MediaMixin and AsyncMediaMixin composing CreateFile/DownloadFile/Publishable mixins (sync and async variants).
Publishable cleanup
mpt_api_client/resources/catalog/mixins/publishable_mixin.py
Removes previously in-file composite mixin classes and unused imports; retains PublishableMixin (and async variant) implementation.
Lint config
pyproject.toml
Removes several per-file WPS lint suppression entries related to mixins and seed files.
New focused tests
tests/unit/resources/catalog/mixins/test_activatable_mixin.py, tests/unit/resources/catalog/mixins/test_document_mixin.py, tests/unit/resources/catalog/mixins/test_publishable_mixin.py
Adds unit tests for Activatable, Document, and Publishable mixins (sync and async), validating HTTP request formation, multipart handling, and response mapping.
Removed consolidated tests
tests/unit/resources/catalog/test_mixins.py
Deletes the previously consolidated mixins test module; tests redistributed into the new focused test modules.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Jira Issue Key In Title ✅ Passed PR title contains exactly one Jira issue key in MPT-XXXX format (MPT-16437) at the beginning.
Test Coverage Required ✅ Passed PR modifies 6 code files in mpt_api_client/resources/catalog/mixins/ and includes 4 modified test files with 3 new test modules providing comprehensive coverage.
Single Commit Required ✅ Passed The pull request contains exactly one commit, meeting the requirement for a clean git history with all resource catalog mixin changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • MPT-16437: Cannot read properties of undefined (reading 'map')

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tests/unit/resources/catalog/mixins/test_publishable_mixin.py`:
- Around line 73-102: Remove the unused input_status parameter from the
parametrization and the test signature in test_custom_resource_actions_no_data:
update the `@pytest.mark.parametrize` tuple to only include ("action",) and change
the function definition to def
test_custom_resource_actions_no_data(publishable_service, action): so the unused
input_status is not passed into the test; apply the same edit to the other
analogous no-data test in this module that was similarly parametrized (the other
test covering the publish/review/unpublish actions).

@albertsola albertsola force-pushed the MPT-16437/Reorganise-resource-catalog-mixins-structure branch from 7d8f7b5 to 53468a5 Compare February 2, 2026 16:41
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tests/unit/resources/catalog/mixins/test_publishable_mixin.py`:
- Around line 135-145: Remove the unused `input_status` parameter from the async
no-data test: update the pytest.mark.parametrize tuple on
`test_async_custom_resource_actions_no_data` to only pass the "action" values
and remove `input_status` from the test function signature (leave
`async_publishable_service` and `action`), ensuring the parametrize decorator
and the test function signature match.
🧹 Nitpick comments (3)
mpt_api_client/resources/catalog/mixins/__init__.py (1)

18-18: Remove unused noqa directive.

Ruff flags WPS410 as unknown. Since the linting configuration was updated in this PR (per pyproject.toml changes), this directive is no longer needed.

🔧 Proposed fix
-__all__ = [  # noqa: WPS410
+__all__ = [
tests/unit/resources/catalog/mixins/test_document_mixin.py (2)

77-77: Remove unused noqa directive.

Ruff flags WPS210 as unknown; this directive has no effect.

🔧 Proposed fix
-def test_document_create_with_file(document_service):  # noqa: WPS210
+def test_document_create_with_file(document_service):

135-135: Remove unused noqa directive.

Ruff flags WPS210 as unknown; this directive has no effect.

🔧 Proposed fix
-async def test_async_document_create_with_file(async_document_service):  # noqa: WPS210
+async def test_async_document_create_with_file(async_document_service):

@albertsola albertsola force-pushed the MPT-16437/Reorganise-resource-catalog-mixins-structure branch from 53468a5 to f9bdf14 Compare February 2, 2026 16:46
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2026

@albertsola albertsola merged commit 8fbe9b9 into main Feb 2, 2026
4 checks passed
@albertsola albertsola deleted the MPT-16437/Reorganise-resource-catalog-mixins-structure branch February 2, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants