Skip to content

Commit

Permalink
feat: delete swap order
Browse files Browse the repository at this point in the history
We are persisting the order slice and if the users are making a lot of orders it can grow quite a lot. Since right now we are mainly interested in the state in order to show correct notifications, we can optimise and delete the order if the state is fulfilled, cancelled or expired. Those are final states - the next time we see them from the txHistory we can ignore them, if our redux state doesn’t have a created, open or presignaturePending state. If the previous state is undefined - this means that we are not interested in those final states and can ignore them.

.
  • Loading branch information
compojoom committed May 3, 2024
1 parent 2a52109 commit 5a3ded0
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 5 deletions.
79 changes: 77 additions & 2 deletions src/store/__tests__/swapOrderSlice.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { listenerMiddlewareInstance } from '@/store'
import { txHistorySlice } from '@/store/txHistorySlice'
import { swapOrderListener, swapOrderStatusListener, setSwapOrder } from '@/store/swapOrderSlice'
import { swapOrderListener, swapOrderStatusListener, setSwapOrder, deleteSwapOrder } from '@/store/swapOrderSlice'
import {
TransactionListItemType,
type TransactionListItem,
Expand Down Expand Up @@ -121,6 +121,37 @@ describe('swapOrderSlice', () => {
}),
)
})

it('should not dispatch setSwapOrder if the old status is undefined and new status is fulfilled, expired, or cancelled', () => {
const swapTransaction = {
type: TransactionListItemType.TRANSACTION,
transaction: {
id: '0x123',
txInfo: {
type: TransactionInfoType.SWAP_ORDER,
uid: 'order1',
status: 'fulfilled', // Test with 'expired' and 'cancelled' as well
},
},
} as unknown as TransactionListItem

const action = txHistorySlice.actions.set({
loading: false,
data: {
results: [swapTransaction],
},
})

const effect = startListeningMock.mock.calls[0][0].effect
effect(action, {
dispatch: mockDispatch,
getOriginalState: () => ({
swapOrders: {}, // Old status is undefined
}),
})

expect(mockDispatch).not.toHaveBeenCalled()
})
})

describe('swapOrderStatusListener', () => {
Expand Down Expand Up @@ -167,7 +198,7 @@ describe('swapOrderSlice', () => {
})
})

