Fix regression with initial sorting after pr205 #229

pull hebasto wants to merge 1 commits into bitcoin-core:master from hebasto:210227-sort changing 2 files +3 −2

hebasto commented at 3:57 pm on February 27, 2021: member

Unfortunately, #205 introduced a regression. After opening the “Receive” or “Transaction” tab at first time despite of the “Date” header is marked as sorted, table rows are not sorted actually:

It appears that sorting the table must be triggered after the QTableView::setModel call.

With this PR (and pre-#205):

qt: Fix regression with initial sorting after pr205

QTableView widget must be explicitly sorted after the setModel call.

c524dc54bb

hebasto commented at 3:58 pm on February 27, 2021: member

Many thanks to @Geremia who reports about this regression.
leonardojobim approved

leonardojobim commented at 9:53 pm on February 27, 2021: none

I can confirm the “Transactions” table rows are not initially ordered on the master branch. And this PR fixes this.

However, in my node, the “Receive” table rows are correctly ordered, both on the master and on this PR branch.

Transactions table on master branch	Transactions table on this PR branch	Both (“Receive” table)

Talkless commented at 1:19 pm on February 28, 2021: none

tACK c524dc54bb985f801b5a3def3c3d51ce497e838e, tested on Debian Sid with Qt 5.15.2. I can confirm @leonardojobim observations.
jonatack commented at 9:23 pm on February 28, 2021: contributor

ACK c524dc54bb985f801b5a3def3c3d51ce497e838e
jarolrod commented at 1:57 am on March 1, 2021: member

ACK c524dc54bb985f801b5a3def3c3d51ce497e838e, tested on macOS 11.1 Qt 5.15.2

Confirming this regression on master and that this PR fixes it. I see the same behavior as shown in @leonardojobim screenshots.

This regression can actually be seen in my review of the PR that introduced the bug: #205#pullrequestreview-585193420. It’s not immediately obvious that the PR introduced this bug because all of the transactions occurred under the same minute. The maturity indicator is the clue that shows that the transactions we’re not sorted by date under this PR.

This highlights the need for increased test coverage of the GUI.
hebasto added the label Bug on Mar 2, 2021
MarcoFalke merged this on Mar 10, 2021
MarcoFalke closed this on Mar 10, 2021
hebasto deleted the branch on Mar 10, 2021
laanwj commented at 4:31 pm on March 10, 2021: member

Thanks for fixing this, can confirm that it works.
sidhujag referenced this in commit 56d2f973d8 on Mar 10, 2021
gwillen referenced this in commit dcc80b82f2 on Jun 28, 2022
bitcoin-core locked this on Aug 16, 2022

Labels
Bug