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

Payment History CSV export download has correct filename (+row sorting and total row) #4346

Merged
merged 2 commits into from
Sep 29, 2016
Merged

Payment History CSV export download has correct filename (+row sorting and total row) #4346

merged 2 commits into from
Sep 29, 2016

Conversation

willy-b
Copy link
Contributor

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

Payment receipt CSV links now suggest a download filename with reconciliation date in it matching UI.

depends on brave/ledger-client@d50d59e

Test Plan:

  1. Open/Enable Brave Payments
  2. Browse pages, wait for or trigger reconciliation
  3. Click 'View Payment History' link
  4. Click a receipt link, e.g. brave_ledger9-23-2016.csv
  5. Observe that a filename is suggested in download box matching link text.
  6. Download CSV
  7. Open from downlloads bar
  8. Observe that rows are sorted alphabetically by Publisher
  9. Observe that there is now a "TOTAL" row at bottom with totals for all non-publisher columns

brave_csv_export_filename_4332

simpler: export is just a data URL link, filename suggested by <a> `download` attribute
depends on brave/ledger-client@d50d59e
fixes #4332
@@ -287,18 +286,6 @@ if (ipc) {
if (balanceTimeoutId) clearTimeout(balanceTimeoutId)
balanceTimeoutId = setTimeout(getBalance, 5 * msecs.second)
})

ipc.on(messages.OPEN_LEDGER_TRANSACTION_CSV, (event, viewingIds, csvFilename) => {
Copy link
Member

Choose a reason for hiding this comment

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

please also remove messages.OPEN_LEDGER_TRANSACTION_CSV and aboutActions.receiptLinkClick

Copy link
Member

Choose a reason for hiding this comment

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

otherwise lgtm once @mrose17 releases brave/ledger-client#17

@diracdeltas diracdeltas self-assigned this Sep 28, 2016
@diracdeltas
Copy link
Member

on second thought, it would be better to move the CSV export methods from ledger-client into js/lib/ledgerExportUtil.js so that #4332 can be fixed without requiring ledger-client updates

@diracdeltas diracdeltas added this to the 0.12.4dev milestone Sep 28, 2016
- closes #4332
- rows are sorted by Publisher
- a TOTAL row is added
- moved the ledger transaction utilities into `js/lib/ledgerExportUtil.js` from `ledger-client/util`
  (see #4346 (comment))

- removed some obsolete code (#4346 (comment))
@willy-b
Copy link
Contributor Author

willy-b commented Sep 29, 2016

@diracdeltas, this is done (including rest of #4332) & everything is pulled into js/lib/ledgerExportUtil.js so you don't need to wait on ledger-client :-)

@willy-b willy-b changed the title Payment History CSV export download has correct filename Payment History CSV export download has correct filename (+row sorting and total row) Sep 29, 2016
* Generates a contribution breakdown by publisher as a CSV data URL from an array of one or more transactions
* @param {Object[]} transactions - array of transactions
*/
let transactionsToCSVDataURL = function (transactions) {
Copy link
Member

Choose a reason for hiding this comment

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

it would be good to add unit tests for this. (see test/unit/lib)

return getTransactionCSVRows(transactions, viewingIds, addTotalRow).join('\n')
}

module.exports = {
Copy link
Member

Choose a reason for hiding this comment

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

for future reference, you can just use es6 object property-value shorthand here. https://ariya.io/2013/02/es6-and-object-literal-property-value-shorthand. i.e.,

{
  transactionsToCSVDataURL,
  getTransactionCSVText,
 ...
}

@diracdeltas
Copy link
Member

thanks! merging for now

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.

5 participants