Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: List NFT collections from account details. #721

Merged
merged 14 commits into from
Jan 24, 2024
Merged

feat: List NFT collections from account details. #721

merged 14 commits into from
Jan 24, 2024

Conversation

Sheng-Long
Copy link
Contributor

@Sheng-Long Sheng-Long commented Oct 11, 2023 •
edited
Loading

Description:
Add Nft list and links to the token balances from account details

Related issue(s):
#525

Fixes #

Notes for reviewer:
PR is built and depends on #692 , 692 should be merged first

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@github-actions
Copy link

github-actions bot commented Oct 11, 2023 •
edited
Loading

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 85.39% 33974 / 39785
🔵 Statements 85.39% 33974 / 39785
🔵 Functions 65.36% 806 / 1233
🔵 Branches 84.25% 3795 / 4504
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
src/router.ts 90.1% 34.48% 62.5% 90.1% 120-123, 234-237, 272, 288-289, 291-292, 294-295, 297-298, 300-301, 303-304, 306-307, 309-310, 312-313, 315-316, 318-319, 321-322, 324, 346-347, 381, 390-391
src/components/account/BalanceTable.vue 96.96% 100% 50% 96.96% 104-107
src/components/account/CollectionTable.vue 75.38% 100% 0% 75.38% 88-119
src/components/account/CollectionTableController.ts 31.39% 0% 0% 31.39% 28-86
src/components/account/NftsTable.vue 96.63% 80% 50% 96.63% 96-99
src/components/allowances/ApproveAllowanceDialog.vue 45.89% 51.51% 5.4% 45.89% 53-56, 100-108, 110-113, 135-143, 145-148, 167-169, 190, 292-293, 300, 314-328, 332-345, 350-353, 357-387, 391-401, 407-417, 420-424, 427-437, 440-444, 447-457, 460-465, 476-494, 514-515, 518-520, 523-587, 590-623, 626-649, 652-677, 680-711, 714-760, 763-813, 816-832
src/pages/AccountBalances.vue 100% 78.57% 100% 100%
src/pages/AccountCollection.vue 79.83% 100% 0% 79.83% 91-114
src/pages/AccountDetails.vue 96.81% 83.49% 100% 96.81% 48-52, 389-390, 403-404, 422-425, 469-470, 483-484
src/pages/ContractDetails.vue 98.93% 91.35% 100% 98.93% 324-327
src/utils/RouteManager.ts 73.98% 95.52% 66.66% 73.98% 64, 86-89, 159-168, 171-180, 198-199, 218-227, 234-237, 240-241, 262-271, 274-278, 281-285, 288-297, 300-309, 320-329, 340-349, 360-369, 380-389, 396-401, 404-405, 416-417, 456-457
src/utils/cache/CacheUtils.ts 100% 100% 100% 100%
src/utils/cache/NftCollectionCache.ts 100% 82.35% 100% 100%
Generated in workflow #666

@Sheng-Long Sheng-Long force-pushed the 00525-NftLinkFromAccount branch 4 times, most recently from e058663 to 3972a0c Compare October 16, 2023 16:28
@Sheng-Long
Copy link
Contributor Author

image

@Sheng-Long
Copy link
Contributor Author

Sheng-Long commented Oct 16, 2023 •
edited
Loading

The above (Table "NFTs owned by") links directly to NftDetails

@Sheng-Long Sheng-Long force-pushed the 00525-NftLinkFromAccount branch 12 times, most recently from 4bca91c to 4aaad8a Compare October 19, 2023 22:36
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 14 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.17) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@Sheng-Long Sheng-Long linked an issue Nov 16, 2023 that may be closed by this pull request
Show NFT Collections #525
Closed
@Sheng-Long
Copy link
Contributor Author

Notes from impromptu review:

Bug: On first load there is an issue with the tokens being displayed, it shows the same tokenID for every collection. This is resolved on refresh

Feature: Collapsible collections on balance page, ellipsis if collection contains too many serial numbers

Feature: New separate page for individual collections owned by account, linked to from the balance page (show all serials owned by account in a certain collection)

@Sheng-Long Sheng-Long force-pushed the 00525-NftLinkFromAccount branch from 4aaad8a to a102bbe Compare December 5, 2023 13:35
@Sheng-Long Sheng-Long force-pushed the 00525-NftLinkFromAccount branch from bfab57c to 057cd0f Compare January 11, 2024 02:15
@ericleponner ericleponner force-pushed the 00525-NftLinkFromAccount branch from 057cd0f to 9311752 Compare January 23, 2024 08:31
@ericleponner ericleponner force-pushed the 00525-NftLinkFromAccount branch from 9311752 to d0ba728 Compare January 23, 2024 10:16
ericleponner and others added 11 commits January 23, 2024 12:34
@ericleponner
Added NftCollectionCache (to be used later).
9af86d0 
Signed-off-by: Eric Le Ponner <[email protected]>
@ericleponner
NftsTable vue now uses NftCollectionCache in place of NftsTableContro…
66a5934 
…ller.

Signed-off-by: Eric Le Ponner <[email protected]>
@ericleponner
Extended AccountBalances unit test (to be validate NftCollectionCache).
3d6ea14 
Signed-off-by: Eric Le Ponner <[email protected]>
@ericleponner
Fixed warning reported by esbuild in ApproveAllowanceDialog vue.
fe67a1e 
Signed-off-by: Eric Le Ponner <[email protected]>
@svienot
'Balance' column -> 'Balance/Nb of NFTs' in BalanceTable.
495f03e 
Signed-off-by: Simon Viénot <[email protected]>
@svienot
'Token Balances for Account' -> 'All Tokens Owned by' in AccountBalan…
ec96e53 
…ces.

Signed-off-by: Simon Viénot <[email protected]>
@svienot
AccountDetails and ContractDetails always display at least a subset o…
8cc2420 
…f the tokens with non-zero balance.

Signed-off-by: Simon Viénot <[email protected]>
@svienot
Display token symbol and account ID in title of AccountCollection.
db443b1 
Signed-off-by: Simon Viénot <[email protected]>
@svienot
Adjust AccountBalances unit test.
f7d40b1 
Signed-off-by: Simon Viénot <[email protected]>
@svienot
Adjust AccountNavigation e2e test.
cd35ba4 
Signed-off-by: Simon Viénot <[email protected]>
@svienot
Change route: '/:network/accountbalances/:accountId/:tokenId'
08e54f8 
to: '/:network/accountcollection/:accountId/:tokenId'

And fix unit test.

Signed-off-by: Simon Viénot <[email protected]>
svienot
svienot approved these changes Jan 24, 2024
View reviewed changes
Copy link
Collaborator

@svienot svienot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions

0.0% Coverage on New Code (required ≥ 80%)
5.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@svienot svienot changed the title 00525 nft link from account feat: List NFT collections from account details. Jan 24, 2024
ericleponner
ericleponner approved these changes Jan 24, 2024
View reviewed changes
Copy link
Collaborator

@ericleponner ericleponner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@ericleponner ericleponner merged commit e32fc94 into main Jan 24, 2024
6 of 7 checks passed
@ericleponner ericleponner deleted the 00525-NftLinkFromAccount branch January 24, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericleponner ericleponner ericleponner approved these changes

@svienot svienot svienot approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Show NFT Collections
3 participants
@Sheng-Long @svienot @ericleponner