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

Only sort ledger transactions once (on initialization) #12448

Merged
merged 1 commit into from
Jan 17, 2018
Merged

Only sort ledger transactions once (on initialization) #12448

merged 1 commit into from
Jan 17, 2018

Conversation

bsclifton
Copy link
Member

Fixes #12119

Auditors: @sergiorojasa

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

  1. New profile, enable payments (this creates a new wallet)
  2. Exit and run npm add-simulated-payment-history 50
  3. Open the ledger-state.json at ~/Library/Application Support/brave-development (or %APPDATA%\brave on Windows)
  4. Manually edit the transactions in the JSON so that some of the newest ones (near the bottom) are at the top of the transactions array
  5. Launch browser, verify transactions look correct
  6. Exit browser, re-open ledger-state.json and notice it's now sorted also

Reviewer Checklist:

  • Request a security/privacy review as needed if one was not already requested.

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

Fixes #12119

Auditors: @sergiorojasa

Test Plan:
1. New profile, enable payments (this creates a new wallet)
2. Exit and run `npm add-simulated-payment-history 50`
3. Open the `ledger-state.json` at ~/Library/Application Support/brave-development (or %APPDATA%\brave on Windows)
4. Manually edit the transactions in the JSON so that some of the newest ones (near the bottom) are at the top of the transactions array
5. Launch browser, verify transactions look correct
6. Exit browser, re-open `ledger-state.json` and notice it's now sorted also
@bsclifton
Copy link
Member Author

bsclifton commented Dec 31, 2017

@NejcZdovc I know we were talking about this and I think this logic is ideal (sort once on launch). Users shouldn't lose history if older than 12 months in my opinion. For example, see my history:
screen shot 2017-12-30 at 9 22 45 pm

There is logic which will kick out when a transaction found is older than a year (when storing in appState)... but this relies on the data being sorted in the first place (which this PR does).

const then = new Date().getTime() - ledgerUtil.milliseconds.year

let transactions = []
if (!parsedData.transactions) {
return state
}
for (let i = parsedData.transactions.length - 1; i >= 0; i--) {
let transaction = parsedData.transactions[i]
if (transaction.stamp < then) break

@bsclifton bsclifton added this to the 0.22.x (Nightly Channel) milestone Dec 31, 2017
@bsclifton
Copy link
Member Author

This is a redo of #12341 by @sergiorojasa. The original fork was destroyed, so I couldn't edit the branch to tweak ☹️

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

++

@NejcZdovc NejcZdovc merged commit 9def174 into brave:master Jan 17, 2018
@bsclifton bsclifton deleted the fix-ledger-transaction-sorting branch February 13, 2018 22:15
@bbondy bbondy modified the milestones: 0.22.x (Developer Channel), 0.23.x (Nightly Channel) Feb 25, 2018
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.

3 participants