Small adjustments to Payment History modal and CSV files

[mrose17]

Items to improve:

1 - in "Your Payment History", each link points to "download.csv regardless of the link name in the list ...The save-as file name should match the link name in the list.

2 - CSV file should be sorted by the "Publisher" value

3 - a final record should be present called "TOTAL" which sums the values in each column

Assigned to @diracdeltas - as the implementor of #3477

cc: @bradleyrichter

[mrose17]

[willy-b]

Will you have to modify WebContents#downloadURL in brave/electron's atom/browser/api/atom_api_web_contents.cc in order to support a suggested filename?

[diracdeltas]

@willy-b that's one way to do it; the other is to send the transaction info to the front-end so you can just use an <a> element with a filename

[willy-b]

Nice. The transaction info is even already present in front-end, so only the transaction->CSV logic needs to be made accessible to js/about/preferences.js.(Then <a href={dataURL} download={csvFilename}>{csvFilename}</a> will work.)

We could either expose the CSV methods as "static" functions in ledger-client so that preferences.js can use them without creating a ledger-client instance, or move the logic to preferences.js. (I'm happy to do either today if you haven't done this.)

Independently of this issue, it would be nice for WebContents#downloadURL to have the option to specify a filename.

[diracdeltas]

@willy-b feel free to start on this issue; i think moving the CSV logic to preferences.js or to a js/lib/ util makes the most sense

[willy-b]

@diracdeltas, I opened a WIP PR (#4346) switching to an <a> w/ data URL and download attribute as you suggested, and the name issue is fixed.

I did move the CSV export stuff out, but to ledger-client/util.js instead of into browser-laptop.
But it is being require-ed all by itself asrequire('ledger-client/util'), without pulling in rest of ledger-client or its dependencies / messing up webpack.

(If that seems awkward to you, etc, we can just move util.js to browser-laptop/js/lib/ledgerExportUtil.js, as it's been factored out -- there are now no ledger-client dependencies.)

[bbondy]

will not be in 0.12.3rc3

[alexwykoff]

what about the edge case of multiple transactions on the same day (as seen in the screenshot)? should there be a differentiator for the file name? otherwise the user is likely to overwrite their own file on export.

[mrose17]

that is possible for developers and others who want to mess with ledger-state.json ... i wouldn't consider getting the overwrite file warning, the expected behavior...

[willy-b]

@alexwykoff, @mrose17 that makes sense.
What do you think about putting the edge case in a new issue so as not to confuse with the changes here which also fix a crash?
I created #4605 to track that edge case, lmk if you prefer not to do it that way