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.

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

[DNM] looking into why "Spam filtering errors unhandled" failed changesets #8418

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/silly-seals-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"ledger-live-desktop": minor
"live-mobile": minor
"@ledgerhq/live-nft-react": minor
---

Fix undefined join on Spam filter
2 changes: 1 addition & 1 deletion .github/workflows/changeset-enforce.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
uses: actions/checkout@v4
with:
ref: ${{ github.head_ref }}
fetch-depth: 200
fetch-depth: 0

- name: Fetch develop branch
run: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
});
});
15 changes: 8 additions & 7 deletions libs/live-nft-react/src/hooks/useCheckNftAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 };
Expand Down
20 changes: 11 additions & 9 deletions libs/live-nft-react/src/tools/helperTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,19 @@ 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) => ({
id: encodeNftId("foo", contract, tokenId, "ethereum"),
tokenId: tokenId,
amount: new BigNumber(0),
contract: contract,
standard: "ERC721" as const,
currencyId: "ethereum",
metadata: undefined,
});
Loading