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

Replace Payment History CSV export with Contribution Statement PDFs #4769

Closed
willy-b opened this issue Oct 14, 2016 · 52 comments
Closed

Replace Payment History CSV export with Contribution Statement PDFs #4769

willy-b opened this issue Oct 14, 2016 · 52 comments
Assignees
Milestone

Comments

@willy-b
Copy link
Contributor

willy-b commented Oct 14, 2016

Follow-up to #3477 where we added CSV export to Payment History

This issue tracks replacing the CSV export with a Contribution Statement PDF per @bradleyrichter 's gorgeous original mockup:
pdf_receipt_mockup

@mrose17
Copy link
Member

mrose17 commented Oct 25, 2016

@willy-b - how is this looking? thanks!

@bradleyrichter
Copy link
Contributor

@mrose17 @willy-b I want to get double-duty with this effort on the publisher side as well. These will be great for pubs accounting departments who often need to file docs. Sometimes even printed.

@willy-b
Copy link
Contributor Author

willy-b commented Oct 25, 2016

@mrose17, I'll be pushing some more changes late tonight, thanks

@willy-b
Copy link
Contributor Author

willy-b commented Nov 12, 2016

Latest version in PR #4771:
brave_contributionstatement_latest

Note: Time Spent values are not available at the transaction level, so the final financial contributions are shown. (This is probably what we want anyway given that this is a receipt and the Contribution Amounts are only proportional to the Time Spent on average (due to randomization) -- my two cents).

Now, working on pagination of PDF (if enough publishers are present) with different note box text per page as in mockup. Thanks!

@bradleyrichter
Copy link
Contributor

@willy-b Looking awesome! I'm excited to get this in. Just in time for 1.0.

@willy-b
Copy link
Contributor Author

willy-b commented Nov 13, 2016

Sweet. Just pushed pagination :-).
brave_contributionstatement_withpagination

@mrose17
Copy link
Member

mrose17 commented Nov 29, 2016

@willy-b - congrats!

@bsclifton
Copy link
Member

bsclifton commented Nov 29, 2016

I meant to close the original PR 😂
Re-opening and this'll auto-close when the other is accepted 😄

@bsclifton bsclifton reopened this Nov 29, 2016
@bsclifton bsclifton added this to the 0.13.0 milestone Nov 29, 2016
@bsclifton
Copy link
Member

Merged! Great job, @willy-b! 😄

@srirambv
Copy link
Collaborator

Unable to open PDF on Windows 10 x64 @willy-b @bsclifton
4769

image

@willy-b
Copy link
Contributor Author

willy-b commented Dec 28, 2016

thanks for info @cezaraugusto. knowing which commits to revert to unblock is a big one.
still working on browser-laptop-bootstrap in background.

FYI, the setTabIndex issue on new tab open is the same as #6442 which was opened and closed this AM. will be fixed with that.

@willy-b
Copy link
Contributor Author

willy-b commented Dec 28, 2016

Okay, after doing workaround for setTabIndex issue I see the printToPDF issue.

Since for #6039 / #6385 we are switching from doing a popup "Save as" to opening the generated PDF in PDF.js, I am going to experiment with just switching that out now.

@bbondy
Copy link
Member

bbondy commented Dec 29, 2016

@willy-b I'm going to fix printToPDF in #6450 so please feel free to close this if it is done otherwise.

@willy-b
Copy link
Contributor Author

willy-b commented Dec 29, 2016

to the best of my knowledge this is fine but for #6450 & #6442 (can't tell until those are fixed though).

thanks for taking #6450, I was working on that right now but I'm sure you'll get it faster.

(opening the payment history pdf in PDF.js will continue in PR #6385 for MTR's issue #6039)

@willy-b
Copy link
Contributor Author

willy-b commented Dec 29, 2016

actually let's leave it open until #6450 and #6442 are in so I or whoever can test this before it goes out.

@willy-b
Copy link
Contributor Author

willy-b commented Dec 30, 2016

if @bbondy's #6450 fix for webview.printToPDF API in brave/muon can't make it into browser-laptop by time next release with Chromium54 goes out then we should probably revert this entire feature because it will be broken.

@bbondy
Copy link
Member

bbondy commented Dec 30, 2016

It'l make it in ;)

@willy-b
Copy link
Contributor Author

willy-b commented Dec 30, 2016

cool, I just don't want this feature (PDF stmt) to make it in if it can't be tested / is broken :-). thanks.

@bbondy
Copy link
Member

bbondy commented Dec 31, 2016

This was committed so next preview should have it in (even maybe some platforms current preview), so closing because I think there's nothing remaining here.

@bbondy bbondy closed this as completed Dec 31, 2016
@willy-b
Copy link
Contributor Author

willy-b commented Dec 31, 2016

let's keep it open until those last Muon changes (brave/muon@465539e) actually hit browser-laptop and can be tested -- there might be a regression or who knows, right?

@willy-b willy-b reopened this Dec 31, 2016
@bbondy
Copy link
Member

bbondy commented Jan 1, 2017

Sorry I should have explained the process better that we follow for QA. When the coding is complete they should be marked completed and QA works off of bugs that are in the completed state to verify them.

@bsclifton
Copy link
Member

@willy-b what @bbondy said 😄 You can watch this issue and see the QA labels that get attached. If everything works out fine, you'll see it get tagged like so:
screen shot 2017-01-01 at 10 03 22 am

@luixxiul
Copy link
Contributor

luixxiul commented Jan 2, 2017

@willy-b would you please specify QA steps or the commit which has them in its commit message?

@luixxiul luixxiul added the needs-info Another team member needs information from the PR/issue opener. label Jan 2, 2017
@luixxiul
Copy link
Contributor

luixxiul commented Jan 2, 2017

Also please indicate which PR has actually closed this issue (because there are several PRs which were referenced with this issue it is hard to find which one it is), thanks.

@luixxiul
Copy link
Contributor

luixxiul commented Jan 2, 2017

Lastly, please add QA/no qa required to issues and PRs which do not require QA.

Thanks!

@willy-b
Copy link
Contributor Author

willy-b commented Jan 2, 2017

@luixxiul the changes which fixed this are @bbondy's from #6450:

In Preview7, already tested:

  • brave/muon@9ea426b in brave/muon fixed webview.printToPDF everywhere but Windows (this was in Preview7, I can confirm this works)

Not in Preview7, not yet tested afaik:

  • brave/muon@465539e supposed to fix webview.printToPDF on Windows
    NOTE: overlaps heavily with previous changes, so should test on all platforms after this comes into brave/browser-laptop

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

8 participants