From 9890ff7c92a5666b64bf0b14ca380533253e8ced Mon Sep 17 00:00:00 2001 From: Juan Cabanas Date: Mon, 29 May 2023 12:59:29 -0300 Subject: [PATCH] PublicDashboards: Audit table redesign (#68137) --- .../dashboard-public-create.spec.ts | 2 +- .../src/selectors/pages.ts | 1 + pkg/services/publicdashboards/api/api.go | 26 +- pkg/services/publicdashboards/api/api_test.go | 2 +- .../publicdashboards/api/query_test.go | 7 +- .../publicdashboards/database/database.go | 2 +- .../database/database_test.go | 8 +- .../publicdashboards/models/models.go | 40 ++- .../public_dashboard_service_mock.go | 10 +- .../public_dashboard_store_mock.go | 3 +- .../publicdashboards/service/query_test.go | 20 +- .../publicdashboards/service/service.go | 136 +++++--- .../publicdashboards/service/service_test.go | 311 +++++++++++++++--- .../validation/validation_test.go | 4 +- .../dashboard/api/publicDashboardApi.ts | 23 +- .../SharePublicDashboard.test.tsx | 2 +- .../PublicDashboardListPage.tsx | 8 +- .../DeletePublicDashboardButton.tsx | 2 +- .../PublicDashboardListTable.test.tsx | 74 +---- .../PublicDashboardListTable.tsx | 278 +++++++++------- 20 files changed, 606 insertions(+), 353 deletions(-) diff --git a/e2e/dashboards-suite/dashboard-public-create.spec.ts b/e2e/dashboards-suite/dashboard-public-create.spec.ts index 5b0bb7b159d..8c0e2b2b407 100644 --- a/e2e/dashboards-suite/dashboard-public-create.spec.ts +++ b/e2e/dashboards-suite/dashboard-public-create.spec.ts @@ -141,7 +141,7 @@ e2e.scenario({ .then((text) => e2e().wrap(text).as('url')); // Save public dashboard - e2e().intercept('PUT', '/api/dashboards/uid/ZqZnVvFZz/public-dashboards/*').as('update'); + e2e().intercept('PATCH', '/api/dashboards/uid/ZqZnVvFZz/public-dashboards/*').as('update'); // Switch off enabling toggle e2e.pages.ShareDashboardModal.PublicDashboard.PauseSwitch().should('be.enabled').click({ force: true }); e2e().wait('@update'); diff --git a/packages/grafana-e2e-selectors/src/selectors/pages.ts b/packages/grafana-e2e-selectors/src/selectors/pages.ts index ca4ab410469..c0f2f36bbf5 100644 --- a/packages/grafana-e2e-selectors/src/selectors/pages.ts +++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts @@ -286,6 +286,7 @@ export const Pages = { linkButton: 'public-dashboard-link-button', configButton: 'public-dashboard-configuration-button', trashcanButton: 'public-dashboard-remove-button', + pauseSwitch: 'data-testid public dashboard pause switch', }, }, UserListPage: { diff --git a/pkg/services/publicdashboards/api/api.go b/pkg/services/publicdashboards/api/api.go index 4e128b692d4..31150e7eb91 100644 --- a/pkg/services/publicdashboards/api/api.go +++ b/pkg/services/publicdashboards/api/api.go @@ -77,7 +77,7 @@ func (api *Api) RegisterAPIEndpoints() { routing.Wrap(api.CreatePublicDashboard)) // Update Public Dashboard - api.RouteRegister.Put("/api/dashboards/uid/:dashboardUid/public-dashboards/:uid", + api.RouteRegister.Patch("/api/dashboards/uid/:dashboardUid/public-dashboards/:uid", auth(accesscontrol.EvalPermission(dashboards.ActionDashboardsPublicWrite, uidScope)), routing.Wrap(api.UpdatePublicDashboard)) @@ -127,22 +127,21 @@ func (api *Api) CreatePublicDashboard(c *contextmodel.ReqContext) response.Respo return response.Err(ErrInvalidUid.Errorf("CreatePublicDashboard: invalid Uid %s", dashboardUid)) } - pd := &PublicDashboard{} - if err := web.Bind(c.Req, pd); err != nil { + pdDTO := &PublicDashboardDTO{} + if err := web.Bind(c.Req, pdDTO); err != nil { return response.Err(ErrBadRequest.Errorf("CreatePublicDashboard: bad request data %v", err)) } // Always set the orgID and userID from the session - pd.OrgId = c.OrgID - dto := SavePublicDashboardDTO{ + pdDTO.OrgId = c.OrgID + dto := &SavePublicDashboardDTO{ UserId: c.UserID, - OrgId: c.OrgID, DashboardUid: dashboardUid, - PublicDashboard: pd, + PublicDashboard: pdDTO, } //Create the public dashboard - pd, err := api.PublicDashboardService.Create(c.Req.Context(), c.SignedInUser, &dto) + pd, err := api.PublicDashboardService.Create(c.Req.Context(), c.SignedInUser, dto) if err != nil { return response.Err(err) } @@ -164,19 +163,18 @@ func (api *Api) UpdatePublicDashboard(c *contextmodel.ReqContext) response.Respo return response.Err(ErrInvalidUid.Errorf("UpdatePublicDashboard: invalid Uid %s", uid)) } - pd := &PublicDashboard{} - if err := web.Bind(c.Req, pd); err != nil { + pdDTO := &PublicDashboardDTO{} + if err := web.Bind(c.Req, pdDTO); err != nil { return response.Err(ErrBadRequest.Errorf("UpdatePublicDashboard: bad request data %v", err)) } // Always set the orgID and userID from the session - pd.OrgId = c.OrgID - pd.Uid = uid + pdDTO.OrgId = c.OrgID + pdDTO.Uid = uid dto := SavePublicDashboardDTO{ UserId: c.UserID, - OrgId: c.OrgID, DashboardUid: dashboardUid, - PublicDashboard: pd, + PublicDashboard: pdDTO, } // Update the public dashboard diff --git a/pkg/services/publicdashboards/api/api_test.go b/pkg/services/publicdashboards/api/api_test.go index a377f8d4de8..a4713c9eb5c 100644 --- a/pkg/services/publicdashboards/api/api_test.go +++ b/pkg/services/publicdashboards/api/api_test.go @@ -572,7 +572,7 @@ func TestAPIUpdatePublicDashboard(t *testing.T) { url := fmt.Sprintf("/api/dashboards/uid/%s/public-dashboards/%s", test.DashboardUid, test.PublicDashboardUid) body := strings.NewReader(fmt.Sprintf(`{ "uid": "%s"}`, test.PublicDashboardUid)) - response := callAPI(testServer, http.MethodPut, url, body, t) + response := callAPI(testServer, http.MethodPatch, url, body, t) assert.Equal(t, test.ExpectedHttpResponse, response.Code) // check whether service called diff --git a/pkg/services/publicdashboards/api/query_test.go b/pkg/services/publicdashboards/api/query_test.go index 26a5db68ee9..5a04831607d 100644 --- a/pkg/services/publicdashboards/api/query_test.go +++ b/pkg/services/publicdashboards/api/query_test.go @@ -310,11 +310,12 @@ func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T) require.NoError(t, err) // Create public dashboard + isEnabled := true savePubDashboardCmd := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, + OrgId: dashboard.OrgID, }, } diff --git a/pkg/services/publicdashboards/database/database.go b/pkg/services/publicdashboards/database/database.go index bdee4da07ea..6882ef2058e 100644 --- a/pkg/services/publicdashboards/database/database.go +++ b/pkg/services/publicdashboards/database/database.go @@ -42,7 +42,7 @@ func (d *PublicDashboardStoreImpl) FindAll(ctx context.Context, orgId int64) ([] "dashboard_public.uid, dashboard_public.access_token, dashboard.uid as dashboard_uid, dashboard_public.is_enabled, dashboard.title"). Join("LEFT", "dashboard", "dashboard.uid = dashboard_public.dashboard_uid AND dashboard.org_id = dashboard_public.org_id"). Where("dashboard_public.org_id = ?", orgId). - OrderBy(" is_enabled DESC, dashboard.title IS NULL, dashboard.title ASC") + OrderBy(" dashboard.title IS NULL, dashboard.title ASC") err := sess.Find(&resp) return err diff --git a/pkg/services/publicdashboards/database/database_test.go b/pkg/services/publicdashboards/database/database_test.go index f918570ceae..9c66558e85f 100644 --- a/pkg/services/publicdashboards/database/database_test.go +++ b/pkg/services/publicdashboards/database/database_test.go @@ -42,14 +42,14 @@ func TestIntegrationListPublicDashboard(t *testing.T) { var orgId int64 = 1 - aDash := insertTestDashboard(t, dashboardStore, "a", orgId, 0, true) bDash := insertTestDashboard(t, dashboardStore, "b", orgId, 0, true) + aDash := insertTestDashboard(t, dashboardStore, "a", orgId, 0, true) cDash := insertTestDashboard(t, dashboardStore, "c", orgId, 0, true) // these are in order of how they should be returned from ListPUblicDashboards - a := insertPublicDashboard(t, publicdashboardStore, bDash.UID, orgId, true, PublicShareType) - b := insertPublicDashboard(t, publicdashboardStore, cDash.UID, orgId, true, PublicShareType) - c := insertPublicDashboard(t, publicdashboardStore, aDash.UID, orgId, false, PublicShareType) + a := insertPublicDashboard(t, publicdashboardStore, aDash.UID, orgId, false, PublicShareType) + b := insertPublicDashboard(t, publicdashboardStore, bDash.UID, orgId, true, PublicShareType) + c := insertPublicDashboard(t, publicdashboardStore, cDash.UID, orgId, true, PublicShareType) // this is case that can happen as of now, however, postgres and mysql sort // null in the exact opposite fashion and there is no shared syntax to sort diff --git a/pkg/services/publicdashboards/models/models.go b/pkg/services/publicdashboards/models/models.go index db2e7b1d56b..47b32e6ded0 100644 --- a/pkg/services/publicdashboards/models/models.go +++ b/pkg/services/publicdashboards/models/models.go @@ -40,20 +40,39 @@ var ( type ShareType string type PublicDashboard struct { - Uid string `json:"uid" xorm:"pk uid"` - DashboardUid string `json:"dashboardUid" xorm:"dashboard_uid"` - OrgId int64 `json:"-" xorm:"org_id"` // Don't ever marshal orgId to Json + Uid string `json:"uid" xorm:"pk uid"` + DashboardUid string `json:"dashboardUid" xorm:"dashboard_uid"` + OrgId int64 `json:"-" xorm:"org_id"` // Don't ever marshal orgId to Json + AccessToken string `json:"accessToken" xorm:"access_token"` + CreatedBy int64 `json:"createdBy" xorm:"created_by"` + UpdatedBy int64 `json:"updatedBy" xorm:"updated_by"` + CreatedAt time.Time `json:"createdAt" xorm:"created_at"` + UpdatedAt time.Time `json:"updatedAt" xorm:"updated_at"` + //config fields TimeSettings *TimeSettings `json:"timeSettings" xorm:"time_settings"` + TimeSelectionEnabled bool `json:"timeSelectionEnabled" xorm:"time_selection_enabled"` IsEnabled bool `json:"isEnabled" xorm:"is_enabled"` - AccessToken string `json:"accessToken" xorm:"access_token"` AnnotationsEnabled bool `json:"annotationsEnabled" xorm:"annotations_enabled"` - TimeSelectionEnabled bool `json:"timeSelectionEnabled" xorm:"time_selection_enabled"` Share ShareType `json:"share" xorm:"share"` Recipients []EmailDTO `json:"recipients,omitempty" xorm:"-"` - CreatedBy int64 `json:"createdBy" xorm:"created_by"` - UpdatedBy int64 `json:"updatedBy" xorm:"updated_by"` - CreatedAt time.Time `json:"createdAt" xorm:"created_at"` - UpdatedAt time.Time `json:"updatedAt" xorm:"updated_at"` +} + +type PublicDashboardDTO struct { + Uid string `json:"uid"` + DashboardUid string `json:"dashboardUid"` + OrgId int64 `json:"-"` // Don't ever marshal orgId to Json + AccessToken string `json:"accessToken"` + CreatedBy int64 `json:"createdBy"` + UpdatedBy int64 `json:"updatedBy"` + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` + //config fields + TimeSettings *TimeSettings `json:"timeSettings"` + TimeSelectionEnabled *bool `json:"timeSelectionEnabled"` + IsEnabled *bool `json:"isEnabled"` + AnnotationsEnabled *bool `json:"annotationsEnabled"` + Share ShareType `json:"share"` + Recipients []EmailDTO `json:"recipients,omitempty"` } type EmailDTO struct { @@ -130,9 +149,8 @@ func (pd PublicDashboard) BuildTimeSettings(dashboard *dashboards.Dashboard, req // DTO for transforming user input in the api type SavePublicDashboardDTO struct { DashboardUid string - OrgId int64 UserId int64 - PublicDashboard *PublicDashboard + PublicDashboard *PublicDashboardDTO } type PublicDashboardQueryDTO struct { diff --git a/pkg/services/publicdashboards/public_dashboard_service_mock.go b/pkg/services/publicdashboards/public_dashboard_service_mock.go index 886cd340ec4..06560513433 100644 --- a/pkg/services/publicdashboards/public_dashboard_service_mock.go +++ b/pkg/services/publicdashboards/public_dashboard_service_mock.go @@ -360,13 +360,13 @@ func (_m *FakePublicDashboardService) GetOrgIdByAccessToken(ctx context.Context, return r0, r1 } -// GetQueryDataResponse provides a mock function with given fields: ctx, skipCache, reqDTO, panelId, accessToken -func (_m *FakePublicDashboardService) GetQueryDataResponse(ctx context.Context, skipCache bool, reqDTO models.PublicDashboardQueryDTO, panelId int64, accessToken string) (*backend.QueryDataResponse, error) { - ret := _m.Called(ctx, skipCache, reqDTO, panelId, accessToken) +// GetQueryDataResponse provides a mock function with given fields: ctx, skipDSCache, reqDTO, panelId, accessToken +func (_m *FakePublicDashboardService) GetQueryDataResponse(ctx context.Context, skipDSCache bool, reqDTO models.PublicDashboardQueryDTO, panelId int64, accessToken string) (*backend.QueryDataResponse, error) { + ret := _m.Called(ctx, skipDSCache, reqDTO, panelId, accessToken) var r0 *backend.QueryDataResponse if rf, ok := ret.Get(0).(func(context.Context, bool, models.PublicDashboardQueryDTO, int64, string) *backend.QueryDataResponse); ok { - r0 = rf(ctx, skipCache, reqDTO, panelId, accessToken) + r0 = rf(ctx, skipDSCache, reqDTO, panelId, accessToken) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*backend.QueryDataResponse) @@ -375,7 +375,7 @@ func (_m *FakePublicDashboardService) GetQueryDataResponse(ctx context.Context, var r1 error if rf, ok := ret.Get(1).(func(context.Context, bool, models.PublicDashboardQueryDTO, int64, string) error); ok { - r1 = rf(ctx, skipCache, reqDTO, panelId, accessToken) + r1 = rf(ctx, skipDSCache, reqDTO, panelId, accessToken) } else { r1 = ret.Error(1) } diff --git a/pkg/services/publicdashboards/public_dashboard_store_mock.go b/pkg/services/publicdashboards/public_dashboard_store_mock.go index 564a6cf755a..7a0b583e231 100644 --- a/pkg/services/publicdashboards/public_dashboard_store_mock.go +++ b/pkg/services/publicdashboards/public_dashboard_store_mock.go @@ -6,8 +6,9 @@ import ( context "context" dashboards "github.com/grafana/grafana/pkg/services/dashboards" - models "github.com/grafana/grafana/pkg/services/publicdashboards/models" mock "github.com/stretchr/testify/mock" + + models "github.com/grafana/grafana/pkg/services/publicdashboards/models" ) // FakePublicDashboardStore is an autogenerated mock type for the Store type diff --git a/pkg/services/publicdashboards/service/query_test.go b/pkg/services/publicdashboards/service/query_test.go index a30186e8ec4..1e19916f258 100644 --- a/pkg/services/publicdashboards/service/query_test.go +++ b/pkg/services/publicdashboards/service/query_test.go @@ -688,14 +688,14 @@ func TestGetQueryDataResponse(t *testing.T) { }} dashboard := insertTestDashboard(t, dashboardStore, "testDashWithHiddenQuery", 1, 0, true, []map[string]interface{}{}, customPanels) + isEnabled := true dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, DashboardUid: "NOTTHESAME", - OrgId: 9999999, + OrgId: dashboard.OrgID, TimeSettings: timeSettings, }, } @@ -1199,11 +1199,11 @@ func TestBuildMetricRequest(t *testing.T) { MaxDataPoints: int64(200), } + isEnabled := true dto := &SavePublicDashboardDTO{ DashboardUid: publicDashboard.UID, - OrgId: publicDashboard.OrgID, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, DashboardUid: "NOTTHESAME", OrgId: 9999999, TimeSettings: timeSettings, @@ -1213,11 +1213,11 @@ func TestBuildMetricRequest(t *testing.T) { publicDashboardPD, err := service.Create(context.Background(), SignedInUser, dto) require.NoError(t, err) + isEnabled = false nonPublicDto := &SavePublicDashboardDTO{ DashboardUid: nonPublicDashboard.UID, - OrgId: nonPublicDashboard.OrgID, - PublicDashboard: &PublicDashboard{ - IsEnabled: false, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, DashboardUid: "NOTTHESAME", OrgId: 9999999, TimeSettings: defaultPubdashTimeSettings, diff --git a/pkg/services/publicdashboards/service/service.go b/pkg/services/publicdashboards/service/service.go index 13f15941748..1e823b4c056 100644 --- a/pkg/services/publicdashboards/service/service.go +++ b/pkg/services/publicdashboards/service/service.go @@ -162,50 +162,24 @@ func (pd *PublicDashboardServiceImpl) Create(ctx context.Context, u *user.Signed return nil, ErrDashboardIsPublic.Errorf("Create: public dashboard for dashboard %s already exists", dto.DashboardUid) } - // set default value for time settings - if dto.PublicDashboard.TimeSettings == nil { - dto.PublicDashboard.TimeSettings = &TimeSettings{} - } - - if dto.PublicDashboard.Share == "" { - dto.PublicDashboard.Share = PublicShareType - } - - uid, err := pd.NewPublicDashboardUid(ctx) - if err != nil { - return nil, err - } - - accessToken, err := pd.NewPublicDashboardAccessToken(ctx) + publicDashboard, err := pd.newCreatePublicDashboard(ctx, dto) if err != nil { return nil, err } cmd := SavePublicDashboardCommand{ - PublicDashboard: PublicDashboard{ - Uid: uid, - DashboardUid: dto.DashboardUid, - OrgId: dto.OrgId, - IsEnabled: dto.PublicDashboard.IsEnabled, - AnnotationsEnabled: dto.PublicDashboard.AnnotationsEnabled, - TimeSelectionEnabled: dto.PublicDashboard.TimeSelectionEnabled, - TimeSettings: dto.PublicDashboard.TimeSettings, - Share: dto.PublicDashboard.Share, - CreatedBy: dto.UserId, - CreatedAt: time.Now(), - AccessToken: accessToken, - }, + PublicDashboard: *publicDashboard, } affectedRows, err := pd.store.Create(ctx, cmd) if err != nil { - return nil, ErrInternalServerError.Errorf("Create: failed to create the public dashboard with Uid %s: %w", uid, err) + return nil, ErrInternalServerError.Errorf("Create: failed to create the public dashboard with Uid %s: %w", publicDashboard.Uid, err) } else if affectedRows == 0 { - return nil, ErrInternalServerError.Errorf("Create: failed to create a database entry for public dashboard with Uid %s. 0 rows changed, no error reported.", uid) + return nil, ErrInternalServerError.Errorf("Create: failed to create a database entry for public dashboard with Uid %s. 0 rows changed, no error reported.", publicDashboard.Uid) } //Get latest public dashboard to return - newPubdash, err := pd.store.Find(ctx, uid) + newPubdash, err := pd.store.Find(ctx, publicDashboard.Uid) if err != nil { return nil, ErrInternalServerError.Errorf("Create: failed to find the public dashboard: %w", err) } @@ -241,27 +215,11 @@ func (pd *PublicDashboardServiceImpl) Update(ctx context.Context, u *user.Signed return nil, ErrPublicDashboardNotFound.Errorf("Update: public dashboard not found by uid: %s", dto.PublicDashboard.Uid) } - // set default value for time settings - if dto.PublicDashboard.TimeSettings == nil { - dto.PublicDashboard.TimeSettings = &TimeSettings{} - } - - if dto.PublicDashboard.Share == "" { - dto.PublicDashboard.Share = existingPubdash.Share - } + publicDashboard := newUpdatePublicDashboard(dto, existingPubdash) // set values to update cmd := SavePublicDashboardCommand{ - PublicDashboard: PublicDashboard{ - Uid: existingPubdash.Uid, - IsEnabled: dto.PublicDashboard.IsEnabled, - AnnotationsEnabled: dto.PublicDashboard.AnnotationsEnabled, - TimeSelectionEnabled: dto.PublicDashboard.TimeSelectionEnabled, - TimeSettings: dto.PublicDashboard.TimeSettings, - Share: dto.PublicDashboard.Share, - UpdatedBy: dto.UserId, - UpdatedAt: time.Now(), - }, + PublicDashboard: *publicDashboard, } // persist @@ -449,3 +407,83 @@ func GenerateAccessToken() (string, error) { } return fmt.Sprintf("%x", token[:]), nil } + +func (pd *PublicDashboardServiceImpl) newCreatePublicDashboard(ctx context.Context, dto *SavePublicDashboardDTO) (*PublicDashboard, error) { + uid, err := pd.NewPublicDashboardUid(ctx) + if err != nil { + return nil, err + } + + accessToken, err := pd.NewPublicDashboardAccessToken(ctx) + if err != nil { + return nil, err + } + + isEnabled := returnValueOrDefault(dto.PublicDashboard.IsEnabled, false) + annotationsEnabled := returnValueOrDefault(dto.PublicDashboard.AnnotationsEnabled, false) + timeSelectionEnabled := returnValueOrDefault(dto.PublicDashboard.TimeSelectionEnabled, false) + + timeSettings := dto.PublicDashboard.TimeSettings + if dto.PublicDashboard.TimeSettings == nil { + timeSettings = &TimeSettings{} + } + + share := dto.PublicDashboard.Share + if dto.PublicDashboard.Share == "" { + share = PublicShareType + } + + return &PublicDashboard{ + Uid: uid, + DashboardUid: dto.DashboardUid, + OrgId: dto.PublicDashboard.OrgId, + IsEnabled: isEnabled, + AnnotationsEnabled: annotationsEnabled, + TimeSelectionEnabled: timeSelectionEnabled, + TimeSettings: timeSettings, + Share: share, + CreatedBy: dto.UserId, + CreatedAt: time.Now(), + AccessToken: accessToken, + }, nil +} + +func newUpdatePublicDashboard(dto *SavePublicDashboardDTO, pd *PublicDashboard) *PublicDashboard { + pubdashDTO := dto.PublicDashboard + timeSelectionEnabled := returnValueOrDefault(pubdashDTO.TimeSelectionEnabled, pd.TimeSelectionEnabled) + isEnabled := returnValueOrDefault(pubdashDTO.IsEnabled, pd.IsEnabled) + annotationsEnabled := returnValueOrDefault(pubdashDTO.AnnotationsEnabled, pd.AnnotationsEnabled) + + timeSettings := pubdashDTO.TimeSettings + if pubdashDTO.TimeSettings == nil { + if pd.TimeSettings == nil { + timeSettings = &TimeSettings{} + } else { + timeSettings = pd.TimeSettings + } + } + + share := pubdashDTO.Share + if pubdashDTO.Share == "" { + share = pd.Share + } + + return &PublicDashboard{ + Uid: pd.Uid, + IsEnabled: isEnabled, + AnnotationsEnabled: annotationsEnabled, + TimeSelectionEnabled: timeSelectionEnabled, + TimeSettings: timeSettings, + Share: share, + UpdatedBy: dto.UserId, + UpdatedAt: time.Now(), + } +} + +func returnValueOrDefault(value *bool, defaultValue bool) bool { + if value != nil { + return *value + } + + return defaultValue +} diff --git a/pkg/services/publicdashboards/service/service_test.go b/pkg/services/publicdashboards/service/service_test.go index 7489d0b28d4..bd92a6d3f12 100644 --- a/pkg/services/publicdashboards/service/service_test.go +++ b/pkg/services/publicdashboards/service/service_test.go @@ -205,17 +205,18 @@ func TestCreatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled, annotationsEnabled, timeSelectionEnabled := true, false, true + dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, - AnnotationsEnabled: false, - TimeSelectionEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, + AnnotationsEnabled: &annotationsEnabled, + TimeSelectionEnabled: &timeSelectionEnabled, Share: EmailShareType, DashboardUid: "NOTTHESAME", - OrgId: 9999999, + OrgId: dashboard.OrgID, TimeSettings: timeSettings, }, } @@ -230,10 +231,10 @@ func TestCreatePublicDashboard(t *testing.T) { assert.Equal(t, dashboard.UID, pubdash.DashboardUid) assert.Equal(t, dashboard.OrgID, pubdash.OrgId) assert.Equal(t, dto.UserId, pubdash.CreatedBy) - assert.Equal(t, dto.PublicDashboard.AnnotationsEnabled, pubdash.AnnotationsEnabled) - assert.Equal(t, dto.PublicDashboard.TimeSelectionEnabled, pubdash.TimeSelectionEnabled) + assert.Equal(t, *dto.PublicDashboard.AnnotationsEnabled, pubdash.AnnotationsEnabled) + assert.Equal(t, *dto.PublicDashboard.TimeSelectionEnabled, pubdash.TimeSelectionEnabled) // ExistsEnabledByDashboardUid set by parameters - assert.Equal(t, dto.PublicDashboard.IsEnabled, pubdash.IsEnabled) + assert.Equal(t, *dto.PublicDashboard.IsEnabled, pubdash.IsEnabled) // CreatedAt set to non-zero time assert.NotEqual(t, &time.Time{}, pubdash.CreatedAt) // Time settings set by db @@ -244,6 +245,81 @@ func TestCreatePublicDashboard(t *testing.T) { require.NoError(t, err, "expected a valid UUID, got %s", pubdash.AccessToken) }) + trueBooleanField := true + + testCases := []struct { + Name string + IsEnabled *bool + TimeSelectionEnabled *bool + AnnotationsEnabled *bool + }{ + { + Name: "isEnabled", + IsEnabled: nil, + TimeSelectionEnabled: &trueBooleanField, + AnnotationsEnabled: &trueBooleanField, + }, + { + Name: "timeSelectionEnabled", + IsEnabled: &trueBooleanField, + TimeSelectionEnabled: nil, + AnnotationsEnabled: &trueBooleanField, + }, + { + Name: "annotationsEnabled", + IsEnabled: &trueBooleanField, + TimeSelectionEnabled: &trueBooleanField, + AnnotationsEnabled: nil, + }, + { + Name: "isEnabled, timeSelectionEnabled and annotationsEnabled", + IsEnabled: nil, + TimeSelectionEnabled: nil, + AnnotationsEnabled: nil, + }, + } + + for _, tt := range testCases { + t.Run(fmt.Sprintf("Create public dashboard with %s null boolean fields stores them as false", tt.Name), func(t *testing.T) { + sqlStore := db.InitTestDB(t) + quotaService := quotatest.New(false, nil) + dashboardStore, err := dashboardsDB.ProvideDashboardStore(sqlStore, sqlStore.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, sqlStore.Cfg), quotaService) + require.NoError(t, err) + publicdashboardStore := database.ProvideStore(sqlStore) + dashboard := insertTestDashboard(t, dashboardStore, "testDashie", 1, 0, true, []map[string]interface{}{}, nil) + serviceWrapper := ProvideServiceWrapper(publicdashboardStore) + + service := &PublicDashboardServiceImpl{ + log: log.New("test.logger"), + store: publicdashboardStore, + serviceWrapper: serviceWrapper, + } + + dto := &SavePublicDashboardDTO{ + DashboardUid: dashboard.UID, + UserId: 7, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: tt.IsEnabled, + TimeSelectionEnabled: tt.TimeSelectionEnabled, + AnnotationsEnabled: tt.AnnotationsEnabled, + Share: PublicShareType, + DashboardUid: "NOTTHESAME", + OrgId: dashboard.OrgID, + TimeSettings: timeSettings, + }, + } + + _, err = service.Create(context.Background(), SignedInUser, dto) + require.NoError(t, err) + pubdash, err := service.FindByDashboardUid(context.Background(), dashboard.OrgID, dashboard.UID) + require.NoError(t, err) + + assertFalseIfNull(t, pubdash.IsEnabled, dto.PublicDashboard.IsEnabled) + assertFalseIfNull(t, pubdash.TimeSelectionEnabled, dto.PublicDashboard.TimeSelectionEnabled) + assertFalseIfNull(t, pubdash.AnnotationsEnabled, dto.PublicDashboard.AnnotationsEnabled) + }) + } + t.Run("Validate pubdash has default time setting value", func(t *testing.T) { sqlStore := db.InitTestDB(t) quotaService := quotatest.New(false, nil) @@ -259,14 +335,14 @@ func TestCreatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled := true dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, DashboardUid: "NOTTHESAME", - OrgId: 9999999, + OrgId: dashboard.OrgID, }, } @@ -294,14 +370,14 @@ func TestCreatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled := true dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, DashboardUid: "NOTTHESAME", - OrgId: 9999999, + OrgId: dashboard.OrgID, }, } @@ -321,7 +397,7 @@ func TestCreatePublicDashboard(t *testing.T) { IsEnabled: true, AnnotationsEnabled: false, DashboardUid: "NOTTHESAME", - OrgId: 9999999, + OrgId: dashboard.OrgID, TimeSettings: timeSettings, } @@ -339,14 +415,14 @@ func TestCreatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled := true dto := &SavePublicDashboardDTO{ DashboardUid: "an-id", - OrgId: 8, UserId: 7, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, DashboardUid: "NOTTHESAME", - OrgId: 9999999, + OrgId: dashboard.OrgID, }, } @@ -376,13 +452,13 @@ func TestCreatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled, annotationsEnabled := true, false dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - AnnotationsEnabled: false, - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + AnnotationsEnabled: &annotationsEnabled, + IsEnabled: &isEnabled, TimeSettings: timeSettings, }, } @@ -408,14 +484,14 @@ func TestCreatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled := true dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, DashboardUid: "NOTTHESAME", - OrgId: 9999999, + OrgId: dashboard.OrgID, }, } @@ -429,6 +505,14 @@ func TestCreatePublicDashboard(t *testing.T) { }) } +func assertFalseIfNull(t *testing.T, expectedValue bool, nullableValue *bool) { + if nullableValue == nil { + assert.Equal(t, expectedValue, false) + } else { + assert.Equal(t, expectedValue, *nullableValue) + } +} + func TestUpdatePublicDashboard(t *testing.T) { t.Run("Updating public dashboard", func(t *testing.T) { sqlStore := db.InitTestDB(t) @@ -445,14 +529,14 @@ func TestUpdatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled, annotationsEnabled, timeSelectionEnabled := true, false, false dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - AnnotationsEnabled: false, - IsEnabled: true, - TimeSelectionEnabled: false, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, + AnnotationsEnabled: &annotationsEnabled, + TimeSelectionEnabled: &timeSelectionEnabled, TimeSettings: timeSettings, }, } @@ -461,21 +545,21 @@ func TestUpdatePublicDashboard(t *testing.T) { savedPubdash, err := service.Create(context.Background(), SignedInUser, dto) require.NoError(t, err) + isEnabled, annotationsEnabled, timeSelectionEnabled = true, true, true // attempt to overwrite settings dto = &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 8, - PublicDashboard: &PublicDashboard{ + PublicDashboard: &PublicDashboardDTO{ Uid: savedPubdash.Uid, OrgId: 9, DashboardUid: "abc1234", CreatedBy: 9, CreatedAt: time.Time{}, - IsEnabled: true, - AnnotationsEnabled: true, - TimeSelectionEnabled: true, + IsEnabled: &isEnabled, + AnnotationsEnabled: &annotationsEnabled, + TimeSelectionEnabled: &timeSelectionEnabled, TimeSettings: timeSettings, AccessToken: "NOTAREALUUID", }, @@ -491,9 +575,9 @@ func TestUpdatePublicDashboard(t *testing.T) { assert.Equal(t, savedPubdash.AccessToken, updatedPubdash.AccessToken) // gets updated - assert.Equal(t, dto.PublicDashboard.IsEnabled, updatedPubdash.IsEnabled) - assert.Equal(t, dto.PublicDashboard.AnnotationsEnabled, updatedPubdash.AnnotationsEnabled) - assert.Equal(t, dto.PublicDashboard.TimeSelectionEnabled, updatedPubdash.TimeSelectionEnabled) + assert.Equal(t, *dto.PublicDashboard.IsEnabled, updatedPubdash.IsEnabled) + assert.Equal(t, *dto.PublicDashboard.AnnotationsEnabled, updatedPubdash.AnnotationsEnabled) + assert.Equal(t, *dto.PublicDashboard.TimeSelectionEnabled, updatedPubdash.TimeSelectionEnabled) assert.Equal(t, dto.PublicDashboard.TimeSettings, updatedPubdash.TimeSettings) assert.Equal(t, dto.UserId, updatedPubdash.UpdatedBy) assert.NotEqual(t, &time.Time{}, updatedPubdash.UpdatedAt) @@ -515,12 +599,12 @@ func TestUpdatePublicDashboard(t *testing.T) { serviceWrapper: serviceWrapper, } + isEnabled := true dto := &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 7, - PublicDashboard: &PublicDashboard{ - IsEnabled: true, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, TimeSettings: timeSettings, }, } @@ -531,17 +615,16 @@ func TestUpdatePublicDashboard(t *testing.T) { // attempt to overwrite settings dto = &SavePublicDashboardDTO{ DashboardUid: dashboard.UID, - OrgId: dashboard.OrgID, UserId: 8, - PublicDashboard: &PublicDashboard{ + PublicDashboard: &PublicDashboardDTO{ Uid: savedPubdash.Uid, OrgId: 9, DashboardUid: "abc1234", CreatedBy: 9, CreatedAt: time.Time{}, - - IsEnabled: true, - AccessToken: "NOTAREALUUID", + IsEnabled: &isEnabled, + TimeSettings: &TimeSettings{}, + AccessToken: "NOTAREALUUID", }, } @@ -550,6 +633,132 @@ func TestUpdatePublicDashboard(t *testing.T) { assert.Equal(t, &TimeSettings{}, updatedPubdash.TimeSettings) }) + + trueBooleanField := true + timeSettings := &TimeSettings{From: "now-8", To: "now"} + shareType := EmailShareType + + testCases := []struct { + Name string + IsEnabled *bool + TimeSelectionEnabled *bool + AnnotationsEnabled *bool + TimeSettings *TimeSettings + ShareType ShareType + }{ + { + Name: "isEnabled", + IsEnabled: nil, + TimeSelectionEnabled: &trueBooleanField, + AnnotationsEnabled: &trueBooleanField, + TimeSettings: timeSettings, + ShareType: shareType, + }, + { + Name: "timeSelectionEnabled", + IsEnabled: &trueBooleanField, + TimeSelectionEnabled: nil, + AnnotationsEnabled: &trueBooleanField, + TimeSettings: timeSettings, + ShareType: shareType, + }, + { + Name: "annotationsEnabled", + IsEnabled: &trueBooleanField, + TimeSelectionEnabled: &trueBooleanField, + AnnotationsEnabled: nil, + TimeSettings: timeSettings, + ShareType: shareType, + }, + { + Name: "isEnabled, timeSelectionEnabled and annotationsEnabled", + IsEnabled: nil, + TimeSelectionEnabled: nil, + AnnotationsEnabled: nil, + TimeSettings: nil, + ShareType: "", + }, + } + + for _, tt := range testCases { + t.Run(fmt.Sprintf("Update public dashboard with %s null boolean fields let those fields with old persisted value", tt.Name), func(t *testing.T) { + sqlStore := db.InitTestDB(t) + quotaService := quotatest.New(false, nil) + dashboardStore, err := dashboardsDB.ProvideDashboardStore(sqlStore, sqlStore.Cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, sqlStore.Cfg), quotaService) + require.NoError(t, err) + publicdashboardStore := database.ProvideStore(sqlStore) + serviceWrapper := ProvideServiceWrapper(publicdashboardStore) + dashboard := insertTestDashboard(t, dashboardStore, "testDashie", 1, 0, true, []map[string]interface{}{}, nil) + + service := &PublicDashboardServiceImpl{ + log: log.New("test.logger"), + store: publicdashboardStore, + serviceWrapper: serviceWrapper, + } + + isEnabled, annotationsEnabled, timeSelectionEnabled := true, true, false + + dto := &SavePublicDashboardDTO{ + DashboardUid: dashboard.UID, + UserId: 7, + PublicDashboard: &PublicDashboardDTO{ + IsEnabled: &isEnabled, + AnnotationsEnabled: &annotationsEnabled, + TimeSelectionEnabled: &timeSelectionEnabled, + TimeSettings: timeSettings, + Share: PublicShareType, + }, + } + + // insert initial pubdash + savedPubdash, err := service.Create(context.Background(), SignedInUser, dto) + require.NoError(t, err) + + // attempt to overwrite settings + dto = &SavePublicDashboardDTO{ + DashboardUid: dashboard.UID, + UserId: 8, + PublicDashboard: &PublicDashboardDTO{ + Uid: savedPubdash.Uid, + OrgId: 9, + DashboardUid: "abc1234", + CreatedBy: 9, + CreatedAt: time.Time{}, + IsEnabled: tt.IsEnabled, + AnnotationsEnabled: tt.AnnotationsEnabled, + TimeSelectionEnabled: tt.TimeSelectionEnabled, + TimeSettings: tt.TimeSettings, + Share: tt.ShareType, + AccessToken: "NOTAREALUUID", + }, + } + updatedPubdash, err := service.Update(context.Background(), SignedInUser, dto) + require.NoError(t, err) + + assertOldValueIfNull(t, updatedPubdash.IsEnabled, savedPubdash.IsEnabled, dto.PublicDashboard.IsEnabled) + assertOldValueIfNull(t, updatedPubdash.AnnotationsEnabled, savedPubdash.AnnotationsEnabled, dto.PublicDashboard.AnnotationsEnabled) + assertOldValueIfNull(t, updatedPubdash.TimeSelectionEnabled, savedPubdash.TimeSelectionEnabled, dto.PublicDashboard.TimeSelectionEnabled) + + if dto.PublicDashboard.TimeSettings == nil { + assert.Equal(t, updatedPubdash.TimeSettings, savedPubdash.TimeSettings) + } else { + assert.Equal(t, updatedPubdash.TimeSettings, dto.PublicDashboard.TimeSettings) + } + if dto.PublicDashboard.Share == "" { + assert.Equal(t, updatedPubdash.Share, savedPubdash.Share) + } else { + assert.Equal(t, updatedPubdash.Share, dto.PublicDashboard.Share) + } + }) + } +} + +func assertOldValueIfNull(t *testing.T, expectedValue bool, oldValue bool, nullableValue *bool) { + if nullableValue == nil { + assert.Equal(t, expectedValue, oldValue) + } else { + assert.Equal(t, expectedValue, *nullableValue) + } } func TestDeletePublicDashboard(t *testing.T) { diff --git a/pkg/services/publicdashboards/validation/validation_test.go b/pkg/services/publicdashboards/validation/validation_test.go index 0d317547171..e55a3e6d530 100644 --- a/pkg/services/publicdashboards/validation/validation_test.go +++ b/pkg/services/publicdashboards/validation/validation_test.go @@ -10,14 +10,14 @@ import ( func TestValidatePublicDashboard(t *testing.T) { t.Run("Returns no error when valid shareType value is received", func(t *testing.T) { - dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: &PublicDashboard{Share: EmailShareType}} + dto := &SavePublicDashboardDTO{DashboardUid: "abc123", UserId: 1, PublicDashboard: &PublicDashboardDTO{Share: EmailShareType}} err := ValidatePublicDashboard(dto) require.NoError(t, err) }) t.Run("Returns error when invalid shareType value", func(t *testing.T) { - dto := &SavePublicDashboardDTO{DashboardUid: "abc123", OrgId: 1, UserId: 1, PublicDashboard: &PublicDashboard{Share: "invalid"}} + dto := &SavePublicDashboardDTO{DashboardUid: "abc123", UserId: 1, PublicDashboard: &PublicDashboardDTO{Share: "invalid"}} err := ValidatePublicDashboard(dto) require.Error(t, err) diff --git a/public/app/features/dashboard/api/publicDashboardApi.ts b/public/app/features/dashboard/api/publicDashboardApi.ts index 762a06e1b15..df98071d404 100644 --- a/public/app/features/dashboard/api/publicDashboardApi.ts +++ b/public/app/features/dashboard/api/publicDashboardApi.ts @@ -85,23 +85,30 @@ export const publicDashboardApi = createApi({ }, invalidatesTags: (result, error, { dashboard }) => [{ type: 'PublicDashboard', id: dashboard.uid }], }), - updatePublicDashboard: builder.mutation({ + updatePublicDashboard: builder.mutation< + PublicDashboard, + { dashboard: Partial; payload: Partial } + >({ query: (params) => ({ url: `/dashboards/uid/${params.dashboard.uid}/public-dashboards/${params.payload.uid}`, - method: 'PUT', + method: 'PATCH', data: params.payload, }), async onQueryStarted({ dashboard, payload }, { dispatch, queryFulfilled }) { const { data } = await queryFulfilled; dispatch(notifyApp(createSuccessNotification('Public dashboard updated!'))); - // Update runtime meta flag - dashboard.updateMeta({ - publicDashboardUid: data.uid, - publicDashboardEnabled: data.isEnabled, - }); + if (dashboard.updateMeta) { + dashboard.updateMeta({ + publicDashboardUid: data.uid, + publicDashboardEnabled: data.isEnabled, + }); + } }, - invalidatesTags: (result, error, { payload }) => [{ type: 'PublicDashboard', id: payload.dashboardUid }], + invalidatesTags: (result, error, { payload }) => [ + { type: 'PublicDashboard', id: payload.dashboardUid }, + 'AuditTablePublicDashboard', + ], }), addRecipient: builder.mutation({ query: () => ({ diff --git a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboard.test.tsx b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboard.test.tsx index 403a88d3a61..5a63467a405 100644 --- a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboard.test.tsx +++ b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard/SharePublicDashboard.test.tsx @@ -306,7 +306,7 @@ describe('SharePublic - Report interactions', () => { jest.clearAllMocks(); server.use(getExistentPublicDashboardResponse()); server.use( - rest.put('/api/dashboards/uid/:dashboardUid/public-dashboards/:uid', (req, res, ctx) => + rest.patch('/api/dashboards/uid/:dashboardUid/public-dashboards/:uid', (req, res, ctx) => res( ctx.status(200), ctx.json({ diff --git a/public/app/features/manage-dashboards/PublicDashboardListPage.tsx b/public/app/features/manage-dashboards/PublicDashboardListPage.tsx index 07f7ab3f950..d65c7cb7ab8 100644 --- a/public/app/features/manage-dashboards/PublicDashboardListPage.tsx +++ b/public/app/features/manage-dashboards/PublicDashboardListPage.tsx @@ -1,15 +1,9 @@ import React from 'react'; -import { Page } from 'app/core/components/Page/Page'; - import { PublicDashboardListTable } from './components/PublicDashboardListTable/PublicDashboardListTable'; export const ListPublicDashboardPage = ({}) => { - return ( - - - - ); + return ; }; export default ListPublicDashboardPage; diff --git a/public/app/features/manage-dashboards/components/PublicDashboardListTable/DeletePublicDashboardButton.tsx b/public/app/features/manage-dashboards/components/PublicDashboardListTable/DeletePublicDashboardButton.tsx index a9b39df8cf0..69a3e93643a 100644 --- a/public/app/features/manage-dashboards/components/PublicDashboardListTable/DeletePublicDashboardButton.tsx +++ b/public/app/features/manage-dashboards/components/PublicDashboardListTable/DeletePublicDashboardButton.tsx @@ -23,7 +23,7 @@ export const DeletePublicDashboardButton = ({ dashboard?: DashboardModel; publicDashboard: PublicDashboardDeletion; loader?: JSX.Element; - children: React.ReactNode; + children?: React.ReactNode; onDismiss?: () => void; } & ButtonProps) => { const [deletePublicDashboard, { isLoading }] = useDeletePublicDashboardMutation(); diff --git a/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.test.tsx b/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.test.tsx index b5b33e6c99f..2b4d3e6071b 100644 --- a/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.test.tsx +++ b/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.test.tsx @@ -2,14 +2,14 @@ import { render, screen, waitForElementToBeRemoved, within } from '@testing-libr import { rest } from 'msw'; import { setupServer } from 'msw/node'; import React from 'react'; -import { Provider } from 'react-redux'; import 'whatwg-fetch'; import { BrowserRouter } from 'react-router-dom'; +import { TestProvider } from 'test/helpers/TestProvider'; +import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; import { selectors as e2eSelectors } from '@grafana/e2e-selectors/src'; import { backendSrv } from 'app/core/services/backend_srv'; import { contextSrv } from 'app/core/services/context_srv'; -import { configureStore } from 'app/store/configureStore'; import { ListPublicDashboardResponse } from '../../types'; @@ -77,23 +77,23 @@ afterEach(() => { const selectors = e2eSelectors.pages.PublicDashboards; const renderPublicDashboardTable = async (waitForListRendering?: boolean) => { - const store = configureStore(); + const context = getGrafanaContextMock(); render( - + - + ); - waitForListRendering && (await waitForElementToBeRemoved(screen.getByTestId('Spinner'), { timeout: 3000 })); + waitForListRendering && (await waitForElementToBeRemoved(screen.getAllByTestId('Spinner')[1], { timeout: 3000 })); }; describe('Show table', () => { it('renders loader spinner while loading', async () => { await renderPublicDashboardTable(); - const spinner = screen.getByTestId('Spinner'); + const spinner = screen.getAllByTestId('Spinner')[1]; expect(spinner).toBeInTheDocument(); await waitForElementToBeRemoved(spinner); @@ -101,8 +101,7 @@ describe('Show table', () => { it('renders public dashboard list items', async () => { await renderPublicDashboardTable(true); - const tableBody = screen.getAllByRole('rowgroup')[1]; - expect(within(tableBody).getAllByRole('row')).toHaveLength(publicDashboardListResponse.length); + expect(screen.getAllByRole('listitem')).toHaveLength(publicDashboardListResponse.length); }); it('renders empty list', async () => { server.use( @@ -113,8 +112,7 @@ describe('Show table', () => { await renderPublicDashboardTable(true); - const tableBody = screen.getAllByRole('rowgroup')[1]; - expect(within(tableBody).queryAllByRole('row')).toHaveLength(0); + expect(screen.queryAllByRole('listitem')).toHaveLength(0); }); it('renders public dashboards in a good way without trashcan', async () => { jest.spyOn(contextSrv, 'hasAccess').mockReturnValue(false); @@ -123,15 +121,6 @@ describe('Show table', () => { publicDashboardListResponse.forEach((pd, idx) => { renderPublicDashboardItemCorrectly(pd, idx, false); }); - - const tableBody = screen.getAllByRole('rowgroup')[1]; - const tableRows = within(tableBody).getAllByRole('row'); - - publicDashboardListResponse.forEach((pd, idx) => { - const tableRow = tableRows[idx]; - const rowDataCells = within(tableRow).getAllByRole('cell'); - expect(within(rowDataCells[2]).queryByTestId(selectors.ListItem.trashcanButton)).toBeNull(); - }); }); it('renders public dashboards in a good way with trashcan', async () => { jest.spyOn(contextSrv, 'hasAccess').mockReturnValue(true); @@ -140,23 +129,6 @@ describe('Show table', () => { publicDashboardListResponse.forEach((pd, idx) => { renderPublicDashboardItemCorrectly(pd, idx, true); }); - - const tableBody = screen.getAllByRole('rowgroup')[1]; - const tableRows = within(tableBody).getAllByRole('row'); - - publicDashboardListResponse.forEach((pd, idx) => { - const tableRow = tableRows[idx]; - const rowDataCells = within(tableRow).getAllByRole('cell'); - expect(within(rowDataCells[2]).getByTestId(selectors.ListItem.trashcanButton)); - }); - }); - it('renders public dashboards items correctly', async () => { - jest.spyOn(contextSrv, 'hasAccess').mockReturnValue(true); - await renderPublicDashboardTable(true); - - publicDashboardListResponse.forEach((pd, idx) => { - renderPublicDashboardItemCorrectly(pd, idx, true); - }); }); }); @@ -165,17 +137,13 @@ describe('Delete public dashboard', () => { jest.spyOn(contextSrv, 'hasAccess').mockReturnValue(false); await renderPublicDashboardTable(true); - const tableBody = screen.getAllByRole('rowgroup')[1]; - expect(within(tableBody).queryAllByTestId(selectors.ListItem.trashcanButton)).toHaveLength(0); + expect(screen.queryAllByTestId(selectors.ListItem.trashcanButton)).toHaveLength(0); }); it('when user has public dashboard write permissions, then dashboards are listed with delete button', async () => { jest.spyOn(contextSrv, 'hasAccess').mockReturnValue(true); await renderPublicDashboardTable(true); - const tableBody = screen.getAllByRole('rowgroup')[1]; - expect(within(tableBody).getAllByTestId(selectors.ListItem.trashcanButton)).toHaveLength( - publicDashboardListResponse.length - ); + expect(screen.getAllByTestId(selectors.ListItem.trashcanButton)).toHaveLength(publicDashboardListResponse.length); }); }); @@ -199,22 +167,14 @@ describe('Orphaned public dashboard', () => { const renderPublicDashboardItemCorrectly = (pd: ListPublicDashboardResponse, idx: number, hasWriteAccess: boolean) => { const isOrphaned = !pd.dashboardUid; - const tableBody = screen.getAllByRole('rowgroup')[1]; - const tableRows = within(tableBody).getAllByRole('row'); + const cardItems = screen.getAllByRole('listitem'); - const tableRow = tableRows[idx]; - const rowDataCells = within(tableRow).getAllByRole('cell'); - expect(rowDataCells).toHaveLength(3); + const linkButton = within(cardItems[idx]).getByTestId(selectors.ListItem.linkButton); + const configButton = within(cardItems[idx]).getByTestId(selectors.ListItem.configButton); + const trashcanButton = within(cardItems[idx]).queryByTestId(selectors.ListItem.trashcanButton); - const statusTag = within(rowDataCells[1]).getByText(pd.isEnabled ? 'enabled' : 'paused'); - const linkButton = within(rowDataCells[2]).getByTestId(selectors.ListItem.linkButton); - const configButton = within(rowDataCells[2]).getByTestId(selectors.ListItem.configButton); - const trashcanButton = within(rowDataCells[2]).queryByTestId(selectors.ListItem.trashcanButton); - - expect(within(rowDataCells[0]).getByText(isOrphaned ? 'Orphaned public dashboard' : pd.title)).toBeInTheDocument(); - expect(statusTag).toBeInTheDocument(); - isOrphaned ? expect(statusTag).toHaveStyle('background-color: rgb(110, 110, 110)') : expect(statusTag).toBeEnabled(); - isOrphaned || !pd.isEnabled + expect(within(cardItems[idx]).getByText(isOrphaned ? 'Orphaned public dashboard' : pd.title)).toBeInTheDocument(); + isOrphaned ? expect(linkButton).toHaveStyle('pointer-events: none') : expect(linkButton).not.toHaveStyle('pointer-events: none'); isOrphaned diff --git a/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx b/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx index 598727fab5b..fb4416b6177 100644 --- a/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx +++ b/public/app/features/manage-dashboards/components/PublicDashboardListTable/PublicDashboardListTable.tsx @@ -1,13 +1,17 @@ import { css } from '@emotion/css'; -import React from 'react'; -import { useWindowSize } from 'react-use'; +import React, { useMemo } from 'react'; +import { useMedia } from 'react-use'; import { GrafanaTheme2 } from '@grafana/data/src'; import { selectors as e2eSelectors } from '@grafana/e2e-selectors/src'; -import { Link, ButtonGroup, LinkButton, Icon, Tag, useStyles2, Tooltip, useTheme2, Spinner } from '@grafana/ui/src'; +import { reportInteraction } from '@grafana/runtime'; +import { LinkButton, useStyles2, Spinner, Card, useTheme2, Tooltip, Icon, Switch } from '@grafana/ui/src'; import { Page } from 'app/core/components/Page/Page'; import { contextSrv } from 'app/core/services/context_srv'; -import { useListPublicDashboardsQuery } from 'app/features/dashboard/api/publicDashboardApi'; +import { + useListPublicDashboardsQuery, + useUpdatePublicDashboardMutation, +} from 'app/features/dashboard/api/publicDashboardApi'; import { generatePublicDashboardConfigUrl, generatePublicDashboardUrl, @@ -19,138 +23,160 @@ import { ListPublicDashboardResponse } from '../../types'; import { DeletePublicDashboardButton } from './DeletePublicDashboardButton'; -export const PublicDashboardListTable = () => { - const { width } = useWindowSize(); - const isMobile = width <= 480; +const PublicDashboardCard = ({ pd }: { pd: ListPublicDashboardResponse }) => { + const styles = useStyles2(getStyles); const theme = useTheme2(); - const styles = useStyles2(() => getStyles(theme, isMobile)); + const isMobile = useMedia(`(max-width: ${theme.breakpoints.values.sm}px)`); - const { data: publicDashboards, isLoading, isFetching } = useListPublicDashboardsQuery(); + const [update, { isLoading: isUpdateLoading }] = useUpdatePublicDashboardMutation(); const selectors = e2eSelectors.pages.PublicDashboards; const hasWritePermissions = contextSrv.hasAccess(AccessControlAction.DashboardsPublicWrite, isOrgAdmin()); - const responsiveSize = isMobile ? 'sm' : 'md'; + const isOrphaned = !pd.dashboardUid; + + const onTogglePause = (pd: ListPublicDashboardResponse, isPaused: boolean) => { + const req = { + dashboard: { uid: pd.dashboardUid }, + payload: { + uid: pd.uid, + isEnabled: !isPaused, + }, + }; + + update(req); + }; + + const CardActions = useMemo(() => (isMobile ? Card.Actions : Card.SecondaryActions), [isMobile]); return ( - - - - - - - - - - - {publicDashboards?.map((pd: ListPublicDashboardResponse) => { - const isOrphaned = !pd.dashboardUid; - return ( - - - - - - ); - })} - -
NameStatus{isFetching && }
- - {!isOrphaned ? ( - - {pd.title} - - ) : ( -
-

Orphaned public dashboard

- -
- )} - - 		- - - - - - - - - - {hasWritePermissions && ( - } - > - - - )} - -
- + + + {!isOrphaned ? ( + {pd.title} + ) : ( + +
+ Orphaned public dashboard + +
+ + )} + + +
+ { + reportInteraction('grafana_dashboards_public_enable_clicked', { + action: e.currentTarget.checked ? 'disable' : 'enable', + }); + onTogglePause(pd, e.currentTarget.checked); + }} + data-testid={selectors.ListItem.pauseSwitch} + /> + Pause sharing +
+ + + {hasWritePermissions && ( + } + data-testid={selectors.ListItem.trashcanButton} + /> + )} + + ); }; -function getStyles(theme: GrafanaTheme2, isMobile: boolean) { - return { - fetchingSpinner: css` - display: flex; - justify-content: end; - `, - link: css` - color: ${theme.colors.primary.text}; - text-decoration: underline; - margin-right: ${theme.spacing()}; - `, - nameTh: css` - width: 20%; - `, - titleTd: css` - max-width: 0; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - `, - buttonGroup: css` - justify-content: ${isMobile ? 'space-between' : 'end'}; - `, - orphanedTitle: css` +export const PublicDashboardListTable = () => { + const styles = useStyles2(getStyles); + + const { data: publicDashboards, isLoading, isFetching } = useListPublicDashboardsQuery(); + + return ( + }> + +
    + {publicDashboards?.map((pd: ListPublicDashboardResponse) => ( +
  • + +
    • + ))} +
+ + + ); +}; + +const getStyles = (theme: GrafanaTheme2) => ({ + list: css` + list-style-type: none; + `, + card: css` + ${theme.breakpoints.up('sm')} { display: flex; - align-items: center; + } + `, + heading: css` + display: flex; + align-items: center; + gap: ${theme.spacing(1)}; + flex: 1; + `, + orphanedTitle: css` + display: flex; + align-items: center; + gap: ${theme.spacing(1)}; + `, + actions: css` + display: flex; + align-items: center; + position: relative; + + gap: ${theme.spacing(0.5)}; + ${theme.breakpoints.up('sm')} { gap: ${theme.spacing(1)}; + } + `, + pauseSwitch: css` + display: flex; + gap: ${theme.spacing(1)}; + align-items: center; + font-size: ${theme.typography.bodySmall.fontSize}; + margin-bottom: 0; + flex: 1; - p { - margin: ${theme.spacing(0)}; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - } - `, - orphanedInfoIcon: css` - color: ${theme.colors.text.link}; - `, - }; -} + ${theme.breakpoints.up('sm')} { + padding-right: ${theme.spacing(2)}; + } + `, +});