From b2ef9a81f2591e78410e658eb4f8c3fa59763c15 Mon Sep 17 00:00:00 2001 From: Ezequiel Victorero Date: Thu, 26 Oct 2023 14:52:27 -0300 Subject: [PATCH] PublicDashboards: Chore refactor api test (#77091) --- pkg/services/publicdashboards/api/api_test.go | 123 +++++++++--------- 1 file changed, 64 insertions(+), 59 deletions(-) diff --git a/pkg/services/publicdashboards/api/api_test.go b/pkg/services/publicdashboards/api/api_test.go index 098b186120e..176bacd7aed 100644 --- a/pkg/services/publicdashboards/api/api_test.go +++ b/pkg/services/publicdashboards/api/api_test.go @@ -27,10 +27,6 @@ var userAdmin = &user.SignedInUser{UserID: 2, OrgID: 1, OrgRole: org.RoleAdmin, var userViewer = &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleViewer, Login: "testViewerUserRBAC", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsRead: {dashboards.ScopeDashboardsAll}}}} var anonymousUser = &user.SignedInUser{IsAnonymous: true} -type JsonErrResponse struct { - Error string `json:"error"` -} - func TestAPIFeatureFlag(t *testing.T) { testCases := []struct { Name string @@ -512,130 +508,139 @@ func TestApiCreatePublicDashboard(t *testing.T) { func TestAPIUpdatePublicDashboard(t *testing.T) { dashboardUid := "abc1234" publicDashboardUid := "1234asdfasdf" - - adminUser := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Login: "testEditorUser", Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {dashboards.ScopeDashboardsAll}}}} - - 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)}}}} - - 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, Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {fmt.Sprintf("dashboards:uid:%s", dashboardUid)}}}} + userEditorAnotherPublicDashboard := &user.SignedInUser{UserID: 4, OrgID: 1, OrgRole: org.RoleEditor, Permissions: map[int64]map[string][]string{1: {dashboards.ActionDashboardsPublicWrite: {"another-uid"}}}} testCases := []struct { Name string User *user.SignedInUser DashboardUid string PublicDashboardUid string - PublicDashboardRes *PublicDashboard - PublicDashboardErr error + Body string + ExpectedResponse *PublicDashboard + ExpectedError interface{} ExpectedHttpResponse int ShouldCallService bool }{ { - Name: "Invalid dashboardUid", - User: adminUser, - DashboardUid: "", - PublicDashboardUid: "", - PublicDashboardRes: nil, - PublicDashboardErr: ErrPublicDashboardIdentifierNotSet.Errorf(""), - ExpectedHttpResponse: http.StatusNotFound, + Name: "Invalid dashboard uid bad request error", + User: userAdmin, + DashboardUid: ".", + PublicDashboardUid: publicDashboardUid, + Body: fmt.Sprintf(`{ "uid": "%s"}`, publicDashboardUid), + ExpectedResponse: nil, + ExpectedError: ErrInvalidUid.Errorf(""), + ExpectedHttpResponse: http.StatusBadRequest, ShouldCallService: false, }, { - Name: "Invalid public dashboard uid", - User: adminUser, + Name: "Invalid public dashboard uid bad request error", + User: userAdmin, DashboardUid: dashboardUid, - PublicDashboardUid: "", - PublicDashboardRes: nil, - PublicDashboardErr: ErrPublicDashboardNotFound.Errorf(""), - ExpectedHttpResponse: http.StatusNotFound, + PublicDashboardUid: ".", + Body: fmt.Sprintf(`{ "uid": "%s"}`, publicDashboardUid), + ExpectedResponse: nil, + ExpectedError: ErrInvalidUid.Errorf(""), + ExpectedHttpResponse: http.StatusBadRequest, ShouldCallService: false, }, { - Name: "Service Error", - User: adminUser, + Name: "Dashboard not found error", + User: userAdmin, DashboardUid: dashboardUid, PublicDashboardUid: publicDashboardUid, - PublicDashboardRes: nil, - PublicDashboardErr: ErrDashboardNotFound.Errorf(""), + Body: fmt.Sprintf(`{ "uid": "%s"}`, publicDashboardUid), + ExpectedResponse: nil, + ExpectedError: ErrDashboardNotFound.Errorf(""), ExpectedHttpResponse: http.StatusNotFound, ShouldCallService: true, }, { Name: "Success", - User: adminUser, + User: userAdmin, DashboardUid: dashboardUid, PublicDashboardUid: publicDashboardUid, - PublicDashboardRes: &PublicDashboard{Uid: "success"}, - PublicDashboardErr: nil, + Body: fmt.Sprintf(`{ "uid": "%s"}`, publicDashboardUid), + ExpectedResponse: &PublicDashboard{Uid: "success"}, + ExpectedError: nil, ExpectedHttpResponse: http.StatusOK, ShouldCallService: true, }, - - // permissions { - Name: "User can update this public dashboard", + Name: "Invalid payload bad request error", + User: userAdmin, + DashboardUid: dashboardUid, + PublicDashboardUid: publicDashboardUid, + Body: `{nonvalidjson,`, + ExpectedResponse: nil, + ExpectedError: ErrBadRequest.Errorf(""), + ExpectedHttpResponse: http.StatusBadRequest, + ShouldCallService: false, + }, + { + Name: "User has permissions to update this public dashboard", User: userEditorPublicDashboard, DashboardUid: dashboardUid, PublicDashboardUid: publicDashboardUid, - PublicDashboardRes: &PublicDashboard{Uid: "success"}, - PublicDashboardErr: nil, + Body: fmt.Sprintf(`{ "uid": "%s"}`, publicDashboardUid), + ExpectedResponse: &PublicDashboard{Uid: "success"}, + ExpectedError: nil, ExpectedHttpResponse: http.StatusOK, ShouldCallService: true, }, { - Name: "User has permissions on another dashboard", + Name: "User has permissions to update another dashboard but not the requested one", User: userEditorAnotherPublicDashboard, + DashboardUid: dashboardUid, PublicDashboardUid: publicDashboardUid, + Body: fmt.Sprintf(`{ "uid": "%s"}`, publicDashboardUid), ExpectedHttpResponse: http.StatusForbidden, ShouldCallService: false, }, { - Name: "Viewer cannot update any dashboard", + Name: "User Viewer cannot update any dashboard", User: userViewer, + DashboardUid: dashboardUid, PublicDashboardUid: publicDashboardUid, + Body: fmt.Sprintf(`{ "uid": "%s"}`, publicDashboardUid), ExpectedHttpResponse: http.StatusForbidden, ShouldCallService: false, }, } for _, test := range testCases { + cfg := setting.NewCfg() + features := featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards) + t.Run(test.Name, func(t *testing.T) { service := publicdashboards.NewFakePublicDashboardService(t) if test.ShouldCallService { service.On("Update", mock.Anything, mock.Anything, mock.Anything). - Return(test.PublicDashboardRes, test.PublicDashboardErr) + Return(test.ExpectedResponse, test.ExpectedError) } - cfg := setting.NewCfg() - features := featuremgmt.WithFeatures(featuremgmt.FlagPublicDashboards) testServer := setupTestServer(t, cfg, features, service, nil, test.User) url := fmt.Sprintf("/api/dashboards/uid/%s/public-dashboards/%s", test.DashboardUid, test.PublicDashboardUid) - body := strings.NewReader(fmt.Sprintf(`{ "uid": "%s"}`, test.PublicDashboardUid)) + body := strings.NewReader(test.Body) response := callAPI(testServer, http.MethodPatch, url, body, t) assert.Equal(t, test.ExpectedHttpResponse, response.Code) - // check whether service called - if !test.ShouldCallService { - service.AssertNotCalled(t, "Update") - } - - fmt.Println(response.Body.String()) - - // check response - if response.Code == http.StatusOK { - val, err := json.Marshal(test.PublicDashboardRes) + // check response when expected response is 200 + if test.ExpectedHttpResponse == http.StatusOK { + val, err := json.Marshal(test.ExpectedResponse) require.NoError(t, err) assert.Equal(t, string(val), response.Body.String()) + } - // verify 4XXs except 403 && 404 - } else if test.ExpectedHttpResponse > 200 && - test.ExpectedHttpResponse != 403 && - test.ExpectedHttpResponse != 404 { - var errResp JsonErrResponse + // forbidden status is returned by middleware and does not have the format of the errutil.PublicError + if test.ExpectedHttpResponse != http.StatusOK && test.ExpectedHttpResponse != http.StatusForbidden { + var errResp errutil.PublicError err := json.Unmarshal(response.Body.Bytes(), &errResp) require.NoError(t, err) - assert.Equal(t, test.PublicDashboardErr.Error(), errResp.Error) + assert.Equal(t, test.ExpectedHttpResponse, errResp.StatusCode) + assert.Equal(t, test.ExpectedError.(errutil.Error).MessageID, errResp.MessageID) } }) }