Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added facet sorting alphabetically to search adaptor #57

Closed
wants to merge 1 commit into from

Conversation

CathLass
Copy link
Collaborator

@CathLass CathLass commented Oct 18, 2024

This is a bug fix raised by Antony but I can't find the ticket. The issue is that facets don't always display in the same order on the FE - they switch around. In a real project we would expect to have a prescribed order from, say, UCD people, which would be addressed as a presentation concern.

@@ -102,7 +103,7 @@ await _searchByKeywordService.SearchAsync<TSearchResult>(
{
Establishments = _searchResultMapper.MapFrom(searchResults.Value.GetResults()),
Facets = searchResults.Value.Facets != null
? _facetsMapper.MapFrom(searchResults.Value.Facets.ToDictionary())
? _facetsMapper.MapFrom(searchResults.Value.Facets.OrderBy(facet => facet.Key).ToDictionary())
Copy link
Member

Choose a reason for hiding this comment

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

Is sort order/sequencing definitely a search prototype concern?

My worry is that this is being implemented too "low level" (as opposed to, for example, treating it as a display/web UI concern and doing the sort at the last-reasonable-moment within the view code).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Roger, I've put the background to this PR in the description, which we should have done in the first place. Maybe it will answer your concern

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it will answer your concern

I've just had a read and I'm not sure it does.

To illustrate the alternative, what if this sorting was performed in the web mvc project (i.e., the project which displays these values in HTML) as opposed to this shared library project?

@CathLass CathLass closed this Oct 31, 2024
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.

2 participants