Feat/paginate payments endpoint #892

lissavxo · 2024-11-30T23:09:52Z

Related to #741

Description

Added server side pagination to payments page, before the payments where fetched all at once in a single api call and the pagination was made in the client, now the client will fetch payments using page and pageSize parameters.
Changed Pagination to fetch tx data from database instead of cache, because it was too complex to paginate and order getting data from cache.

Test plan

Run server and check payments page pagination, try to change pageSize and pass between the pages

Klakurka · 2024-11-30T23:19:13Z

Tests failing.

Klakurka

Viewing /payments initially works but if I do an F5 then it throws an error:

Klakurka

Can't get it to build:

chedieck · 2024-12-10T21:08:47Z

Can't get it to build:

This is happening on master, I put #897 up to fix it

Klakurka · 2024-12-14T18:26:56Z

Needs a rebase.

chedieck

One thing about this PR is that it breaks the sorting using other columns. E.g, if we want to order by amount, there is not really a way to do that without looking at all the payments and then sorting them by amount.

Sorting by network would still be possible in this way, since the network of the address is in the cache key name.

The idea of the issue was to avoid fetching all this data at once, which is possible when the user has it ordered by date or network, but not in the other cases (amount & buttons). I don't really see a way out of it, so maybe we can use pagination only in the case that the ordering is set to happen by date and fetch the whole lot of the data if the user wants it sorted in some other way.

chedieck · 2024-12-19T11:19:18Z

redis/paymentCache.ts

