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

feat: integrate NFT api to display names for nfts within simulations for ERC721s #25692

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
a8337d0
Integrate NFT api to display names for nfts within simulations
OGPoyraz Jul 5, 2024
4d0971e
Fix lint
OGPoyraz Jul 5, 2024
7420114
remove log
OGPoyraz Jul 5, 2024
e149537
Fix type
OGPoyraz Jul 5, 2024
58a6bcd
Add nftcollection name only if collection is not spam
OGPoyraz Jul 9, 2024
5a19514
Fix memoised contracts
OGPoyraz Jul 9, 2024
92cd60f
Removed type
OGPoyraz Jul 9, 2024
10419b4
Fix lint
OGPoyraz Jul 9, 2024
1b04416
Fix tests
OGPoyraz Jul 9, 2024
fb766cd
Fix test constants
OGPoyraz Jul 9, 2024
61d7a7c
Fix lint
OGPoyraz Jul 9, 2024
a2b5ce3
Filter nft requests for useNftCollectionsMetadata
OGPoyraz Jul 10, 2024
af7329f
Fix hook tests
OGPoyraz Jul 10, 2024
384b394
Fix useDisplayName tests
OGPoyraz Jul 10, 2024
edf40ec
Simplify use of collection metadata
OGPoyraz Jul 10, 2024
43ddeaf
Fix lint
OGPoyraz Jul 10, 2024
dd1ee4d
Limit use of NFT api only for ERC721s
OGPoyraz Jul 15, 2024
9f9ad5c
Lint fix
OGPoyraz Jul 15, 2024
402f713
Fix lint
OGPoyraz Jul 15, 2024
7845d42
Fix lint
OGPoyraz Jul 15, 2024
d0d2187
Fix lint
OGPoyraz Jul 15, 2024
9b8220a
remove patch
OGPoyraz Sep 2, 2024
c9e1e77
Merge branch 'develop' into 2507-integrate-nft-api-to-display-names-f…
OGPoyraz Sep 2, 2024
ddd9642
Merge branch 'develop' into 2507-integrate-nft-api-to-display-names-f…
OGPoyraz Sep 19, 2024
780a63b
Fix performance
OGPoyraz Sep 19, 2024
efeafda
Fix tests
OGPoyraz Sep 19, 2024
271bb25
Merge branch 'develop' into 2507-integrate-nft-api-to-display-names-f…
OGPoyraz Sep 19, 2024
d8f2b41
Fix lint
OGPoyraz Sep 19, 2024
c0c3417
Fix suggestions
OGPoyraz Sep 20, 2024
22a8e80
Merge branch 'develop' into 2507-integrate-nft-api-to-display-names-f…
OGPoyraz Sep 20, 2024
883ffc6
Merge branch 'develop' into 2507-integrate-nft-api-to-display-names-f…
OGPoyraz Sep 20, 2024
b6c4881
Merge branch 'develop' into 2507-integrate-nft-api-to-display-names-f…
OGPoyraz Sep 23, 2024
ab8f93f
Fix tests
OGPoyraz Sep 23, 2024
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
2 changes: 2 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -3432,6 +3432,8 @@ export default class MetamaskController extends EventEmitter {
nftController,
),

getNFTContractInfo: nftController.getNFTContractInfo.bind(nftController),

isNftOwner: nftController.isNftOwner.bind(nftController),

// AddressController
Expand Down
22 changes: 22 additions & 0 deletions ui/components/app/name/__snapshots__/name.test.tsx.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Name renders address with image 1`] = `
<div>
<div>
<div
class="name name__saved"
>
<img
alt=""
class="identicon"
src="test-image"
style="height: 16px; width: 16px; border-radius: 8px;"
/>
<p
class="mm-box mm-text name__name mm-text--body-md mm-box--color-text-default"
>
TestName
</p>
</div>
</div>
</div>
`;

exports[`Name renders address with no saved name 1`] = `
<div>
<div>
Expand Down
15 changes: 15 additions & 0 deletions ui/components/app/name/name.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,21 @@ describe('Name', () => {
expect(container).toMatchSnapshot();
});

it('renders address with image', () => {
useDisplayNameMock.mockReturnValue({
name: SAVED_NAME_MOCK,
hasPetname: true,
image: 'test-image',
});

const { container } = renderWithProvider(
<Name type={NameType.ETHEREUM_ADDRESS} value={ADDRESS_SAVED_NAME_MOCK} />,
store,
);

expect(container).toMatchSnapshot();
});

