Skip to content

Conversation

@JPrevost
Copy link
Member

@JPrevost JPrevost commented Jan 8, 2021

Why are these changes being introduced:

  • The order was rather arbitrary to begin with, and we decided to make
    the order match at least one real world use case, which is our own
    DISCO UI
  • Updates nokogiri (unrelated to this work but it's a security patch so it should be updated)

Relevant ticket(s):

How does this address that need:

  • This returns the aggregations in the order our DISCO UI currently
    desires. Future changes to the display order in any UI should probably
    be handled in the UI itself, but since the initial order was not for a
    specific purpose this was both easier and fine.

Requires Database Migrations?

NO

Includes new or updated dependencies?

YES

Why are these changes being introduced:

* The order was rather arbitrary to begin with, and we decided to make
  the order match at least one real world use case, which is our own
  DISCO UI

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/DISCO-221

How does this address that need:

* This returns the aggregations in the order our DISCO UI currently
  desires. Future changes to the display order in any UI should probably
  be handled in the UI itself, but since the initial order was not for a
  specific purpose this was both easier and fine.
@mitlib mitlib temporarily deployed to timdex-pipel-disco-221--fvw45i January 8, 2021 17:00 Inactive
@jazairi jazairi self-assigned this Jan 8, 2021
Copy link
Contributor

@jazairi jazairi left a comment

Choose a reason for hiding this comment

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

Confirmed that TIMDEX returns aggregations in the desired order. :shipit:

@JPrevost JPrevost merged commit fc68f5b into main Jan 11, 2021
@JPrevost JPrevost deleted the DISCO-221_facet_display_order branch January 11, 2021 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants