Skip to content

Commit

Permalink
graphql: Minor delete mutation msg fix (#5316)
Browse files Browse the repository at this point in the history
This PR changes the delete msg reported for the case when no nodes are deleted to No nodes were deleted. In addition, it corrects some tests which were wrong earlier.

Fixes #GRAPHQL-423.

Co-authored-by: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com>
  • Loading branch information
vardhanapoorv and abhimanyusinghgaur authored May 12, 2020
1 parent 401e052 commit 47f2f51
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 173 deletions.
47 changes: 13 additions & 34 deletions ee/acl/acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func checkUserCount(t *testing.T, resp []byte, expected int) {
require.Equal(t, expected, len(r.AddUser.User))
}

func deleteUser(t *testing.T, accessToken, username string) {
func deleteUser(t *testing.T, accessToken, username string, confirmDeletion bool) {
// TODO - Verify that only one uid got deleted once numUids are returned as part of the payload.
delUser := `
mutation deleteUser($name: String!) {
Expand All @@ -104,7 +104,9 @@ func deleteUser(t *testing.T, accessToken, username string) {
resp := makeRequest(t, accessToken, params)
resp.RequireNoGraphQLErrors(t)

require.JSONEq(t, `{"deleteUser":{"msg":"Deleted"}}`, string(resp.Data))
if confirmDeletion {
require.JSONEq(t, `{"deleteUser":{"msg":"Deleted"}}`, string(resp.Data))
}
}

func deleteGroup(t *testing.T, accessToken, name string) {
Expand Down Expand Up @@ -164,20 +166,14 @@ func TestPasswordReturn(t *testing.T) {
}

func TestGetCurrentUser(t *testing.T) {
accessJwt, _, err := testutil.HttpLogin(&testutil.LoginParams{
Endpoint: adminEndpoint,
UserID: "groot",
Passwd: "password",
})
require.NoError(t, err, "login failed")

accessJwt, _ := testutil.GrootHttpLogin(adminEndpoint)
currentUser := getCurrentUser(t, accessJwt)
currentUser.RequireNoGraphQLErrors(t)
require.Equal(t, string(currentUser.Data), `{"getCurrentUser":{"name":"groot"}}`)

// clean up the user to allow repeated running of this test
userid := "hamilton"
deleteUser(t, accessJwt, userid)
deleteUser(t, accessJwt, userid, false)
glog.Infof("cleaned up db user state")

resp := createUser(t, accessJwt, userid, userpassword)
Expand All @@ -197,31 +193,19 @@ func TestGetCurrentUser(t *testing.T) {
}

func TestCreateAndDeleteUsers(t *testing.T) {
accessJwt, _, err := testutil.HttpLogin(&testutil.LoginParams{
Endpoint: adminEndpoint,
UserID: "groot",
Passwd: "password",
})
require.NoError(t, err, "login failed")

// clean up the user to allow repeated running of this test
deleteUser(t, accessJwt, userid)
glog.Infof("cleaned up db user state")

resp := createUser(t, accessJwt, userid, userpassword)
resp.RequireNoGraphQLErrors(t)
checkUserCount(t, resp.Data, 1)
resetUser(t)

// adding the user again should fail
resp = createUser(t, accessJwt, userid, userpassword)
accessJwt, _ := testutil.GrootHttpLogin(adminEndpoint)
resp := createUser(t, accessJwt, userid, userpassword)
require.Equal(t, x.GqlErrorList{{
Message: "couldn't rewrite query for mutation addUser because id alice already exists" +
" for type User",
}}, resp.Errors)
checkUserCount(t, resp.Data, 0)

// delete the user
deleteUser(t, accessJwt, userid)
deleteUser(t, accessJwt, userid, true)

resp = createUser(t, accessJwt, userid, userpassword)
resp.RequireNoGraphQLErrors(t)
Expand All @@ -230,15 +214,10 @@ func TestCreateAndDeleteUsers(t *testing.T) {
}

func resetUser(t *testing.T) {
accessJwt, _, err := testutil.HttpLogin(&testutil.LoginParams{
Endpoint: adminEndpoint,
UserID: "groot",
Passwd: "password",
})
require.NoError(t, err, "login failed")
accessJwt, _ := testutil.GrootHttpLogin(adminEndpoint)

// clean up the user to allow repeated running of this test
deleteUser(t, accessJwt, userid)
deleteUser(t, accessJwt, userid, false)
glog.Infof("deleted user")

resp := createUser(t, accessJwt, userid, userpassword)
Expand Down Expand Up @@ -1599,7 +1578,7 @@ func TestDeleteUserShouldDeleteUserFromGroup(t *testing.T) {
})
require.NoError(t, err, "login failed")

deleteUser(t, accessJwt, userid)
deleteUser(t, accessJwt, userid, true)

gqlQuery := `
query {
Expand Down
4 changes: 2 additions & 2 deletions graphql/e2e/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ func TestDeleteAuthRule(t *testing.T) {
"anyofterms": "Sensitive information",
},
},
result: `{"deleteUserSecret":{"msg":"Deleted","numUids":0}}`,
result: `{"deleteUserSecret":{"msg":"No nodes were deleted","numUids":0}}`,
},
}
query := `
Expand Down Expand Up @@ -666,7 +666,7 @@ func TestDeleteDeepAuthRule(t *testing.T) {
"anyofterms": "Ticket2",
},
},
result: `{"deleteTicket":{"msg":"Deleted","numUids":0}}`,
result: `{"deleteTicket":{"msg":"No nodes were deleted","numUids":0}}`,
},
{
name: "ticket with edit permission",
Expand Down
Loading

0 comments on commit 47f2f51

Please sign in to comment.