[MPT-16678] Added missing billing statement attachments endpoints #176
Conversation
robcsegal
requested review from
albertsola,
alephsur,
d3rky,
jentyk,
ruben-sebrango and
svazquezco
as code owners
📝 WalkthroughWalkthroughAdds a Statement Attachments API client and accessor on Statements; consolidates multiple attachment-related mixins into unified AttachmentMixin/AsyncAttachmentMixin; updates several attachment service classes to use the new mixins; extends and adds unit tests to validate attachment methods and endpoints. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (14)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-12T15:02:20.732ZApplied to files:
🧬 Code graph analysis (6)tests/unit/resources/billing/test_ledger_attachments.py (2)
mpt_api_client/resources/billing/journal_attachments.py (1)
tests/unit/resources/billing/test_statements.py (1)
mpt_api_client/resources/billing/invoice_attachments.py (1)
mpt_api_client/resources/billing/mixins.py (1)
mpt_api_client/resources/billing/ledger_attachments.py (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (15)
Comment |
✅ Found Jira issue key in the title: MPT-16678 Tests mirroring check (created files only)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/resources/billing/test_statement_attachments.py (1)
23-38: Consider simplifying the assertion pattern.The tests use an intermediate variable to store the comparison result before asserting. This can be simplified to direct assertions for better readability.
🔎 Proposed simplification
def test_endpoint(statement_attachments_service) -> None: - result = ( - statement_attachments_service.path - == "/public/v1/billing/statements/STM-0000-0001/attachments" - ) - - assert result is True + assert ( + statement_attachments_service.path + == "/public/v1/billing/statements/STM-0000-0001/attachments" + ) def test_async_endpoint(async_statement_attachments_service) -> None: - result = ( - async_statement_attachments_service.path - == "/public/v1/billing/statements/STM-0000-0001/attachments" - ) - - assert result is True + assert ( + async_statement_attachments_service.path + == "/public/v1/billing/statements/STM-0000-0001/attachments" + )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
mpt_api_client/resources/billing/statement_attachments.pympt_api_client/resources/billing/statements.pytests/unit/resources/billing/test_statement_attachments.pytests/unit/resources/billing/test_statements.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T15:02:20.732Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 160
File: tests/e2e/commerce/agreement/attachment/test_async_agreement_attachment.py:55-58
Timestamp: 2025-12-12T15:02:20.732Z
Learning: In pytest with pytest-asyncio, if a test function uses async fixtures but contains no await, declare the test function as def (synchronous) instead of async def. Pytest-asyncio will resolve the async fixtures automatically; this avoids linter complaints about unnecessary async functions. This pattern applies to any test file under the tests/ directory that uses such fixtures.
Applied to files:
tests/unit/resources/billing/test_statement_attachments.pytests/unit/resources/billing/test_statements.py
🧬 Code graph analysis (3)
mpt_api_client/resources/billing/statements.py (8)
mpt_api_client/resources/billing/statement_attachments.py (2)
AsyncStatementAttachmentsService(46-56)StatementAttachmentsService(33-43)mpt_api_client/resources/billing/journals.py (2)
attachments(79-84)attachments(137-142)mpt_api_client/resources/commerce/agreements.py (2)
attachments(43-55)attachments(68-80)mpt_api_client/resources/catalog/pricing_policies.py (2)
attachments(38-43)attachments(80-85)mpt_api_client/resources/billing/credit_memos.py (2)
attachments(41-46)attachments(59-64)mpt_api_client/resources/billing/invoices.py (2)
attachments(41-46)attachments(59-64)mpt_api_client/resources/billing/custom_ledgers.py (2)
attachments(59-64)attachments(90-95)mpt_api_client/resources/billing/ledgers.py (2)
attachments(49-54)attachments(73-78)
tests/unit/resources/billing/test_statement_attachments.py (3)
mpt_api_client/resources/billing/statement_attachments.py (2)
AsyncStatementAttachmentsService(46-56)StatementAttachmentsService(33-43)tests/unit/conftest.py (2)
http_client(17-18)async_http_client(22-23)mpt_api_client/http/base_service.py (1)
path(28-30)
tests/unit/resources/billing/test_statements.py (2)
tests/unit/resources/billing/test_billing.py (1)
billing(23-24)mpt_api_client/resources/billing/statement_attachments.py (2)
AsyncStatementAttachmentsService(46-56)StatementAttachmentsService(33-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
tests/unit/resources/billing/test_statement_attachments.py (2)
9-20: LGTM!The fixtures correctly instantiate both synchronous and asynchronous statement attachments services with the required endpoint parameters.
41-52: LGTM!The parametrized tests correctly verify the presence of all six expected methods (get, create, update, delete, iterate, download) on both synchronous and asynchronous services. This aligns with the mixins used in the service definitions.
tests/unit/resources/billing/test_statements.py (2)
3-65: LGTM!The imports and parametrization updates correctly integrate the new statement attachments functionality into the existing test suite. The changes follow the established patterns and ensure comprehensive coverage.
68-93: LGTM!The test cases for the attachments accessor methods correctly verify service instantiation and parameter passing for both synchronous and asynchronous variants. The implementation mirrors the existing charges tests.
mpt_api_client/resources/billing/statements.py (2)
12-15: LGTM!The imports are correctly placed and necessary for the new attachments accessor methods.
51-81: LGTM!The attachments accessor methods are correctly implemented for both synchronous and asynchronous services. The implementation follows the established pattern used by the charges methods and is consistent with other attachment accessors throughout the codebase (ledgers, journals, invoices, etc.).
mpt_api_client/resources/billing/statement_attachments.py (4)
1-16: LGTM!All necessary imports are present for implementing the statement attachments service with full CRUD and file operations support.
19-20: LGTM!The StatementAttachment model follows the standard pattern used for attachment resources throughout the codebase.
23-30: LGTM!The service configuration is correctly defined with the appropriate endpoint pattern and standard configuration values for file upload operations.
33-56: LGTM!Both synchronous and asynchronous service classes are correctly implemented with all necessary mixins for complete CRUD and file operations (create, read, update, delete, iterate, download). The mixin composition and inheritance order follow established patterns and match the test expectations.
robcsegal
force-pushed
the
MPT-16678-add-missing-billing-statements-attachments-endpoints
branch
from
3657841 to
9392d9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mpt_api_client/resources/billing/credit_memo_attachments.py (1)
14-19: Missing upload configuration keys required byCreateFileMixin.The
CreditMemoAttachmentsServiceConfigclass inherits fromAttachmentMixin, which includesCreateFileMixin. TheCreateFileMixin.create()method requires_upload_file_keyand_upload_data_keyattributes to be defined, but they are missing from this config class. This will cause anAttributeErrorat runtime if thecreate()method is called with a file parameter.
JournalAttachmentsServiceConfigandStatementAttachmentsServiceConfigboth define these keys as_upload_file_key = "file"and_upload_data_key = "attachment". Add the same configuration toCreditMemoAttachmentsServiceConfig, or removeCreateFileMixinfromAttachmentMixinif credit memo attachments do not support file uploads.Note: This issue also affects
InvoiceAttachmentsServiceConfig,LedgerAttachmentsServiceConfig, andCustomLedgerAttachmentsServiceConfig.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
mpt_api_client/resources/billing/credit_memo_attachments.pympt_api_client/resources/billing/custom_ledger_attachments.pympt_api_client/resources/billing/invoice_attachments.pympt_api_client/resources/billing/journal_attachments.pympt_api_client/resources/billing/ledger_attachments.pympt_api_client/resources/billing/mixins.pympt_api_client/resources/billing/statement_attachments.pympt_api_client/resources/billing/statements.pytests/unit/resources/billing/test_credit_memo_attachments.pytests/unit/resources/billing/test_custom_ledger_attachments.pytests/unit/resources/billing/test_invoice_attachments.pytests/unit/resources/billing/test_ledger_attachments.pytests/unit/resources/billing/test_statement_attachments.pytests/unit/resources/billing/test_statements.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T15:02:20.732Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 160
File: tests/e2e/commerce/agreement/attachment/test_async_agreement_attachment.py:55-58
Timestamp: 2025-12-12T15:02:20.732Z
Learning: In pytest with pytest-asyncio, if a test function uses async fixtures but contains no await, declare the test function as def (synchronous) instead of async def. Pytest-asyncio will resolve the async fixtures automatically; this avoids linter complaints about unnecessary async functions. This pattern applies to any test file under the tests/ directory that uses such fixtures.
Applied to files:
tests/unit/resources/billing/test_statements.pytests/unit/resources/billing/test_custom_ledger_attachments.pytests/unit/resources/billing/test_ledger_attachments.pytests/unit/resources/billing/test_credit_memo_attachments.pytests/unit/resources/billing/test_invoice_attachments.pytests/unit/resources/billing/test_statement_attachments.py
🧬 Code graph analysis (14)
tests/unit/resources/billing/test_statements.py (1)
mpt_api_client/resources/billing/statement_attachments.py (2)
AsyncStatementAttachmentsService(33-39)StatementAttachmentsService(24-30)
tests/unit/resources/billing/test_custom_ledger_attachments.py (16)
tests/unit/resources/billing/test_credit_memo_attachments.py (1)
test_methods_present(42-45)tests/unit/resources/billing/test_invoice_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/billing/test_ledger_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/billing/test_statement_attachments.py (1)
test_methods_present(42-45)tests/unit/resources/billing/test_journal_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/commerce/test_subscriptions.py (1)
test_methods_present(22-25)tests/unit/resources/catalog/test_price_list_items.py (1)
test_methods_present(40-43)tests/unit/resources/catalog/test_pricing_policy_attachments.py (1)
test_methods_present(44-47)tests/unit/resources/catalog/test_product_term_variants.py (1)
test_methods_present(47-50)tests/unit/resources/catalog/test_product_terms.py (1)
test_methods_present(42-45)tests/unit/resources/catalog/test_products_documents.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_media.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_item_groups.py (1)
test_methods_present(34-37)tests/unit/resources/catalog/test_products_parameter_groups.py (1)
test_methods_present(39-42)tests/unit/resources/catalog/test_products_parameters.py (1)
test_methods_present(34-37)tests/unit/resources/commerce/test_agreements_attachments.py (1)
test_methods_present(36-39)
mpt_api_client/resources/billing/credit_memo_attachments.py (1)
mpt_api_client/resources/billing/mixins.py (2)
AsyncAttachmentMixin(419-426)AttachmentMixin(409-416)
mpt_api_client/resources/billing/custom_ledger_attachments.py (1)
mpt_api_client/resources/billing/mixins.py (2)
AsyncAttachmentMixin(419-426)AttachmentMixin(409-416)
mpt_api_client/resources/billing/journal_attachments.py (1)
mpt_api_client/resources/billing/mixins.py (2)
AsyncAttachmentMixin(419-426)AttachmentMixin(409-416)
mpt_api_client/resources/billing/ledger_attachments.py (1)
mpt_api_client/resources/billing/mixins.py (2)
AsyncAttachmentMixin(419-426)AttachmentMixin(409-416)
mpt_api_client/resources/billing/statements.py (4)
mpt_api_client/resources/billing/statement_attachments.py (2)
AsyncStatementAttachmentsService(33-39)StatementAttachmentsService(24-30)mpt_api_client/resources/billing/journals.py (2)
attachments(79-84)attachments(137-142)mpt_api_client/resources/billing/invoices.py (2)
attachments(41-46)attachments(59-64)mpt_api_client/resources/billing/ledgers.py (2)
attachments(49-54)attachments(73-78)
tests/unit/resources/billing/test_ledger_attachments.py (16)
tests/unit/resources/billing/test_credit_memo_attachments.py (1)
test_methods_present(42-45)tests/unit/resources/billing/test_custom_ledger_attachments.py (1)
test_methods_present(40-43)tests/unit/resources/billing/test_invoice_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/billing/test_statement_attachments.py (1)
test_methods_present(42-45)tests/unit/resources/billing/test_journal_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/commerce/test_subscriptions.py (1)
test_methods_present(22-25)tests/unit/resources/catalog/test_price_list_items.py (1)
test_methods_present(40-43)tests/unit/resources/catalog/test_pricing_policy_attachments.py (1)
test_methods_present(44-47)tests/unit/resources/catalog/test_product_term_variants.py (1)
test_methods_present(47-50)tests/unit/resources/catalog/test_product_terms.py (1)
test_methods_present(42-45)tests/unit/resources/catalog/test_products_documents.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_media.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_item_groups.py (1)
test_methods_present(34-37)tests/unit/resources/catalog/test_products_parameter_groups.py (1)
test_methods_present(39-42)tests/unit/resources/catalog/test_products_parameters.py (1)
test_methods_present(34-37)tests/unit/resources/commerce/test_agreements_attachments.py (1)
test_methods_present(36-39)
tests/unit/resources/billing/test_credit_memo_attachments.py (16)
tests/unit/resources/billing/test_custom_ledger_attachments.py (1)
test_methods_present(40-43)tests/unit/resources/billing/test_invoice_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/billing/test_ledger_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/billing/test_statement_attachments.py (1)
test_methods_present(42-45)tests/unit/resources/billing/test_journal_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/commerce/test_subscriptions.py (1)
test_methods_present(22-25)tests/unit/resources/catalog/test_price_list_items.py (1)
test_methods_present(40-43)tests/unit/resources/catalog/test_pricing_policy_attachments.py (1)
test_methods_present(44-47)tests/unit/resources/catalog/test_product_term_variants.py (1)
test_methods_present(47-50)tests/unit/resources/catalog/test_product_terms.py (1)
test_methods_present(42-45)tests/unit/resources/catalog/test_products_documents.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_media.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_item_groups.py (1)
test_methods_present(34-37)tests/unit/resources/catalog/test_products_parameter_groups.py (1)
test_methods_present(39-42)tests/unit/resources/catalog/test_products_parameters.py (1)
test_methods_present(34-37)tests/unit/resources/commerce/test_agreements_attachments.py (1)
test_methods_present(36-39)
mpt_api_client/resources/billing/mixins.py (2)
mpt_api_client/http/mixins.py (10)
AsyncCreateFileMixin(187-216)AsyncDeleteMixin(273-283)AsyncDownloadFileMixin(303-325)AsyncGetMixin(380-395)AsyncUpdateMixin(286-300)CreateFileMixin(115-144)DeleteMixin(29-38)DownloadFileMixin(58-80)GetMixin(361-377)UpdateMixin(41-55)mpt_api_client/models/model.py (1)
Model(65-125)
mpt_api_client/resources/billing/invoice_attachments.py (1)
mpt_api_client/resources/billing/mixins.py (2)
AsyncAttachmentMixin(419-426)AttachmentMixin(409-416)
tests/unit/resources/billing/test_invoice_attachments.py (16)
tests/unit/resources/billing/test_credit_memo_attachments.py (1)
test_methods_present(42-45)tests/unit/resources/billing/test_custom_ledger_attachments.py (1)
test_methods_present(40-43)tests/unit/resources/billing/test_ledger_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/billing/test_statement_attachments.py (1)
test_methods_present(42-45)tests/unit/resources/billing/test_journal_attachments.py (1)
test_methods_present(41-44)tests/unit/resources/commerce/test_subscriptions.py (1)
test_methods_present(22-25)tests/unit/resources/catalog/test_price_list_items.py (1)
test_methods_present(40-43)tests/unit/resources/catalog/test_pricing_policy_attachments.py (1)
test_methods_present(44-47)tests/unit/resources/catalog/test_product_term_variants.py (1)
test_methods_present(47-50)tests/unit/resources/catalog/test_product_terms.py (1)
test_methods_present(42-45)tests/unit/resources/catalog/test_products_documents.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_media.py (1)
test_methods_present(37-40)tests/unit/resources/catalog/test_products_item_groups.py (1)
test_methods_present(34-37)tests/unit/resources/catalog/test_products_parameter_groups.py (1)
test_methods_present(39-42)tests/unit/resources/catalog/test_products_parameters.py (1)
test_methods_present(34-37)tests/unit/resources/commerce/test_agreements_attachments.py (1)
test_methods_present(36-39)
tests/unit/resources/billing/test_statement_attachments.py (3)
mpt_api_client/resources/billing/statement_attachments.py (2)
AsyncStatementAttachmentsService(33-39)StatementAttachmentsService(24-30)tests/unit/conftest.py (2)
http_client(17-18)async_http_client(22-23)mpt_api_client/http/base_service.py (1)
path(28-30)
mpt_api_client/resources/billing/statement_attachments.py (5)
mpt_api_client/http/async_service.py (1)
AsyncService(12-80)mpt_api_client/http/service.py (1)
Service(12-80)mpt_api_client/http/mixins.py (2)
AsyncCollectionMixin(578-645)CollectionMixin(509-575)mpt_api_client/models/model.py (1)
Model(65-125)mpt_api_client/resources/billing/mixins.py (2)
AsyncAttachmentMixin(419-426)AttachmentMixin(409-416)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (21)
tests/unit/resources/billing/test_invoice_attachments.py (1)
40-51: LGTM!The expanded parametrization correctly tests for the newly added
iterateanddownloadmethods from theAttachmentMixin. The test structure is consistent with other attachment test files across the codebase.tests/unit/resources/billing/test_custom_ledger_attachments.py (1)
39-50: LGTM!The test coverage correctly extends to include
iterateanddownloadmethods, aligning with theAttachmentMixincapabilities now used by the custom ledger attachments service.tests/unit/resources/billing/test_credit_memo_attachments.py (1)
41-52: LGTM!Test parametrization correctly extended to verify
iterateanddownloadmethods for both sync and async credit memo attachments services.tests/unit/resources/billing/test_statement_attachments.py (1)
1-52: LGTM!Well-structured new test file for statement attachments. The test coverage includes:
- Endpoint path verification for both sync and async services
- Method presence checks for all attachment operations (
get,create,update,delete,iterate,download)The file follows the established testing patterns from other attachment service tests.
tests/unit/resources/billing/test_statements.py (3)
3-6: LGTM!Correctly imports the new
StatementAttachmentsServiceandAsyncStatementAttachmentsServiceclasses to support the new test cases.
26-38: LGTM!The parametrized method lists are properly extended to include
attachmentsandcharges, verifying the statement services expose these accessor methods.Also applies to: 48-60
72-93: LGTM!The property service tests correctly verify that:
attachments()returns the appropriateStatementAttachmentsService/AsyncStatementAttachmentsService- The
endpoint_paramsare correctly set withstatement_idtests/unit/resources/billing/test_ledger_attachments.py (1)
40-51: LGTM!Test parametrization correctly extended to verify
iterateanddownloadmethods for ledger attachments services, consistent with the newAttachmentMixincapabilities.mpt_api_client/resources/billing/mixins.py (2)
1-12: LGTM!Clean import additions for the mixin components that will be composed into the new
AttachmentMixinandAsyncAttachmentMixinclasses.
409-426: LGTM!The new
AttachmentMixinandAsyncAttachmentMixinclasses effectively consolidate attachment-related operations:
CreateFileMixin/AsyncCreateFileMixinfor file uploadsUpdateMixin/AsyncUpdateMixinfor metadata updatesDeleteMixin/AsyncDeleteMixinfor removalDownloadFileMixin/AsyncDownloadFileMixinfor file downloadsGetMixin/AsyncGetMixinfor retrievalThis reduces duplication across multiple attachment service implementations.
mpt_api_client/resources/billing/invoice_attachments.py (2)
7-7: LGTM!Correctly imports the new consolidated attachment mixins from the billing mixins module.
22-37: LGTM!The service classes now use the consolidated
AttachmentMixinandAsyncAttachmentMixin, which simplifies the inheritance chain while maintaining full attachment functionality. The generic type parameterInvoiceAttachmentis correctly applied.mpt_api_client/resources/billing/credit_memo_attachments.py (1)
22-28: Mixin refactor looks good.The switch from individual mixins to the consolidated
AttachmentMixinandAsyncAttachmentMixinis consistent with the pattern established inmpt_api_client/resources/billing/mixins.py(lines 408-425). This provides a cleaner inheritance hierarchy while maintaining the same functionality (create, update, delete, download, get).Also applies to: 31-37
mpt_api_client/resources/billing/statements.py (2)
12-15: Import addition looks correct.The import of
StatementAttachmentsServiceandAsyncStatementAttachmentsServicealigns with the new statement attachments module being introduced.
51-56: Attachments accessor methods follow established patterns.Both synchronous and asynchronous
attachments()methods mirror the existingcharges()method pattern and are consistent with similar accessors in other billing modules (e.g.,ledgers.py:48-53,journals.py:78-83,invoices.py:40-45). The implementation correctly passeshttp_clientandendpoint_paramswith the appropriatestatement_idkey.Also applies to: 76-81
mpt_api_client/resources/billing/journal_attachments.py (1)
7-7: Clean refactor to AttachmentMixin.The migration from individual file-operation mixins to the unified
AttachmentMixinandAsyncAttachmentMixinsimplifies the inheritance chain while maintaining equivalent functionality. The configuration properly includes_upload_file_keyand_upload_data_keyfor file upload support.Also applies to: 24-30, 33-39
mpt_api_client/resources/billing/statement_attachments.py (1)
1-39: Well-structured new attachment module.The new
statement_attachments.pymodule follows the established patterns for billing attachment services:
- Complete configuration with
_endpoint,_model_class,_collection_key,_upload_file_key, and_upload_data_key- Proper inheritance from
AttachmentMixin/AsyncAttachmentMixin,CollectionMixin/AsyncCollectionMixin, andService/AsyncService- Consistent with other attachment modules like
journal_attachments.pympt_api_client/resources/billing/custom_ledger_attachments.py (2)
14-19: Potential missing upload configuration keys.Similar to
CreditMemoAttachmentsServiceConfig, this config is missing_upload_file_keyand_upload_data_key. TheAttachmentMixinincludesCreateFileMixinwhich may require these keys for file upload operations. Compare withJournalAttachmentsServiceConfigandStatementAttachmentsServiceConfigwhich define these keys.Verify whether this is intentional or if the keys should be added.
22-28: Mixin refactor is consistent with other modules.The service classes correctly inherit from the consolidated
AttachmentMixinandAsyncAttachmentMixin, maintaining consistency with the broader refactoring effort.Also applies to: 31-37
mpt_api_client/resources/billing/ledger_attachments.py (2)
14-19: Missing upload configuration keys - same pattern as other attachment configs.This is the third attachment config missing
_upload_file_keyand_upload_data_key(along withCreditMemoAttachmentsServiceConfigandCustomLedgerAttachmentsServiceConfig). If theCreateFileMixinfromAttachmentMixinrequires these keys for file upload, they should be added for consistency.🔎 Suggested addition if uploads are supported
class LedgerAttachmentsServiceConfig: """Ledger Attachments service configuration.""" _endpoint = "/public/v1/billing/ledgers/{ledger_id}/attachments" _model_class = LedgerAttachment _collection_key = "data" + _upload_file_key = "file" + _upload_data_key = "attachment"
22-28: Mixin migration is correct.The refactor to use
AttachmentMixinandAsyncAttachmentMixinis implemented correctly and consistently with other attachment modules.Also applies to: 31-37
robcsegal
force-pushed
the
MPT-16678-add-missing-billing-statements-attachments-endpoints
branch
from
9392d9e to
74e3a58
Compare
|
d3rky
deleted the
MPT-16678-add-missing-billing-statements-attachments-endpoints
branch
Added missing billing statement attachments endpoints Even though mixins will be re-organized, I added an attachment mixin because there's almost half a dozen of attachment resources. https://softwareone.atlassian.net/browse/MPT-16678 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Added statement attachments support with upload/download and create/list/get/delete operations (sync and async). * Added attachments accessors on statements to obtain per-statement attachment services. * **Refactor** * Replaced multiple per-operation attachment mixins with unified attachment mixins for consistent behavior across attachment services. * **Tests** * Expanded unit tests for statement attachments and added iterate/download checks across attachment services. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Added missing billing statement attachments endpoints
Even though mixins will be re-organized, I added an attachment mixin because there's almost half a dozen of attachment resources.
https://softwareone.atlassian.net/browse/MPT-16678
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.