Skip to content

Fix sorting Quote Source column of quote sort list#9658

Merged
Gudahtt merged 1 commit intodevelopfrom
fix-quote-sort-list-sorting
Oct 20, 2020
Merged

Fix sorting Quote Source column of quote sort list#9658
Gudahtt merged 1 commit intodevelopfrom
fix-quote-sort-list-sorting

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Oct 20, 2020

Sorting was broken for the "Quote Source" column of the quote sort list. Attempting to sort by this column would arrange the quotes in a seemingly random order.

It appears that this was due to this column being programmed to sort by a property called liquiditySource, which does not exist in the quote data. I'm unsure what the difference between liquiditySource and quoteSource was supposed to be; the values in the mocks are all identical.

All references to liquiditySource have been updated to refer to quoteSource instead, and the sorting now works correctly.

@Gudahtt Gudahtt requested a review from a team as a code owner October 20, 2020 04:47
@Gudahtt Gudahtt requested a review from danjm October 20, 2020 04:47
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [62103e6]
Page Load Metrics (754 ± 133 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaintNaNNaNNaNNaNNaN
domContentLoaded4321397751276133
load4331399754276133
domInteractive4321397751276133

danjm
danjm previously approved these changes Oct 20, 2020
Copy link
Copy Markdown
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

I'm unsure what the difference between liquiditySource and quoteSource

There is no difference. At some point the property was renamed in quotesToRenderableData() and the correct updates were not made in other locations.

This fix is correct.

Sorting was broken for the "Quote Source" column of the quote sort
list. Attempting to sort by this column would arrange the quotes in a
seemingly random order.

It appears that this was due to this column being programmed to sort by
a property called `liquiditySource`, which does not exist in the quote
data. I'm unsure what the difference between `liquiditySource` and
`quoteSource` was supposed to be; the values in the mocks are all
identical.

All references to `liquiditySource` have been updated to refer to
`quoteSource` instead, and the sorting now works correctly.
@Gudahtt
Copy link
Copy Markdown
Member Author

Gudahtt commented Oct 20, 2020

I just had to update this to resolve conflicts; the diff should still be the same as it was before.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [fcc98af]
Page Load Metrics (465 ± 62 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint31533963
domContentLoaded29375746312861
load29476346512862
domInteractive29375746312861

@Gudahtt Gudahtt merged commit 5789bd8 into develop Oct 20, 2020
@Gudahtt Gudahtt deleted the fix-quote-sort-list-sorting branch October 20, 2020 14:44
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants