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

fix: Fixes in NFT listing label and values #29046

Merged
merged 14 commits into from
Jan 8, 2025
3 changes: 3 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions test/e2e/tests/confirmations/signatures/nft-permit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('Confirmation Signature - NFT Permit @no-mmi', function (this: Suite) {
signatureType: 'eth_signTypedData_v4',
primaryType: 'Permit',
uiCustomizations: ['redesigned_confirmation', 'permit'],
decodingChangeTypes: ['RECEIVE', 'LISTING'],
decodingChangeTypes: ['LISTING', 'RECEIVE'],
decodingResponse: 'CHANGE',
});

Expand Down Expand Up @@ -115,7 +115,7 @@ describe('Confirmation Signature - NFT Permit @no-mmi', function (this: Suite) {
primaryType: 'Permit',
uiCustomizations: ['redesigned_confirmation', 'permit'],
location: 'confirmation',
decodingChangeTypes: ['RECEIVE', 'LISTING'],
decodingChangeTypes: ['LISTING', 'RECEIVE'],
decodingResponse: 'CHANGE',
});
},
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/tests/confirmations/signatures/permit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('Confirmation Signature - Permit @no-mmi', function (this: Suite) {
signatureType: 'eth_signTypedData_v4',
primaryType: 'Permit',
uiCustomizations: ['redesigned_confirmation', 'permit'],
decodingChangeTypes: ['RECEIVE', 'LISTING'],
decodingChangeTypes: ['LISTING', 'RECEIVE'],
decodingResponse: 'CHANGE',
});

