Skip to content

Commit

Permalink
fix(TransactionFeedV2): Fix persisted feed storing unmerged transacti…
Browse files Browse the repository at this point in the history
…ons (#6205)

### Description
While working on a different feed issue, I've noticed there's a bug in
storing the first page of the feed to the persisted storage. TLDR,
currently it stores "raw" transactions from pagination, which are not
merged with the stand by transactions. This still sometimes causes
transactions to rearrange on the first load.

This fix moves updating of the feed from to component level. It
contradicts the current
[Redux-driven](https://redux.js.org/style-guide/#put-as-much-logic-as-possible-in-reducers)
approach as it moves processing that can be managed on reducer level
back to the component level but in order to make it follow that
guideline we will need to:
- move `paginatedData` to Redux
- move `confirmedTransactions` construction to Redux
- keep `paginatedData` away from persisted storage with an extra
dependency [like
this](https://github.com/edy/redux-persist-transform-filter) or tweak
the current persist settings as per their [nested persist
guide](https://github.com/rt2zz/redux-persist?tab=readme-ov-file#nested-persists)
which might cause issues with existing persistence-related tests
therefore create more friction to implement this seemingly simple fix.

[Initial PR review
discussion](#6157 (review))
made sense at the time of working on that PR but back then I overlooked
the fact that I've implemented it with this very bug as I was putting
the wrong `data.transactions` data into the feed instead of the right
`paginatedData[FIRST_PAGE_CURSOR]`.

### Test plan
Existing test to check proper rehydration of the persisted feed passes.

### Related issues

- Relates to RET-1207

### Backwards compatibility
Yes

### Network scalability

If a new NetworkId and/or Network are added in the future, the changes
in this PR will:

- [x] Continue to work without code changes, OR trigger a compilation
error (guaranteeing we find it when a new network is added)
  • Loading branch information
sviderock authored Nov 5, 2024
1 parent fd30a91 commit 52fb491
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
15 changes: 14 additions & 1 deletion src/transactions/feed/TransactionFeedV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import TokenApprovalFeedItem from 'src/transactions/feed/TokenApprovalFeedItem'
import TransferFeedItem from 'src/transactions/feed/TransferFeedItem'
import NoActivity from 'src/transactions/NoActivity'
import { allStandbyTransactionsSelector, feedFirstPageSelector } from 'src/transactions/selectors'
import { updateFeedFirstPage } from 'src/transactions/slice'
import {
FeeType,
TokenTransactionTypeV2,
Expand Down Expand Up @@ -202,7 +203,8 @@ function mergeStandByTransactionsInRange({
return inRange || newTransaction || veryOldTransaction
})
const deduplicatedTransactions = deduplicateTransactions([...transactions, ...standByInRange])
const transactionsFromAllowedNetworks = deduplicatedTransactions.filter((tx) =>
const sortedTransactions = sortTransactions(deduplicatedTransactions)
const transactionsFromAllowedNetworks = sortedTransactions.filter((tx) =>
allowedNetworks.includes(tx.networkId)
)

Expand Down Expand Up @@ -426,6 +428,17 @@ export default function TransactionFeedV2() {
[newlyCompletedCrossChainSwaps]
)

useEffect(
function updatePersistedFeedFirstPage() {
const isFirstPage = !data?.pageInfo.hasPreviousPage
if (isFirstPage) {
const firstPageData = paginatedData[FIRST_PAGE_CURSOR]
dispatch(updateFeedFirstPage({ transactions: firstPageData }))
}
},
[paginatedData, data?.pageInfo]
)

const confirmedTransactions = useMemo(() => {
const flattenedPages = Object.values(paginatedData).flat()
const deduplicatedTransactions = deduplicateTransactions(flattenedPages)
Expand Down
1 change: 1 addition & 0 deletions src/transactions/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,5 @@ export const pendingStandbyTxHashesByNetworkIdSelector = createSelector(
)

const feedFirstPage = (state: RootState) => state.transactions.feedFirstPage

export const feedFirstPageSelector = createSelector(feedFirstPage, (feed) => feed)
18 changes: 11 additions & 7 deletions src/transactions/slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,11 @@ const slice = createSlice({
standbyTransactions: updatedStandbyTransactions,
}
},

updateFeedFirstPage: (state, action: PayloadAction<{ transactions: TokenTransaction[] }>) => ({
...state,
feedFirstPage: action.payload.transactions,
}),
},

extraReducers: (builder) => {
Expand All @@ -221,10 +226,6 @@ const slice = createSlice({
* Whenever we get new data from the feed pagination - we need to perform updates on some portion
* of our reducer data, as side-effects. These scenarios include:
*
* - Updating "feedFirstPage" whenever we get new data for the first page. We use this to instantly
* show the user something that can be interacted with while we're actually refetching the latest
* state in the background.
*
* - In order to avoid bloating stand by transactions with confirmed transactions that are already
* present in the feed via pagination – we need to clean them up. This must run for every page
* as standByTransaction might include very old transactions. We should use the chance whenever
Expand All @@ -233,14 +234,12 @@ const slice = createSlice({
builder.addMatcher(
transactionFeedV2Api.endpoints.transactionFeedV2.matchFulfilled,
(state, { payload, meta }) => {
const isFirstPage = meta.arg.originalArgs.endCursor === undefined
const confirmedTransactionsFromNewPage = payload.transactions
.filter((tx) => tx.status !== TransactionStatus.Pending)
.map((tx) => tx.transactionHash)

return {
...state,
feedFirstPage: isFirstPage ? payload.transactions : state.feedFirstPage,
standbyTransactions: state.standbyTransactions.filter((tx) => {
/**
* - ignore empty hashes as there's no way to compare them
Expand All @@ -256,7 +255,12 @@ const slice = createSlice({
},
})

export const { addStandbyTransaction, transactionConfirmed, updateTransactions } = slice.actions
export const {
addStandbyTransaction,
transactionConfirmed,
updateTransactions,
updateFeedFirstPage,
} = slice.actions

export const { actions } = slice

Expand Down

0 comments on commit 52fb491

Please sign in to comment.