From d33e305af979c7d33ffe0ca13fe299f3b9463635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Mon, 21 Apr 2025 18:45:30 -0700 Subject: [PATCH] fix(api): `hide_globally` categories field should be a boolean --- Makefile | 2 +- client/client.go | 39 ++++- client/model.go | 21 ++- internal/api/api_integration_test.go | 133 +++++++++++++++++- internal/api/category.go | 20 +-- internal/googlereader/handler.go | 34 ++--- internal/model/category.go | 22 ++- internal/model/model.go | 4 + internal/reader/opml/handler.go | 2 +- internal/storage/category.go | 13 +- .../templates/views/edit_category.html | 2 +- internal/ui/category_edit.go | 8 +- internal/ui/category_save.go | 6 +- internal/ui/category_update.go | 6 +- internal/ui/form/category.go | 4 +- internal/validator/category.go | 16 ++- 16 files changed, 262 insertions(+), 70 deletions(-) diff --git a/Makefile b/Makefile index b5a79f09..ee6d1e4e 100644 --- a/Makefile +++ b/Makefile @@ -144,7 +144,7 @@ integration-test: ADMIN_PASSWORD=test123 \ CREATE_ADMIN=1 \ RUN_MIGRATIONS=1 \ - DEBUG=1 \ + LOG_LEVEL=debug \ ./miniflux-test >/tmp/miniflux.log 2>&1 & echo "$$!" > "/tmp/miniflux.pid" while ! nc -z localhost 8080; do sleep 1; done diff --git a/client/client.go b/client/client.go index 838af02e..5cdbd1b3 100644 --- a/client/client.go +++ b/client/client.go @@ -239,8 +239,8 @@ func (c *Client) Categories() (Categories, error) { // CreateCategory creates a new category. func (c *Client) CreateCategory(title string) (*Category, error) { - body, err := c.request.Post("/v1/categories", map[string]interface{}{ - "title": title, + body, err := c.request.Post("/v1/categories", &CategoryCreationRequest{ + Title: title, }) if err != nil { return nil, err @@ -255,10 +255,25 @@ func (c *Client) CreateCategory(title string) (*Category, error) { return category, nil } +// CreateCategoryWithOptions creates a new category with options. +func (c *Client) CreateCategoryWithOptions(createRequest *CategoryCreationRequest) (*Category, error) { + body, err := c.request.Post("/v1/categories", createRequest) + if err != nil { + return nil, err + } + defer body.Close() + + var category *Category + if err := json.NewDecoder(body).Decode(&category); err != nil { + return nil, fmt.Errorf("miniflux: response error (%v)", err) + } + return category, nil +} + // UpdateCategory updates a category. func (c *Client) UpdateCategory(categoryID int64, title string) (*Category, error) { - body, err := c.request.Put(fmt.Sprintf("/v1/categories/%d", categoryID), map[string]interface{}{ - "title": title, + body, err := c.request.Put(fmt.Sprintf("/v1/categories/%d", categoryID), &CategoryModificationRequest{ + Title: SetOptionalField(title), }) if err != nil { return nil, err @@ -273,6 +288,22 @@ func (c *Client) UpdateCategory(categoryID int64, title string) (*Category, erro return category, nil } +// UpdateCategoryWithOptions updates a category with options. +func (c *Client) UpdateCategoryWithOptions(categoryID int64, categoryChanges *CategoryModificationRequest) (*Category, error) { + body, err := c.request.Put(fmt.Sprintf("/v1/categories/%d", categoryID), categoryChanges) + if err != nil { + return nil, err + } + defer body.Close() + + var category *Category + if err := json.NewDecoder(body).Decode(&category); err != nil { + return nil, fmt.Errorf("miniflux: response error (%v)", err) + } + + return category, nil +} + // MarkCategoryAsRead marks all unread entries in a category as read. func (c *Client) MarkCategoryAsRead(categoryID int64) error { _, err := c.request.Put(fmt.Sprintf("/v1/categories/%d/mark-all-as-read", categoryID), nil) diff --git a/client/model.go b/client/model.go index 8f257e6a..df49283a 100644 --- a/client/model.go +++ b/client/model.go @@ -97,9 +97,12 @@ type Users []User // Category represents a feed category. type Category struct { - ID int64 `json:"id,omitempty"` - Title string `json:"title,omitempty"` - UserID int64 `json:"user_id,omitempty"` + ID int64 `json:"id"` + Title string `json:"title"` + UserID int64 `json:"user_id,omitempty"` + HideGlobally bool `json:"hide_globally,omitempty"` + FeedCount *int `json:"feed_count,omitempty"` + TotalUnread *int `json:"total_unread,omitempty"` } func (c Category) String() string { @@ -109,6 +112,18 @@ func (c Category) String() string { // Categories represents a list of categories. type Categories []*Category +// CategoryCreationRequest represents the request to create a category. +type CategoryCreationRequest struct { + Title string `json:"title"` + HideGlobally bool `json:"hide_globally"` +} + +// CategoryModificationRequest represents the request to update a category. +type CategoryModificationRequest struct { + Title *string `json:"title"` + HideGlobally *bool `json:"hide_globally"` +} + // Subscription represents a feed subscription. type Subscription struct { Title string `json:"title"` diff --git a/internal/api/api_integration_test.go b/internal/api/api_integration_test.go index 0ea3f017..53f4d367 100644 --- a/internal/api/api_integration_test.go +++ b/internal/api/api_integration_test.go @@ -824,6 +824,10 @@ func TestCreateCategoryEndpoint(t *testing.T) { if category.Title != categoryName { t.Errorf(`Invalid title, got "%v" instead of "%v"`, category.Title, categoryName) } + + if category.HideGlobally { + t.Errorf(`Invalid hide globally value, got "%v"`, category.HideGlobally) + } } func TestCreateCategoryWithEmptyTitle(t *testing.T) { @@ -865,7 +869,49 @@ func TestCannotCreateDuplicatedCategory(t *testing.T) { } } -func TestUpdateCatgoryEndpoint(t *testing.T) { +func TestCreateCategoryWithOptions(t *testing.T) { + testConfig := newIntegrationTestConfig() + if !testConfig.isConfigured() { + t.Skip(skipIntegrationTestsMessage) + } + + adminClient := miniflux.NewClient(testConfig.testBaseURL, testConfig.testAdminUsername, testConfig.testAdminPassword) + + regularTestUser, err := adminClient.CreateUser(testConfig.genRandomUsername(), testConfig.testRegularPassword, false) + if err != nil { + t.Fatal(err) + } + defer adminClient.DeleteUser(regularTestUser.ID) + + regularUserClient := miniflux.NewClient(testConfig.testBaseURL, regularTestUser.Username, testConfig.testRegularPassword) + + newCategory, err := regularUserClient.CreateCategoryWithOptions(&miniflux.CategoryCreationRequest{ + Title: "My category", + HideGlobally: true, + }) + if err != nil { + t.Fatalf(`Creating a category with options should not raise an error: %v`, err) + } + + categories, err := regularUserClient.Categories() + if err != nil { + t.Fatal(err) + } + + for _, category := range categories { + if category.ID == newCategory.ID { + if category.Title != newCategory.Title { + t.Errorf(`Invalid title, got %q instead of %q`, category.Title, newCategory.Title) + } + if category.HideGlobally != true { + t.Errorf(`Invalid hide globally value, got "%v"`, category.HideGlobally) + } + break + } + } +} + +func TestUpdateCategoryEndpoint(t *testing.T) { testConfig := newIntegrationTestConfig() if !testConfig.isConfigured() { t.Skip(skipIntegrationTestsMessage) @@ -903,6 +949,91 @@ func TestUpdateCatgoryEndpoint(t *testing.T) { if updatedCategory.Title != "new title" { t.Errorf(`Invalid title, got "%v" instead of "%v"`, updatedCategory.Title, "new title") } + + if updatedCategory.HideGlobally { + t.Errorf(`Invalid hide globally value, got "%v"`, updatedCategory.HideGlobally) + } +} + +func TestUpdateCategoryWithOptions(t *testing.T) { + testConfig := newIntegrationTestConfig() + if !testConfig.isConfigured() { + t.Skip(skipIntegrationTestsMessage) + } + + adminClient := miniflux.NewClient(testConfig.testBaseURL, testConfig.testAdminUsername, testConfig.testAdminPassword) + + regularTestUser, err := adminClient.CreateUser(testConfig.genRandomUsername(), testConfig.testRegularPassword, false) + if err != nil { + t.Fatal(err) + } + defer adminClient.DeleteUser(regularTestUser.ID) + + regularUserClient := miniflux.NewClient(testConfig.testBaseURL, regularTestUser.Username, testConfig.testRegularPassword) + + newCategory, err := regularUserClient.CreateCategoryWithOptions(&miniflux.CategoryCreationRequest{ + Title: "My category", + }) + if err != nil { + t.Fatalf(`Creating a category with options should not raise an error: %v`, err) + } + + updatedCategory, err := regularUserClient.UpdateCategoryWithOptions(newCategory.ID, &miniflux.CategoryModificationRequest{ + Title: miniflux.SetOptionalField("new title"), + }) + if err != nil { + t.Fatal(err) + } + + if updatedCategory.ID != newCategory.ID { + t.Errorf(`Invalid categoryID, got "%v"`, updatedCategory.ID) + } + + if updatedCategory.Title != "new title" { + t.Errorf(`Invalid title, got "%v" instead of "%v"`, updatedCategory.Title, "new title") + } + + if updatedCategory.HideGlobally { + t.Errorf(`Invalid hide globally value, got "%v"`, updatedCategory.HideGlobally) + } + + updatedCategory, err = regularUserClient.UpdateCategoryWithOptions(newCategory.ID, &miniflux.CategoryModificationRequest{ + HideGlobally: miniflux.SetOptionalField(true), + }) + if err != nil { + t.Fatal(err) + } + + if updatedCategory.ID != newCategory.ID { + t.Errorf(`Invalid categoryID, got "%v"`, updatedCategory.ID) + } + + if updatedCategory.Title != "new title" { + t.Errorf(`Invalid title, got "%v" instead of "%v"`, updatedCategory.Title, "new title") + } + + if !updatedCategory.HideGlobally { + t.Errorf(`Invalid hide globally value, got "%v"`, updatedCategory.HideGlobally) + } + + updatedCategory, err = regularUserClient.UpdateCategoryWithOptions(newCategory.ID, &miniflux.CategoryModificationRequest{ + HideGlobally: miniflux.SetOptionalField(false), + }) + if err != nil { + t.Fatal(err) + } + + if updatedCategory.ID != newCategory.ID { + t.Errorf(`Invalid categoryID, got %q`, updatedCategory.ID) + } + + if updatedCategory.Title != "new title" { + t.Errorf(`Invalid title, got %q instead of %q`, updatedCategory.Title, "new title") + } + + if updatedCategory.HideGlobally { + t.Errorf(`Invalid hide globally value, got "%v"`, updatedCategory.HideGlobally) + } } func TestUpdateInexistingCategory(t *testing.T) { diff --git a/internal/api/category.go b/internal/api/category.go index 7b47e2a3..3ae2cdc4 100644 --- a/internal/api/category.go +++ b/internal/api/category.go @@ -19,18 +19,18 @@ import ( func (h *handler) createCategory(w http.ResponseWriter, r *http.Request) { userID := request.UserID(r) - var categoryRequest model.CategoryRequest - if err := json_parser.NewDecoder(r.Body).Decode(&categoryRequest); err != nil { + var categoryCreationRequest model.CategoryCreationRequest + if err := json_parser.NewDecoder(r.Body).Decode(&categoryCreationRequest); err != nil { json.BadRequest(w, r, err) return } - if validationErr := validator.ValidateCategoryCreation(h.store, userID, &categoryRequest); validationErr != nil { + if validationErr := validator.ValidateCategoryCreation(h.store, userID, &categoryCreationRequest); validationErr != nil { json.BadRequest(w, r, validationErr.Error()) return } - category, err := h.store.CreateCategory(userID, &categoryRequest) + category, err := h.store.CreateCategory(userID, &categoryCreationRequest) if err != nil { json.ServerError(w, r, err) return @@ -54,20 +54,20 @@ func (h *handler) updateCategory(w http.ResponseWriter, r *http.Request) { return } - var categoryRequest model.CategoryRequest - if err := json_parser.NewDecoder(r.Body).Decode(&categoryRequest); err != nil { + var categoryModificationRequest model.CategoryModificationRequest + if err := json_parser.NewDecoder(r.Body).Decode(&categoryModificationRequest); err != nil { json.BadRequest(w, r, err) return } - if validationErr := validator.ValidateCategoryModification(h.store, userID, category.ID, &categoryRequest); validationErr != nil { + if validationErr := validator.ValidateCategoryModification(h.store, userID, category.ID, &categoryModificationRequest); validationErr != nil { json.BadRequest(w, r, validationErr.Error()) return } - categoryRequest.Patch(category) - err = h.store.UpdateCategory(category) - if err != nil { + categoryModificationRequest.Patch(category) + + if err := h.store.UpdateCategory(category); err != nil { json.ServerError(w, r, err) return } diff --git a/internal/googlereader/handler.go b/internal/googlereader/handler.go index c5e6e537..2e1b168e 100644 --- a/internal/googlereader/handler.go +++ b/internal/googlereader/handler.go @@ -736,17 +736,16 @@ func getFeed(stream Stream, store *storage.Storage, userID int64) (*model.Feed, return store.FeedByID(userID, feedID) } -func getOrCreateCategory(category Stream, store *storage.Storage, userID int64) (*model.Category, error) { +func getOrCreateCategory(streamCategory Stream, store *storage.Storage, userID int64) (*model.Category, error) { switch { - case category.ID == "": + case streamCategory.ID == "": return store.FirstCategory(userID) - case store.CategoryTitleExists(userID, category.ID): - return store.CategoryByTitle(userID, category.ID) + case store.CategoryTitleExists(userID, streamCategory.ID): + return store.CategoryByTitle(userID, streamCategory.ID) default: - catRequest := model.CategoryRequest{ - Title: category.ID, - } - return store.CreateCategory(userID, &catRequest) + return store.CreateCategory(userID, &model.CategoryCreationRequest{ + Title: streamCategory.ID, + }) } } @@ -1144,20 +1143,23 @@ func (h *handler) renameTagHandler(w http.ResponseWriter, r *http.Request) { json.NotFound(w, r) return } - categoryRequest := model.CategoryRequest{ - Title: destination.ID, + + categoryModificationRequest := model.CategoryModificationRequest{ + Title: model.SetOptionalField(destination.ID), } - verr := validator.ValidateCategoryModification(h.store, userID, category.ID, &categoryRequest) - if verr != nil { - json.BadRequest(w, r, verr.Error()) + + if validationError := validator.ValidateCategoryModification(h.store, userID, category.ID, &categoryModificationRequest); validationError != nil { + json.BadRequest(w, r, validationError.Error()) return } - categoryRequest.Patch(category) - err = h.store.UpdateCategory(category) - if err != nil { + + categoryModificationRequest.Patch(category) + + if err := h.store.UpdateCategory(category); err != nil { json.ServerError(w, r, err) return } + OK(w, r) } diff --git a/internal/model/category.go b/internal/model/category.go index b48ffda2..9e85de6d 100644 --- a/internal/model/category.go +++ b/internal/model/category.go @@ -19,16 +19,24 @@ func (c *Category) String() string { return fmt.Sprintf("ID=%d, UserID=%d, Title=%s", c.ID, c.UserID, c.Title) } -// CategoryRequest represents the request to create or update a category. -type CategoryRequest struct { +type CategoryCreationRequest struct { Title string `json:"title"` - HideGlobally string `json:"hide_globally"` + HideGlobally bool `json:"hide_globally"` } -// Patch updates category fields. -func (cr *CategoryRequest) Patch(category *Category) { - category.Title = cr.Title - category.HideGlobally = cr.HideGlobally != "" +type CategoryModificationRequest struct { + Title *string `json:"title"` + HideGlobally *bool `json:"hide_globally"` +} + +func (c *CategoryModificationRequest) Patch(category *Category) { + if c.Title != nil { + category.Title = *c.Title + } + + if c.HideGlobally != nil { + category.HideGlobally = *c.HideGlobally + } } // Categories represents a list of categories. diff --git a/internal/model/model.go b/internal/model/model.go index 86f1e370..64f3400c 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -20,3 +20,7 @@ func OptionalString(value string) *string { } return nil } + +func SetOptionalField[T any](value T) *T { + return &value +} diff --git a/internal/reader/opml/handler.go b/internal/reader/opml/handler.go index 5a81d069..c8dc610b 100644 --- a/internal/reader/opml/handler.go +++ b/internal/reader/opml/handler.go @@ -61,7 +61,7 @@ func (h *Handler) Import(userID int64, data io.Reader) error { } if category == nil { - category, err = h.store.CreateCategory(userID, &model.CategoryRequest{Title: subscription.CategoryName}) + category, err = h.store.CreateCategory(userID, &model.CategoryCreationRequest{Title: subscription.CategoryName}) if err != nil { return fmt.Errorf(`opml: unable to create this category: %q`, subscription.CategoryName) } diff --git a/internal/storage/category.go b/internal/storage/category.go index c7270e9b..4f6ee56e 100644 --- a/internal/storage/category.go +++ b/internal/storage/category.go @@ -165,27 +165,30 @@ func (s *Storage) CategoriesWithFeedCount(userID int64) (model.Categories, error } // CreateCategory creates a new category. -func (s *Storage) CreateCategory(userID int64, request *model.CategoryRequest) (*model.Category, error) { +func (s *Storage) CreateCategory(userID int64, request *model.CategoryCreationRequest) (*model.Category, error) { var category model.Category query := ` INSERT INTO categories - (user_id, title) + (user_id, title, hide_globally) VALUES - ($1, $2) + ($1, $2, $3) RETURNING id, user_id, - title + title, + hide_globally ` err := s.db.QueryRow( query, userID, request.Title, + request.HideGlobally, ).Scan( &category.ID, &category.UserID, &category.Title, + &category.HideGlobally, ) if err != nil { @@ -197,7 +200,7 @@ func (s *Storage) CreateCategory(userID int64, request *model.CategoryRequest) ( // UpdateCategory updates an existing category. func (s *Storage) UpdateCategory(category *model.Category) error { - query := `UPDATE categories SET title=$1, hide_globally = $2 WHERE id=$3 AND user_id=$4` + query := `UPDATE categories SET title=$1, hide_globally=$2 WHERE id=$3 AND user_id=$4` _, err := s.db.Exec( query, category.Title, diff --git a/internal/template/templates/views/edit_category.html b/internal/template/templates/views/edit_category.html index 6c8eccf1..55c2f558 100644 --- a/internal/template/templates/views/edit_category.html +++ b/internal/template/templates/views/edit_category.html @@ -31,7 +31,7 @@ diff --git a/internal/ui/category_edit.go b/internal/ui/category_edit.go index 2edc8e13..469243cc 100644 --- a/internal/ui/category_edit.go +++ b/internal/ui/category_edit.go @@ -20,8 +20,7 @@ func (h *handler) showEditCategoryPage(w http.ResponseWriter, r *http.Request) { return } - categoryID := request.RouteInt64Param(r, "categoryID") - category, err := h.store.Category(request.UserID(r), categoryID) + category, err := h.store.Category(request.UserID(r), request.RouteInt64Param(r, "categoryID")) if err != nil { html.ServerError(w, r, err) return @@ -34,10 +33,7 @@ func (h *handler) showEditCategoryPage(w http.ResponseWriter, r *http.Request) { categoryForm := form.CategoryForm{ Title: category.Title, - HideGlobally: "", - } - if category.HideGlobally { - categoryForm.HideGlobally = "checked" + HideGlobally: category.HideGlobally, } sess := session.New(h.store, request.SessionID(r)) diff --git a/internal/ui/category_save.go b/internal/ui/category_save.go index 22787e16..e23c2d39 100644 --- a/internal/ui/category_save.go +++ b/internal/ui/category_save.go @@ -33,15 +33,15 @@ func (h *handler) saveCategory(w http.ResponseWriter, r *http.Request) { view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID)) view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID)) - categoryRequest := &model.CategoryRequest{Title: categoryForm.Title} + categoryCreationRequest := &model.CategoryCreationRequest{Title: categoryForm.Title} - if validationErr := validator.ValidateCategoryCreation(h.store, loggedUser.ID, categoryRequest); validationErr != nil { + if validationErr := validator.ValidateCategoryCreation(h.store, loggedUser.ID, categoryCreationRequest); validationErr != nil { view.Set("errorMessage", validationErr.Translate(loggedUser.Language)) html.OK(w, r, view.Render("create_category")) return } - if _, err = h.store.CreateCategory(loggedUser.ID, categoryRequest); err != nil { + if _, err = h.store.CreateCategory(loggedUser.ID, categoryCreationRequest); err != nil { html.ServerError(w, r, err) return } diff --git a/internal/ui/category_update.go b/internal/ui/category_update.go index e701a0bd..7403bd63 100644 --- a/internal/ui/category_update.go +++ b/internal/ui/category_update.go @@ -46,9 +46,9 @@ func (h *handler) updateCategory(w http.ResponseWriter, r *http.Request) { view.Set("countUnread", h.store.CountUnreadEntries(loggedUser.ID)) view.Set("countErrorFeeds", h.store.CountUserFeedsWithErrors(loggedUser.ID)) - categoryRequest := &model.CategoryRequest{ - Title: categoryForm.Title, - HideGlobally: categoryForm.HideGlobally, + categoryRequest := &model.CategoryModificationRequest{ + Title: model.SetOptionalField(categoryForm.Title), + HideGlobally: model.SetOptionalField(categoryForm.HideGlobally), } if validationErr := validator.ValidateCategoryModification(h.store, loggedUser.ID, category.ID, categoryRequest); validationErr != nil { diff --git a/internal/ui/form/category.go b/internal/ui/form/category.go index addd368f..72e5b05b 100644 --- a/internal/ui/form/category.go +++ b/internal/ui/form/category.go @@ -10,13 +10,13 @@ import ( // CategoryForm represents a feed form in the UI type CategoryForm struct { Title string - HideGlobally string + HideGlobally bool } // NewCategoryForm returns a new CategoryForm. func NewCategoryForm(r *http.Request) *CategoryForm { return &CategoryForm{ Title: r.FormValue("title"), - HideGlobally: r.FormValue("hide_globally"), + HideGlobally: r.FormValue("hide_globally") == "1", } } diff --git a/internal/validator/category.go b/internal/validator/category.go index 7be076db..3257db6b 100644 --- a/internal/validator/category.go +++ b/internal/validator/category.go @@ -10,7 +10,7 @@ import ( ) // ValidateCategoryCreation validates category creation. -func ValidateCategoryCreation(store *storage.Storage, userID int64, request *model.CategoryRequest) *locale.LocalizedError { +func ValidateCategoryCreation(store *storage.Storage, userID int64, request *model.CategoryCreationRequest) *locale.LocalizedError { if request.Title == "" { return locale.NewLocalizedError("error.title_required") } @@ -23,13 +23,15 @@ func ValidateCategoryCreation(store *storage.Storage, userID int64, request *mod } // ValidateCategoryModification validates category modification. -func ValidateCategoryModification(store *storage.Storage, userID, categoryID int64, request *model.CategoryRequest) *locale.LocalizedError { - if request.Title == "" { - return locale.NewLocalizedError("error.title_required") - } +func ValidateCategoryModification(store *storage.Storage, userID, categoryID int64, request *model.CategoryModificationRequest) *locale.LocalizedError { + if request.Title != nil { + if *request.Title == "" { + return locale.NewLocalizedError("error.title_required") + } - if store.AnotherCategoryExists(userID, categoryID, request.Title) { - return locale.NewLocalizedError("error.category_already_exists") + if store.AnotherCategoryExists(userID, categoryID, *request.Title) { + return locale.NewLocalizedError("error.category_already_exists") + } } return nil