diff --git a/pkg/services/publicdashboards/api/api.go b/pkg/services/publicdashboards/api/api.go index 61e867ad287..1837911e62d 100644 --- a/pkg/services/publicdashboards/api/api.go +++ b/pkg/services/publicdashboards/api/api.go @@ -1,8 +1,6 @@ package api import ( - "context" - "errors" "net/http" "github.com/grafana/grafana-plugin-sdk-go/backend" @@ -89,50 +87,49 @@ func (api *Api) RegisterAPIEndpoints() { routing.Wrap(api.DeletePublicDashboard)) } -// ListPublicDashboards Gets list of public dashboards for an org -// GET /api/dashboards/public +// ListPublicDashboards Gets list of public dashboards by orgId +// GET /api/dashboards/public-dashboards func (api *Api) ListPublicDashboards(c *models.ReqContext) response.Response { resp, err := api.PublicDashboardService.FindAll(c.Req.Context(), c.SignedInUser, c.OrgID) if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "ListPublicDashboards: failed to list public dashboards", err) + return response.Err(err) } return response.JSON(http.StatusOK, resp) } // GetPublicDashboard Gets public dashboard for dashboard -// GET /api/dashboards/uid/:uid/public-dashboards +// GET /api/dashboards/uid/:dashboardUid/public-dashboards func (api *Api) GetPublicDashboard(c *models.ReqContext) response.Response { // exit if we don't have a valid dashboardUid dashboardUid := web.Params(c.Req)[":dashboardUid"] if !tokens.IsValidShortUID(dashboardUid) { - api.handleError(c.Req.Context(), http.StatusBadRequest, "GetPublicDashboard: no valid dashboardUid", dashboards.ErrDashboardIdentifierNotSet) + return response.Err(ErrPublicDashboardIdentifierNotSet.Errorf("GetPublicDashboard: no dashboard Uid for public dashboard specified")) } - pd, err := api.PublicDashboardService.FindByDashboardUid(c.Req.Context(), c.OrgID, web.Params(c.Req)[":dashboardUid"]) - + pd, err := api.PublicDashboardService.FindByDashboardUid(c.Req.Context(), c.OrgID, dashboardUid) if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "GetPublicDashboard: failed to get public dashboard ", err) + return response.Err(err) } if pd == nil { - return api.handleError(c.Req.Context(), http.StatusNotFound, "GetPublicDashboard: public dashboard not found", ErrPublicDashboardNotFound) + response.Err(ErrPublicDashboardNotFound.Errorf("GetPublicDashboard: public dashboard not found")) } return response.JSON(http.StatusOK, pd) } // CreatePublicDashboard Sets public dashboard for dashboard -// POST /api/dashboards/uid/:uid/public-dashboards +// POST /api/dashboards/uid/:dashboardUid/public-dashboards func (api *Api) CreatePublicDashboard(c *models.ReqContext) response.Response { // exit if we don't have a valid dashboardUid dashboardUid := web.Params(c.Req)[":dashboardUid"] if !tokens.IsValidShortUID(dashboardUid) { - return api.handleError(c.Req.Context(), http.StatusBadRequest, "CreatePublicDashboard: invalid dashboardUid", dashboards.ErrDashboardIdentifierInvalid) + return response.Err(ErrInvalidUid.Errorf("CreatePublicDashboard: invalid Uid %s", dashboardUid)) } pd := &PublicDashboard{} if err := web.Bind(c.Req, pd); err != nil { - return api.handleError(c.Req.Context(), http.StatusBadRequest, "CreatePublicDashboard: bad request data", err) + return response.Err(ErrBadRequest.Errorf("CreatePublicDashboard: bad request data %v", err)) } // Always set the orgID and userID from the session @@ -147,29 +144,29 @@ func (api *Api) CreatePublicDashboard(c *models.ReqContext) response.Response { //Create the public dashboard pd, err := api.PublicDashboardService.Create(c.Req.Context(), c.SignedInUser, &dto) if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "CreatePublicDashboard: failed to create public dashboard", err) + return response.Err(err) } return response.JSON(http.StatusOK, pd) } // UpdatePublicDashboard Sets public dashboard for dashboard -// PUT /api/dashboards/uid/:uid/public-dashboards +// PUT /api/dashboards/uid/:dashboardUid/public-dashboards/:uid func (api *Api) UpdatePublicDashboard(c *models.ReqContext) response.Response { // exit if we don't have a valid dashboardUid dashboardUid := web.Params(c.Req)[":dashboardUid"] if !tokens.IsValidShortUID(dashboardUid) { - return api.handleError(c.Req.Context(), http.StatusBadRequest, "UpdatePublicDashboard: invalid dashboardUid", dashboards.ErrDashboardIdentifierInvalid) + return response.Err(ErrInvalidUid.Errorf("UpdatePublicDashboard: invalid dashboard Uid %s", dashboardUid)) } uid := web.Params(c.Req)[":uid"] if !tokens.IsValidShortUID(uid) { - return api.handleError(c.Req.Context(), http.StatusBadRequest, "UpdatePublicDashboard: invalid public dashboard uid", ErrPublicDashboardIdentifierNotSet) + return response.Err(ErrInvalidUid.Errorf("UpdatePublicDashboard: invalid Uid %s", uid)) } pd := &PublicDashboard{} if err := web.Bind(c.Req, pd); err != nil { - return api.handleError(c.Req.Context(), http.StatusBadRequest, "UpdatePublicDashboard: bad request data", err) + return response.Err(ErrBadRequest.Errorf("UpdatePublicDashboard: bad request data %v", err)) } // Always set the orgID and userID from the session @@ -182,10 +179,10 @@ func (api *Api) UpdatePublicDashboard(c *models.ReqContext) response.Response { PublicDashboard: pd, } - // Save the public dashboard + // Update the public dashboard pd, err := api.PublicDashboardService.Update(c.Req.Context(), c.SignedInUser, &dto) if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "UpdatePublicDashboard: failed to update public dashboard", err) + return response.Err(err) } return response.JSON(http.StatusOK, pd) @@ -196,38 +193,17 @@ func (api *Api) UpdatePublicDashboard(c *models.ReqContext) response.Response { func (api *Api) DeletePublicDashboard(c *models.ReqContext) response.Response { uid := web.Params(c.Req)[":uid"] if !tokens.IsValidShortUID(uid) { - return api.handleError(c.Req.Context(), http.StatusBadRequest, "DeletePublicDashboard: invalid dashboard uid", dashboards.ErrDashboardIdentifierNotSet) + return response.Err(ErrInvalidUid.Errorf("UpdatePublicDashboard: invalid Uid %s", uid)) } err := api.PublicDashboardService.Delete(c.Req.Context(), c.OrgID, uid) if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "DeletePublicDashboard: failed to delete public dashboard", err) + return response.Err(err) } return response.JSON(http.StatusOK, nil) } -// util to help us unpack dashboard and publicdashboard errors or use default http code and message -// we should look to do some future refactoring of these errors as publicdashboard err is the same as a dashboarderr, just defined in a -// different package. -func (api *Api) handleError(ctx context.Context, code int, message string, err error) response.Response { - var publicDashboardErr PublicDashboardErr - ctxLogger := api.Log.FromContext(ctx) - ctxLogger.Error(message, "error", err.Error()) - - // handle public dashboard error - if ok := errors.As(err, &publicDashboardErr); ok { - return response.Error(publicDashboardErr.StatusCode, publicDashboardErr.Error(), publicDashboardErr) - } - - var dashboardErr dashboards.DashboardErr - if ok := errors.As(err, &dashboardErr); ok { - return response.Error(dashboardErr.StatusCode, dashboardErr.Error(), dashboardErr) - } - - return response.Error(code, message, err) -} - // Copied from pkg/api/metrics.go func toJsonStreamingResponse(features *featuremgmt.FeatureManager, qdr *backend.QueryDataResponse) response.Response { statusWhenError := http.StatusBadRequest diff --git a/pkg/services/publicdashboards/api/api_test.go b/pkg/services/publicdashboards/api/api_test.go index 25b279cf978..226274d7f1b 100644 --- a/pkg/services/publicdashboards/api/api_test.go +++ b/pkg/services/publicdashboards/api/api_test.go @@ -8,6 +8,10 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" @@ -15,9 +19,7 @@ import ( . "github.com/grafana/grafana/pkg/services/publicdashboards/models" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/util/errutil" ) var userAdmin = &user.SignedInUser{UserID: 1, OrgID: 1, OrgRole: org.RoleAdmin, Login: "testAdminUser"} @@ -121,7 +123,7 @@ func TestAPIListPublicDashboard(t *testing.T) { Name: "Handles Service error", User: userViewer, Response: nil, - ResponseErr: errors.New("error, service broken"), + ResponseErr: ErrInternalServerError.Errorf(""), ExpectedHttpResponse: http.StatusInternalServerError, }, } @@ -148,16 +150,150 @@ func TestAPIListPublicDashboard(t *testing.T) { } if test.ResponseErr != nil { - var errResp JsonErrResponse + var errResp errutil.PublicError err := json.Unmarshal(response.Body.Bytes(), &errResp) require.NoError(t, err) - assert.Equal(t, "error, service broken", errResp.Error) + assert.Equal(t, "Internal server error", errResp.Message) + assert.Equal(t, "publicdashboards.internalServerError", errResp.MessageID) service.AssertNotCalled(t, "FindAll") } }) } } +func TestAPIDeletePublicDashboard(t *testing.T) { + dashboardUid := "abc1234" + publicDashboardUid := "1234asdfasdf" + userEditorAllPublicDashboard := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Login: "testEditorUser", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {dashboards.ScopeDashboardsAll}}}} + userEditorAnotherPublicDashboard := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Login: "testEditorUser", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {"another-uid"}}}} + userEditorPublicDashboard := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Login: "testEditorUser", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {fmt.Sprintf("dashboards:uid:%s", dashboardUid)}}}} + + testCases := []struct { + Name string + User *user.SignedInUser + DashboardUid string + PublicDashboardUid string + ResponseErr error + ExpectedHttpResponse int + ExpectedMessageResponse string + ShouldCallService bool + }{ + { + Name: "User viewer cannot delete public dashboard", + User: userViewer, + DashboardUid: dashboardUid, + PublicDashboardUid: publicDashboardUid, + ResponseErr: nil, + ExpectedHttpResponse: http.StatusForbidden, + ShouldCallService: false, + }, + { + Name: "User editor without specific dashboard access cannot delete public dashboard", + User: userEditorAnotherPublicDashboard, + DashboardUid: dashboardUid, + PublicDashboardUid: publicDashboardUid, + ResponseErr: nil, + ExpectedHttpResponse: http.StatusForbidden, + ShouldCallService: false, + }, + { + Name: "User editor with all dashboard accesses can delete public dashboard", + User: userEditorAllPublicDashboard, + DashboardUid: dashboardUid, + PublicDashboardUid: publicDashboardUid, + ResponseErr: nil, + ExpectedHttpResponse: http.StatusOK, + ShouldCallService: true, + }, + { + Name: "User editor with dashboard access can delete public dashboard", + User: userEditorPublicDashboard, + DashboardUid: dashboardUid, + PublicDashboardUid: publicDashboardUid, + ResponseErr: nil, + ExpectedHttpResponse: http.StatusOK, + ShouldCallService: true, + }, + { + Name: "Internal server error returns an error", + User: userEditorPublicDashboard, + DashboardUid: dashboardUid, + PublicDashboardUid: publicDashboardUid, + ResponseErr: ErrInternalServerError.Errorf(""), + ExpectedHttpResponse: ErrInternalServerError.Errorf("").Reason.Status().HTTPStatus(), + ExpectedMessageResponse: ErrInternalServerError.Errorf("").PublicMessage, + ShouldCallService: true, + }, + { + Name: "PublicDashboard error returns correct status code instead of 500", + User: userEditorPublicDashboard, + DashboardUid: dashboardUid, + PublicDashboardUid: publicDashboardUid, + ResponseErr: ErrPublicDashboardIdentifierNotSet.Errorf(""), + ExpectedHttpResponse: ErrPublicDashboardIdentifierNotSet.Errorf("").Reason.Status().HTTPStatus(), + ExpectedMessageResponse: ErrPublicDashboardIdentifierNotSet.Errorf("").PublicMessage, + ShouldCallService: true, + }, + { + Name: "Invalid publicDashboardUid throws an error", + User: userEditorPublicDashboard, + DashboardUid: dashboardUid, + PublicDashboardUid: "inv@lid-publicd@shboard-uid!", + ResponseErr: nil, + ExpectedHttpResponse: http.StatusBadRequest, + ShouldCallService: false, + }, + { + Name: "Public dashboard uid does not exist", + User: userEditorPublicDashboard, + DashboardUid: dashboardUid, + PublicDashboardUid: "UIDDOESNOTEXIST", + ResponseErr: ErrPublicDashboardNotFound.Errorf(""), + ExpectedHttpResponse: ErrPublicDashboardNotFound.Errorf("").Reason.Status().HTTPStatus(), + ExpectedMessageResponse: ErrPublicDashboardNotFound.Errorf("").PublicMessage, + ShouldCallService: true, + }, + } + + for _, test := range testCases { + t.Run(test.Name, func(t *testing.T) { + service := publicdashboards.NewFakePublicDashboardService(t) + + if test.ShouldCallService { + service.On("Delete", mock.Anything, mock.Anything, mock.Anything). + Return(test.ResponseErr) + } + + cfg := setting.NewCfg() + + features := featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards) + testServer := setupTestServer(t, cfg, features, service, nil, test.User) + + response := callAPI(testServer, http.MethodDelete, fmt.Sprintf("/api/dashboards/uid/%s/public-dashboards/%s", test.DashboardUid, test.PublicDashboardUid), nil, t) + assert.Equal(t, test.ExpectedHttpResponse, response.Code) + + if test.ExpectedHttpResponse == http.StatusOK { + var jsonResp any + err := json.Unmarshal(response.Body.Bytes(), &jsonResp) + require.NoError(t, err) + assert.Equal(t, jsonResp, nil) + } + + if !test.ShouldCallService { + service.AssertNotCalled(t, "Delete") + } + + if test.ResponseErr != nil { + var errResp errutil.PublicError + err := json.Unmarshal(response.Body.Bytes(), &errResp) + require.NoError(t, err) + assert.Equal(t, test.ExpectedHttpResponse, errResp.StatusCode) + assert.Equal(t, test.ExpectedMessageResponse, errResp.Message) + } + }) + } +} + func TestAPIGetPublicDashboard(t *testing.T) { pubdash := &PublicDashboard{IsEnabled: true} @@ -186,7 +322,7 @@ func TestAPIGetPublicDashboard(t *testing.T) { DashboardUid: "77777", ExpectedHttpResponse: http.StatusNotFound, PublicDashboardResult: nil, - PublicDashboardErr: dashboards.ErrDashboardNotFound, + PublicDashboardErr: ErrDashboardNotFound.Errorf(""), User: userViewer, AccessControlEnabled: false, ShouldCallService: true, @@ -288,7 +424,7 @@ func TestApiCreatePublicDashboard(t *testing.T) { Name: "returns 500 when not persisted", ExpectedHttpResponse: http.StatusInternalServerError, publicDashboard: &PublicDashboard{}, - SaveDashboardErr: errors.New("backend failed to save"), + SaveDashboardErr: ErrInternalServerError.Errorf(""), User: userAdmin, AccessControlEnabled: false, ShouldCallService: true, @@ -297,7 +433,7 @@ func TestApiCreatePublicDashboard(t *testing.T) { Name: "returns 404 when dashboard not found", ExpectedHttpResponse: http.StatusNotFound, publicDashboard: &PublicDashboard{}, - SaveDashboardErr: dashboards.ErrDashboardNotFound, + SaveDashboardErr: ErrDashboardNotFound.Errorf(""), User: userAdmin, AccessControlEnabled: false, ShouldCallService: true, @@ -316,7 +452,7 @@ func TestApiCreatePublicDashboard(t *testing.T) { Name: "returns 403 when no permissions", ExpectedHttpResponse: http.StatusForbidden, publicDashboard: &PublicDashboard{IsEnabled: true}, - SaveDashboardErr: nil, + SaveDashboardErr: ErrInternalServerError.Errorf("default error"), User: userViewer, AccessControlEnabled: false, ShouldCallService: false, @@ -400,7 +536,7 @@ func TestAPIUpdatePublicDashboard(t *testing.T) { DashboardUid: "", PublicDashboardUid: "", PublicDashboardRes: nil, - PublicDashboardErr: dashboards.ErrDashboardIdentifierInvalid, + PublicDashboardErr: ErrPublicDashboardIdentifierNotSet.Errorf(""), ExpectedHttpResponse: http.StatusNotFound, ShouldCallService: false, }, @@ -410,7 +546,7 @@ func TestAPIUpdatePublicDashboard(t *testing.T) { DashboardUid: dashboardUid, PublicDashboardUid: "", PublicDashboardRes: nil, - PublicDashboardErr: ErrPublicDashboardNotFound, + PublicDashboardErr: ErrPublicDashboardNotFound.Errorf(""), ExpectedHttpResponse: http.StatusNotFound, ShouldCallService: false, }, @@ -420,7 +556,7 @@ func TestAPIUpdatePublicDashboard(t *testing.T) { DashboardUid: dashboardUid, PublicDashboardUid: publicDashboardUid, PublicDashboardRes: nil, - PublicDashboardErr: dashboards.ErrDashboardNotFound, + PublicDashboardErr: ErrDashboardNotFound.Errorf(""), ExpectedHttpResponse: http.StatusNotFound, ShouldCallService: true, }, @@ -505,131 +641,3 @@ func TestAPIUpdatePublicDashboard(t *testing.T) { }) } } - -func TestAPIDeletePublicDashboard(t *testing.T) { - dashboardUid := "abc1234" - publicDashboardUid := "1234asdfasdf" - userEditorAllPublicDashboard := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Login: "testEditorUser", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {dashboards.ScopeDashboardsAll}}}} - userEditorAnotherPublicDashboard := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Login: "testEditorUser", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {"another-uid"}}}} - userEditorPublicDashboard := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Login: "testEditorUser", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {fmt.Sprintf("dashboards:uid:%s", dashboardUid)}}}} - - testCases := []struct { - Name string - User *user.SignedInUser - DashboardUid string - PublicDashboardUid string - ResponseErr error - ExpectedHttpResponse int - ShouldCallService bool - }{ - { - Name: "User viewer cannot delete public dashboard", - User: userViewer, - DashboardUid: dashboardUid, - PublicDashboardUid: publicDashboardUid, - ResponseErr: nil, - ExpectedHttpResponse: http.StatusForbidden, - ShouldCallService: false, - }, - { - Name: "User editor without specific dashboard access cannot delete public dashboard", - User: userEditorAnotherPublicDashboard, - DashboardUid: dashboardUid, - PublicDashboardUid: publicDashboardUid, - ResponseErr: nil, - ExpectedHttpResponse: http.StatusForbidden, - ShouldCallService: false, - }, - { - Name: "User editor with all dashboard accesses can delete public dashboard", - User: userEditorAllPublicDashboard, - DashboardUid: dashboardUid, - PublicDashboardUid: publicDashboardUid, - ResponseErr: nil, - ExpectedHttpResponse: http.StatusOK, - ShouldCallService: true, - }, - { - Name: "User editor with dashboard access can delete public dashboard", - User: userEditorPublicDashboard, - DashboardUid: dashboardUid, - PublicDashboardUid: publicDashboardUid, - ResponseErr: nil, - ExpectedHttpResponse: http.StatusOK, - ShouldCallService: true, - }, - { - Name: "Internal server error returns an error", - User: userEditorPublicDashboard, - DashboardUid: dashboardUid, - PublicDashboardUid: publicDashboardUid, - ResponseErr: errors.New("server error"), - ExpectedHttpResponse: http.StatusInternalServerError, - ShouldCallService: true, - }, - { - Name: "PublicDashboard error returns correct status code instead of 500", - User: userEditorPublicDashboard, - DashboardUid: dashboardUid, - PublicDashboardUid: publicDashboardUid, - ResponseErr: ErrPublicDashboardIdentifierNotSet, - ExpectedHttpResponse: ErrPublicDashboardIdentifierNotSet.StatusCode, - ShouldCallService: true, - }, - { - Name: "Invalid publicDashboardUid throws an error", - User: userEditorPublicDashboard, - DashboardUid: dashboardUid, - PublicDashboardUid: "inv@lid-publicd@shboard-uid!", - ResponseErr: nil, - ExpectedHttpResponse: ErrPublicDashboardIdentifierNotSet.StatusCode, - ShouldCallService: false, - }, - { - Name: "Public dashboard uid does not exist", - User: userEditorPublicDashboard, - DashboardUid: dashboardUid, - PublicDashboardUid: "UIDDOESNOTEXIST", - ResponseErr: ErrPublicDashboardNotFound, - ExpectedHttpResponse: ErrPublicDashboardNotFound.StatusCode, - ShouldCallService: true, - }, - } - - for _, test := range testCases { - t.Run(test.Name, func(t *testing.T) { - service := publicdashboards.NewFakePublicDashboardService(t) - - if test.ShouldCallService { - service.On("Delete", mock.Anything, mock.Anything, mock.Anything). - Return(test.ResponseErr) - } - - cfg := setting.NewCfg() - - features := featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards) - testServer := setupTestServer(t, cfg, features, service, nil, test.User) - - response := callAPI(testServer, http.MethodDelete, fmt.Sprintf("/api/dashboards/uid/%s/public-dashboards/%s", test.DashboardUid, test.PublicDashboardUid), nil, t) - assert.Equal(t, test.ExpectedHttpResponse, response.Code) - - if test.ExpectedHttpResponse == http.StatusOK { - var jsonResp any - err := json.Unmarshal(response.Body.Bytes(), &jsonResp) - require.NoError(t, err) - assert.Equal(t, jsonResp, nil) - } - - if !test.ShouldCallService { - service.AssertNotCalled(t, "Delete") - } - - if test.ResponseErr != nil { - var errResp JsonErrResponse - err := json.Unmarshal(response.Body.Bytes(), &errResp) - require.NoError(t, err) - assert.Equal(t, test.ResponseErr.Error(), errResp.Error) - } - }) - } -} diff --git a/pkg/services/publicdashboards/api/query.go b/pkg/services/publicdashboards/api/query.go index bab07c64445..582bec042db 100644 --- a/pkg/services/publicdashboards/api/query.go +++ b/pkg/services/publicdashboards/api/query.go @@ -16,9 +16,8 @@ import ( // GET /api/public/dashboards/:accessToken func (api *Api) ViewPublicDashboard(c *models.ReqContext) response.Response { accessToken := web.Params(c.Req)[":accessToken"] - if !tokens.IsValidAccessToken(accessToken) { - return response.Error(http.StatusBadRequest, "Invalid Access Token", nil) + return response.Err(ErrInvalidAccessToken.Errorf("ViewPublicDashboard: invalid access token")) } pubdash, dash, err := api.PublicDashboardService.FindPublicDashboardAndDashboardByAccessToken( @@ -26,7 +25,7 @@ func (api *Api) ViewPublicDashboard(c *models.ReqContext) response.Response { accessToken, ) if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "ViewPublicDashboard: failed to get public dashboard", err) + return response.Err(err) } meta := dtos.DashboardMeta{ @@ -56,22 +55,22 @@ func (api *Api) ViewPublicDashboard(c *models.ReqContext) response.Response { func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response { accessToken := web.Params(c.Req)[":accessToken"] if !tokens.IsValidAccessToken(accessToken) { - return response.Error(http.StatusBadRequest, "Invalid Access Token", nil) + return response.Err(ErrInvalidAccessToken.Errorf("QueryPublicDashboard: invalid access token")) } panelId, err := strconv.ParseInt(web.Params(c.Req)[":panelId"], 10, 64) if err != nil { - return response.Error(http.StatusBadRequest, "QueryPublicDashboard: invalid panel ID", err) + return response.Err(ErrInvalidPanelId.Errorf("QueryPublicDashboard: error parsing panelId %v", err)) } reqDTO := PublicDashboardQueryDTO{} if err = web.Bind(c.Req, &reqDTO); err != nil { - return response.Error(http.StatusBadRequest, "QueryPublicDashboard: bad request data", err) + return response.Err(ErrBadRequest.Errorf("QueryPublicDashboard: error parsing request: %v", err)) } resp, err := api.PublicDashboardService.GetQueryDataResponse(c.Req.Context(), c.SkipCache, reqDTO, panelId, accessToken) if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "QueryPublicDashboard: error running public dashboard panel queries", err) + return response.Err(err) } return toJsonStreamingResponse(api.Features, resp) @@ -82,7 +81,7 @@ func (api *Api) QueryPublicDashboard(c *models.ReqContext) response.Response { func (api *Api) GetAnnotations(c *models.ReqContext) response.Response { accessToken := web.Params(c.Req)[":accessToken"] if !tokens.IsValidAccessToken(accessToken) { - return response.Error(http.StatusBadRequest, "Invalid Access Token", nil) + return response.Err(ErrInvalidAccessToken.Errorf("GetAnnotations: invalid access token")) } reqDTO := AnnotationsQueryDTO{ @@ -91,9 +90,8 @@ func (api *Api) GetAnnotations(c *models.ReqContext) response.Response { } annotations, err := api.PublicDashboardService.FindAnnotations(c.Req.Context(), reqDTO, accessToken) - if err != nil { - return api.handleError(c.Req.Context(), http.StatusInternalServerError, "error getting public dashboard annotations", err) + return response.Err(err) } return response.JSON(http.StatusOK, annotations) diff --git a/pkg/services/publicdashboards/api/query_test.go b/pkg/services/publicdashboards/api/query_test.go index 92b87dec2ed..c5aa5caa787 100644 --- a/pkg/services/publicdashboards/api/query_test.go +++ b/pkg/services/publicdashboards/api/query_test.go @@ -31,6 +31,7 @@ import ( "github.com/grafana/grafana/pkg/services/tag/tagimpl" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/util/errutil" "github.com/grafana/grafana/pkg/web" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -65,7 +66,7 @@ func TestAPIViewPublicDashboard(t *testing.T) { AccessToken: validAccessToken, ExpectedHttpResponse: http.StatusNotFound, DashboardResult: nil, - Err: ErrPublicDashboardNotFound, + Err: ErrPublicDashboardNotFound.Errorf(""), FixedErrorResponse: "", }, { @@ -74,7 +75,7 @@ func TestAPIViewPublicDashboard(t *testing.T) { ExpectedHttpResponse: http.StatusBadRequest, DashboardResult: nil, Err: nil, - FixedErrorResponse: "{\"message\":\"Invalid Access Token\"}", + FixedErrorResponse: "{\"message\":\"Invalid access token\", \"messageId\":\"publicdashboards.invalidAccessToken\", \"statusCode\":400, \"traceID\":\"\"}", }, } @@ -115,12 +116,13 @@ func TestAPIViewPublicDashboard(t *testing.T) { assert.Equal(t, false, dashResp.Meta.CanSave) } else if test.FixedErrorResponse != "" { require.Equal(t, test.ExpectedHttpResponse, response.Code) - require.JSONEq(t, "{\"message\":\"Invalid Access Token\"}", response.Body.String()) + require.JSONEq(t, "{\"message\":\"Invalid access token\", \"messageId\":\"publicdashboards.invalidAccessToken\", \"statusCode\":400, \"traceID\":\"\"}", response.Body.String()) } else { - var errResp JsonErrResponse + var errResp errutil.PublicError err := json.Unmarshal(response.Body.Bytes(), &errResp) require.NoError(t, err) - assert.Equal(t, test.Err.Error(), errResp.Error) + assert.Equal(t, "Public dashboard not found", errResp.Message) + assert.Equal(t, "publicdashboards.notFound", errResp.MessageID) } }) } @@ -208,19 +210,19 @@ func TestAPIQueryPublicDashboard(t *testing.T) { server, _ := setup(true) resp := callAPI(server, http.MethodPost, getValidQueryPath("SomeInvalidAccessToken"), strings.NewReader("{}"), t) require.Equal(t, http.StatusBadRequest, resp.Code) - require.JSONEq(t, "{\"message\":\"Invalid Access Token\"}", resp.Body.String()) + require.JSONEq(t, "{\"message\":\"Invalid access token\", \"messageId\":\"publicdashboards.invalidAccessToken\", \"statusCode\":400, \"traceID\":\"\"}", resp.Body.String()) }) t.Run("Status code is 400 when the intervalMS is lesser than 0", func(t *testing.T) { server, fakeDashboardService := setup(true) - fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), validAccessToken).Return(&backend.QueryDataResponse{}, ErrPublicDashboardBadRequest) + fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), validAccessToken).Return(&backend.QueryDataResponse{}, ErrBadRequest.Errorf("")) resp := callAPI(server, http.MethodPost, getValidQueryPath(validAccessToken), strings.NewReader(`{"intervalMs":-100,"maxDataPoints":1000}`), t) require.Equal(t, http.StatusBadRequest, resp.Code) }) t.Run("Status code is 400 when the maxDataPoints is lesser than 0", func(t *testing.T) { server, fakeDashboardService := setup(true) - fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), validAccessToken).Return(&backend.QueryDataResponse{}, ErrPublicDashboardBadRequest) + fakeDashboardService.On("GetQueryDataResponse", mock.Anything, true, mock.Anything, int64(2), validAccessToken).Return(&backend.QueryDataResponse{}, ErrBadRequest.Errorf("")) resp := callAPI(server, http.MethodPost, getValidQueryPath(validAccessToken), strings.NewReader(`{"intervalMs":100,"maxDataPoints":-1000}`), t) require.Equal(t, http.StatusBadRequest, resp.Code) }) diff --git a/pkg/services/publicdashboards/models/errors.go b/pkg/services/publicdashboards/models/errors.go new file mode 100644 index 00000000000..51ff7a1def5 --- /dev/null +++ b/pkg/services/publicdashboards/models/errors.go @@ -0,0 +1,22 @@ +package models + +import "github.com/grafana/grafana/pkg/util/errutil" + +var ( + ErrInternalServerError = errutil.NewBase(errutil.StatusInternal, "publicdashboards.internalServerError", errutil.WithPublicMessage("Internal server error")) + + ErrPublicDashboardNotFound = errutil.NewBase(errutil.StatusNotFound, "publicdashboards.notFound", errutil.WithPublicMessage("Public dashboard not found")) + ErrDashboardNotFound = errutil.NewBase(errutil.StatusNotFound, "publicdashboards.dashboardNotFound", errutil.WithPublicMessage("Dashboard not found")) + ErrPanelNotFound = errutil.NewBase(errutil.StatusNotFound, "publicdashboards.panelNotFound", errutil.WithPublicMessage("Public dashboard panel not found")) + + ErrBadRequest = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.badRequest") + ErrPanelQueriesNotFound = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.panelQueriesNotFound", errutil.WithPublicMessage("Failed to extract queries from panel")) + ErrInvalidAccessToken = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidAccessToken", errutil.WithPublicMessage("Invalid access token")) + ErrInvalidPanelId = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidPanelId", errutil.WithPublicMessage("Invalid panel id")) + ErrInvalidUid = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidUid", errutil.WithPublicMessage("Invalid Uid")) + + ErrPublicDashboardIdentifierNotSet = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.identifierNotSet", errutil.WithPublicMessage("No Uid for public dashboard specified")) + ErrPublicDashboardHasTemplateVariables = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.hasTemplateVariables", errutil.WithPublicMessage("Public dashboard has template variables")) + ErrInvalidInterval = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.invalidInterval", errutil.WithPublicMessage("intervalMS should be greater than 0")) + ErrInvalidMaxDataPoints = errutil.NewBase(errutil.StatusBadRequest, "publicdashboards.maxDataPoints", errutil.WithPublicMessage("maxDataPoints should be greater than 0")) +) diff --git a/pkg/services/publicdashboards/models/models.go b/pkg/services/publicdashboards/models/models.go index bfdbebe0417..4afa3513856 100644 --- a/pkg/services/publicdashboards/models/models.go +++ b/pkg/services/publicdashboards/models/models.go @@ -30,43 +30,6 @@ const QueryFailure = "failure" var QueryResultStatuses = []string{QuerySuccess, QueryFailure} -var ( - ErrPublicDashboardFailedGenerateUniqueUid = PublicDashboardErr{ - Reason: "failed to generate unique public dashboard id", - StatusCode: 500, - } - ErrPublicDashboardFailedGenerateAccessToken = PublicDashboardErr{ - Reason: "failed to create public dashboard", - StatusCode: 500, - } - ErrPublicDashboardNotFound = PublicDashboardErr{ - Reason: "public dashboard not found", - StatusCode: 404, - Status: "not-found", - } - ErrPublicDashboardPanelNotFound = PublicDashboardErr{ - Reason: "panel not found in dashboard", - StatusCode: 404, - Status: "not-found", - } - ErrPublicDashboardIdentifierNotSet = PublicDashboardErr{ - Reason: "no Uid for public dashboard specified", - StatusCode: 400, - } - ErrPublicDashboardHasTemplateVariables = PublicDashboardErr{ - Reason: "public dashboard has template variables", - StatusCode: 422, - } - ErrPublicDashboardBadRequest = PublicDashboardErr{ - Reason: "bad Request", - StatusCode: 400, - } - ErrNoPanelQueriesFound = PublicDashboardErr{ - Reason: "failed to extract queries from panel", - StatusCode: 400, - } -) - type PublicDashboard struct { Uid string `json:"uid" xorm:"pk uid"` DashboardUid string `json:"dashboardUid" xorm:"dashboard_uid"` diff --git a/pkg/services/publicdashboards/service/query.go b/pkg/services/publicdashboards/service/query.go index d063660b518..d5c94b86993 100644 --- a/pkg/services/publicdashboards/service/query.go +++ b/pkg/services/publicdashboards/service/query.go @@ -17,7 +17,7 @@ import ( "github.com/grafana/grafana/pkg/tsdb/grafanads" ) -// GetAnnotations returns annotations for a public dashboard +// FindAnnotations returns annotations for a public dashboard func (pd *PublicDashboardServiceImpl) FindAnnotations(ctx context.Context, reqDTO models.AnnotationsQueryDTO, accessToken string) ([]models.AnnotationEvent, error) { pub, dash, err := pd.FindPublicDashboardAndDashboardByAccessToken(ctx, accessToken) if err != nil { @@ -30,7 +30,7 @@ func (pd *PublicDashboardServiceImpl) FindAnnotations(ctx context.Context, reqDT annoDto, err := UnmarshalDashboardAnnotations(dash.Data) if err != nil { - return nil, err + return nil, models.ErrInternalServerError.Errorf("FindAnnotations: failed to unmarshal dashboard annotations: %w", err) } anonymousUser := buildAnonymousUser(ctx, dash) @@ -59,7 +59,7 @@ func (pd *PublicDashboardServiceImpl) FindAnnotations(ctx context.Context, reqDT annotationItems, err := pd.AnnotationsRepo.Find(ctx, annoQuery) if err != nil { - return nil, err + return nil, models.ErrInternalServerError.Errorf("FindAnnotations: failed to find annotations: %w", err) } for _, item := range annotationItems { @@ -131,7 +131,7 @@ func (pd *PublicDashboardServiceImpl) GetQueryDataResponse(ctx context.Context, } if len(metricReq.Queries) == 0 { - return nil, models.ErrNoPanelQueriesFound + return nil, models.ErrPanelQueriesNotFound.Errorf("GetQueryDataResponse: failed to extract queries from panel") } anonymousUser := buildAnonymousUser(ctx, dashboard) @@ -155,7 +155,7 @@ func (pd *PublicDashboardServiceImpl) buildMetricRequest(ctx context.Context, da queriesByPanel := groupQueriesByPanelId(dashboard.Data) queries, ok := queriesByPanel[panelId] if !ok { - return dtos.MetricRequest{}, models.ErrPublicDashboardPanelNotFound + return dtos.MetricRequest{}, models.ErrPanelNotFound.Errorf("buildMetricRequest: public dashboard panel not found") } ts := publicDashboard.BuildTimeSettings(dashboard) diff --git a/pkg/services/publicdashboards/service/query_test.go b/pkg/services/publicdashboards/service/query_test.go index 366fe40c259..884b82d8d59 100644 --- a/pkg/services/publicdashboards/service/query_test.go +++ b/pkg/services/publicdashboards/service/query_test.go @@ -915,7 +915,7 @@ func TestBuildMetricRequest(t *testing.T) { publicDashboardQueryDTO, ) - require.ErrorContains(t, err, ErrPublicDashboardPanelNotFound.Reason) + require.ErrorContains(t, err, ErrPanelNotFound.Error()) }) t.Run("metric request built without hidden query", func(t *testing.T) { diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index 29685c36bc4..daab65439bd 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -65,33 +65,29 @@ func ProvideService( func (pd *PublicDashboardServiceImpl) FindDashboard(ctx context.Context, orgId int64, dashboardUid string) (*models.Dashboard, error) { dash, err := pd.store.FindDashboard(ctx, orgId, dashboardUid) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("FindDashboard: failed to find dashboard by orgId: %d and dashboardUid: %s: %w", orgId, dashboardUid, err) } if dash == nil { - return nil, dashboards.ErrDashboardNotFound + return nil, ErrDashboardNotFound.Errorf("FindDashboard: dashboard not found by orgId: %d and dashboardUid: %s", orgId, dashboardUid) } return dash, nil } -// FindPublicDashboardAndDashboardByAccessToken Gets public dashboard via access token +// FindPublicDashboardAndDashboardByAccessToken Gets public dashboard and a dashboard by access token func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessToken(ctx context.Context, accessToken string) (*PublicDashboard, *models.Dashboard, error) { - ctxLogger := pd.log.FromContext(ctx) - pubdash, err := pd.store.FindByAccessToken(ctx, accessToken) if err != nil { - return nil, nil, err + return nil, nil, ErrInternalServerError.Errorf("FindPublicDashboardAndDashboardByAccessToken: failed to find a public dashboard: %w", err) } if pubdash == nil { - ctxLogger.Error("FindPublicDashboardAndDashboardByAccessToken: Public dashboard not found", "accessToken", accessToken) - return nil, nil, ErrPublicDashboardNotFound + return nil, nil, ErrPublicDashboardNotFound.Errorf("FindPublicDashboardAndDashboardByAccessToken: Public dashboard not found accessToken: %s", accessToken) } if !pubdash.IsEnabled { - ctxLogger.Error("FindPublicDashboardAndDashboardByAccessToken: Public dashboard is disabled", "accessToken", accessToken) - return nil, nil, ErrPublicDashboardNotFound + return nil, nil, ErrPublicDashboardNotFound.Errorf("FindPublicDashboardAndDashboardByAccessToken: Public dashboard is disabled accessToken: %s", accessToken) } dash, err := pd.store.FindDashboard(ctx, pubdash.OrgId, pubdash.DashboardUid) @@ -100,8 +96,7 @@ func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessTok } if dash == nil { - ctxLogger.Error("FindPublicDashboardAndDashboardByAccessToken: Dashboard not found", "accessToken", accessToken) - return nil, nil, ErrPublicDashboardNotFound + return nil, nil, ErrPublicDashboardNotFound.Errorf("FindPublicDashboardAndDashboardByAccessToken: Dashboard not found accessToken: %s", accessToken) } return pubdash, dash, nil @@ -111,11 +106,11 @@ func (pd *PublicDashboardServiceImpl) FindPublicDashboardAndDashboardByAccessTok func (pd *PublicDashboardServiceImpl) FindByDashboardUid(ctx context.Context, orgId int64, dashboardUid string) (*PublicDashboard, error) { pubdash, err := pd.store.FindByDashboardUid(ctx, orgId, dashboardUid) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("FindByDashboardUid: failed to find a public dashboard by orgId: %d and dashboardUid: %s: %w", orgId, dashboardUid, err) } if pubdash == nil { - return nil, ErrPublicDashboardNotFound + return nil, ErrPublicDashboardNotFound.Errorf("FindByDashboardUid: Public dashboard not found by orgId: %d and dashboardUid: %s", orgId, dashboardUid) } return pubdash, nil @@ -144,9 +139,9 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed // request existingPubdash, err := pd.store.Find(ctx, dto.PublicDashboard.Uid) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("Create: failed to find the public dashboard: %w", err) } else if existingPubdash != nil { - return nil, ErrPublicDashboardBadRequest + return nil, ErrBadRequest.Errorf("Create: public dashboard already exists: %s", dto.PublicDashboard.Uid) } uid, err := pd.NewPublicDashboardUid(ctx) @@ -175,13 +170,13 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed _, err = pd.store.Create(ctx, cmd) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("Create: failed to create the public dashboard: %w", err) } //Get latest public dashboard to return newPubdash, err := pd.store.Find(ctx, uid) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("Create: failed to find the public dashboard: %w", err) } pd.logIsEnabledChanged(existingPubdash, newPubdash, u) @@ -189,16 +184,16 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed return newPubdash, err } -// Updates an existing public dashboard based on publicdashboard.Uid +// Update: updates an existing public dashboard based on publicdashboard.Uid func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.SignedInUser, dto *SavePublicDashboardDTO) (*PublicDashboard, error) { // validate if the dashboard exists dashboard, err := pd.FindDashboard(ctx, u.OrgID, dto.DashboardUid) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("Update: failed to find dashboard by orgId: %d and dashboardUid: %s: %w", u.OrgID, dto.DashboardUid, err) } if dashboard == nil { - return nil, dashboards.ErrDashboardNotFound + return nil, ErrDashboardNotFound.Errorf("Update: dashboard not found by orgId: %d and dashboardUid: %s", u.OrgID, dto.DashboardUid) } // set default value for time settings @@ -209,9 +204,9 @@ func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.Signed // get existing public dashboard if exists existingPubdash, err := pd.store.Find(ctx, dto.PublicDashboard.Uid) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("Update: failed to find public dashboard by uid: %s: %w", dto.PublicDashboard.Uid, err) } else if existingPubdash == nil { - return nil, ErrPublicDashboardNotFound + return nil, ErrPublicDashboardNotFound.Errorf("Update: public dashboard not found by uid: %s", dto.PublicDashboard.Uid) } // validate dashboard @@ -235,23 +230,23 @@ func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.Signed // persist affectedRows, err := pd.store.Update(ctx, cmd) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("Update: failed to update public dashboard: %w", err) } // 404 if not found if affectedRows == 0 { - return nil, ErrPublicDashboardNotFound + return nil, ErrPublicDashboardNotFound.Errorf("Update: failed to update public dashboard not found by uid: %s", dto.PublicDashboard.Uid) } // get latest public dashboard to return newPubdash, err := pd.store.Find(ctx, existingPubdash.Uid) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("Update: failed to find public dashboard by uid: %s: %w", existingPubdash.Uid, err) } pd.logIsEnabledChanged(existingPubdash, newPubdash, u) - return newPubdash, err + return newPubdash, nil } // NewPublicDashboardUid Generates a unique uid to create a public dashboard. Will make 3 attempts and fail if it cannot find an unused uid @@ -265,7 +260,7 @@ func (pd *PublicDashboardServiceImpl) NewPublicDashboardUid(ctx context.Context) return uid, nil } } - return "", ErrPublicDashboardFailedGenerateUniqueUid + return "", ErrInternalServerError.Errorf("failed to generate a unique uid for public dashboard") } // NewPublicDashboardAccessToken Generates a unique accessToken to create a public dashboard. Will make 3 attempts and fail if it cannot find an unused access token @@ -283,14 +278,14 @@ func (pd *PublicDashboardServiceImpl) NewPublicDashboardAccessToken(ctx context. return accessToken, nil } } - return "", ErrPublicDashboardFailedGenerateAccessToken + return "", ErrInternalServerError.Errorf("failed to generate a unique accesssToken for public dashboard") } // FindAll Returns a list of public dashboards by orgId func (pd *PublicDashboardServiceImpl) FindAll(ctx context.Context, u *user.SignedInUser, orgId int64) ([]PublicDashboardListResponse, error) { publicDashboards, err := pd.store.FindAll(ctx, orgId) if err != nil { - return nil, err + return nil, ErrInternalServerError.Errorf("FindAll: %w", err) } return pd.filterDashboardsByPermissions(ctx, u, publicDashboards) @@ -311,11 +306,11 @@ func (pd *PublicDashboardServiceImpl) GetOrgIdByAccessToken(ctx context.Context, func (pd *PublicDashboardServiceImpl) Delete(ctx context.Context, orgId int64, uid string) error { affectedRows, err := pd.store.Delete(ctx, orgId, uid) if err != nil { - return err + return ErrInternalServerError.Errorf("Delete: failed to delete a public dashboard by orgId: %d and Uid: %s %w", orgId, uid, err) } if affectedRows == 0 { - return ErrPublicDashboardNotFound + return ErrPublicDashboardNotFound.Errorf("Delete: Public dashboard not found by orgId: %d and Uid: %s", orgId, uid) } return nil @@ -369,7 +364,7 @@ func (pd *PublicDashboardServiceImpl) filterDashboardsByPermissions(ctx context. hasAccess, err := pd.ac.Evaluate(ctx, u, accesscontrol.EvalPermission(dashboards.ActionDashboardsRead, dashboards.ScopeDashboardsProvider.GetResourceScopeUID(publicDashboards[i].DashboardUid))) // If original dashboard does not exist, the public dashboard is an orphan. We want to list it anyway if err != nil && !errors.Is(err, dashboards.ErrDashboardNotFound) { - return nil, err + return nil, ErrInternalServerError.Errorf("filterDashboardsByPermissions: error evaluating permissions %w", err) } // If user has access to the original dashboard or the dashboard does not exist, add the pubdash to the result diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 82dec54da3d..caafbad220e 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -258,9 +258,8 @@ func TestCreatePublicDashboard(t *testing.T) { } _, err := service.Create(context.Background(), SignedInUser, dto) - require.Error(t, err) - require.Equal(t, err, ErrPublicDashboardFailedGenerateAccessToken) + require.Equal(t, err, ErrInternalServerError.Errorf("failed to generate a unique accesssToken for public dashboard")) publicDashboardStore.AssertNotCalled(t, "Create") }) @@ -309,7 +308,8 @@ func TestCreatePublicDashboard(t *testing.T) { } _, err = service.Create(context.Background(), SignedInUser, dto) - assert.Equal(t, ErrPublicDashboardBadRequest, err) + require.Error(t, err) + assert.True(t, ErrBadRequest.Is(err)) }) } @@ -428,33 +428,33 @@ func TestDeletePublicDashboard(t *testing.T) { testCases := []struct { Name string AffectedRowsResp int64 - ErrResp error - ExpectedErr error + ExpectedErrResp error + StoreRespErr error }{ { Name: "Successfully deletes a public dashboards", AffectedRowsResp: 1, - ErrResp: nil, - ExpectedErr: nil, + ExpectedErrResp: nil, + StoreRespErr: nil, }, { Name: "Public dashboard not found", AffectedRowsResp: 0, - ErrResp: nil, - ExpectedErr: ErrPublicDashboardNotFound, + ExpectedErrResp: ErrPublicDashboardNotFound.Errorf("Delete: Public dashboard not found by orgId: 13 and Uid: uid"), + StoreRespErr: nil, }, { Name: "Database error", AffectedRowsResp: 0, - ErrResp: errors.New("db error!"), - ExpectedErr: errors.New("db error!"), + ExpectedErrResp: ErrInternalServerError.Errorf("Delete: failed to delete a public dashboard by orgId: 13 and Uid: uid db error!"), + StoreRespErr: errors.New("db error!"), }, } for _, tt := range testCases { t.Run(tt.Name, func(t *testing.T) { store := NewFakePublicDashboardStore(t) - store.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tt.AffectedRowsResp, tt.ExpectedErr) + store.On("Delete", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tt.AffectedRowsResp, tt.StoreRespErr) service := &PublicDashboardServiceImpl{ log: log.New("test.logger"), @@ -462,7 +462,12 @@ func TestDeletePublicDashboard(t *testing.T) { } err := service.Delete(context.Background(), 13, "uid") - assert.Equal(t, tt.ExpectedErr, err) + if tt.ExpectedErrResp != nil { + assert.Equal(t, tt.ExpectedErrResp.Error(), err.Error()) + assert.Equal(t, tt.ExpectedErrResp.Error(), err.Error()) + } else { + assert.NoError(t, err) + } }) } } @@ -833,7 +838,7 @@ func TestPublicDashboardServiceImpl_NewPublicDashboardUid(t *testing.T) { store.AssertNumberOfCalls(t, "Find", 1) } else { store.AssertNumberOfCalls(t, "Find", 3) - assert.True(t, errors.Is(err, ErrPublicDashboardFailedGenerateUniqueUid)) + assert.True(t, ErrInternalServerError.Is(err)) } }) } @@ -897,7 +902,7 @@ func TestPublicDashboardServiceImpl_NewPublicDashboardAccessToken(t *testing.T) store.AssertNumberOfCalls(t, "FindByAccessToken", 1) } else { store.AssertNumberOfCalls(t, "FindByAccessToken", 3) - assert.True(t, errors.Is(err, ErrPublicDashboardFailedGenerateAccessToken)) + assert.True(t, ErrInternalServerError.Is(err)) } }) } diff --git a/pkg/services/publicdashboards/validation/validation.go b/pkg/services/publicdashboards/validation/validation.go index d0b4936625e..727e2371580 100644 --- a/pkg/services/publicdashboards/validation/validation.go +++ b/pkg/services/publicdashboards/validation/validation.go @@ -1,15 +1,13 @@ package validation import ( - "fmt" - "github.com/grafana/grafana/pkg/models" . "github.com/grafana/grafana/pkg/services/publicdashboards/models" ) func ValidatePublicDashboard(dto *SavePublicDashboardDTO, dashboard *models.Dashboard) error { if hasTemplateVariables(dashboard) { - return ErrPublicDashboardHasTemplateVariables + return ErrPublicDashboardHasTemplateVariables.Errorf("ValidateSavePublicDashboard: public dashboard has template variables") } return nil @@ -23,11 +21,11 @@ func hasTemplateVariables(dashboard *models.Dashboard) bool { func ValidateQueryPublicDashboardRequest(req PublicDashboardQueryDTO) error { if req.IntervalMs < 0 { - return fmt.Errorf("intervalMS should be greater than 0") + return ErrInvalidInterval.Errorf("ValidateQueryPublicDashboardRequest: intervalMS should be greater than 0") } if req.MaxDataPoints < 0 { - return fmt.Errorf("maxDataPoints should be greater than 0") + return ErrInvalidMaxDataPoints.Errorf("ValidateQueryPublicDashboardRequest: maxDataPoints should be greater than 0") } return nil diff --git a/pkg/services/publicdashboards/validation/validation_test.go b/pkg/services/publicdashboards/validation/validation_test.go index 1273ca3191f..dbae8acc7db 100644 --- a/pkg/services/publicdashboards/validation/validation_test.go +++ b/pkg/services/publicdashboards/validation/validation_test.go @@ -25,7 +25,7 @@ func TestValidatePublicDashboard(t *testing.T) { dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: nil} err := ValidatePublicDashboard(dto, dashboard) - require.ErrorContains(t, err, ErrPublicDashboardHasTemplateVariables.Reason) + require.ErrorContains(t, err, ErrPublicDashboardHasTemplateVariables.Error()) }) t.Run("Returns no validation error when dashboard has no template variables", func(t *testing.T) {