mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-21 05:54:10 +08:00
Fix view store test IDs and improve error handling in app layer
- Use model.NewId() for linked property IDs in testUpdateView to fix validation failure (IsValid rejects non-UUID strings) - Fix import grouping in app/view.go (stdlib imports in one block) - Return 404 instead of 500 when Update/Delete store calls return ErrNotFound (e.g. concurrent deletion TOCTOU race)
This commit is contained in:
@@ -5,9 +5,8 @@ package app
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"github.com/mattermost/mattermost/server/public/model"
|
||||
"github.com/mattermost/mattermost/server/public/shared/mlog"
|
||||
@@ -59,6 +58,10 @@ func (a *App) UpdateView(rctx request.CTX, viewID string, patch *model.ViewPatch
|
||||
view.Patch(patch)
|
||||
updated, err := a.Srv().Store().View().Update(view)
|
||||
if err != nil {
|
||||
var nfErr *store.ErrNotFound
|
||||
if errors.As(err, &nfErr) {
|
||||
return nil, model.NewAppError("UpdateView", "app.view.update.app_error", nil, "", http.StatusNotFound).Wrap(err)
|
||||
}
|
||||
return nil, model.NewAppError("UpdateView", "app.view.update.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
|
||||
}
|
||||
|
||||
@@ -74,6 +77,10 @@ func (a *App) DeleteView(rctx request.CTX, viewID string) *model.AppError {
|
||||
}
|
||||
|
||||
if err := a.Srv().Store().View().Delete(viewID, model.GetMillis()); err != nil {
|
||||
var nfErr *store.ErrNotFound
|
||||
if errors.As(err, &nfErr) {
|
||||
return model.NewAppError("DeleteView", "app.view.delete.app_error", nil, "", http.StatusNotFound).Wrap(err)
|
||||
}
|
||||
return model.NewAppError("DeleteView", "app.view.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
|
||||
}
|
||||
|
||||
|
||||
@@ -198,8 +198,10 @@ func testUpdateView(t *testing.T, ss store.Store) {
|
||||
fetched, err := ss.View().Get(saved.Id)
|
||||
require.NoError(t, err)
|
||||
|
||||
propA := model.NewId()
|
||||
propB := model.NewId()
|
||||
fetched.Props = &model.ViewBoardProps{
|
||||
LinkedProperties: []string{"prop-a", "prop-b"},
|
||||
LinkedProperties: []string{propA, propB},
|
||||
Subviews: []model.Subview{{Title: "Kanban", Type: model.SubviewTypeKanban}},
|
||||
}
|
||||
_, err = ss.View().Update(fetched)
|
||||
@@ -207,7 +209,7 @@ func testUpdateView(t *testing.T, ss store.Store) {
|
||||
|
||||
refetched, err := ss.View().Get(saved.Id)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, []string{"prop-a", "prop-b"}, refetched.Props.LinkedProperties)
|
||||
assert.Equal(t, []string{propA, propB}, refetched.Props.LinkedProperties)
|
||||
})
|
||||
|
||||
t.Run("returns not found for unknown ID", func(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user