rmrk-team / rmrk-substrate

[WIP] RMRK Substrate pallets
https://rmrk.app
Other
74 stars 36 forks source link

Get all NFTs owned by account #263

Closed Szegoo closed 1 year ago

Szegoo commented 1 year ago

Closes: #253

This PR adds two new functions to the RmrkApi.

The reason why I separated the two is that if we were to unite the two the return type would be too long and the readability of the code would suffer a bit. And also we already have this kind of separation by having two separate functions in the RmrkApi, nft_by_id and nft_properties.

Both of these functions have pagination implemented since the list of the NFTs a user owns may become very large.

The change I have made in the integration test:

Szegoo commented 1 year ago

@ilionic I have added integration tests.

ilionic commented 1 year ago

Hey @Szegoo getting two failed integration tests:

  109 passing (7m)
  2 failing

  1) integration test: get owned NFTs
       "before all" hook for "fetch all NFTs owned by a user over multiple collections":
     Error: uniques.InUse

  2) integration test: get properties of owned NFTs
       "before all" hook for "fetch all the properites of the NFTs owned by a user over multiple collections":
     Error: uniques.InUse
Szegoo commented 1 year ago

Hey @Szegoo getting two failed integration tests:

Yes, this was a silly mistake, I used the same collection ids in the integration tests. Also, there seem to be some other fixes I need to do in the code.

Szegoo commented 1 year ago

@ilionic The integration tests should be passing now :)

Szegoo commented 1 year ago

@HashWarlock could you please review this?