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

Entire ledger should be sorted before display #12119

Closed
bsclifton opened this issue Nov 28, 2017 · 8 comments
Closed

Entire ledger should be sorted before display #12119

bsclifton opened this issue Nov 28, 2017 · 8 comments

Comments

@bsclifton
Copy link
Member

bsclifton commented Nov 28, 2017

Test plan

#12448 (comment)


Description

Naturally (ex: just by using Brave), the transactions shown in History under Preferences > Payments will be in order. The newest transactions will be at the top and the older ones towards the bottom. However, you can (for testing purposes) generate a fake history which would put things in the wrong order (this was fixed with #12118)

The original sorting fix was done by @josiah-keller when fixing #11300 and works great. However, it has the following problems:

  • it runs each time the view is shown

  • it only sorts what is stored in ledgerData.get('transactions'). If the state has things out of order, this could potentially leave out entries. Thanks to @NejcZdovc for sharing this logic in getStateInfo:

    const getStateInfo = (state, parsedData) => {
    const info = parsedData.paymentInfo
    const then = new Date().getTime() - miliseconds.year

    If the timestamp is older than a year, the for loop stops ☹️

    if (transaction.stamp < then) break

The ideal fix would:

  • run one time
  • sort on startup

The change could go here:

const onInitRead = (state, parsedData) => {

parsedData contains the raw JSON blob for the ledger-state.json, which includes all the transactions. The sort that was done in #12118 could be done here too, before the call to getStateInfo

Brave Version

about:brave info:

Reproducible on current live release:
yes- but only by hand-editing the ledger-state.json

Additional Information

cc: @josiah-keller in case you wanted to give this one a shot 😄

@sergio-rojasa
Copy link

@bsclifton Can I work on this bug.

@NejcZdovc
Copy link
Contributor

@sergiorojasa sure, if you need you need any help don't hesitate to reach out. Thank you

@bsclifton
Copy link
Member Author

+1 to what @NejcZdovc said! Let us know if you have any questions, @sergiorojasa 😄 👍

@sergio-rojasa
Copy link

@bsclifton What properties of the ledger-state.json do I hand-editing?

@sergio-rojasa
Copy link

sergio-rojasa commented Dec 14, 2017

@bsclifton @NejcZdovc Should the sorting on parsedData affect the ledger-state.json or should I create a new array and fill the array element with the transaction from parsedData and then sort the new array and pass it to getStateInfo().

@NejcZdovc
Copy link
Contributor

@sergiorojasa yeah I think that we should only sort data from parsedData when we read it

@bsclifton
Copy link
Member Author

bsclifton commented Dec 19, 2017

@sergiorojasa you can just modify parsedData directly before getStateInfo gets called 😄

const onInitRead = (state, parsedData) => {
  parsedData = {{sort-goes-here}}
  state = getStateInfo(state, parsedData)
  // ...

@srirambv
Copy link
Collaborator

srirambv commented Jun 19, 2018

Verified on Windows 10 x64 using

  • 0.23.11 - 6565c06
  • Muon - 7.1.0
  • libchromiumcontent - 67.0.3396.87

Verified on Ubuntu 17.10 x64

  • 0.23.11 - 6565c06
  • Muon - 7.1.0
  • libchromiumcontent - 67.0.3396.87

Verified with macOS 10.12.6 using

  • 0.23.11 6565c06
  • Muon 7.1.0
  • libchromiumcontent 67.0.3396.87

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