From 325f7a789ea940bdbfb1ae37432e7b071196abcf Mon Sep 17 00:00:00 2001 From: idafurjes <36131195+idafurjes@users.noreply.github.com> Date: Tue, 3 Jan 2023 15:25:35 +0100 Subject: [PATCH] Chore: Delete duplicate models for user (#60906) * Delete duplicate models for user * Use new models in some tests * Add auth model conversion back --- pkg/api/user.go | 16 +-- pkg/api/user_test.go | 16 +-- pkg/models/user.go | 133 ------------------ .../contexthandler/auth_proxy_test.go | 18 +-- .../notifications/notifications_test.go | 4 +- pkg/services/org/orgimpl/store.go | 10 +- pkg/services/sqlstore/mockstore/mockstore.go | 2 +- 7 files changed, 24 insertions(+), 175 deletions(-) diff --git a/pkg/api/user.go b/pkg/api/user.go index 15c6fa586a4..df12cd38b1a 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -94,14 +94,14 @@ func (hs *HTTPServer) GetUserByLoginOrEmail(c *models.ReqContext) response.Respo } return response.Error(500, "Failed to get user", err) } - result := models.UserProfileDTO{ - Id: usr.ID, + result := user.UserProfileDTO{ + ID: usr.ID, Name: usr.Name, Email: usr.Email, Login: usr.Login, Theme: usr.Theme, IsGrafanaAdmin: usr.IsAdmin, - OrgId: usr.OrgID, + OrgID: usr.OrgID, UpdatedAt: usr.Updated, CreatedAt: usr.Created, } @@ -560,7 +560,7 @@ type UpdateSignedInUserParams struct { // To change the email, name, login, theme, provide another one. // in:body // required:true - Body models.UpdateUserCommand `json:"body"` + Body user.UpdateUserCommand `json:"body"` } // swagger:parameters userSetUsingOrg @@ -582,7 +582,7 @@ type ChangeUserPasswordParams struct { // To change the email, name, login, theme, provide another one. // in:body // required:true - Body models.ChangeUserPasswordCommand `json:"body"` + Body user.ChangeUserPasswordCommand `json:"body"` } // swagger:parameters getUserByID @@ -619,7 +619,7 @@ type UpdateUserParams struct { // To change the email, name, login, theme, provide another one. // in:body // required:true - Body models.UpdateUserCommand `json:"body"` + Body user.UpdateUserCommand `json:"body"` // in:path // required:true UserID int64 `json:"user_id"` @@ -629,14 +629,14 @@ type UpdateUserParams struct { type SearchUsersResponse struct { // The response message // in: body - Body models.SearchUserQueryResult `json:"body"` + Body user.SearchUserQueryResult `json:"body"` } // swagger:response userResponse type UserResponse struct { // The response message // in: body - Body models.UserProfileDTO `json:"body"` + Body user.UserProfileDTO `json:"body"` } // swagger:response getUserOrgListResponse diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 7f733b7dc9a..fdd26f987d9 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -76,7 +76,7 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { Login: "loginuser", IsAdmin: true, } - user, err := userSvc.CreateUserForTests(context.Background(), &createUserCmd) + usr, err := userSvc.CreateUserForTests(context.Background(), &createUserCmd) require.NoError(t, err) sc.handlerFunc = hs.GetUserByID @@ -92,7 +92,7 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { login := "loginuser" query := &models.GetUserByAuthInfoQuery{AuthModule: "test", AuthId: "test", UserLookupParams: models.UserLookupParams{Login: &login}} cmd := &models.UpdateAuthInfoCommand{ - UserId: user.ID, + UserId: usr.ID, AuthId: query.AuthId, AuthModule: query.AuthModule, OAuthToken: token, @@ -100,14 +100,14 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { err = srv.UpdateAuthInfo(context.Background(), cmd) require.NoError(t, err) avatarUrl := dtos.GetGravatarUrl("@test.com") - sc.fakeReqWithParams("GET", sc.url, map[string]string{"id": fmt.Sprintf("%v", user.ID)}).exec() + sc.fakeReqWithParams("GET", sc.url, map[string]string{"id": fmt.Sprintf("%v", usr.ID)}).exec() - expected := models.UserProfileDTO{ - Id: 1, + expected := user.UserProfileDTO{ + ID: 1, Email: "user@test.com", Name: "user", Login: "loginuser", - OrgId: 1, + OrgID: 1, IsGrafanaAdmin: true, AuthLabels: []string{}, CreatedAt: fakeNow, @@ -115,7 +115,7 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { AvatarUrl: avatarUrl, } - var resp models.UserProfileDTO + var resp user.UserProfileDTO require.Equal(t, http.StatusOK, sc.resp.Code) err = json.Unmarshal(sc.resp.Body.Bytes(), &resp) require.NoError(t, err) @@ -147,7 +147,7 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { hs.userService = userMock sc.fakeReqWithParams("GET", sc.url, map[string]string{"loginOrEmail": "admin@test.com"}).exec() - var resp models.UserProfileDTO + var resp user.UserProfileDTO require.Equal(t, http.StatusOK, sc.resp.Code) err = json.Unmarshal(sc.resp.Body.Bytes(), &resp) require.NoError(t, err) diff --git a/pkg/models/user.go b/pkg/models/user.go index 01f68ad63d6..b2af1e0837d 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -1,144 +1,11 @@ package models -import ( - "time" - - "github.com/grafana/grafana/pkg/services/user" -) - type Password string func (p Password) IsWeak() bool { return len(p) <= 4 } -type UpdateUserCommand struct { - Name string `json:"name"` - Email string `json:"email"` - Login string `json:"login"` - Theme string `json:"theme"` - - UserId int64 `json:"-"` -} - -type ChangeUserPasswordCommand struct { - OldPassword string `json:"oldPassword"` - NewPassword string `json:"newPassword"` - - UserId int64 `json:"-"` -} - -type DisableUserCommand struct { - UserId int64 - IsDisabled bool -} - -type BatchDisableUsersCommand struct { - UserIds []int64 - IsDisabled bool -} - -type DeleteUserCommand struct { - UserId int64 -} - -type SetUsingOrgCommand struct { - UserId int64 - OrgId int64 -} - -// ---------------------- -// QUERIES - -type GetUserByLoginQuery struct { - LoginOrEmail string - Result *user.User -} - -type GetUserByEmailQuery struct { - Email string - Result *user.User -} - -type GetUserByIdQuery struct { - Id int64 - Result *user.User -} - -type GetSignedInUserQuery struct { - UserId int64 - Login string - Email string - OrgId int64 - Result *user.SignedInUser -} - -type GetUserProfileQuery struct { - UserId int64 - Result UserProfileDTO -} - -type SearchUsersQuery struct { - SignedInUser *user.SignedInUser - OrgId int64 - Query string - Page int - Limit int - AuthModule string - Filters []user.Filter - - IsDisabled *bool - - Result SearchUserQueryResult -} - -type SearchUserQueryResult struct { - TotalCount int64 `json:"totalCount"` - Users []*UserSearchHitDTO `json:"users"` - Page int `json:"page"` - PerPage int `json:"perPage"` -} - -type GetUserOrgListQuery struct { - UserId int64 - Result []*UserOrgDTO -} - -type UpdateUserLastSeenAtCommand struct { - UserId int64 -} - -type UserProfileDTO struct { - Id int64 `json:"id"` - Email string `json:"email"` - Name string `json:"name"` - Login string `json:"login"` - Theme string `json:"theme"` - OrgId int64 `json:"orgId,omitempty"` - IsGrafanaAdmin bool `json:"isGrafanaAdmin"` - IsDisabled bool `json:"isDisabled"` - IsExternal bool `json:"isExternal"` - AuthLabels []string `json:"authLabels"` - UpdatedAt time.Time `json:"updatedAt"` - CreatedAt time.Time `json:"createdAt"` - AvatarUrl string `json:"avatarUrl"` - AccessControl map[string]bool `json:"accessControl,omitempty"` -} - -type UserSearchHitDTO struct { - Id int64 `json:"id"` - Name string `json:"name"` - Login string `json:"login"` - Email string `json:"email"` - AvatarUrl string `json:"avatarUrl"` - IsAdmin bool `json:"isAdmin"` - IsDisabled bool `json:"isDisabled"` - LastSeenAt time.Time `json:"lastSeenAt"` - LastSeenAtAge string `json:"lastSeenAtAge"` - AuthLabels []string `json:"authLabels"` - AuthModule AuthModuleConversion `json:"-"` -} - type UserIdDTO struct { Id int64 `json:"id"` Message string `json:"message"` diff --git a/pkg/services/contexthandler/auth_proxy_test.go b/pkg/services/contexthandler/auth_proxy_test.go index b53d2935273..72981b2e32b 100644 --- a/pkg/services/contexthandler/auth_proxy_test.go +++ b/pkg/services/contexthandler/auth_proxy_test.go @@ -101,7 +101,7 @@ func getContextHandler(t *testing.T) *ContextHandler { } orgService := orgtest.NewOrgServiceFake() - authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, &userService, &FakeGetSignUserStore{}) + authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, &userService, nil) authenticator := &fakeAuthenticator{} return ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, @@ -109,22 +109,6 @@ func getContextHandler(t *testing.T) *ContextHandler { &userService, orgService, nil, nil, &authntest.FakeService{}) } -type FakeGetSignUserStore struct { - db.DB -} - -func (f *FakeGetSignUserStore) GetSignedInUser(ctx context.Context, query *models.GetSignedInUserQuery) error { - if query.UserId != userID { - return user.ErrUserNotFound - } - - query.Result = &user.SignedInUser{ - UserID: userID, - OrgID: orgID, - } - return nil -} - type fakeAuthenticator struct{} func (fa *fakeAuthenticator) AuthenticateUser(c context.Context, query *models.LoginUserQuery) error { diff --git a/pkg/services/notifications/notifications_test.go b/pkg/services/notifications/notifications_test.go index eca10a8fe5c..9ebada7cb81 100644 --- a/pkg/services/notifications/notifications_test.go +++ b/pkg/services/notifications/notifications_test.go @@ -206,9 +206,7 @@ func TestSendEmailAsync(t *testing.T) { // verify code query := models.ValidateResetPasswordCodeQuery{Code: code} getUserByLogin := func(ctx context.Context, login string) (*user.User, error) { - query := models.GetUserByLoginQuery{LoginOrEmail: login} - query.Result = &testuser - return query.Result, nil + return &testuser, nil } err = sut.ValidateResetPasswordCode(context.Background(), &query, getUserByLogin) require.NoError(t, err) diff --git a/pkg/services/org/orgimpl/store.go b/pkg/services/org/orgimpl/store.go index 09936a3195d..e4418ddb8c0 100644 --- a/pkg/services/org/orgimpl/store.go +++ b/pkg/services/org/orgimpl/store.go @@ -767,7 +767,7 @@ func (ss *sqlStore) RemoveOrgUser(ctx context.Context, cmd *org.RemoveOrgUserCom } } else if cmd.ShouldDeleteOrphanedUser { // no other orgs, delete the full user - if err := ss.deleteUserInTransaction(sess, &models.DeleteUserCommand{UserId: usr.ID}); err != nil { + if err := ss.deleteUserInTransaction(sess, &user.DeleteUserCommand{UserID: usr.ID}); err != nil { return err } @@ -784,9 +784,9 @@ func (ss *sqlStore) RemoveOrgUser(ctx context.Context, cmd *org.RemoveOrgUserCom }) } -func (ss *sqlStore) deleteUserInTransaction(sess *db.Session, cmd *models.DeleteUserCommand) error { +func (ss *sqlStore) deleteUserInTransaction(sess *db.Session, cmd *user.DeleteUserCommand) error { // Check if user exists - usr := user.User{ID: cmd.UserId} + usr := user.User{ID: cmd.UserID} has, err := sess.Where(ss.notServiceAccountFilter()).Get(&usr) if err != nil { return err @@ -795,13 +795,13 @@ func (ss *sqlStore) deleteUserInTransaction(sess *db.Session, cmd *models.Delete return user.ErrUserNotFound } for _, sql := range ss.userDeletions() { - _, err := sess.Exec(sql, cmd.UserId) + _, err := sess.Exec(sql, cmd.UserID) if err != nil { return err } } - return deleteUserAccessControl(sess, cmd.UserId) + return deleteUserAccessControl(sess, cmd.UserID) } func deleteUserAccessControl(sess *db.Session, userID int64) error { diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 6f6de30c37a..01795da6704 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -71,7 +71,7 @@ func (m *SQLStoreMock) CreateUser(ctx context.Context, cmd user.CreateUserComman return nil, m.ExpectedError } -func (m *SQLStoreMock) GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error { +func (m *SQLStoreMock) GetUserProfile(ctx context.Context, query *user.GetUserProfileQuery) error { return m.ExpectedError }