Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

fix: history polling next/transactions remaining pending #3992

Merged
merged 7 commits into from
Jun 22, 2022
Merged

Conversation

iamacook
Copy link
Member

What it solves

History pointers polling next

How this PR fixes it

loadHistoryTransactions has been split back to loadHistoryTransactions and loadPagedHistoryTransactions, the latter used for loading next+ pageUrls.

How to test it

  1. Open a Safe with a large history, scroll to the second page. Enable a filter that also has a large amount. Observe the correct endpoints was queried and that is is correctly paginated. Clearing the filter should return to the history results, which should also be paginated correctly.
  2. Open a Safe with a large history, create and execute a transaction. Observe the transaction no longer remains pending but moves to the history when successfully indexed.

@iamacook iamacook self-assigned this Jun 22, 2022
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Jun 22, 2022

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 1 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@coveralls
Copy link

coveralls commented Jun 22, 2022

Pull Request Test Coverage Report for Build 2542661777

  • 25 of 38 (65.79%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 37.62%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/routes/safe/components/Transactions/TxList/Filter/index.tsx 7 12 58.33%
src/logic/safe/store/actions/transactions/fetchTransactions/loadGatewayTransactions.ts 18 26 69.23%
Files with Coverage Reduction New Missed Lines %
src/logic/safe/store/actions/transactions/fetchTransactions/loadGatewayTransactions.ts 1 49.4%
Totals Coverage Status
Change from base Build 2540671449: 0.3%
Covered Lines: 4058
Relevant Lines: 9816

💛 - Coveralls

@github-actions
Copy link

Deployment links

Mainnet     🟣 Polygon     🟠 Rinkeby

Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK, let's test it 👍

@francovenica
Copy link
Contributor

Ok, so the issue with tx getting stuck in pending is fixed, but there are issues with the filter now.

If you search for something like "incomming" > "amount" It will show the tx that meet the criteria, but then it will load a page with what it seems no filtering criteria (basically just looking for the next page)

Try in this safe:
https://pr3992--safereact.review-safe.gnosisdev.com/app/rin:0xFfDC1BcdeC18b1196e7FA04246295DE3A17972Ac/transactions/history

Filter by:
Incomming > Amount > 10
Only 1 tx should meet the criteria, but a bunch of tx are shown

gif
test1

@iamacook
Copy link
Member Author

If you search for something like "incomming" > "amount" It will show the tx that meet the criteria, but then it will load a page with what it seems no filtering criteria (basically just looking for the next page)

I've found the issue and am looking into it now.

@francovenica
Copy link
Contributor

Everything works now.
The tx with the pending status are no more, which was the main issue that this PR needed to solve
The filtering is working fine, the issue reported in the previous comment was fixed

The following gif is just to showcase a few filters in action. Everything worked as expected:
test1

@iamacook iamacook merged commit a56f2de into dev Jun 22, 2022
@iamacook iamacook deleted the history-polling branch June 22, 2022 14:27
@github-actions github-actions bot locked and limited conversation to collaborators Jun 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants