Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Payment history CSV receipt links #3477 (WIP) #4312

Closed
wants to merge 4 commits into from
Closed

Payment history CSV receipt links #3477 (WIP) #4312

wants to merge 4 commits into from

Conversation

willy-b
Copy link
Contributor

@willy-b willy-b commented Sep 26, 2016

^^ tests on the way pending general review of this PR

  • Ran git rebase -i to squash commits (if needed).

^^ will probably squash after adding tests

Test Plan:
Needs tests. Not sure if CSV export is desired in interim and so getting that feedback while writing tests to save time.

Willy Bruns added 3 commits September 24, 2016 00:37
for issue #3477
- Adds "Receipt Link" column to Payment History dialog
- Clicking link opens CSV with 5 columns breaking down your contribution by Publisher:
  "Publisher","Votes","Fraction","BTC","USD"

- depends on ledger-client changes NOT in master:
 see branch: https://github.com/willy-b/ledger-client/tree/support-browser-laptop-3477
 specifically https://github.com/willy-b/ledger-client/commit/8c9e728de1f8f1b801dcba4baddc876f54c9167a
- link is now an <a> element
- added an error check to OPEN_LEDGER_TRANSACTION_CSV handler in app/ledger.js
for #3477
@willy-b
Copy link
Contributor Author

willy-b commented Sep 26, 2016

depends on a PR in ledger-client: brave/ledger-client#15


ipc.on(messages.OPEN_LEDGER_TRANSACTION_CSV, (event, viewingIds, csvFilename) => {
if (client) {
var txCsvText = client._getTransactionCSVText(viewingIds)
Copy link
Member

Choose a reason for hiding this comment

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

i would replace all of the below with win.webContents.downloadURL(url), where url is txCsvText as a Blob object URL or perhaps a data URL. then the CSV file will be automatically written to the default downloads folder, and the user can click on the downloads bar to open it.


const win = electron.BrowserWindow.getFocusedWindow()
if (win && win.webContents) {
win.webContents.downloadURL(txCsvTextDataURI)
Copy link
Member

Choose a reason for hiding this comment

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

unfortunately there seems to be no way to specify the filename so it defaults to download.csv. but i'm fine with merging this as-is and fixing the filename in a later issue. thanks!

@diracdeltas diracdeltas added this to the 0.12.3dev milestone Sep 27, 2016
@diracdeltas
Copy link
Member

@mrose17 this looks good to me to merge once ledger-client is updated.

@mrose17
Copy link
Member

mrose17 commented Sep 27, 2016

@willy-b i'm going to merge brave/ledger-client#15 and make a few changes, then push the new version.

@diracdeltas - i will ping you when that is done so you can merge this PR. thanks!

@mrose17
Copy link
Member

mrose17 commented Sep 27, 2016

@diracdeltas - i have published ledger-client@0.8.78 - please update and commit as you see fit. thanks!

@diracdeltas
Copy link
Member

fixing and rebasing this PR now

@mrose17
Copy link
Member

mrose17 commented Sep 27, 2016

+1 thanks!

@diracdeltas
Copy link
Member

cherry-picked 7c44c28

@willy-b
Copy link
Contributor Author

willy-b commented Sep 27, 2016

thanks @diracdeltas & @mrose17!

@diracdeltas diracdeltas removed this from the 0.12.3dev milestone Oct 3, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants