From 4877b81544add1543480d74563a42dee83afa464 Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Fri, 27 Dec 2024 17:24:41 +0300 Subject: [PATCH 1/4] Feat: transaction note from tx origin Null -> undefined --- .../transaction-details.entity.ts | 2 + ...ultisig-transaction-details.mapper.spec.ts | 5 ++ .../multisig-transaction-details.mapper.ts | 4 ++ .../multisig-transaction-note.mapper.spec.ts | 46 +++++++++++++++++++ .../multisig-transaction-note.mapper.ts | 23 ++++++++++ .../transactions/transactions.module.ts | 2 + 6 files changed, 82 insertions(+) create mode 100644 src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts create mode 100644 src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts diff --git a/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts b/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts index b6842f6e24..9029ea4800 100644 --- a/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts +++ b/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts @@ -46,4 +46,6 @@ export class TransactionDetails { txHash!: string | null; @ApiPropertyOptional({ type: SafeAppInfo, nullable: true }) safeAppInfo!: SafeAppInfo | null; + @ApiPropertyOptional({ type: String, nullable: true }) + note?: string; } diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.spec.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.spec.ts index 0d2a35d4c0..778db733c0 100644 --- a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.spec.ts +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.spec.ts @@ -39,6 +39,10 @@ const multisigExecutionDetailsMapper = jest.mocked({ mapMultisigExecutionDetails: jest.fn(), } as jest.MockedObjectDeep); +const multisigTransactionNoteMapper = jest.mocked({ + mapTxNote: jest.fn(), +}); + describe('MultisigTransactionDetails mapper (Unit)', () => { let mapper: MultisigTransactionDetailsMapper; @@ -51,6 +55,7 @@ describe('MultisigTransactionDetails mapper (Unit)', () => { transactionDataMapper, safeAppInfoMapper, multisigExecutionDetailsMapper, + multisigTransactionNoteMapper, ); }); diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.ts index 034ed1d108..595e8c2b55 100644 --- a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.ts +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-details.mapper.ts @@ -15,6 +15,7 @@ import { TransactionDataMapper } from '@/routes/transactions/mappers/common/tran import { MultisigTransactionInfoMapper } from '@/routes/transactions/mappers/common/transaction-info.mapper'; import { MultisigTransactionExecutionDetailsMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-execution-details.mapper'; import { MultisigTransactionStatusMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-status.mapper'; +import { MultisigTransactionNoteMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper'; @Injectable() export class MultisigTransactionDetailsMapper { @@ -25,6 +26,7 @@ export class MultisigTransactionDetailsMapper { private readonly transactionDataMapper: TransactionDataMapper, private readonly safeAppInfoMapper: SafeAppInfoMapper, private readonly multisigTransactionExecutionDetailsMapper: MultisigTransactionExecutionDetailsMapper, + private readonly noteMapper: MultisigTransactionNoteMapper, ) {} async mapDetails( @@ -33,6 +35,7 @@ export class MultisigTransactionDetailsMapper { safe: Safe, ): Promise { const txStatus = this.statusMapper.mapTransactionStatus(transaction, safe); + const note = this.noteMapper.mapTxNote(transaction); const [ isTrustedDelegateCall, addressInfoIndex, @@ -79,6 +82,7 @@ export class MultisigTransactionDetailsMapper { txHash: transaction.transactionHash, detailedExecutionInfo, safeAppInfo, + note, }; } diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts new file mode 100644 index 0000000000..9878e07487 --- /dev/null +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts @@ -0,0 +1,46 @@ +import { multisigTransactionBuilder } from '@/domain/safe/entities/__tests__/multisig-transaction.builder'; +import { MultisigTransactionNoteMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper'; + +const mapper = new MultisigTransactionNoteMapper(); + +describe('Multisig Transaction note mapper (Unit)', () => { + it('should parse transaction `origin` and return a note', () => { + const transaction = multisigTransactionBuilder() + .with('origin', '{"name":"{\\"note\\":\\"This is a note\\"}"}') + .build(); + + const note = mapper.mapTxNote(transaction); + + expect(note).toBe('This is a note'); + }); + + it('should return undefined if `origin` is not a valid JSON', () => { + const transaction = multisigTransactionBuilder() + .with('origin', 'invalid-json') + .build(); + + const note = mapper.mapTxNote(transaction); + + expect(note).toBeUndefined(); + }); + + it('should return undefined if `origin` does not contain a note', () => { + const transaction = multisigTransactionBuilder() + .with('origin', '{"url":"uniswap.org","name":"Uniswap"}') + .build(); + + const note = mapper.mapTxNote(transaction); + + expect(note).toBeUndefined(); + }); + + it('should return undefined if `origin` is null', () => { + const transaction = multisigTransactionBuilder() + .with('origin', null) + .build(); + + const note = mapper.mapTxNote(transaction); + + expect(note).toBeUndefined(); + }); +}); diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts new file mode 100644 index 0000000000..68b47cebae --- /dev/null +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts @@ -0,0 +1,23 @@ +import { Injectable } from '@nestjs/common'; +import { MultisigTransaction } from '@/domain/safe/entities/multisig-transaction.entity'; + +@Injectable() +export class MultisigTransactionNoteMapper { + mapTxNote(transaction: MultisigTransaction): string | undefined { + let note: string | undefined; + + if (transaction.origin) { + try { + const origin = JSON.parse(transaction.origin); + const parsedName = origin.name && JSON.parse(String(origin.name)); + if (typeof parsedName.note === 'string') { + note = parsedName.note; + } + } catch { + // Ignore, no note + } + } + + return note; + } +} diff --git a/src/routes/transactions/transactions.module.ts b/src/routes/transactions/transactions.module.ts index c99ec524d7..187dde3b0a 100644 --- a/src/routes/transactions/transactions.module.ts +++ b/src/routes/transactions/transactions.module.ts @@ -35,6 +35,7 @@ import { MultisigTransactionDetailsMapper } from '@/routes/transactions/mappers/ import { MultisigTransactionExecutionDetailsMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-execution-details.mapper'; import { MultisigTransactionExecutionInfoMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-execution-info.mapper'; import { MultisigTransactionStatusMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-status.mapper'; +import { MultisigTransactionNoteMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper'; import { MultisigTransactionMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction.mapper'; import { QueuedItemsMapper } from '@/routes/transactions/mappers/queued-items/queued-items.mapper'; import { TransactionPreviewMapper } from '@/routes/transactions/mappers/transaction-preview.mapper'; @@ -86,6 +87,7 @@ import { Module } from '@nestjs/common'; MultisigTransactionInfoMapper, MultisigTransactionMapper, MultisigTransactionStatusMapper, + MultisigTransactionNoteMapper, NativeCoinTransferMapper, NativeStakingMapper, QueuedItemsMapper, From 9a68808de4140f0dd41bdd2135cdff1326ac693b Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Mon, 30 Dec 2024 12:32:13 +0300 Subject: [PATCH 2/4] Code style --- .../multisig-transaction-note.mapper.spec.ts | 9 +++++++-- .../multisig-transaction-note.mapper.ts | 6 +----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts index 9878e07487..90c8297d74 100644 --- a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts @@ -1,3 +1,4 @@ +import { faker } from '@faker-js/faker'; import { multisigTransactionBuilder } from '@/domain/safe/entities/__tests__/multisig-transaction.builder'; import { MultisigTransactionNoteMapper } from '@/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper'; @@ -5,13 +6,17 @@ const mapper = new MultisigTransactionNoteMapper(); describe('Multisig Transaction note mapper (Unit)', () => { it('should parse transaction `origin` and return a note', () => { + const noteText = faker.lorem.sentence(); const transaction = multisigTransactionBuilder() - .with('origin', '{"name":"{\\"note\\":\\"This is a note\\"}"}') + .with( + 'origin', + JSON.stringify({ name: JSON.stringify({ note: noteText }) }), + ) .build(); const note = mapper.mapTxNote(transaction); - expect(note).toBe('This is a note'); + expect(note).toBe(noteText); }); it('should return undefined if `origin` is not a valid JSON', () => { diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts index 68b47cebae..5d6e984b45 100644 --- a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts @@ -4,20 +4,16 @@ import { MultisigTransaction } from '@/domain/safe/entities/multisig-transaction @Injectable() export class MultisigTransactionNoteMapper { mapTxNote(transaction: MultisigTransaction): string | undefined { - let note: string | undefined; - if (transaction.origin) { try { const origin = JSON.parse(transaction.origin); const parsedName = origin.name && JSON.parse(String(origin.name)); if (typeof parsedName.note === 'string') { - note = parsedName.note; + return parsedName.note; } } catch { // Ignore, no note } } - - return note; } } From bf7c9c665f4fe87ebbebdd6687b378b2cb3a37ee Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Mon, 30 Dec 2024 12:39:47 +0300 Subject: [PATCH 3/4] Undefined -> null --- .../get-transaction-by-id.transactions.controller.spec.ts | 2 ++ .../transaction-details/transaction-details.entity.ts | 2 +- .../module-transaction-details.mapper.spec.ts | 2 ++ .../module-transaction-details.mapper.ts | 1 + .../multisig-transaction-note.mapper.spec.ts | 6 +++--- .../multisig-transaction-note.mapper.ts | 3 ++- .../mappers/transfers/transfer-details.mapper.spec.ts | 1 + .../mappers/transfers/transfer-details.mapper.ts | 1 + 8 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts b/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts index f7442fad17..f02b966166 100644 --- a/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts +++ b/src/routes/transactions/__tests__/controllers/get-transaction-by-id.transactions.controller.spec.ts @@ -271,6 +271,7 @@ describe('Get by id - Transactions Controller (Unit)', () => { }, txHash: moduleTransaction.transactionHash, safeAppInfo: null, + note: null, }); }); }); @@ -371,6 +372,7 @@ describe('Get by id - Transactions Controller (Unit)', () => { detailedExecutionInfo: null, txHash: transfer.transactionHash, safeAppInfo: null, + note: null, }); }); }); diff --git a/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts b/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts index 9029ea4800..4dbd81fc34 100644 --- a/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts +++ b/src/routes/transactions/entities/transaction-details/transaction-details.entity.ts @@ -47,5 +47,5 @@ export class TransactionDetails { @ApiPropertyOptional({ type: SafeAppInfo, nullable: true }) safeAppInfo!: SafeAppInfo | null; @ApiPropertyOptional({ type: String, nullable: true }) - note?: string; + note!: string | null; } diff --git a/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.spec.ts b/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.spec.ts index 06a701c8fe..944ad279bf 100644 --- a/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.spec.ts +++ b/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.spec.ts @@ -80,6 +80,7 @@ describe('ModuleTransactionDetails mapper (Unit)', () => { txHash: transaction.transactionHash, detailedExecutionInfo: new ModuleExecutionDetails(addressInfo), safeAppInfo: null, + note: null, }); }); @@ -127,6 +128,7 @@ describe('ModuleTransactionDetails mapper (Unit)', () => { txHash: transaction.transactionHash, detailedExecutionInfo: new ModuleExecutionDetails(addressInfo), safeAppInfo: null, + note: null, }); }); }); diff --git a/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.ts b/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.ts index 2c0f9a0dcf..546288fdb2 100644 --- a/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.ts +++ b/src/routes/transactions/mappers/module-transactions/module-transaction-details.mapper.ts @@ -44,6 +44,7 @@ export class ModuleTransactionDetailsMapper { txHash: transaction.transactionHash, detailedExecutionInfo: new ModuleExecutionDetails(moduleAddress), safeAppInfo: null, + note: null, }; } diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts index 90c8297d74..24ad8a9f38 100644 --- a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts @@ -26,7 +26,7 @@ describe('Multisig Transaction note mapper (Unit)', () => { const note = mapper.mapTxNote(transaction); - expect(note).toBeUndefined(); + expect(note).toBeNull(); }); it('should return undefined if `origin` does not contain a note', () => { @@ -36,7 +36,7 @@ describe('Multisig Transaction note mapper (Unit)', () => { const note = mapper.mapTxNote(transaction); - expect(note).toBeUndefined(); + expect(note).toBeNull(); }); it('should return undefined if `origin` is null', () => { @@ -46,6 +46,6 @@ describe('Multisig Transaction note mapper (Unit)', () => { const note = mapper.mapTxNote(transaction); - expect(note).toBeUndefined(); + expect(note).toBeNull(); }); }); diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts index 5d6e984b45..bdc1216f24 100644 --- a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.ts @@ -3,7 +3,7 @@ import { MultisigTransaction } from '@/domain/safe/entities/multisig-transaction @Injectable() export class MultisigTransactionNoteMapper { - mapTxNote(transaction: MultisigTransaction): string | undefined { + mapTxNote(transaction: MultisigTransaction): string | null { if (transaction.origin) { try { const origin = JSON.parse(transaction.origin); @@ -15,5 +15,6 @@ export class MultisigTransactionNoteMapper { // Ignore, no note } } + return null; } } diff --git a/src/routes/transactions/mappers/transfers/transfer-details.mapper.spec.ts b/src/routes/transactions/mappers/transfers/transfer-details.mapper.spec.ts index 0e2a17ee82..73d33186b8 100644 --- a/src/routes/transactions/mappers/transfers/transfer-details.mapper.spec.ts +++ b/src/routes/transactions/mappers/transfers/transfer-details.mapper.spec.ts @@ -36,6 +36,7 @@ describe('TransferDetails mapper (Unit)', () => { detailedExecutionInfo: null, txHash: transfer.transactionHash, safeAppInfo: null, + note: null, }); }); }); diff --git a/src/routes/transactions/mappers/transfers/transfer-details.mapper.ts b/src/routes/transactions/mappers/transfers/transfer-details.mapper.ts index 16526978b6..07c819528a 100644 --- a/src/routes/transactions/mappers/transfers/transfer-details.mapper.ts +++ b/src/routes/transactions/mappers/transfers/transfer-details.mapper.ts @@ -32,6 +32,7 @@ export class TransferDetailsMapper { detailedExecutionInfo: null, txHash: transfer.transactionHash, safeAppInfo: null, + note: null, }; } } From a3fb125c31b28b42ab996238a5859918aa9b7c63 Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Mon, 30 Dec 2024 13:59:57 +0300 Subject: [PATCH 4/4] More faker --- .../multisig-transaction-note.mapper.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts index 24ad8a9f38..7343a4de0f 100644 --- a/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts +++ b/src/routes/transactions/mappers/multisig-transactions/multisig-transaction-note.mapper.spec.ts @@ -31,7 +31,10 @@ describe('Multisig Transaction note mapper (Unit)', () => { it('should return undefined if `origin` does not contain a note', () => { const transaction = multisigTransactionBuilder() - .with('origin', '{"url":"uniswap.org","name":"Uniswap"}') + .with( + 'origin', + JSON.stringify({ name: faker.word.noun(), url: faker.internet.url() }), + ) .build(); const note = mapper.mapTxNote(transaction);