Export wallet history as CSV

[rleed]

Added the green button to download a CSV file (closes #217):

The downloaded file looks something like this:

The filter options are taken into effect for the CSV file.

[huumn]

Nice! As far as I can tell though, this will only allow you to export the visible page. People probably want to export everything.

The csv will probably need to be made on the server like our rss feed, lib/rss.js

[rleed]

Ah ok, makes sense.

[rleed]

I think this is ready for (re)review now.

[huumn]

Looking good!

The walletHistory resolver by default returns a single page (21 items) so you'll need to page over it to return the full walletHistory.

[huumn]

Btw I'm going to start doing pr related work on the weekend. So I'll go through all prs this weekend and merge anything that's ready.

[huumn]

I put this in draft until we get paging in it.

[huumn]

This is going to require more work before deploying. I've enhanced it but it's still not there for prod yet. I'm going to put it in draft.

At the very least it needs:

• rate limits

Ideally it'd also have:

• cancelation
• caching

[rleed]

Good point about rate limit. I expected there'd be ongoing improvements to this functionality beyond the scope of this PR/issue, but I see that rate limiting is important to include before deploying on the live dataset.

Do we already have guidelines/requirements about rate limits? In the simplest case I could introduce a paging delay so that time becomes the limiting factor for large datasets. This should at least provide basic protection (leaving room for future improvement in UX), but I'm not sure if this is enough because those who care most about this feature are the ones who are likely to have large amounts of data.

A progress indicator would also be important I think.

[huumn]

Do we already have guidelines/requirements about rate limits? In the simplest case I could introduce a paging delay so that time becomes the limiting factor for large datasets. This should at least provide basic protection (leaving room for future improvement in UX), but I'm not sure if this is enough because those who care most about this feature are the ones who are likely to have large amounts of data.

If an operation is very expensive, then we don't do it or do it infrequently. This operation is very expensive. For my account, a full wallet history CSV is 2.2MB.

A paging delay solves a certain kind of DoS vector, but it doesn't solve the one where someone requests tens of thousands of csv downloads simultaneously.

[rleed]

Requests can be queued for files to be generated inexpensively in the background and a notification sent when ready. Realistically I think what people need is to do a monthly download of a month-long time range or a yearly download of a year-long time range, or a download of everything since the last download, or a download of everything once. All of those scenarios equate to the same: a single download of all data once per user. More than that, charge sats...it's the ideal rate-limiter!

[huumn]

Requests can be queued for files to be generated inexpensively in the background and a notification sent when ready.

This is a great idea!

If we cache these files, new downloads can also check the cache for existing ones and append any new data to it.

[rleed]

I seem to have hosed my system somehow by switching builds. Now even after a fresh clone of the original stackernews repo and docker-compose up --build I still get the following error repeating in the log:

2023-08-30 16:09:22 app | prisma:error
2023-08-30 16:09:22 app | Invalid prisma.user.update() invocation:
2023-08-30 16:09:22 app |
2023-08-30 16:09:22 app |
2023-08-30 16:09:22 app | An operation failed because it depends on one or more records that were required but not found. Record to update not found.
2023-08-30 16:09:22 app | PrismaClientKnownRequestError:
2023-08-30 16:09:22 app | Invalid prisma.user.update() invocation:
2023-08-30 16:09:22 app |
2023-08-30 16:09:22 app |
2023-08-30 16:09:22 app | An operation failed because it depends on one or more records that were required but not found. Record to update not found.
2023-08-30 16:09:22 app | at Hr.handleRequestError (/app/node_modules/@prisma/client/runtime/library.js:122:6999)
2023-08-30 16:09:22 app | at Hr.handleAndLogRequestError (/app/node_modules/@prisma/client/runtime/library.js:122:6388)
2023-08-30 16:09:22 app | at Hr.request (/app/node_modules/@prisma/client/runtime/library.js:122:6108)
2023-08-30 16:09:22 app | at async l (/app/node_modules/@prisma/client/runtime/library.js:126:10298) {
2023-08-30 16:09:22 app | code: 'P2025',
2023-08-30 16:09:22 app | clientVersion: '5.1.1',
2023-08-30 16:09:22 app | meta: { cause: 'Record to update not found.' }
2023-08-30 16:09:22 app | }

I've tried docker-compose rm -f without success. Any tips on what else I can do to reset the whole thing and get a fresh start?

[huumn]

docker-compose down --rmi all -v might work

[rleed]

That was annoying... it turned out to be caused by the browser, that's why no amount of cleaning helped, not even the command you suggested. Clearing all browser data finally fixed it.

[ekzyis]

That was annoying... it turned out to be caused by the browser, that's why no amount of cleaning helped, not even the command you suggested. Clearing all browser data finally fixed it.

Sorry for not chiming in earlier. I think you mentioned a similar problem somewhere else; not sure.

Clearing the browser data fixed it because you probably still had a cookie for the old database data. So it tried to update an user with a valid cookie which doesn't exist.

But regarding the branch / PR status: Did the branch got messed up? I see duplicate commits (same commit messages) with changes that don't seem to be related to this PR. Maybe something got messed up during a merge?

I would recommend a clean rebase on master, no merge.

[rleed]

Clearing the browser data fixed it because you probably still had a cookie for the old database data. So it tried to update an user with a valid cookie which doesn't exist.

Thanks for the insight!

But regarding the branch / PR status: Did the branch got messed up? I see duplicate commits (same commit messages) with changes that don't seem to be related to this PR. Maybe something got messed up during a merge?

Yes, sorry.

I would recommend a clean rebase on master, no merge.

Agreed! I will take care of it.

I am hoping to push some updated code for this PR today as well, to hook up to the worker process!

[SatsAllDay]

I'm very eager to see what this PR turns into. I think the idea of doing this kind of data export with async jobs in the background is very interesting. Keep it up!

[rleed]

Partial update...still in work.
These diagrams document how the interactions work...

[ekzyis]

Going to take a look at this within the next 24 hours. Mentioning this here to keep myself accountable, lol

[rleed]

Going to take a look at this within the next 24 hours. Mentioning this here to keep myself accountable, lol

Great! Appreciate experienced feedback at this point. I think its working well, but I feel like some parts of the code still have room for improvement/simplification where I might have overlooked some better ways of doing things.

• Files are currently created in the root folder and shared between worker and app. Ok or can we improve?
• Is there a way to share GQL queries between app (ES6) and worker (non-module)?
• I added a user id variable to the query for when it is called by the worker. Is there a better way? (to provide "me"?)
• Do you like the UI (button color changes, anim, etc)? Maybe it's too much complexity again for UX?

Surely you will find other things too, but those are areas I am questioning.

[ekzyis]

Okay, I have taken a quick look over everything and tested generating + downloading CSVs. That worked like a charm and I like the "generating CSV" animation :)

Left some comments and will do another pass after I slept since it got late for me.

[ekzyis]

You can use lib/constants.js for these variables. You will have to use CommonJS though since some constants are imported by the worker which only supports CommonJS so far

[rleed]

Ahhh wonderful. I was looking for a way to share code between app and worker... now I understand how to do it via CommonJS. I've now done so with the enum constants.

I would still like a solution for sharing the GQL template literals...I don't think putting those in the lib/constants.js file would be the right organization. Do you have any suggestions?

[ekzyis]

I would still like a solution for sharing the GQL template literals...I don't think putting those in the lib/constants.js file would be the right organization. Do you have any suggestions?

My answer here should answer your question: I don't think you need to share GQL template literals with the worker

[huumn]

This is solid. There are some changes I'll want to make before shipping to prod, which are nontrivial like storing the CSV on S3 and giving authenticated access to the link, but in general @rleed you can consider this PR done.

I'm going to move to draft until I get around to those changes (which might take me a couple weeks).

[rleed]

Awesome. Thank you!

[huumn]

Remaining todos (for me):

• use s3 and authenticated links in prod