Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new 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 |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/e2e/program/program/test_sync_program.py (1)
55-61: Strengthen the delete test with a post-delete assertion.Right now this only verifies the call path. Consider asserting a follow-up
getfails to confirm the resource is actually gone.Suggested enhancement
def test_delete_program(mpt_vendor, created_program): - program_data = created_program - - result = mpt_vendor.program.programs - - result.delete(program_data.id) + mpt_vendor.program.programs.delete(created_program.id) + with pytest.raises(MPTAPIError): + mpt_vendor.program.programs.get(created_program.id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/program/program/test_sync_program.py` around lines 55 - 61, The test_delete_program currently only calls result.delete(program_data.id); after that call, perform a follow-up verification by attempting to retrieve the deleted program with result.get(program_data.id) and assert the expected failure (e.g., raise a NotFound/HTTP 404 exception or return None) to confirm the resource was actually removed; update the test_delete_program to catch the expected exception or assert the not-found response accordingly.tests/e2e/program/program/test_async_program.py (1)
55-61: Strengthen the delete test to assert actual deletion.The test currently passes if
delete()does not raise, but it does not verify that the resource is truly gone. Prefer asserting the post-delete read fails.Suggested refactor
async def test_delete_program(async_mpt_vendor, created_program): - program_data = created_program - - result = async_mpt_vendor.program.programs - - await result.delete(program_data.id) + await async_mpt_vendor.program.programs.delete(created_program.id) + with pytest.raises(MPTAPIError): + await async_mpt_vendor.program.programs.get(created_program.id)Based on learnings: “Reserve isolated fixtures for destructive tests that require per-test creation and cleanup.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/program/program/test_async_program.py` around lines 55 - 61, The test_delete_program currently only calls async_mpt_vendor.program.programs.delete(created_program.id) and doesn't verify deletion; update test_delete_program to assert the resource is gone by attempting to read/fetch the program after deletion (e.g., call async_mpt_vendor.program.programs.get or fetch by id) and assert it raises the expected NotFound/HTTPError or returns a 404/None result; also switch to an isolated/per-test fixture for created_program if this is destructive to avoid affecting other tests. Use the symbols test_delete_program, async_mpt_vendor.program.programs, delete, and created_program.id to locate and change the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 151: Add the missing production per-file-ignores entry for the program
resource by adding the pattern "mpt_api_client/resources/program/*.py: WPS202
WPS210 WPS218" to the pyproject.toml alongside the existing
"tests/unit/resources/program/*.py: WPS202 WPS210 WPS218" entry; ensure the new
entry uses the same three rules (WPS202, WPS210, WPS218) and matches the
formatting of the other resource production entries (e.g., accounts, billing) so
lint configuration remains consistent.
---
Nitpick comments:
In `@tests/e2e/program/program/test_async_program.py`:
- Around line 55-61: The test_delete_program currently only calls
async_mpt_vendor.program.programs.delete(created_program.id) and doesn't verify
deletion; update test_delete_program to assert the resource is gone by
attempting to read/fetch the program after deletion (e.g., call
async_mpt_vendor.program.programs.get or fetch by id) and assert it raises the
expected NotFound/HTTPError or returns a 404/None result; also switch to an
isolated/per-test fixture for created_program if this is destructive to avoid
affecting other tests. Use the symbols test_delete_program,
async_mpt_vendor.program.programs, delete, and created_program.id to locate and
change the test.
In `@tests/e2e/program/program/test_sync_program.py`:
- Around line 55-61: The test_delete_program currently only calls
result.delete(program_data.id); after that call, perform a follow-up
verification by attempting to retrieve the deleted program with
result.get(program_data.id) and assert the expected failure (e.g., raise a
NotFound/HTTP 404 exception or return None) to confirm the resource was actually
removed; update the test_delete_program to catch the expected exception or
assert the not-found response accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e17c454b-59b5-4e89-8ff4-f4ae535aa5ee
📒 Files selected for processing (18)
e2e_config.test.jsonmpt_api_client/mpt_client.pympt_api_client/resources/__init__.pympt_api_client/resources/program/__init__.pympt_api_client/resources/program/mixins/__init__.pympt_api_client/resources/program/mixins/publishable_mixin.pympt_api_client/resources/program/program.pympt_api_client/resources/program/programs.pypyproject.tomltests/e2e/program/__init__.pytests/e2e/program/conftest.pytests/e2e/program/program/conftest.pytests/e2e/program/program/test_async_program.pytests/e2e/program/program/test_sync_program.pytests/unit/resources/program/mixin/test_publishable_mixin.pytests/unit/resources/program/test_program.pytests/unit/resources/program/test_programs.pytests/unit/test_mpt_client.py



This pull request introduces a new "Program" resource to the API client, including both sync and async support, resource mixins for publish/unpublish actions, and comprehensive tests. It also updates configuration and test files to integrate the new resource.
New Program Resource Integration
ProgramandAsyncProgrammodules to the client, exposing them through the mainMPTClientandAsyncMPTClientinterfaces. [1] [2] [3] [4] [5] [6] [7] [8] [9]Program Service and Model
ProgramsServiceandAsyncProgramsServicewith full CRUD, file upload, publish/unpublish, and settings update capabilities for theProgrammodel. The model includes attributes such as name, website, eligibility, and vendor.PublishableMixinandAsyncPublishableMixinto encapsulate publish/unpublish logic for resources, and exposed them in the package. [1] [2]Testing
Programresource actions, including create, update, get, filter/select, delete, publish, and unpublish, for both sync and async clients. [1] [2] [3] [4] [5]These changes collectively provide a robust, fully tested "Program" resource in the API client with all standard and custom actions.
Closes MPT-20323
Release Notes
ProgramandAsyncProgramresource classes with full API client integrationProgramsServiceandAsyncProgramsServiceproviding CRUD operations, file upload support, and settings managementPublishableMixinandAsyncPublishableMixinto encapsulate publish/unpublish operations for the Program resourceprogramproperty on bothMPTClientandAsyncMPTClientfor accessing program operationsupdate_settings()method to manage program-specific configuration via dedicated API endpoint