Skip to content

Conversation

xuatz
Copy link
Collaborator

@xuatz xuatz commented May 12, 2025

Summary

Notes

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.

@MohamedBassem
Copy link
Collaborator

oops, I was trying to rebase the branch and kinda ruined the history :D

Copy link
Collaborator

@MohamedBassem MohamedBassem left a 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!

@xuatz
Copy link
Collaborator Author

xuatz commented May 18, 2025

Let me rebase in my fork before we merge!

@xuatz xuatz force-pushed the feat/update-api-sortorder-docs branch from 8f1939e to f29d38e Compare May 18, 2025 08:48
@xuatz xuatz force-pushed the feat/update-api-sortorder-docs branch from 97423f8 to b62305a Compare May 18, 2025 14:35
@xuatz
Copy link
Collaborator Author

xuatz commented May 18, 2025

@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),
Copy link
Collaborator Author

@xuatz xuatz May 18, 2025

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({
Copy link
Collaborator Author

@xuatz xuatz May 18, 2025

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.

@MohamedBassem MohamedBassem merged commit 4e06ea7 into karakeep-app:main May 18, 2025
5 checks passed
@MohamedBassem
Copy link
Collaborator

Looks good to me, thank you!

@MohamedBassem MohamedBassem mentioned this pull request May 18, 2025
1 task
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