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: transaction note from tx origin #2223

Merged
merged 4 commits into from
Dec 30, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ describe('Get by id - Transactions Controller (Unit)', () => {
},
txHash: moduleTransaction.transactionHash,
safeAppInfo: null,
note: null,
});
});
});
Expand Down Expand Up @@ -371,6 +372,7 @@ describe('Get by id - Transactions Controller (Unit)', () => {
detailedExecutionInfo: null,
txHash: transfer.transactionHash,
safeAppInfo: null,
note: null,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 | null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ describe('ModuleTransactionDetails mapper (Unit)', () => {
txHash: transaction.transactionHash,
detailedExecutionInfo: new ModuleExecutionDetails(addressInfo),
safeAppInfo: null,
note: null,
});
});

Expand Down Expand Up @@ -127,6 +128,7 @@ describe('ModuleTransactionDetails mapper (Unit)', () => {
txHash: transaction.transactionHash,
detailedExecutionInfo: new ModuleExecutionDetails(addressInfo),
safeAppInfo: null,
note: null,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export class ModuleTransactionDetailsMapper {
txHash: transaction.transactionHash,
detailedExecutionInfo: new ModuleExecutionDetails(moduleAddress),
safeAppInfo: null,
note: null,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ const multisigExecutionDetailsMapper = jest.mocked({
mapMultisigExecutionDetails: jest.fn(),
} as jest.MockedObjectDeep<MultisigTransactionExecutionDetailsMapper>);

const multisigTransactionNoteMapper = jest.mocked({
mapTxNote: jest.fn(),
});

describe('MultisigTransactionDetails mapper (Unit)', () => {
let mapper: MultisigTransactionDetailsMapper;

Expand All @@ -51,6 +55,7 @@ describe('MultisigTransactionDetails mapper (Unit)', () => {
transactionDataMapper,
safeAppInfoMapper,
multisigExecutionDetailsMapper,
multisigTransactionNoteMapper,
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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(
Expand All @@ -33,6 +35,7 @@ export class MultisigTransactionDetailsMapper {
safe: Safe,
): Promise<TransactionDetails> {
const txStatus = this.statusMapper.mapTransactionStatus(transaction, safe);
const note = this.noteMapper.mapTxNote(transaction);
const [
isTrustedDelegateCall,
addressInfoIndex,
Expand Down Expand Up @@ -79,6 +82,7 @@ export class MultisigTransactionDetailsMapper {
txHash: transaction.transactionHash,
detailedExecutionInfo,
safeAppInfo,
note,
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
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';

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',
JSON.stringify({ name: JSON.stringify({ note: noteText }) }),
)
.build();

const note = mapper.mapTxNote(transaction);

expect(note).toBe(noteText);
});

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).toBeNull();
});

it('should return undefined if `origin` does not contain a note', () => {
const transaction = multisigTransactionBuilder()
.with(
'origin',
JSON.stringify({ name: faker.word.noun(), url: faker.internet.url() }),
)
.build();

const note = mapper.mapTxNote(transaction);

expect(note).toBeNull();
});

it('should return undefined if `origin` is null', () => {
const transaction = multisigTransactionBuilder()
.with('origin', null)
.build();

const note = mapper.mapTxNote(transaction);

expect(note).toBeNull();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Injectable } from '@nestjs/common';
import { MultisigTransaction } from '@/domain/safe/entities/multisig-transaction.entity';

@Injectable()
export class MultisigTransactionNoteMapper {
mapTxNote(transaction: MultisigTransaction): string | null {
if (transaction.origin) {
try {
const origin = JSON.parse(transaction.origin);
const parsedName = origin.name && JSON.parse(String(origin.name));
if (typeof parsedName.note === 'string') {
return parsedName.note;
}
} catch {
// Ignore, no note
}
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('TransferDetails mapper (Unit)', () => {
detailedExecutionInfo: null,
txHash: transfer.transactionHash,
safeAppInfo: null,
note: null,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export class TransferDetailsMapper {
detailedExecutionInfo: null,
txHash: transfer.transactionHash,
safeAppInfo: null,
note: null,
};
}
}
2 changes: 2 additions & 0 deletions src/routes/transactions/transactions.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -86,6 +87,7 @@ import { Module } from '@nestjs/common';
MultisigTransactionInfoMapper,
MultisigTransactionMapper,
MultisigTransactionStatusMapper,
MultisigTransactionNoteMapper,
NativeCoinTransferMapper,
NativeStakingMapper,
QueuedItemsMapper,
Expand Down
Loading