Răsfoiți Sursa

fix(api): `hide_globally` categories field should be a boolean

Frédéric Guillot 1 an în urmă
părinte
comite
d33e305af9

+ 1 - 1
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

+ 35 - 4
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)

+ 18 - 3
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"`

+ 132 - 1
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) {

+ 10 - 10
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
 	}

+ 18 - 16
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)
 }
 

+ 15 - 7
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"`
+}
+
+type CategoryModificationRequest struct {
+	Title        *string `json:"title"`
+	HideGlobally *bool   `json:"hide_globally"`
 }
 
-// Patch updates category fields.
-func (cr *CategoryRequest) Patch(category *Category) {
-	category.Title = cr.Title
-	category.HideGlobally = cr.HideGlobally != ""
+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.

+ 4 - 0
internal/model/model.go

@@ -20,3 +20,7 @@ func OptionalString(value string) *string {
 	}
 	return nil
 }
+
+func SetOptionalField[T any](value T) *T {
+	return &value
+}

+ 1 - 1
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)
 					}

+ 8 - 5
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,

+ 1 - 1
internal/template/templates/views/edit_category.html

@@ -31,7 +31,7 @@
     <input type="text" name="title" id="form-title" value="{{ .form.Title }}" required autofocus>
 
     <label>
-        <input type="checkbox" name="hide_globally" {{ if .form.HideGlobally }}checked{{ end }}>
+        <input type="checkbox" name="hide_globally" {{ if .form.HideGlobally }}checked{{ end }} value="1">
         {{ t "form.category.hide_globally" }}
     </label>
 

+ 2 - 6
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))

+ 3 - 3
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
 	}

+ 3 - 3
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 {

+ 2 - 2
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",
 	}
 }

+ 10 - 8
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")
-	}
-
-	if store.AnotherCategoryExists(userID, categoryID, request.Title) {
-		return locale.NewLocalizedError("error.category_already_exists")
+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")
+		}
 	}
 
 	return nil