From ef0be315798df3a96a85afdb1d77dc4b1e65acff Mon Sep 17 00:00:00 2001 From: olaszakos Date: Tue, 5 Mar 2024 11:11:12 +0100 Subject: [PATCH] feat: add description field to wallet (#151) Enable users to add an optional summary to transfer requests. Summary data is stored in Proposal::summary. Updated create transfer form, it is also prepared to load and show an existing transfer along with the summary from its proposal. To support this I added the proposal_id to the Transfer DTO. ![image](https://github.com/dfinity/orbit-wallet/assets/9403182/fafa7ec4-6ff5-432e-88d0-dba340a9d939) Display the summary on the Proposal details dialog: ![image](https://github.com/dfinity/orbit-wallet/assets/9403182/f0094810-1599-4613-9f30-8ddf295dd77d) --- .../integration-tests/src/transfer_tests.rs | 39 +++- .../accounts/TransferDialog.spec.ts | 167 ++++++++++++++++++ .../components/accounts/TransferDialog.vue | 76 ++++++-- .../src/components/accounts/TransferForm.vue | 3 + .../ui/src/generated/wallet/wallet.did.d.ts | 2 + .../ui/src/generated/wallet/wallet.did.js | 2 + canisters/ui/src/services/wallet.service.ts | 4 +- canisters/wallet/api/spec.did | 4 + canisters/wallet/api/src/transfer.rs | 2 + canisters/wallet/impl/src/mappers/account.rs | 19 +- .../wallet/impl/src/mappers/address_book.rs | 3 +- canisters/wallet/impl/src/mappers/transfer.rs | 15 +- 12 files changed, 282 insertions(+), 54 deletions(-) create mode 100644 canisters/ui/src/components/accounts/TransferDialog.spec.ts diff --git a/canisters/integration-tests/src/transfer_tests.rs b/canisters/integration-tests/src/transfer_tests.rs index ef5ca61cb..cb63e0ac1 100644 --- a/canisters/integration-tests/src/transfer_tests.rs +++ b/canisters/integration-tests/src/transfer_tests.rs @@ -11,9 +11,9 @@ use std::time::Duration; use wallet_api::{ AccountPoliciesDTO, AddAccountOperationInput, ApiErrorDTO, ApprovalThresholdDTO, CreateProposalInput, CreateProposalResponse, CriteriaDTO, GetProposalInput, - GetProposalResponse, ListAccountTransfersInput, ListAccountTransfersResponse, MeResponse, - ProposalExecutionScheduleDTO, ProposalOperationDTO, ProposalOperationInput, ProposalStatusDTO, - TransferOperationInput, UserSpecifierDTO, + GetProposalResponse, GetTransfersInput, GetTransfersResponse, ListAccountTransfersInput, + ListAccountTransfersResponse, MeResponse, ProposalExecutionScheduleDTO, ProposalOperationDTO, + ProposalOperationInput, ProposalStatusDTO, TransferOperationInput, UserSpecifierDTO, }; #[test] @@ -163,7 +163,7 @@ fn make_transfer_successful() { // check transfer proposal status let get_proposal_args = GetProposalInput { - proposal_id: proposal_dto.id, + proposal_id: proposal_dto.id.clone(), }; let res: (Result,) = update_candid_as( &env, @@ -185,15 +185,38 @@ fn make_transfer_successful() { }; // proposal has the transfer id filled out - match new_proposal_dto.operation { - ProposalOperationDTO::Transfer(transfer) => { - transfer.transfer_id.expect("transfer id must be set") - } + let transfer_id = match new_proposal_dto.operation { + ProposalOperationDTO::Transfer(transfer) => transfer + .transfer_id + .expect("transfer id must be set for completed transfer"), _ => { panic!("proposal must be Transfer"); } }; + // fetch the transfer and check if its proposal id matches the proposal id that created it + let res: (Result,) = query_candid_as( + &env, + canister_ids.wallet, + WALLET_ADMIN_USER, + "get_transfers", + (GetTransfersInput { + transfer_ids: vec![transfer_id], + },), + ) + .unwrap(); + + let proposal_id_in_transfer_dto = res + .0 + .unwrap() + .transfers + .first() + .expect("One transaction must be returned") + .proposal_id + .clone(); + + assert_eq!(proposal_id_in_transfer_dto, proposal_dto.id); + // check beneficiary balance after completed transfer let new_beneficiary_balance = get_icp_balance(&env, beneficiary_id); assert_eq!(new_beneficiary_balance, ICP); diff --git a/canisters/ui/src/components/accounts/TransferDialog.spec.ts b/canisters/ui/src/components/accounts/TransferDialog.spec.ts new file mode 100644 index 000000000..9483497eb --- /dev/null +++ b/canisters/ui/src/components/accounts/TransferDialog.spec.ts @@ -0,0 +1,167 @@ +import { describe, expect, it, vi } from 'vitest'; +import { mount } from '~/test.utils'; +import TransferDialog from './TransferDialog.vue'; +import { Account, GetProposalResult, Proposal, Transfer } from '~/generated/wallet/wallet.did'; +import DataLoaderVue from '../DataLoader.vue'; +import { flushPromises } from '@vue/test-utils'; +import { services } from '~/plugins/services.plugin'; +import { ExtractOk } from '~/types/helper.types'; + +vi.mock('~/services/wallet.service', () => ({ + WalletService: vi.fn().mockImplementation(() => { + return { + transfer: vi.fn(() => { + return Promise.resolve({} as Proposal); + }), + }; + }), +})); + +describe('TransferDialog', () => { + it('renders correctly', () => { + const wrapper = mount(TransferDialog, { + props: { + account: { + id: '1', + decimals: 1, + } as Account, + open: true, + }, + }); + expect(wrapper.exists()).toBe(true); + }); + + it('shows empty form when transferId not specified', async () => { + const wrapper = mount(TransferDialog, { + props: { + account: { + id: '1', + decimals: 1, + } as Account, + open: true, + }, + }); + await wrapper.vm.$nextTick(); + await flushPromises(); + await wrapper.vm.$nextTick(); + + const dataLoader = wrapper.findComponent(DataLoaderVue); + const form = dataLoader.find(`[data-test-id="transfer-dialog-form"]`); + + const transferId = form.find(`[data-test-id="transfer-form-transfer-id"]`); + const amount = form.find(`[data-test-id="transfer-form-amount"]`); + const destination = form.find(`[data-test-id="transfer-form-destination-address"]`); + const summary = form.find(`[data-test-id="transfer-dialog-proposal-summary"]`); + + // transferId should not be visible when not specified as a prop + expect(transferId.exists()).toBe(false); + + expect(amount.exists()).toBe(true); + expect(destination.exists()).toBe(true); + expect(summary.exists()).toBe(true); + + // amount field should be empty + expect(amount.find('input').element.value).toBe(''); + + // destination field should be empty + expect(destination.find('input').element.value).toBe(''); + + // summary should be empty + expect(summary.find('input').element.value).toBe(''); + }); + + it('creates transfer proposal with summary', async () => { + const wrapper = mount(TransferDialog, { + props: { + account: { + id: '1', + decimals: 1, + } as Account, + open: true, + }, + }); + await wrapper.vm.$nextTick(); + await flushPromises(); + await wrapper.vm.$nextTick(); + + const dataLoader = wrapper.findComponent(DataLoaderVue); + const form = dataLoader.find(`[data-test-id="transfer-dialog-form"]`); + + const amount = form.find(`[data-test-id="transfer-form-amount"]`); + const destination = form.find(`[data-test-id="transfer-form-destination-address"]`); + const summary = form.find(`[data-test-id="transfer-dialog-proposal-summary"]`); + + const submitButton = form.find(`[data-test-id="transfer-dialog-save-button"]`); + + amount.find('input').setValue('1'); + destination.find('input').setValue('destination address'); + summary.find('input').setValue('test summary'); + + await flushPromises(); + + await submitButton.trigger('click'); + + await wrapper.vm.$nextTick(); + await flushPromises(); + + expect(services().wallet.transfer).toHaveBeenCalledWith( + expect.objectContaining({ + amount: 10n, + to: 'destination address', + }), + 'test summary', + ); + }); + + it('loads the corresponding objects to display the transfer and summary if transferId is specified', async () => { + services().wallet.getProposal = vi.fn(() => + Promise.resolve({ + proposal: { + summary: ['test summary'], // it's an opt + } as unknown as Proposal, + } as ExtractOk), + ); + services().wallet.getTransfer = vi.fn(() => + Promise.resolve({ + id: 'transfer-id', + to: 'destination address', + amount: 123n, + proposal_id: 'proposal-id', + } as Transfer), + ); + + const wrapper = mount(TransferDialog, { + props: { + account: { + id: '1', + decimals: 2, + } as Account, + open: true, + transferId: 'transfer-id', + }, + }); + await wrapper.vm.$nextTick(); + await flushPromises(); + await wrapper.vm.$nextTick(); + + expect(services().wallet.getTransfer).toHaveBeenCalledWith('transfer-id'); + expect(services().wallet.getProposal).toHaveBeenCalledWith({ + proposal_id: 'proposal-id', + }); + + const dataLoader = wrapper.findComponent(DataLoaderVue); + const form = dataLoader.find(`[data-test-id="transfer-dialog-form"]`); + + const transferId = form.find(`[data-test-id="transfer-form-transfer-id"]`); + const amount = form.find(`[data-test-id="transfer-form-amount"]`); + const destination = form.find(`[data-test-id="transfer-form-destination-address"]`); + const summary = form.find(`[data-test-id="transfer-dialog-proposal-summary"]`); + + expect(transferId.exists()).toBe(true); + expect(transferId.find('input').element.value).toBe('transfer-id'); + + expect(amount.find('input').element.value).toBe('1.23'); + expect(destination.find('input').element.value).toBe('destination address'); + expect(summary.find('input').element.value).toBe('test summary'); + }); +}); diff --git a/canisters/ui/src/components/accounts/TransferDialog.vue b/canisters/ui/src/components/accounts/TransferDialog.vue index 8cdd820c9..af6adfe8a 100644 --- a/canisters/ui/src/components/accounts/TransferDialog.vue +++ b/canisters/ui/src/components/accounts/TransferDialog.vue @@ -10,9 +10,12 @@ v-slot="{ data }" :load="loadTransfer" @loading="loading = $event" - @loaded="transfer = $event.transfer" + @loaded=" + transfer = $event.transfer; + proposal = $event.proposal; + " > - + {{ $t('terms.transfer') }} @@ -23,10 +26,27 @@ v-model="transfer" v-model:trigger-submit="triggerSubmit" :account="props.account.value" - :disabled="props.readonly.value" + :mode="props.readonly.value ? 'view' : 'edit'" @submit="save" @valid="valid = $event" /> + + + + + + @@ -34,6 +54,7 @@ v-if="!props.readonly.value" :disabled="!canSave" :loading="saving" + data-test-id="transfer-dialog-save-button" @click="triggerSubmit = true" > {{ props.transferId.value ? $t('terms.save') : $t('terms.create') }} @@ -44,7 +65,7 @@