+  return allPayments
+}
+
+function sortByDate (keys: string[], order: 'ascending' | 'descending'): string[] {


more clear name: sortWeekKeysByDate, since this function is specific for it.

more clear name: sortWeekKeysByDate, since this function is specific for it.

No need to do that anymore, since this is obsolete, it can be deleted. However, if you think this could be useful for a future task, just put a comment indicating that and rename it for more clarity.

lissavxo · 2024-12-19T18:59:44Z

One thing about this PR is that it breaks the sorting using other columns. E.g, if we want to order by amount, there is not really a way to do that without looking at all the payments and then sorting them by amount.

Sorting by network would still be possible in this way, since the network of the address is in the cache key name.

The idea of the issue was to avoid fetching all this data at once, which is possible when the user has it ordered by date or network, but not in the other cases (amount & buttons). I don't really see a way out of it, so maybe we can use pagination only in the case that the ordering is set to happen by date and fetch the whole lot of the data if the user wants it sorted in some other way.

don’t you think having two types of pagination might end up looking a bit messy?

Because for backend pagination, it’s one component, and for frontend, it’s another. I’d have to create something super complex. Is it really worth it?

chedieck · 2024-12-20T04:11:09Z

don’t you think having two types of pagination might end up looking a bit messy?

Because for backend pagination, it’s one component, and for frontend, it’s another. I’d have to create something super complex. Is it really worth it?

Yeah, would be messy indeed.

The main reason for this issue is that with many payments we get messages like "API response shouldn't be > 4MB", but the overall behavior is better the way already is, I think.

chedieck · 2024-12-20T04:16:16Z

I have an idea: when e.g. requesting it sorted by amount, fetch the list of payments normally (from the DB, in the server), sort it, and then serving it to the client paginated. This fixes the issue of having large API responses and keeps the better answer time if sorted by date. If sorted by amount the answer time will be basically the same as it was already, but is not bad already for what I've tested, even for large addresses.

Klakurka · 2024-12-23T18:19:01Z

I'll wait for Estevao's review.

chedieck

Payments for a button with only ecash address are showing as being bitcoin cash payments

chedieck · 2024-12-27T02:53:27Z

Bug I mentioned looks solved, but now tests are failing.

chedieck

Amount ordering is wrong.

I am thinking is because the ordering is on the cripto amount, not on the converted value taking into account the price at the time of the tx.

services/transactionService.ts

chedieck

Looks good, but when ordering by button name, the network shows BCH icon where it should show the XEC icon.

chedieck

Added requests for the code, many things reminiscent of earlier stages of this PR that need not be created anymore.

services/transactionService.ts

chedieck · 2024-12-29T03:20:26Z

redis/paymentCache.ts

+  return allPayments
+}
+
+function sortByDate (keys: string[], order: 'ascending' | 'descending'): string[] {


more clear name: sortWeekKeysByDate, since this function is specific for it.

No need to do that anymore, since this is obsolete, it can be deleted. However, if you think this could be useful for a future task, just put a comment indicating that and rename it for more clarity.

redis/index.ts

redis/paymentCache.ts

lissavxo · 2024-12-29T18:46:53Z

Looks good, but when ordering by button name, the network shows BCH icon where it should show the XEC icon.

there was a typo in the query, fixed

chedieck

Looks good!

lissavxo requested a review from chedieck November 30, 2024 23:09

lissavxo requested a review from Klakurka December 1, 2024 17:16

Klakurka requested changes Dec 3, 2024

View reviewed changes

lissavxo force-pushed the feat/paginate-payments-endpoint branch from 734222c to 2d87333 Compare December 9, 2024 15:33

lissavxo requested a review from Klakurka December 9, 2024 15:34

Klakurka requested changes Dec 10, 2024

View reviewed changes

lissavxo force-pushed the feat/paginate-payments-endpoint branch from 2d87333 to f32747f Compare December 15, 2024 16:15

lissavxo requested a review from Klakurka December 15, 2024 16:15

Klakurka previously approved these changes Dec 15, 2024

View reviewed changes

chedieck dismissed Klakurka’s stale review via 1fdf64c December 16, 2024 14:43

Klakurka previously approved these changes Dec 16, 2024

View reviewed changes

lissavxo dismissed Klakurka’s stale review via 8162403 December 17, 2024 22:47

Klakurka previously approved these changes Dec 17, 2024

View reviewed changes

chedieck requested changes Dec 19, 2024

View reviewed changes

lissavxo dismissed Klakurka’s stale review via 9b9aa10 December 23, 2024 17:15

lissavxo requested review from Klakurka and chedieck December 23, 2024 17:33

lissavxo force-pushed the feat/paginate-payments-endpoint branch from 4a6a95b to 729de7d Compare December 26, 2024 15:58

chedieck requested changes Dec 26, 2024

View reviewed changes

lissavxo force-pushed the feat/paginate-payments-endpoint branch from 84f3d90 to 3dbf6cb Compare December 26, 2024 21:08

lissavxo force-pushed the feat/paginate-payments-endpoint branch from 3dbf6cb to 02c386c Compare December 27, 2024 16:04

chedieck requested changes Dec 27, 2024

View reviewed changes

services/transactionService.ts Outdated Show resolved Hide resolved

lissavxo requested a review from chedieck December 27, 2024 21:50

lissavxo force-pushed the feat/paginate-payments-endpoint branch 3 times, most recently from 045c4d5 to 74b82d9 Compare December 27, 2024 22:03

lissavxo and others added 10 commits December 27, 2024 19:28

feat: add getCachedPaymentsForUserWithPagination method

d719d03

feat: use getCachedPaymentsForUserWithPagination method

bfeb299

feat: paginate payments

01e784b

fix: localstorage undefined

cca15af

refactor: better variable names

22f108a

feat: add sorting payments

1be130c

feat: fetch payments from db

4df12c4

tests: fix integration test

f6afd8c

fix: networkid payments filter

80d341b

fix: amount sort

2315345

lissavxo force-pushed the feat/paginate-payments-endpoint branch from 74b82d9 to 2315345 Compare December 27, 2024 22:29

chedieck requested changes Dec 29, 2024

View reviewed changes

lissavxo added 2 commits December 29, 2024 15:35

chore: clean up

8288e65

fix: type networkId query

0a3babe

chedieck approved these changes Dec 29, 2024

View reviewed changes

Klakurka approved these changes Dec 31, 2024

View reviewed changes

Klakurka merged commit 32dd993 into master Dec 31, 2024
2 checks passed

Feat/paginate payments endpoint #892

Feat/paginate payments endpoint #892

Uh oh!

Conversation

lissavxo commented Nov 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Test plan

Uh oh!

Klakurka commented Nov 30, 2024

Uh oh!

Klakurka left a comment

Choose a reason for hiding this comment

Uh oh!

Klakurka left a comment

Choose a reason for hiding this comment

Uh oh!

chedieck commented Dec 10, 2024

Uh oh!

Klakurka commented Dec 14, 2024

Uh oh!

chedieck left a comment

Choose a reason for hiding this comment

Uh oh!

chedieck Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

chedieck Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lissavxo commented Dec 19, 2024

Uh oh!

chedieck commented Dec 20, 2024

Uh oh!

chedieck commented Dec 20, 2024

Uh oh!

Klakurka commented Dec 23, 2024

Uh oh!

chedieck left a comment

Choose a reason for hiding this comment

Uh oh!

chedieck commented Dec 27, 2024

Uh oh!

chedieck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chedieck left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chedieck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chedieck Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lissavxo commented Dec 29, 2024

Uh oh!

chedieck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lissavxo commented Nov 30, 2024 •

edited

Loading

chedieck Dec 29, 2024 •

edited

Loading

chedieck left a comment •

edited

Loading

chedieck Dec 29, 2024 •

edited

Loading