Merge pull request 'Return an error list when bookmark filters contain errors' (#981) from fix/975-filter-errors into release

Reviewed-on: https://codeberg.org/readeck/readeck/pulls/981

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## [Unreleased]
+### Fixed
+- Return a 422 with errors on /api/bookmarks when the filters contain errors
+
 ## [0.21.3] - 2025-11-26
 ### Fixed
 - User management command would create users without UID

docs/api/bookmarks/routes-bookmarks.yaml

@@ -40,6 +40,7 @@ routes:
       $merge:
         - "traits.yaml#scope-bookmarks-read"
         - "traits.yaml#.authenticated"
+        - "traits.yaml#.validator"
         - "traits.yaml#.paginated"
         - "bookmarks/traits.yaml#.sortable"
 

internal/bookmarks/routes/api_bookmarks.go

@@ -703,7 +703,14 @@ func (api *apiRouter) withBookmarkListSelectDataset(next http.Handler) http.Hand
 		filterForm := newContextFilterForm(r.Context(), server.Locale(r))
 		forms.BindURL(filterForm, r)
 
-		if filterForm.IsValid() {
+		if !filterForm.IsValid() {
+			// When the form is invalid and we're not in a view, render the form's error list.
+			// Not having a base template context is a good indicator for that.
+			if _, ok := checkBaseContext(r.Context()); !ok {
+				server.Render(w, r, http.StatusUnprocessableEntity, filterForm)
+				return
+			}
+		} else {
 			filters := bookmarks.NewFiltersFromForm(filterForm)
 			filters.UpdateForm(filterForm)
 			ds = filters.ToSelectDataSet(ds)

internal/bookmarks/routes/http.go

@@ -22,7 +22,6 @@ import (
 // apiRouter is the base bookmark API router.
 type apiRouter struct {
 	chi.Router
-	srv *server.Server
 }
 
 type viewsRouter struct {
@@ -44,7 +43,7 @@ func SetupRoutes(s *server.Server) {
 	s.AddRoute("/bm", mediaRoutes(s))
 
 	// API routes
-	api := newAPIRouter(s)
+	api := newAPIRouter()
 	s.AddRoute("/api/bookmarks", api)
 
 	// Website routes
@@ -55,10 +54,10 @@ func SetupRoutes(s *server.Server) {
 }
 
 // newAPIRouter returns an apiRouter with all the routes set up.
-func newAPIRouter(s *server.Server) *apiRouter {
+func newAPIRouter() *apiRouter {
 	r := server.AuthenticatedRouter()
 
-	api := &apiRouter{r, s}
+	api := &apiRouter{r}
 
 	// Bookmark API
 	r.With(server.WithPermission("api:bookmarks", "read")).Group(func(r chi.Router) {

internal/bookmarks/routes/opds.go

@@ -26,9 +26,9 @@ type opdsRouter struct {
 
 // NewOPDSRouteHandler returns a chi Router handler with the OPDS
 // routes for the bookmark domain.
-func NewOPDSRouteHandler(s *server.Server) func(r chi.Router) {
+func NewOPDSRouteHandler() func(r chi.Router) {
 	return func(r chi.Router) {
-		h := &opdsRouter{r, newAPIRouter(s)}
+		h := &opdsRouter{r, newAPIRouter()}
 
 		r.With(server.WithPermission("api:bookmarks", "read")).Group(func(r chi.Router) {
 			r.With(h.withCollectionFilters, h.withBookmarkList).Get("/all", h.bookmarkList)

internal/opds/http.go

@@ -33,7 +33,7 @@ func SetupRoutes(s *server.Server) {
 	h.Use(middleware.GetHead)
 	h.With(server.WithPermission("api:opds", "read")).Group(func(r chi.Router) {
 		r.Get("/", h.mainCatalog)
-		r.Route("/bookmarks", bookmark_routes.NewOPDSRouteHandler(s))
+		r.Route("/bookmarks", bookmark_routes.NewOPDSRouteHandler())
 	})
 
 	s.AddRoute("/opds", h)