Repository navigation
MPT-24263 add accounts services service - #388
Conversation
|
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: Essentials Run ID: 📒 Files selected for processing (8)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved⚙️ CodeRabbit configuration file Files:
🧠 Learnings (1)📚 Learning: 2026-02-02T13:05:41.144ZApplied to files:
🔇 Additional comments (8)
📝 WalkthroughWalkthroughAdded synchronous and asynchronous accounts services resources. The change defines service identity models, exposes account accessors, configures the services endpoint, and adds unit and end-to-end tests. ChangesAccounts services access
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The new read-only services client has synchronous, asynchronous, unit, and end-to-end coverage in the supplied context. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Full details: Documentation Up To DateExplanation The PR adds documented public behavior without updating the matching user documentation. The diff adds Resolution Update
Comment |
✅ Found Jira issue key in the title: MPT-24263 |
a30a3a0 to
69f1c19
Compare
550aa77 to
1201063
Compare
The accounts group had no service for the top-level
/public/v1/accounts/account-users and /public/v1/accounts/services
endpoints, so both collections were unreachable from MPTClient and
AsyncMPTClient.
Add AccountUsersService and ServicesService (plus their async mirrors)
following the existing resource-service pattern, and expose them as
`account_users` and `services` properties on both Accounts and
AsyncAccounts.
Mixins are picked from what the OpenAPI spec documents for each
endpoint rather than assuming full CRUD: account-users supports
list, create, get, delete and the invite actions but no update, and
services is read-only with list and get.
The new module is account_users.py, distinct from the pre-existing
accounts_users.py, which serves the nested
/public/v1/accounts/accounts/{account_id}/users resource.
Streaming support for these endpoints is tracked separately in
MPT-24241 and is not part of this change.
MPT-24263
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1201063 to
e86bc88
Compare
Cover the new ServicesService against TEST: get by id, 404 on an unknown id, paged listing and RQL filter with iterate, in both the sync and async flavours. Register the service identity the tests read in e2e_config.test.json. The account-users service gets no e2e coverage on purpose. The endpoint is tagged AccountUsersLegacy and marked deprecated in the OpenAPI spec served by TEST, and every route under /public/v1/accounts/account-users answers 404 there for the vendor, client and operations tokens, so no live test can exercise it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jentyk
changed the title
…ervices
Remove AccountUsersService and its async mirror before they ship. The
OpenAPI document served by TEST tags every /v1/accounts/account-users
operation AccountUsersLegacy with deprecated: true, and on TEST every
route under /public/v1/accounts/account-users answers 404 for the
vendor, client and operations tokens alike, so the service would target
an endpoint the platform no longer routes. The live replacement is the
nested /accounts/accounts/{accountId}/users family, which
AccountsUsersService already covers.
Add revision to the ServiceIdentity model to match ServiceQueryModel in
the spec and the sibling exchange models, which declare it too.
No published version exposed accounts.account_users, so this is not a
breaking change for consumers.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
f851d20 to
2505450
Compare
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
🤖 AI-generated PR — Please review carefully.
What was done
/public/v1/accounts/servicesis documented in the Marketplace OpenAPI spec but had no clientservice, so it was unreachable from
MPTClient/AsyncMPTClient. This PR adds it./public/v1/accounts/servicesmpt_api_client/resources/accounts/services.pyaccounts.servicesServicesService/AsyncServicesServiceThe module follows the existing resource-service pattern: a
Modelsubclass with thedocumented attributes, a shared
ServicesServiceConfig(_endpoint,_model_class,_collection_key = "data"), and a sync service plus its async mirror, exposed as@propertyon
AccountsandAsyncAccounts.Verified against the spec served by TEST
The swagger served by
api.s1.showdocumentsGETon the collection and on/{id}only, so theservice is read-only:
GetMixin+CollectionMixin, no create/update/delete. Live checks agree:POST,PUTandDELETEanswer405.ServiceIdentitydeclares everyServiceQueryModelproperty:name,status,description,icon,revision,audit. Streaming comes withCollectionMixin:stream_snapshot()onTEST returns all 16 service identities with a matching
MPT-Item-Count.The model is named
ServiceIdentityrather thanServicebecausempt_api_client.http.Serviceis the service base class imported in the same module. The spec itself calls the resource a
"service identity" (
List service identities/Get service identity by ID).Why
account-userswas dropped from this PRMPT-24263 also named
/public/v1/accounts/account-users. AnAccountUsersServicewas added inthe first commit and removed again after verification against TEST:
/v1/accounts/account-users*operationAccountUsersLegacyand marks itdeprecated: true;/public/v1/accounts/account-usersanswers404 Not Foundfor thevendor, client and operations tokens alike — paged and
MPT-Streamingcollection GET, GET byid, and POST with a valid body.
The platform no longer routes the endpoint, so shipping a client for it would give consumers a
service that cannot succeed. The non-deprecated replacement, the nested
/accounts/accounts/{accountId}/usersfamily, is already covered byaccounts_users.py(
AccountsUsersService), which is untouched.E2E tests
tests/e2e/accounts/services/coversServicesService/AsyncServicesServiceagainst TEST:get by id, 404 on an unknown id, paged listing, and RQL filter with
iterate(). The serviceidentity the tests read (
SVC-0001, theplatformservice) is registered asaccounts.service.idine2e_config.test.json. All 8 tests pass locally against TEST.Documentation
No docs were changed, deliberately. Neither
docs/usage.mdnordocs/architecture.mdenumerates individual services:
usage.mdhas no service list, and the resource tree inarchitecture.mdis group-granular with ellipses(
accounts/ # Account, Users, Buyers, Sellers, API Tokens, …), so the new service is alreadycovered by the existing text.
Testing
make checkpasses — ruff format, ruff, flake8/WPS, mypy,uv lock --check. Full unit suite:3422 passed, 99% coverage,
services.pyat 100%.Unit tests:
tests/unit/resources/accounts/test_services.py— path,getpresent, write methods absent.tests/unit/resources/accounts/test_accounts.py—servicesadded to the sync and asyncparametrized group-property lists.
No
pyproject.tomlchange was needed:mpt_api_client/resources/accounts/*.pyalready carriesWPS214/WPS235in the per-file ignores.MPT-24263
🤖 Generated with Claude Code
/public/v1/accounts/servicessupport toMPTClientandAsyncMPTClient.ServiceIdentity,ServicesService, andAsyncServicesService.Accounts.servicesandAsyncAccounts.services./accounts/account-usersservice.