-
-
Notifications
You must be signed in to change notification settings - Fork 854
feat(api): enable ?sortOrder= for relevant resources #1398
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
feat(api): enable ?sortOrder= for relevant resources #1398
Conversation
oops, I was trying to rebase the branch and kinda ruined the history :D |
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.
The PR is good to go though!
Let me rebase in my fork before we merge! |
8f1939e
to
f29d38e
Compare
97423f8
to
b62305a
Compare
@MohamedBassem failing tests are now fixed, with some some typing changes, so will need another review over the newly added changes. |
@@ -14,7 +12,6 @@ export const GET = (req: NextRequest) => | |||
searchParamsSchema: z | |||
.object({ | |||
q: z.string(), | |||
sortOrder: zSortOrder.optional().default(zSortOrder.Enum.relevance), |
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.
already present via .and(zGetBookmarkSearchParamsSchema)
export const zGetBookmarkSearchParamsSchema = z.object({ | ||
export const zGetBookmarkQueryParamsSchema = z.object({ |
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.
zGetBookmarkQueryParamsSchema
: not used in /bookmarks/search
zGetBookmarkSearchParamsSchema
: used in /bookmarks/search
let me know if you have a better approach for this.
Looks good to me, thank you! |
Summary
sortOrder
functionality in the api for relevant resourcesNotes
Technically this should be merged after #1392
Btw I'm not sure how much CI magic has been setup and if there are more docs or tasks that I also need to update for this PR, so please let me know if I missed anything.