From 5d21c9b3b53a0f78373951664a154369b2b1aedc Mon Sep 17 00:00:00 2001 From: Julien Tant Date: Thu, 26 Feb 2026 07:10:52 -0500 Subject: [PATCH] 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) --- server/channels/app/view.go | 11 +++++++++-- server/channels/store/storetest/view_store.go | 6 ++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/server/channels/app/view.go b/server/channels/app/view.go index be31a9ca57b..a1cf8c38c53 100644 --- a/server/channels/app/view.go +++ b/server/channels/app/view.go @@ -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) } diff --git a/server/channels/store/storetest/view_store.go b/server/channels/store/storetest/view_store.go index bc665a51d23..bbfabedc2c1 100644 --- a/server/channels/store/storetest/view_store.go +++ b/server/channels/store/storetest/view_store.go @@ -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) {