only display time range in statement PDF when range is available (#6896, #6897) #6933
Conversation
- fix path to "ledger-state.json" for "NODE_ENV=development" "app.getPath(\"userData\")" runs in "./tools/lib/utilApp/index.js" and "./tools/utilAppRunner.js" was running brave with "--user-data-dir=brave-development" causing brave to look in "./tools/lib/utilApp/brave-development" instead of "./brave-development" - fix parsing of optional argument for # of payments to simulate
…lable if this is the 1st payment in the payment history, do not display statement date range, per @NejcZdovc / @mrose17 suggestion fixes #6897
| @@ -14,7 +14,7 @@ function runUtilApp (cmd, file, stdioOptions) { | |||
| } | |||
| cmd = cmd.split(' ') | |||
| if (process.env.NODE_ENV === 'development') { | |||
| cmd.push('--user-data-dir=brave-development') | |||
| cmd.push('--user-data-dir=../../../brave-development') | |||
There was a problem hiding this comment.
@willy-b - what's the motivation behind this change, i think it might break some other tests...
There was a problem hiding this comment.
Good question, @mrose17. This change ensures Brave has the right path to ledger-state.json when NODE_ENV is set to development.
Reason:
For these utility scripts, the userData path is retrieved via app.getPath(\"userData\") run in ./tools/lib/utilApp/index.js (three directory levels removed from the base of the repo).
Before this change, ./tools/utilAppRunner.js was running brave with --user-data-dir=brave-development, which would cause Brave to look in ./tools/lib/utilApp/brave-development instead of ./brave-development.
What tests do you think it might break?
|
@mrose17 have you had a chance to check this PR out? (test it out, etc) Once it's in a state that you approve of, I'll gladly review / check tests and if good to go, merge 😄 |
|
it is fine for merge. |
|
Most tests passed CI; the ones that didn't I re-ran. Some (like punycode) are perma-failing... but not related to this change. Merging 😄 |
|
thanks! |
bsclifton
added
QA/test-plan-specified
release-notes/include
and removed
PR/needs-QA-attention ☕
labels
Test Plan
npm run add-simulated-payment-historyabout:preferences#payments)Description
for #6896
(replaces and comes from discussion in #6897 and in https://github.com/willy-b/browser-laptop/commit/a510c87d1598b6e75043bf2924e36643355e4e7e#commitcomment-20667505 , where @mrose17 reviewed this change)
Example: Two consecutive statements
where 1st has no time range (since 1st payment):
2nd does have a time range (begins with date of last statement):
