-
Notifications
You must be signed in to change notification settings - Fork 8
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
Get Collection Items extension - facets - filter queries - sort/type #230
Get Collection Items extension - facets - filter queries - sort/type #230
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, I just have some minor comments below
src/collections/infra/repositories/transformers/collectionTransformers.ts
Outdated
Show resolved
Hide resolved
@ekraffmiller all changes addressed, thanks for the review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, approved!
@g-saracca I realized there is one more change needed, there is a sort type "Relevance" used when the CollectionSearchCriteria contains search text. For the API, I'm going to put the PR back to InReview, sorry about that. |
Hi @ekraffmiller , in the api endpoint docs there is nothing about sort by relevance, there is |
Ah ok, so when the user chooses sort by relevance, that is the same as doing a text search without a sort option. Makes sense, I don't think we need to update the API, I was just confused by the url params in the frontend. |
Right, when you search something the url looks like this: |
Tests are passing - Merging PR |
What this PR does / why we need it:
Extends the Get Collection Items use case.
Now returns the
facets
according to the search.Also now accepts 3 more properties in the
CollectionSearchCriteria
-sort
,order
andfilterQueries
.sort
: 'name' or 'date'type:
'asc' or 'desc'filterQueries
: an strings Array of typestring
:string
Which issue(s) this PR closes:
Suggestions on how to test this:
Visual code inspection and run tests.
Is there a release notes update needed for this change?:
No