From f3e481e9cc7a315e3fa137c5af1545de3ce09f40 Mon Sep 17 00:00:00 2001 From: Martin Cayuelas Date: Tue, 19 Nov 2024 16:28:01 +0100 Subject: [PATCH] =?UTF-8?q?bugfix:=20=F0=9F=90=9B=20Spam=20filtering=20err?= =?UTF-8?q?ors=20unhandled?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/silly-seals-argue.md | 7 +++ .../hooks/nfts/useSyncNFTsWithAccounts.ts | 6 +- .../SimpleHashTools/SpamScore/index.tsx | 2 +- .../src/hooks/nfts/useSyncNFTsWithAccounts.ts | 6 +- .../__tests__/useCheckNftAccount.test.tsx | 55 ++++++++++++++++++- .../src/hooks/useCheckNftAccount.ts | 15 ++--- libs/live-nft-react/src/tools/helperTests.tsx | 20 +++---- 7 files changed, 86 insertions(+), 25 deletions(-) create mode 100644 .changeset/silly-seals-argue.md diff --git a/.changeset/silly-seals-argue.md b/.changeset/silly-seals-argue.md new file mode 100644 index 000000000000..6a4fc5833a23 --- /dev/null +++ b/.changeset/silly-seals-argue.md @@ -0,0 +1,7 @@ +--- +"ledger-live-desktop": minor +"live-mobile": minor +"@ledgerhq/live-nft-react": minor +--- + +Fix undefined join on Spam filter diff --git a/apps/ledger-live-desktop/src/renderer/hooks/nfts/useSyncNFTsWithAccounts.ts b/apps/ledger-live-desktop/src/renderer/hooks/nfts/useSyncNFTsWithAccounts.ts index b795bb62fc0c..e9f06ead91c2 100644 --- a/apps/ledger-live-desktop/src/renderer/hooks/nfts/useSyncNFTsWithAccounts.ts +++ b/apps/ledger-live-desktop/src/renderer/hooks/nfts/useSyncNFTsWithAccounts.ts @@ -66,12 +66,12 @@ export function useSyncNFTsWithAccounts() { const [, setCurrentIndex] = useState(0); const { refetch } = useCheckNftAccount({ - addresses: groupToFetch.join(","), + addresses: groupToFetch?.join(",") || "", nftsOwned, chains: SUPPORTED_NFT_CURRENCIES, threshold, action: hideSpamCollection, - enabled, + enabled: enabled && groupToFetch.length > 0, }); // Refetch with new last group when addressGroups length changes @@ -91,7 +91,7 @@ export function useSyncNFTsWithAccounts() { const interval = setInterval(() => { setCurrentIndex(prevIndex => { const nextIndex = (prevIndex + 1) % addressGroups.length; - setGroupToFetch(addressGroups[nextIndex]); + setGroupToFetch(addressGroups[nextIndex] || []); return nextIndex; }); }, TIMER); diff --git a/apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/SimpleHashTools/SpamScore/index.tsx b/apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/SimpleHashTools/SpamScore/index.tsx index 76bc9e4cf834..c5a8b392cb99 100644 --- a/apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/SimpleHashTools/SpamScore/index.tsx +++ b/apps/ledger-live-desktop/src/renderer/screens/settings/sections/Developer/SimpleHashTools/SpamScore/index.tsx @@ -72,7 +72,7 @@ export default function SpamScore(props: HookResult) { return (error as Error).message; }; - const getScore = (data?: SimpleHashResponse) => data?.nfts[0].collection.spam_score; + const getScore = (data?: SimpleHashResponse) => data?.nfts[0]?.collection.spam_score || 100; const text = checkSpamScore.isError ? getErrorText(checkSpamScore.error) diff --git a/apps/ledger-live-mobile/src/hooks/nfts/useSyncNFTsWithAccounts.ts b/apps/ledger-live-mobile/src/hooks/nfts/useSyncNFTsWithAccounts.ts index 354731e034f2..2dd05c31e5f3 100644 --- a/apps/ledger-live-mobile/src/hooks/nfts/useSyncNFTsWithAccounts.ts +++ b/apps/ledger-live-mobile/src/hooks/nfts/useSyncNFTsWithAccounts.ts @@ -66,12 +66,12 @@ export function useSyncNFTsWithAccounts() { const [, setCurrentIndex] = useState(0); const { refetch } = useCheckNftAccount({ - addresses: groupToFetch.join(","), + addresses: groupToFetch?.join(",") || "", nftsOwned, chains: SUPPORTED_NFT_CURRENCIES, threshold, action: hideSpamCollection, - enabled, + enabled: enabled && groupToFetch.length > 0, }); // Refetch with new last group when addressGroups length changes @@ -91,7 +91,7 @@ export function useSyncNFTsWithAccounts() { const interval = setInterval(() => { setCurrentIndex(prevIndex => { const nextIndex = (prevIndex + 1) % addressGroups.length; - setGroupToFetch(addressGroups[nextIndex]); + setGroupToFetch(addressGroups[nextIndex] || []); return nextIndex; }); }, TIMER); diff --git a/libs/live-nft-react/src/hooks/__tests__/useCheckNftAccount.test.tsx b/libs/live-nft-react/src/hooks/__tests__/useCheckNftAccount.test.tsx index fd09fe0ea9c1..c435b198f25d 100644 --- a/libs/live-nft-react/src/hooks/__tests__/useCheckNftAccount.test.tsx +++ b/libs/live-nft-react/src/hooks/__tests__/useCheckNftAccount.test.tsx @@ -2,7 +2,7 @@ import { waitFor, renderHook } from "@testing-library/react"; import { SimpleHashResponse } from "@ledgerhq/live-nft/api/types"; import { notifyManager } from "@tanstack/react-query"; -import { wrapper, generateNftsOwned } from "../../tools/helperTests"; +import { wrapper, generateNftsOwned, generateNft } from "../../tools/helperTests"; import { useCheckNftAccount } from "../useCheckNftAccount"; jest.setTimeout(30000); @@ -79,4 +79,57 @@ describe("useCheckNftAccount", () => { expect(callCount).toBe(nftsOwned.length / pagedBy); expect(result.current.nfts.length).toEqual(nftsOwned.length); }); + + it("should call action", async () => { + const addresses = "0x34"; + const chains = ["ethereum"]; + + const actionMockMulti = jest.fn(); + + const { result: resultBis } = renderHook( + () => + useCheckNftAccount({ + addresses, + nftsOwned: [ + generateNft("0x1221", "1"), + generateNft("0x2321", "7"), + generateNft("0x27B21", "3"), + ], + chains, + threshold: 80, + action: actionMockMulti, + }), + { + wrapper, + }, + ); + + await waitFor(() => !resultBis.current.hasNextPage); + + expect(actionMockMulti).toHaveBeenCalledTimes(3); + }); + it("should not call action", async () => { + const addresses = "0x34"; + const chains = ["ethereum"]; + + const actionMockUnique = jest.fn(); + + const { result } = renderHook( + () => + useCheckNftAccount({ + addresses, + nftsOwned, + chains, + threshold: 80, + action: actionMockUnique, + }), + { + wrapper, + }, + ); + + await waitFor(() => !result.current.hasNextPage); + + expect(actionMockUnique).not.toHaveBeenCalled(); + }); }); diff --git a/libs/live-nft-react/src/hooks/useCheckNftAccount.ts b/libs/live-nft-react/src/hooks/useCheckNftAccount.ts index 6d67b3f56ef4..ef31ce5b196c 100644 --- a/libs/live-nft-react/src/hooks/useCheckNftAccount.ts +++ b/libs/live-nft-react/src/hooks/useCheckNftAccount.ts @@ -65,7 +65,7 @@ export function useCheckNftAccount({ const processingNFTs = queryResult.data?.pages.flatMap(page => page.nfts); - if (!queryResult.hasNextPage && processingNFTs) { + if (!queryResult.hasNextPage && processingNFTs?.length) { for (const nft of processingNFTs) { const hash = hashProtoNFT(nft.contract_address, nft.token_id, nft.chain); const existing = nftsWithProperties.get(hash); @@ -76,14 +76,15 @@ export function useCheckNftAccount({ if (action) { const spams = nftsOwned.filter(nft => !nfts.some(ownedNft => ownedNft.id === nft.id)); - const collections = nftsByCollections(spams); - Object.entries(collections).map(([contract, nfts]: [string, ProtoNFT[]]) => { - const { accountId } = decodeNftId(nfts[0].id); - const collection = `${accountId}|${contract}`; - action(collection); - }); + if (spams.length > 0) { + Object.entries(collections).map(([contract, nfts]: [string, ProtoNFT[]]) => { + const { accountId } = decodeNftId(nfts[0].id); + const collection = `${accountId}|${contract}`; + action(collection); + }); + } } } return { ...queryResult, nfts }; diff --git a/libs/live-nft-react/src/tools/helperTests.tsx b/libs/live-nft-react/src/tools/helperTests.tsx index 0697b401507a..69994ff9b37e 100644 --- a/libs/live-nft-react/src/tools/helperTests.tsx +++ b/libs/live-nft-react/src/tools/helperTests.tsx @@ -17,7 +17,6 @@ export type FakeNFTRaw = { contract: string; standard: "ERC721"; currencyId: string; - metadata: undefined; }; export const generateNftsOwned = () => { @@ -25,17 +24,18 @@ export const generateNftsOwned = () => { NFTs.forEach(nft => { for (let i = 1; i <= 20; i++) { - nfts.push({ - id: encodeNftId("foo", nft.collection.contract, String(i), "ethereum"), - tokenId: String(i), - amount: new BigNumber(0), - contract: nft.collection.contract, - standard: "ERC721" as const, - currencyId: "ethereum", - metadata: undefined, - }); + nfts.push(generateNft(nft.collection.contract, String(i))); } }); return nfts; }; + +export const generateNft = (contract: string, tokenId: string): FakeNFTRaw => ({ + id: encodeNftId("foo", contract, tokenId, "ethereum"), + tokenId: tokenId, + amount: new BigNumber(0), + contract: contract, + standard: "ERC721" as const, + currencyId: "ethereum", +});