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

Remove contribution range from PDF statement #6897

Closed
wants to merge 2 commits into from
Closed

Conversation

mrose17
Copy link
Member

@mrose17 mrose17 commented Jan 27, 2017

Fixes #6896

Auditor: @bsclifton

Test Plan:

  1. Generate a PDF contribution statement
  2. Verify that the contribution range you see here as the last line isn't there any more.

729d24fa-e49a-11e6-8b9f-a831ba1ff6e0

@mrose17 mrose17 added this to the 0.13.2 milestone Jan 27, 2017
@mrose17 mrose17 self-assigned this Jan 27, 2017
@mrose17 mrose17 requested a review from bsclifton January 27, 2017 22:21
@willy-b
Copy link
Contributor

willy-b commented Jan 27, 2017

note: this does give a range if the current contribution is not your first one (then it is not redundant). not disagreeing with change, just giving context :-).

@NejcZdovc
Copy link
Contributor

Maybe keep it and only display it when range is available?

@mrose17
Copy link
Member Author

mrose17 commented Jan 29, 2017

@NejcZdovc - you've convinced me. i'm going to withdraw this issue and the PR. could you make the line more useful? i'm also not really keen on the design placement either, but that's another matter!

@mrose17 mrose17 closed this Jan 29, 2017
@mrose17 mrose17 deleted the issue-6896 branch January 29, 2017 03:08
@luixxiul luixxiul removed this from the 0.13.2 milestone Jan 29, 2017
@NejcZdovc
Copy link
Contributor

NejcZdovc commented Jan 29, 2017

@willy-b can you tackle this?

@willy-b
Copy link
Contributor

willy-b commented Jan 29, 2017

@NejcZdovc sure, happy to!

@willy-b
Copy link
Contributor

willy-b commented Jan 30, 2017

@mrose17 , I fixed this as per @NejcZdovc's suggestion (https://github.com/willy-b/browser-laptop/commit/a510c87d1598b6e75043bf2924e36643355e4e7e).

Example: Two consecutive statements

  • where 1st has no date range (since 1st payment):
    statementpdf_norange

  • 2nd does have a date range (begins with date of last statement):
    statementpdf_yesrange

should I open a new PR against #6896?

@willy-b
Copy link
Contributor

willy-b commented Jan 30, 2017

note: @mrose17 responded to my question in a comment on my commit (https://github.com/willy-b/browser-laptop/commit/a510c87d1598b6e75043bf2924e36643355e4e7e#commitcomment-20667505):
"thanks! go ahead and submit an issue/PR for this fix and we should be good."

so, opening a separate PR

bsclifton added a commit that referenced this pull request Feb 16, 2017
only display time range in statement PDF when range is available (#6896, #6897)
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.

4 participants