Expand Down Expand Up @@ -107,7 +107,7 @@ describe('Confirmation Signature - Permit @no-mmi', function (this: Suite) {
primaryType: 'Permit',
uiCustomizations: ['redesigned_confirmation', 'permit'],
location: 'confirmation',
decodingChangeTypes: ['RECEIVE', 'LISTING'],
decodingChangeTypes: ['LISTING', 'RECEIVE'],
decodingResponse: 'CHANGE',
});
},
Expand All @@ -126,12 +126,12 @@ describe('Confirmation Signature - Permit @no-mmi', function (this: Suite) {
const simulationSection = driver.findElement({
text: 'Estimated changes',
});
const receiveChange = driver.findElement({ text: 'You receive' });
const receiveChange = driver.findElement({ text: 'Listing price' });
const listChange = driver.findElement({ text: 'You list' });
const listChangeValue = driver.findElement({ text: '#2101' });

assert.ok(await simulationSection, 'Estimated changes');
assert.ok(await receiveChange, 'You receive');
assert.ok(await receiveChange, 'Listing price');
assert.ok(await listChange, 'You list');
assert.ok(await listChangeValue, '#2101');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import {
import { getMockTypedSignConfirmStateForRequest } from '../../../../../../../../../test/data/confirmations/helper';
import { renderWithConfirmContextProvider } from '../../../../../../../../../test/lib/confirmations/render-helpers';
import { permitSignatureMsg } from '../../../../../../../../../test/data/confirmations/typed_sign';
import PermitSimulation, { getStateChangeToolip } from './decoded-simulation';
import PermitSimulation, {
getStateChangeType,
getStateChangeToolip,
StateChangeType,
} from './decoded-simulation';

const decodingData = {
stateChanges: [
Expand Down Expand Up @@ -57,21 +61,40 @@ const decodingDataListingERC1155: DecodingDataStateChanges = [
tokenID: '2233',
},
];
const decodingDataBidding: DecodingDataStateChanges = [

const nftListing: DecodingDataStateChanges = [
{
assetType: 'ERC721',
changeType: DecodingDataChangeType.Listing,
address: '',
amount: '',
contractAddress: '0x922dC160f2ab743312A6bB19DD5152C1D3Ecca33',
tokenID: '189',
},
{
assetType: 'ERC20',
changeType: DecodingDataChangeType.Receive,
address: '',
amount: '900000000000000000',
contractAddress: '',
amount: '950000000000000000',
contractAddress: '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2',
},
];

const nftBidding: DecodingDataStateChanges = [
{
assetType: 'Native',
assetType: 'ERC20',
changeType: DecodingDataChangeType.Bidding,
address: '',
amount: '',
contractAddress: '0xafd4896984CA60d2feF66136e57f958dCe9482d5',
tokenID: '2101',
contractAddress: '0x922dC160f2ab743312A6bB19DD5152C1D3Ecca33',
tokenID: '189',
},
{
assetType: 'ERC721',
changeType: DecodingDataChangeType.Receive,
address: '',
amount: '950000000000000000',
contractAddress: '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2',
},
];

Expand Down Expand Up @@ -137,7 +160,7 @@ describe('DecodedSimulation', () => {
);

expect(await findByText('Estimated changes')).toBeInTheDocument();
expect(await findByText('You receive')).toBeInTheDocument();
expect(await findByText('Listing price')).toBeInTheDocument();
expect(await findByText('You list')).toBeInTheDocument();
expect(await findByText('#2101')).toBeInTheDocument();
});
Expand Down Expand Up @@ -174,24 +197,74 @@ describe('DecodedSimulation', () => {
expect(await findByText('Unavailable')).toBeInTheDocument();
});

it('renders label only once if there are multiple state changes of same changeType', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: false,
decodingData: {
stateChanges: [
decodingData.stateChanges[0],
decodingData.stateChanges[0],
decodingData.stateChanges[0],
],
},
});
const mockStore = configureMockStore([])(state);

const { findAllByText } = renderWithConfirmContextProvider(
<PermitSimulation />,
mockStore,
);

expect(await findAllByText('Spending cap')).toHaveLength(1);
});

it('for NFT permit label for receive should be "Listing price"', async () => {
const state = getMockTypedSignConfirmStateForRequest({
...permitSignatureMsg,
decodingLoading: false,
decodingData: {
stateChanges: nftListing,
},
});
const mockStore = configureMockStore([])(state);

const { findAllByText } = renderWithConfirmContextProvider(
<PermitSimulation />,
mockStore,
);

expect(await findAllByText('Listing price')).toHaveLength(1);
});

describe('getStateChangeToolip', () => {
it('return correct tooltip when permit is for listing NFT', async () => {
const tooltip = getStateChangeToolip(
decodingDataListing,
decodingDataListing?.[0],
StateChangeType.NFTListingReceive,
(str: string) => str,
);
expect(tooltip).toBe('signature_decoding_list_nft_tooltip');
});

it('return correct tooltip when permit is for bidding NFT', async () => {
const tooltip = getStateChangeToolip(
StateChangeType.NFTBiddingReceive,
(str: string) => str,
);
expect(tooltip).toBe('signature_decoding_bid_nft_tooltip');
});
});

it('return correct tooltip when permit is for bidding NFT', async () => {
const tooltip = getStateChangeToolip(
decodingDataBidding,
decodingDataBidding?.[0],
(str: string) => str,
);
expect(tooltip).toBe('signature_decoding_bid_nft_tooltip');
describe('getStateChangeType', () => {
it('return correct state change type for NFT listing receive', async () => {
const stateChange = getStateChangeType(nftListing, nftListing[1]);
expect(stateChange).toBe(StateChangeType.NFTListingReceive);
});

it('return correct state change type for NFT bidding receive', async () => {
const stateChange = getStateChangeType(nftBidding, nftBidding[1]);
expect(stateChange).toBe(StateChangeType.NFTBiddingReceive);
});
});

it('renders label only once if there are multiple state changes of same changeType', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@ import StaticSimulation from '../../../shared/static-simulation/static-simulatio
import TokenValueDisplay from '../value-display/value-display';
import NativeValueDisplay from '../native-value-display/native-value-display';

export const getStateChangeToolip = (
export enum StateChangeType {
NFTListingReceive = 'NFTListingReceive',
NFTBiddingReceive = 'NFTBiddingReceive',
}

export const getStateChangeType = (
stateChangeList: DecodingDataStateChanges | null,
stateChange: DecodingDataStateChange,
t: ReturnType<typeof useI18nContext>,
): string | undefined => {
): StateChangeType | undefined => {
if (stateChange.changeType === DecodingDataChangeType.Receive) {
if (
stateChangeList?.some(
Expand All @@ -29,32 +33,58 @@ export const getStateChangeToolip = (
change.assetType === TokenStandard.ERC721,
)
) {
return t('signature_decoding_list_nft_tooltip');
return StateChangeType.NFTListingReceive;
}
if (
stateChange.assetType === TokenStandard.ERC721 &&
stateChangeList?.some(
(change) => change.changeType === DecodingDataChangeType.Bidding,
)
) {
return t('signature_decoding_bid_nft_tooltip');
return StateChangeType.NFTBiddingReceive;
}
}
return undefined;
};

export const getStateChangeToolip = (
nftTransactionType: StateChangeType | undefined,
t: ReturnType<typeof useI18nContext>,
): string | undefined => {
if (nftTransactionType === StateChangeType.NFTListingReceive) {
return t('signature_decoding_list_nft_tooltip');
} else if (nftTransactionType === StateChangeType.NFTBiddingReceive) {
return t('signature_decoding_bid_nft_tooltip');
}
return undefined;
};

const stateChangeOrder = {
Copy link
Member

Choose a reason for hiding this comment

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

Minor, capitals and underscores and top of file as constant?

[DecodingDataChangeType.Transfer]: 1,
[DecodingDataChangeType.Listing]: 2,
[DecodingDataChangeType.Approve]: 3,
[DecodingDataChangeType.Revoke]: 4,
[DecodingDataChangeType.Bidding]: 5,
[DecodingDataChangeType.Receive]: 6,
};

const getStateChangeLabelMap = (
t: ReturnType<typeof useI18nContext>,
changeType: string,
) =>
({
stateChangeType?: StateChangeType,
) => {
return {
[DecodingDataChangeType.Transfer]: t('permitSimulationChange_transfer'),
[DecodingDataChangeType.Receive]: t('permitSimulationChange_receive'),
[DecodingDataChangeType.Receive]:
stateChangeType === StateChangeType.NFTListingReceive
? t('permitSimulationChange_nft_listing')
: t('permitSimulationChange_receive'),
[DecodingDataChangeType.Approve]: t('permitSimulationChange_approve'),
[DecodingDataChangeType.Revoke]: t('permitSimulationChange_revoke'),
[DecodingDataChangeType.Bidding]: t('permitSimulationChange_bidding'),
[DecodingDataChangeType.Listing]: t('permitSimulationChange_listing'),
}[changeType]);
}[changeType];
};

const StateChangeRow = ({
stateChangeList,
Expand All @@ -70,14 +100,19 @@ const StateChangeRow = ({
const t = useI18nContext();
const { assetType, changeType, amount, contractAddress, tokenID } =
stateChange;
const tooltip = getStateChangeToolip(stateChangeList, stateChange, t);
const nftTransactionType = getStateChangeType(stateChangeList, stateChange);
const tooltip = getStateChangeToolip(nftTransactionType, t);
const canDisplayValueAsUnlimited =
assetType === TokenStandard.ERC20 &&
(changeType === DecodingDataChangeType.Approve ||
changeType === DecodingDataChangeType.Revoke);
return (
<ConfirmInfoRow
label={shouldDisplayLabel ? getStateChangeLabelMap(t, changeType) : ''}
label={
shouldDisplayLabel
? getStateChangeLabelMap(t, changeType, nftTransactionType)
: ''
}
tooltip={tooltip}
>
{(assetType === TokenStandard.ERC20 ||
Expand All @@ -88,7 +123,10 @@ const StateChangeRow = ({
value={amount}
chainId={chainId}
tokenId={tokenID}
credit={changeType === DecodingDataChangeType.Receive}
credit={
nftTransactionType !== StateChangeType.NFTListingReceive &&
changeType === DecodingDataChangeType.Receive
}
debit={changeType === DecodingDataChangeType.Transfer}
canDisplayValueAsUnlimited={canDisplayValueAsUnlimited}
/>
Expand All @@ -97,7 +135,10 @@ const StateChangeRow = ({
<NativeValueDisplay
value={amount}
chainId={chainId}
credit={changeType === DecodingDataChangeType.Receive}
credit={
nftTransactionType !== StateChangeType.NFTListingReceive &&
changeType === DecodingDataChangeType.Receive
}
debit={changeType === DecodingDataChangeType.Transfer}
/>
)}
Expand All @@ -112,8 +153,13 @@ const DecodedSimulation: React.FC<object> = () => {
const { decodingLoading, decodingData } = currentConfirmation;

const stateChangeFragment = useMemo(() => {
const orderedStateChanges = decodingData?.stateChanges?.sort((c1, c2) =>
stateChangeOrder[c1.changeType] > stateChangeOrder[c2.changeType]
? 1
: -1,
);
const stateChangesGrouped: Record<string, DecodingDataStateChange[]> = (
decodingData?.stateChanges ?? []
orderedStateChanges ?? []
).reduce<Record<string, DecodingDataStateChange[]>>(
(result, stateChange) => {
result[stateChange.changeType] = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ describe('PermitSimulation', () => {
mockStore,
);

expect(await findByText('You receive')).toBeInTheDocument();
expect(await findByText('Listing price')).toBeInTheDocument();
expect(await findByText('You list')).toBeInTheDocument();
});
});
Expand Down
Loading