MPT-13307: implement exchange currencies resource#274
Merged
albertsola merged 1 commit intomainfrom Apr 2, 2026
Merged
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: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds an exchange resource package with a Currency model, sync/async Currencies services (CRUD + icon download), and Exchange/AsyncExchange accessors on both client classes. Updates package exports, lint ignore rules, and adds unit tests for the new resources. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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-13307/exchange-currencies-resource
branch
from
b3f1274 to
d1d4429
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/unit/resources/exchange/test_currencies.py (1)
85-270: Consider extracting repeated endpoint strings into fixtures/constants.This would reduce duplication and make future endpoint changes easier to maintain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/resources/exchange/test_currencies.py` around lines 85 - 270, Many tests repeat the same hardcoded URLs (e.g., "https://api.example.com/public/v1/exchange/currencies/{currency_id}" and the "/icon" variant) which should be centralized; add a module-level constant (e.g., CURRENCIES_BASE = "https://api.example.com/public/v1/exchange/currencies") or a fixture (e.g., currencies_endpoint(currency_id) and currency_icon_endpoint(currency_id)) in this test module or conftest, then update each test (functions like test_get_currency, test_async_get_currency, test_create_currency, test_update_currency, test_delete_currency, test_download_icon_returns_file_model, etc.) to use the constant/fixture instead of embedding the raw string so future URL changes are made in one place.
🤖 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/unit/resources/exchange/test_currencies.py`:
- Around line 85-270: Many tests repeat the same hardcoded URLs (e.g.,
"https://api.example.com/public/v1/exchange/currencies/{currency_id}" and the
"/icon" variant) which should be centralized; add a module-level constant (e.g.,
CURRENCIES_BASE = "https://api.example.com/public/v1/exchange/currencies") or a
fixture (e.g., currencies_endpoint(currency_id) and
currency_icon_endpoint(currency_id)) in this test module or conftest, then
update each test (functions like test_get_currency, test_async_get_currency,
test_create_currency, test_update_currency, test_delete_currency,
test_download_icon_returns_file_model, etc.) to use the constant/fixture instead
of embedding the raw string so future URL changes are made in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 52f541a9-e019-445b-866b-8796eb3032e7
📒 Files selected for processing (8)
mpt_api_client/mpt_client.pympt_api_client/resources/__init__.pympt_api_client/resources/exchange/__init__.pympt_api_client/resources/exchange/currencies.pympt_api_client/resources/exchange/exchange.pypyproject.tomltests/unit/resources/exchange/__init__.pytests/unit/resources/exchange/test_currencies.py
- Add Currency model with all fields (name, code, precision, statistics, status, icon, revision, audit) - Add CurrenciesService and AsyncCurrenciesService with list, get, create, update, delete and download_icon operations - Add Exchange and AsyncExchange module classes - Register exchange module on MPTClient and AsyncMPTClient - Add per-file-ignores for exchange module in pyproject.toml - Add unit tests following AAA and single-assertion practices Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
albertsola
force-pushed
the
MPT-13307/exchange-currencies-resource
branch
from
d1d4429 to
cb368b2
Compare
|
robcsegal
approved these changes
Apr 2, 2026
This was referenced Apr 2, 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
2 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
exchangedomain module with full support for the Currency endpoints exposed by the MPT API (/public/v1/exchange/currencies).Changes
New files
mpt_api_client/resources/exchange/currencies.py—Currencymodel +CurrenciesService/AsyncCurrenciesServicempt_api_client/resources/exchange/exchange.py—Exchange/AsyncExchangemodule classesmpt_api_client/resources/exchange/__init__.pytests/unit/resources/exchange/test_currencies.py— 31 unit testsUpdated files
mpt_api_client/resources/__init__.py— exportsExchange,AsyncExchangempt_api_client/mpt_client.py— adds.exchangeproperty toMPTClientandAsyncMPTClientpyproject.toml— per-file-ignores for theexchangemoduleAPI Coverage
/public/v1/exchange/currenciesiterate()/public/v1/exchange/currenciescreate(data, file=icon)/public/v1/exchange/currencies/{id}get(id)/public/v1/exchange/currencies/{id}update(id, data, file=icon)/public/v1/exchange/currencies/{id}delete(id)/public/v1/exchange/currencies/{id}/icondownload_icon(id)Testing
make check-allclean (ruff, flake8, mypy)Closes MPT-13307
exchangedomain module with full support for Currency endpoints (/public/v1/exchange/currencies)Currencymodel with typed fields:name,code,precision,statistics,status,icon,revision, andauditCurrenciesServiceandAsyncCurrenciesServicewith sync/async support for list (iterate), create (with icon upload), get, update (with icon upload), delete, and icon download endpointsExchangeandAsyncExchangemodule classes exposing acurrenciesproperty.exchangeproperty toMPTClientandAsyncMPTClientfor accessing the exchange resourcesExchangeandAsyncExchangeviampt_api_client.resourcesand packagempt_api_client.resources.exchange