describe('metrics', () => {
// @ts-expect-error This is missing from the Mocha type definitions
it.each([
Expand Down
16 changes: 8 additions & 8 deletions ui/components/app/name/name.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@ export type NameProps = {
/** Whether this is being rendered inside the NameDetails modal. */
internal?: boolean;

/** The type of value, e.g. NameType.ETHEREUM_ADDRESS */
type: NameType;

/** The raw value to display the name of. */
value: string;

/**
* Applies to recognized contracts with no petname saved:
* If true the contract symbol (e.g. WBTC) will be used instead of the contract name.
*/
preferContractSymbol?: boolean;

/** The type of value, e.g. NameType.ETHEREUM_ADDRESS */
type: NameType;

/** The raw value to display the name of. */
value: string;
};

function formatValue(value: string, type: NameType): string {
Expand Down Expand Up @@ -65,7 +65,7 @@ const Name = memo(
const [modalOpen, setModalOpen] = useState(false);
const trackEvent = useContext(MetaMetricsContext);

const { name, hasPetname } = useDisplayName(
const { name, hasPetname, image } = useDisplayName(
value,
type,
preferContractSymbol,
Expand Down Expand Up @@ -112,7 +112,7 @@ const Name = memo(
onClick={handleClick}
>
{hasDisplayName ? (
<Identicon address={value} diameter={16} />
<Identicon address={value} diameter={16} image={image} />
) : (
<Icon
name={IconName.Question}
Expand Down
73 changes: 68 additions & 5 deletions ui/hooks/useDisplayName.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import { NameEntry, NameType } from '@metamask/name-controller';
import { NftContract } from '@metamask/assets-controllers';
import { renderHook } from '@testing-library/react-hooks';
import { getRemoteTokens } from '../selectors';
import { getNftContractsByAddressOnCurrentChain } from '../selectors/nft';
import { useDisplayName } from './useDisplayName';
import { useNames } from './useName';
import { useFirstPartyContractNames } from './useFirstPartyContractName';
import { useNftCollectionsMetadata } from './useNftCollectionsMetadata';

jest.mock('react-redux', () => ({
// TODO: Replace `any` with type
Expand All @@ -20,6 +22,10 @@ jest.mock('./useFirstPartyContractName', () => ({
useFirstPartyContractNames: jest.fn(),
}));

jest.mock('./useNftCollectionsMetadata', () => ({
useNftCollectionsMetadata: jest.fn(),
}));

jest.mock('../selectors', () => ({
getRemoteTokens: jest.fn(),
getCurrentChainId: jest.fn(),
Expand Down Expand Up @@ -62,6 +68,7 @@ describe('useDisplayName', () => {
const getNftContractsByAddressOnCurrentChainMock = jest.mocked(
getNftContractsByAddressOnCurrentChain,
);
const useNftCollectionsMetadataMock = jest.mocked(useNftCollectionsMetadata);

beforeEach(() => {
jest.resetAllMocks();
Expand All @@ -78,10 +85,12 @@ describe('useDisplayName', () => {
getNftContractsByAddressOnCurrentChainMock.mockReturnValue(
NO_WATCHED_NFT_NAME_FOUND_RETURN_VALUE,
);
useNftCollectionsMetadataMock.mockReturnValue({});
});

it('handles no name found', () => {
expect(useDisplayName(VALUE_MOCK, TYPE_MOCK)).toEqual({
const { result } = renderHook(() => useDisplayName(VALUE_MOCK, TYPE_MOCK));
expect(result.current).toEqual({
name: null,
hasPetname: false,
});
Expand All @@ -101,7 +110,9 @@ describe('useDisplayName', () => {
WATCHED_NFT_FOUND_RETURN_VALUE,
);

expect(useDisplayName(VALUE_MOCK, TYPE_MOCK)).toEqual({
const { result } = renderHook(() => useDisplayName(VALUE_MOCK, TYPE_MOCK));

expect(result.current).toEqual({
name: NAME_MOCK,
hasPetname: true,
contractDisplayName: CONTRACT_NAME_MOCK,
Expand All @@ -119,7 +130,9 @@ describe('useDisplayName', () => {
WATCHED_NFT_FOUND_RETURN_VALUE,
);

expect(useDisplayName(VALUE_MOCK, TYPE_MOCK)).toEqual({
const { result } = renderHook(() => useDisplayName(VALUE_MOCK, TYPE_MOCK));

expect(result.current).toEqual({
name: FIRST_PARTY_CONTRACT_NAME_MOCK,
hasPetname: false,
});
Expand All @@ -135,7 +148,9 @@ describe('useDisplayName', () => {
WATCHED_NFT_FOUND_RETURN_VALUE,
);

expect(useDisplayName(VALUE_MOCK, TYPE_MOCK)).toEqual({
const { result } = renderHook(() => useDisplayName(VALUE_MOCK, TYPE_MOCK));

expect(result.current).toEqual({
name: CONTRACT_NAME_MOCK,
hasPetname: false,
contractDisplayName: CONTRACT_NAME_MOCK,
Expand All @@ -147,9 +162,57 @@ describe('useDisplayName', () => {
WATCHED_NFT_FOUND_RETURN_VALUE,
);

expect(useDisplayName(VALUE_MOCK, TYPE_MOCK)).toEqual({
const { result } = renderHook(() => useDisplayName(VALUE_MOCK, TYPE_MOCK));

expect(result.current).toEqual({
name: WATCHED_NFT_NAME_MOCK,
hasPetname: false,
});
});

it('returns nft collection name from metadata if no other name is found', () => {
const IMAGE_MOCK = 'url';

useNftCollectionsMetadataMock.mockReturnValue({
[VALUE_MOCK.toLowerCase()]: {
name: CONTRACT_NAME_MOCK,
image: IMAGE_MOCK,
isSpam: false,
},
});

const { result } = renderHook(() =>
useDisplayName(VALUE_MOCK, TYPE_MOCK, false),
);

expect(result.current).toEqual({
name: CONTRACT_NAME_MOCK,
hasPetname: false,
contractDisplayName: undefined,
image: IMAGE_MOCK,
});
});

it('does not return nft collection name if collection is marked as spam', () => {
const IMAGE_MOCK = 'url';

useNftCollectionsMetadataMock.mockReturnValue({
[VALUE_MOCK.toLowerCase()]: {
name: CONTRACT_NAME_MOCK,
image: IMAGE_MOCK,
isSpam: true,
},
});

const { result } = renderHook(() =>
useDisplayName(VALUE_MOCK, TYPE_MOCK, false),
);

expect(result.current).toEqual(
expect.objectContaining({
name: null,
image: undefined,
}),
);
});
});
32 changes: 24 additions & 8 deletions ui/hooks/useDisplayName.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,36 @@
import { useMemo } from 'react';
import { NameType } from '@metamask/name-controller';
import { useSelector } from 'react-redux';
import { getRemoteTokens } from '../selectors';
import { getNftContractsByAddressOnCurrentChain } from '../selectors/nft';
import { useNames } from './useName';
import { useFirstPartyContractNames } from './useFirstPartyContractName';
import { useNftCollectionsMetadata } from './useNftCollectionsMetadata';

export type UseDisplayNameRequest = {
value: string;
type: NameType;
preferContractSymbol?: boolean;
type: NameType;
};

export type UseDisplayNameResponse = {
name: string | null;
hasPetname: boolean;
contractDisplayName?: string;
image?: string;
};

export function useDisplayNames(
requests: UseDisplayNameRequest[],
): UseDisplayNameResponse[] {
const nameRequests = requests.map(({ value, type }) => ({
value,
type,
}));
const nameRequests = useMemo(
() => requests.map(({ value, type }) => ({ value, type })),
[requests],
);

const nameEntries = useNames(nameRequests);
const firstPartyContractNames = useFirstPartyContractNames(nameRequests);
const nftCollections = useNftCollectionsMetadata(nameRequests);
const values = requests.map(({ value }) => value);

const contractInfo = useSelector((state) =>
Expand All @@ -42,6 +46,16 @@ export function useDisplayNames(
const firstPartyContractName = firstPartyContractNames[index];
const singleContractInfo = contractInfo[index];
const watchedNftName = watchedNftNames[value.toLowerCase()]?.name;
const nftCollectionProperties = nftCollections[value.toLowerCase()];

const isNotSpam = nftCollectionProperties?.isSpam === false;

const nftCollectionName = isNotSpam
? nftCollectionProperties?.name
: undefined;
const nftCollectionImage = isNotSpam
? nftCollectionProperties?.image
: undefined;

const contractDisplayName =
preferContractSymbol && singleContractInfo?.symbol
Expand All @@ -51,6 +65,7 @@ export function useDisplayNames(
const name =
nameEntry?.name ||
firstPartyContractName ||
nftCollectionName ||
contractDisplayName ||
watchedNftName ||
null;
Expand All @@ -61,15 +76,16 @@ export function useDisplayNames(
name,
hasPetname,
contractDisplayName,
image: nftCollectionImage,
};
});
}

/**
* Attempts to resolve the name for the given parameters.
*
* @param value
* @param type
* @param value - The address or contract address to resolve.
* @param type - The type of value, e.g. NameType.ETHEREUM_ADDRESS.
* @param preferContractSymbol - Applies to recognized contracts when no petname is saved:
* If true the contract symbol (e.g. WBTC) will be used instead of the contract name.
* @returns An object with two properties:
Expand All @@ -81,5 +97,5 @@ export function useDisplayName(
type: NameType,
preferContractSymbol: boolean = false,
): UseDisplayNameResponse {
return useDisplayNames([{ value, type, preferContractSymbol }])[0];
return useDisplayNames([{ preferContractSymbol, type, value }])[0];
}
Loading