it('should dispatch a notification if the swapOrder status is created and threshold is more than 1', () => {
it('should dispatch a notification if the swapOrder status is created and there is nothing about this swap in the state and threshold is more than 1', () => {
const swapOrder = {
orderUid: 'order1',
status: 'created' as const,
Expand Down Expand Up @@ -199,6 +230,43 @@ describe('swapOrderSlice', () => {
})
})

it('should dispatch a notification if the swapOrder status is open and we have old status and threshold is 1', () => {

Check failure on line 233 in src/store/__tests__/swapOrderSlice.test.ts

View workflow job for this annotation

GitHub Actions / Tests annotations (🧪 jest-coverage-report-action)

swapOrderSlice > swapOrderStatusListener > should dispatch a notification if the swapOrder status is open and we have old status and threshold is 1

Error: expect(jest.fn()).toHaveBeenCalledWith(...expected) - Expected + Received Object { "groupKey": "swap-order-status", - "message": "Waiting for the transaction to be executed", - "title": "Order created", + "message": "Waiting for order execution by the CoW Protocol", + "title": "Order transaction confirmed", "variant": "info", }, Number of calls: 1 at Object.toHaveBeenCalledWith (/home/runner/work/safe-wallet-web/safe-wallet-web/src/store/__tests__/swapOrderSlice.test.ts:262:35) at Promise.then.completed (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/utils.js:298:28) at new Promise (<anonymous>) at callAsyncCircusFn (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/utils.js:231:10) at _callCircusTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:316:40) at processTicksAndRejections (node:internal/process/task_queues:95:5) at _runTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:252:3) at _runTestsForDescribeBlock (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:126:9) at _runTestsForDescribeBlock (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:121:9) at _runTestsForDescribeBlock (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:121:9) at run (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/run.js:71:3) at runAndTransformResultsToJestFormat (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21) at jestAdapter (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19) at runTestInternal (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-runner/build/runTest.js:367:16) at runTest (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-runner/build/runTest.js:444:34) at Object.worker (/home/runner/work/safe-wallet-web/safe-wallet-web/node_modules/jest-runner/build/testWorker.js:106:12)
const swapOrder = {
orderUid: 'order1',
status: 'open' as const,
txId: '0x123',
}

const action = setSwapOrder(swapOrder)

const effect = startListeningMock.mock.calls[0][0].effect
effect(action, {
dispatch: mockDispatch,
getState: () => ({
safeInfo: {
data: {
threshold: 1,
},
},
}),
getOriginalState: () => ({
swapOrders: {
order1: {
orderUid: 'order1',
status: 'created', // Old status is not undefined
},
},
}),
})

expect(showNotificationSpy).toHaveBeenCalledWith({
title: 'Order created',
message: 'Waiting for the transaction to be executed',
groupKey: 'swap-order-status',
variant: 'info',
})
})

it('should dispatch a notification if the swapOrder status is presignaturePending', () => {
const swapOrder = {
orderUid: 'order1',
Expand Down Expand Up @@ -325,6 +393,8 @@ describe('swapOrderSlice', () => {
groupKey: 'swap-order-status',
variant: 'info',
})

expect(mockDispatch).toHaveBeenCalledWith(deleteSwapOrder('order1'))
})

it('should not dispatch a notification if the swapOrder status is expired and old status is undefined', () => {
Expand Down Expand Up @@ -352,6 +422,7 @@ describe('swapOrderSlice', () => {
})

expect(showNotificationSpy).not.toHaveBeenCalled()
expect(mockDispatch).not.toHaveBeenCalledWith(deleteSwapOrder('order1'))
})

it('should dispatch a notification if the swapOrder status is expired and old status is not undefined', () => {
Expand Down Expand Up @@ -389,6 +460,8 @@ describe('swapOrderSlice', () => {
groupKey: 'swap-order-status',
variant: 'warning',
})

expect(mockDispatch).toHaveBeenCalledWith(deleteSwapOrder('order1'))
})

it('should not dispatch a notification if the swapOrder status is cancelled and old status is undefined', () => {
Expand Down Expand Up @@ -416,6 +489,7 @@ describe('swapOrderSlice', () => {
})

expect(showNotificationSpy).not.toHaveBeenCalled()
expect(mockDispatch).not.toHaveBeenCalledWith(deleteSwapOrder('order1'))
})

it('should dispatch a notification if the swapOrder status is cancelled and old status is not undefined', () => {
Expand Down Expand Up @@ -453,6 +527,7 @@ describe('swapOrderSlice', () => {
groupKey: 'swap-order-status',
variant: 'warning',
})
expect(mockDispatch).toHaveBeenCalledWith(deleteSwapOrder('order1'))
})
})
})
15 changes: 12 additions & 3 deletions src/store/swapOrderSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,15 @@ const slice = createSlice({
},
}
},
deleteSwapOrder: (state, { payload }: { payload: string }): SwapOrderState => {
const newState = { ...state }
delete newState[payload]
return newState
},
},
})

export const { setSwapOrder } = slice.actions
export const { setSwapOrder, deleteSwapOrder } = slice.actions
const selector = (state: RootState) => state[slice.name]
export const swapOrderSlice = slice
export const selectAllSwapOrderStatuses = selector
Expand Down Expand Up @@ -72,7 +77,7 @@ export const swapOrderStatusListener = (listenerMiddleware: typeof listenerMiddl
const oldStatus = selectSwapOrderStatus(listenerApi.getOriginalState(), swapOrder.orderUid)
const newStatus = swapOrder.status

if (oldStatus === newStatus) {
if (oldStatus === newStatus || newStatus === undefined) {
return
}

Expand Down Expand Up @@ -125,6 +130,7 @@ export const swapOrderStatusListener = (listenerMiddleware: typeof listenerMiddl
variant: 'info',
}),
)
dispatch(slice.actions.deleteSwapOrder(swapOrder.orderUid))
break
case 'expired':
if (oldStatus === undefined) {
Expand All @@ -138,6 +144,7 @@ export const swapOrderStatusListener = (listenerMiddleware: typeof listenerMiddl
variant: 'warning',
}),
)
dispatch(slice.actions.deleteSwapOrder(swapOrder.orderUid))
break
case 'cancelled':
if (oldStatus === undefined) {
Expand All @@ -151,6 +158,7 @@ export const swapOrderStatusListener = (listenerMiddleware: typeof listenerMiddl
variant: 'warning',
}),
)
dispatch(slice.actions.deleteSwapOrder(swapOrder.orderUid))
break
}
},
Expand Down Expand Up @@ -178,7 +186,8 @@ export const swapOrderListener = (listenerMiddleware: typeof listenerMiddlewareI
const swapOrder = result.transaction.txInfo
const oldStatus = selectSwapOrderStatus(listenerApi.getOriginalState(), swapOrder.uid)

if (oldStatus === swapOrder.status) {
const finalStatuses: AllStatuses[] = ['fulfilled', 'expired', 'cancelled']
if (oldStatus === swapOrder.status || (oldStatus === undefined && finalStatuses.includes(swapOrder.status))) {
continue
}

Expand Down

0 comments on commit 5a3ded0

Please sign in to comment.