Feature: Voucher integrations #225
Conversation
| @@ -61,6 +61,7 @@ | |||
| safe_getattr, | |||
| ) | |||
| from .abstract import AlephClient | |||
| from .services.voucher import Vouchers | |||
There was a problem hiding this comment.
I think this import should be in the same form than other ones below:
https://github.com/aleph-im/aleph-sdk-python/pull/225/files#diff-98b2f220317fe0379eef848b853313affd30595845e3c4483da9fdb54cdc3a4bR36-R40
| "https://claim.twentysix.cloud/sbt/metadata/{}.json" | ||
| ) | ||
| VOUCHER_SOL_REGISTRY: str = "https://api.claim.twentysix.cloud/v1/registry/sol" | ||
| VOUCHER_SENDER: str = "0xB34f25f2c935bCA437C061547eA12851d719dEFb" |
There was a problem hiding this comment.
I think that instead VOUCHER_SENDER should be something like VOUCHER_ORIGIN_ADDRESS
| VOUCHER_SENDER: str = "0xB34f25f2c935bCA437C061547eA12851d719dEFb" | |
| VOUCHER_ORIGIN_ADDRESS: str = "0xB34f25f2c935bCA437C061547eA12851d719dEFb" |
There was a problem hiding this comment.
These test are useless because are mocking everything. Instead to be useful needs to instantiate a real Account and check that method returns the result expected.
There was a problem hiding this comment.
Well logic of Voucher is tested,
The logic of voucher is still tested since:
- We mocked the get_posts to return mocked_data
- We mocked the accounts (where it's only to get address.get_address) to match the mocked address
- We mocked the http request to metdata / solana voucher
But we do not mocked any logic of voucher they still process just with mocked data
There was a problem hiding this comment.
The thing is that on vouchers we use more logic from the account side and inside the implementation, so instead of mocking everything, it's good to test also other parts of the logic at the same time if it doesn't involve infrastructure calls like DB or HTTP calls.
|
|
||
| @pytest.mark.asyncio | ||
| async def test_get_evm_vouchers(mock_post_response, make_mock_aiohttp_session): | ||
| mock_client = MagicMock() |
There was a problem hiding this comment.
Why mocking the entire client? Instead just mock the method you need to get the results to ensure that there is not an error on the instantiation or other calls.
There was a problem hiding this comment.
Well yeah could only mock get_posts but since it's only this func i did it light that
Will fix i
| with patch( | ||
| "aiohttp.ClientSession", side_effect=[registry_session, metadata_session] | ||
| ): |
There was a problem hiding this comment.
Same as above, if you already mock the aiohttp call, is not needed to mock the Client.
There was a problem hiding this comment.
I mocked this client since it's not used on that specific case
we call :
https://api.claim.twentysix.cloud/v1/registry/sol
(in evm case we call .get_posts but here client is not even used)
So did though it's was usefull to put an Actual client but i can change, it's don't have any impacts
There was a problem hiding this comment.
It's not a good pattern to mock everything, we need to mock only elements not involved on the logic we want to test, like http responses, calls to database, etc.
Related to ALEPH-542
This pull request introduces a new voucher service for managing and retrieving vouchers across EVM and Solana chains, along with tests for the functionality. The most significant changes include the creation of
AuthenticatedVoucherandVouchersclasses, the addition of voucher-related types, configuration updates, and comprehensive unit tests.Voucher Service Implementation:
src/aleph/sdk/client/services/authenticated_voucher.py: Added theAuthenticatedVoucherclass, which extendsVouchers. It allows retrieval of vouchers without explicitly passing an address by using the account address as a fallback. This includes methods for fetching vouchers (get_vouchers,get_evm_vouchers,get_solana_vouchers) and resolving addresses.src/aleph/sdk/client/services/voucher.py: Introduced theVouchersclass for fetching voucher data from EVM and Solana chains. It includes utilities for resolving addresses, fetching metadata, and retrieving vouchers by chain.Voucher Types and Configuration:
src/aleph/sdk/types.py: AddedVoucher,VoucherMetadata, andVoucherAttributetypes to represent voucher data and metadata. These types encapsulate attributes like name, description, external URL, image, and traits.src/aleph/sdk/conf.py: Added configuration settings for voucher metadata URLs, Solana registry URLs, and the voucher sender address. These settings are used by the voucher services to interact with external resources.Unit Tests:
tests/unit/services/test_authenticated_voucher.py: Added tests for theAuthenticatedVoucherclass, verifying address resolution logic and voucher retrieval for both EVM and Solana chains. Tests ensure fallback mechanisms work correctly when no address is provided.tests/unit/services/test_voucher.py: Added tests for theVouchersclass, covering metadata fetching, voucher retrieval by chain, and chain detection logic for both EVM and Solana addresses.