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

ledger transaction history should have receipt links #3477

Closed
diracdeltas opened this issue Aug 27, 2016 · 18 comments
Closed

ledger transaction history should have receipt links #3477

diracdeltas opened this issue Aug 27, 2016 · 18 comments
Assignees
Milestone

Comments

@diracdeltas
Copy link
Member

follow-up to #3473, #2994

@diracdeltas
Copy link
Member Author

@willy-b are you taking this one?

@willy-b
Copy link
Contributor

willy-b commented Aug 27, 2016

yep! thanks

@mrose17
Copy link
Member

mrose17 commented Sep 7, 2016

@willy-b - do you have a timeframe for this one? just checking, thanks!

@willy-b
Copy link
Contributor

willy-b commented Sep 7, 2016

yes, will update this afternoon (asked Q's in slack instead, updating today 09/10/16)

(pardon the radio silence, went down a bit of a rabbit hole messing with ledger-client to try and make it easier to generate the synopsis data locally to test this. always feel free to prod me if you want an update :-)! )

@mrose17
Copy link
Member

mrose17 commented Sep 7, 2016

@willy-b - thanks. no worries, just checking in... i integrated your changes into the ledger-publisher package with an update to handle the usual corner cases (-;

@willy-b
Copy link
Contributor

willy-b commented Sep 10, 2016

Hey @mrose17, this is NOT at all a blocker, but do we still plan to have a ballots field on the transactions object like we used to? (I have a workaround, so not depending on this.)

In a comment when you were adding the contribution field, you gave an example transaction (#2994 (comment)):

 {
      "viewingId": "f0dfd6dc-b8ac-4635-b0df-36ffde8524dd",
      "contribution": {
        "fiat": {
          "amount": 5,
          "currency": "USD"
        },
        "rates": {
          "USD": 581.33
        },
        "satoshis": 850362,
        "fee": 9735
      },
      "submissionStamp": 1472170952704,
      "count": 49,
      /** BALLOTS FIELD BELOW! **/
      "ballots": {
        "wsj.com": 3,
        "st-george-melkite.org": 1,
        "jsonprettyprint.com": 1
      }
    }

Which has a ballots field with vote counts.

The current transaction I am seeing (LMK if my code is just out of date, etc), has no ballots field (BUT does have enough data for me to work around this):

{ viewingId: '51812a6b-677b-4e41-aec8-67bb20727f4b',
  surveyorId: '6+BcBKuLcgzYginI1+HJtOGk/QVpuTN1gLXWTiPl2xJ',
  contribution: 
   { fiat: { amount: 5, currency: 'USD' },
     rates: { USD: 626.37 },
     satoshis: 788524,
     fee: 9726 },
  submissionStamp: 1473465726257,
  submissionDate: '2016-09-10T00:02:06.257Z',
  submissionId: '133d4a975cf4d58f298529b09dcc67dedc8e468c3655f64c754be8dff0af56f2',
  credential: '[...]',
  surveyorIds: 
   [ '4xMzVmq3h3uovZKA0JT7mQRp1VTyA4vVQWD7qgZ5w2g',
     'D46qlIjWeYYoQ4PsoX6z2oyIO361bwkTbEmA/gkJD9J',
     '8Bq3sja8bvx9SIG5UoXs6Exlb9AHcxJikAYhBW1aPg5' ],
  count: 3,
  satoshis: 788524,
  votes: 3 
 /** NO BALLOTS HERE **/
}

I am working around this by associating surveyorIds with client.state.ballots myself but this will be unnecessary if the ballots field is added.

@diracdeltas
Copy link
Member Author

@mrose17 what's the status on ^ ? just want to make sure @willy-b is not blocked

@mrose17
Copy link
Member

mrose17 commented Sep 13, 2016

@diracdeltas - my understanding is that this isn't a blocker.

@willy-b do you agree? please let me know, thanks!

@diracdeltas diracdeltas modified the milestones: 0.12.2dev, 1.0.0 Sep 16, 2016
@diracdeltas
Copy link
Member Author

@willy-b i moved this to 0.12.2; let me know if that's a reasonable timeframe. if not i can pick up this issue.

@willy-b
Copy link
Contributor

willy-b commented Sep 16, 2016

Yes, @diracdeltas, 0.12.2 works for me.

Sorry for delay -- was on a cross-country bus trip & broke my phone, completely lost internet access for a couple of days for first time in a while :-).

Confirming: ballots field issue I mentioned to @mrose17 is not a blocker, though I have changes in my fork of ledger-client that'll have to come in.

@mrose17 mrose17 modified the milestones: 0.12.3dev, 0.12.2dev Sep 17, 2016
@mrose17 mrose17 closed this as completed Sep 22, 2016
@mrose17 mrose17 reopened this Sep 22, 2016
@willy-b
Copy link
Contributor

willy-b commented Sep 22, 2016

Yeah, sorry, this is still not ready. @diracdeltas, @mrose17 feel free to take this if you want to -- I've been holding it a while.
I'm getting bogged down with issues around reconciliation and a very, very slow testing cycle.
Whenever the ledger data format / related code changes upstream it can take me up to ~1.5hrs to get Brave to reconcile and allocate a payment in order to start testing the Payment History UI.
Obviously, I/we should set up mock ledger data (always in latest format) so we can skip the slow reconciliation step for pure UI work / tests.

@mrose17
Copy link
Member

mrose17 commented Sep 22, 2016

sorry, i closed the issue earlier by accident...

@willy-b: understood, thanks for the update!

@diracdeltas
Copy link
Member Author

no problem, i'll assign to myself

@diracdeltas diracdeltas self-assigned this Sep 22, 2016
@willy-b
Copy link
Contributor

willy-b commented Sep 23, 2016

Thanks, sorry! Didn't want to hold up dev on this (especially now that I have ended up somewhere where I can't even get on the internet reliably).

@willy-b
Copy link
Contributor

willy-b commented Sep 24, 2016

@diracdeltas, in case you were busy with other issues and didn't dig into this yet, I pushed a branch that implements the CSV export of contribution breakdown by publisher:
https://github.com/willy-b/browser-laptop/tree/payment-history-receipt-links-3477

This depends on a ledger-client branch:
https://github.com/willy-b/ledger-client/tree/support-browser-laptop-3477

You can ignore these if you have your own WIP or want to wait to do the PDF, you can use it for reference, or I can turn them into PRs against browser-laptop & ledger-client. Just sharing in case it is useful / etc!

@willy-b
Copy link
Contributor

willy-b commented Sep 24, 2016

So I used a file:// link to the generated CSV, and it's a little awkward (it triggers a download instead of opening):
brave_csv_export_test

Seems like there must be a better way.
(With a PDF this won't be an issue, it'll open in-browser, in pdfjs.)
Is there a different IPC message that would trigger opening the local file directly in the 3rd party app?

diracdeltas pushed a commit that referenced this issue Sep 27, 2016
for issue #3477
- Adds "Receipt Link" column to Payment History dialog
- Clicking link downloads CSV with 5 columns breaking down your contribution by Publisher:
  "Publisher","Votes","Fraction","BTC","USD"

auditor: @diracdeltas
test plan:
1. open payments panel
2. click 'transaction history'
3. click on receipt link in the right column
4. open the downloaded receipt and make sure it has payment info
@diracdeltas
Copy link
Member Author

closed by 7c44c28

@aekeus
Copy link
Member

aekeus commented Sep 28, 2016

screen shot 2016-09-28 at 7 14 04 pm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants