Skip to content
This repository has been archived by the owner on Nov 27, 2018. It is now read-only.

Helper methods for summarizing contributions and CSV export (browser-laptop#3477) #15

Merged
merged 4 commits into from
Sep 27, 2016
Merged

Helper methods for summarizing contributions and CSV export (browser-laptop#3477) #15

merged 4 commits into from
Sep 27, 2016

Conversation

willy-b
Copy link
Contributor

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

Adds CSV export for ledger transaction/contribution data by publisher
supports brave/browser-laptop#3477
needed for PR brave/browser-laptop#4312

A couple contribution summary helper methods:

  • Client#_publisherVoteData([viewingIds])
    get a contribution summary organized by publisher for 1, N (an array), or all transactions
    (useful for ledger transaction history should have receipt links browser-laptop#3477)
  • Client#_totalContribution([viewingIds])
    get the total amount contributed in BTC and fiat for 1, N (an array), or all transactions
  • Client#_transactionByViewingId(viewingId) (self-explanatory)

Willy Bruns added 4 commits September 16, 2016 10:39
- Client#_publisherVoteData([viewingIds])
  get a contribution summary organized by publisher for 1, N (an array), or all transactions
  (useful for brave/browser-laptop#3477)

- Client#_totalContribution([viewingIds])
  get the total amount contributed in BTC and fiat for 1, N (an array), or all transactions

- Client#_transactionByViewingId(viewingId) (self-explanatory)
@@ -275,6 +275,160 @@ Client.prototype.ballots = function (viewingId) {
return count
}

Client.prototype._totalContribution = function (viewingIds) {
Copy link
Member

Choose a reason for hiding this comment

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

what is the entry point that users of this module are going to access? it should be the function that doesn't start with a "_" and is defined between ballots and vote, the other functions should be defined between _updatedRules and _roundtrip

Copy link
Member

Choose a reason for hiding this comment

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

it's _getTransactionCSVText, which shouldn't begin with an underscore


var rows = [headerRow]

rows = rows.concat(publishers.map(function (pub) {
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't there be some kind of CSV quoting for the publisher name?

pubRow.votes,
pubRow.fraction,
pubRow.contribution.satoshis / Math.pow(10, 10),
pubRow.contribution.fiat.toFixed(2) + ' ' + pubRow.contribution.currency
Copy link
Member

Choose a reason for hiding this comment

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

you should add a comment such as

    // TBD: update later to reflect correct number of fixed points for fiat currency in use

@mrose17 mrose17 merged commit a0b9a9f into brave:master Sep 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants