users prune - Paginate graphql site users - fix bug for soft deleted users (#909)

* fixes bug in which request is made to delete users that have already been deleted but exist in the aggregated_user_stats table

* implemented some pagination -- totalCount of nodes is always 0 -- needing to fix that for break condition

* fixed totalCount issue -- switched DoRaw methods to Do

* remove development print statements; hide users table behind flag; improve comments

* gofmt

* added a few more comments

* style improvements

* better graphQL querry formatting for single use queries

Author: DaedalusG

cmd/src/users.go

@@ -97,4 +97,5 @@ type SiteUser struct {
 	Email        string
 	SiteAdmin    bool
 	LastActiveAt string
+	DeletedAt    string
 }

cmd/src/users_prune.go

@@ -31,11 +31,12 @@ Examples:
 		fmt.Println(usage)
 	}
 	var (
-		daysToDelete       = flagSet.Int("days", 60, "Days threshold on which to remove users, must be 60 days or greater and defaults to this value ")
-		removeAdmin        = flagSet.Bool("remove-admin", false, "prune admin accounts")
-		removeNoLastActive = flagSet.Bool("remove-null-users", false, "removes users with no last active value")
-		skipConfirmation   = flagSet.Bool("force", false, "skips user confirmation step allowing programmatic use")
-		apiFlags           = api.NewFlags(flagSet)
+		daysToDelete         = flagSet.Int("days", 60, "Days threshold on which to remove users, must be 60 days or greater and defaults to this value ")
+		removeAdmin          = flagSet.Bool("remove-admin", false, "prune admin accounts")
+		removeNoLastActive   = flagSet.Bool("remove-null-users", false, "removes users with no last active value")
+		skipConfirmation     = flagSet.Bool("force", false, "skips user confirmation step allowing programmatic use")
+		displayUsersToDelete = flagSet.Bool("display-users", false, "display table of users to be deleted by prune")
+		apiFlags             = api.NewFlags(flagSet)
 	)
 
 	handler := func(args []string) error {
@@ -50,73 +51,109 @@ Examples:
 		ctx := context.Background()
 		client := cfg.apiClient(apiFlags, flagSet.Output())
 
-		currentUserQuery := `
-query getCurrentUser {
-	currentUser {
-		username
-	}
-}
-`
+		// get current user so as not to delete issuer of the prune request
+		currentUserQuery := `query getCurrentUser { currentUser { username }}`
 		var currentUserResult struct {
-			Data struct {
-				CurrentUser struct {
-					Username string
+			CurrentUser struct {
+				Username string
+			}
+		}
+		if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(currentUserQuery, nil).Do(context.Background(), &currentUserResult); err != nil || !ok {
+			return err
+		}
+
+		// get total users to paginate over
+		totalUsersQuery := `query getTotalUsers { site { users { totalCount }}}`
+		var totalUsers struct {
+			Site struct {
+				Users struct {
+					TotalCount float64
 				}
 			}
 		}
-		if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(currentUserQuery, nil).DoRaw(context.Background(), &currentUserResult); err != nil || !ok {
+		if ok, err := cfg.apiClient(apiFlags, flagSet.Output()).NewRequest(totalUsersQuery, nil).Do(context.Background(), &totalUsers); err != nil || !ok {
 			return err
 		}
 
+		// get 100 site users
 		getInactiveUsersQuery := `
-query getInactiveUsers {
+		query getInactiveUsers($limit: Int $offset: Int) {
 	site {
 		users {
-			nodes {
+			nodes (limit: $limit offset: $offset) {
+				id
 				username
 				email
 				siteAdmin
 				lastActiveAt
+				deletedAt
 			}
 		}
 	}
 }
 `
 
-		var usersResult struct {
-			Site struct {
-				Users struct {
-					Nodes []SiteUser
-				}
+		// paginate through users
+		var aggregatedUsers []SiteUser
+		// pagination variables, limit set to maximum possible users returned per request
+		offset := 0
+		const limit int = 100
+
+		// paginate requests until all site users have been checked -- this includes soft deleted users
+		for len(aggregatedUsers) < int(totalUsers.Site.Users.TotalCount) {
+			pagVars := map[string]interface{}{
+				"offset": offset,
+				"limit":  limit,
 			}
-		}
 
-		if ok, err := client.NewRequest(getInactiveUsersQuery, nil).Do(ctx, &usersResult); err != nil || !ok {
-			return err
+			var usersResult struct {
+				Site struct {
+					Users struct {
+						Nodes []SiteUser
+					}
+					TotalCount float64
+				}
+			}
+			if ok, err := client.NewRequest(getInactiveUsersQuery, pagVars).Do(ctx, &usersResult); err != nil || !ok {
+				return err
+			}
+			// increment graphql request offset by the length of the last user set returned
+			offset = offset + len(usersResult.Site.Users.Nodes)
+			// append graphql user results to aggregated users to be processed against user removal conditions
+			aggregatedUsers = append(aggregatedUsers, usersResult.Site.Users.Nodes...)
 		}
 
+		// filter users for deletion
 		usersToDelete := make([]UserToDelete, 0)
-		for _, user := range usersResult.Site.Users.Nodes {
+		for _, user := range aggregatedUsers {
+			// never remove user issuing command
+			if user.Username == currentUserResult.CurrentUser.Username {
+				continue
+			}
+			// filter out soft deleted users returned by site graphql endpoint
+			if user.DeletedAt != "" {
+				continue
+			}
+			//compute days since last use
 			daysSinceLastUse, hasLastActive, err := computeDaysSinceLastUse(user)
 			if err != nil {
 				return err
 			}
-			// never remove user issuing command
-			if user.Username == currentUserResult.Data.CurrentUser.Username {
-				continue
-			}
+			// don't remove users with no last active value unless option flag is set
 			if !hasLastActive && !*removeNoLastActive {
 				continue
 			}
+			// don't remove admins unless option flag is set
 			if !*removeAdmin && user.SiteAdmin {
 				continue
 			}
+			// remove users who have been inactive for longer than the threshold set by the -days flag
 			if daysSinceLastUse <= *daysToDelete && hasLastActive {
 				continue
 			}
-			deleteUser := UserToDelete{user, daysSinceLastUse}
-
-			usersToDelete = append(usersToDelete, deleteUser)
+			// serialize user to print in table as part of confirmUserRemoval, add to delete slice
+			userToDelete := UserToDelete{user, daysSinceLastUse}
+			usersToDelete = append(usersToDelete, userToDelete)
 		}
 
 		if *skipConfirmation {
@@ -129,7 +166,7 @@ query getInactiveUsers {
 		}
 
 		// confirm and remove users
-		if confirmed, _ := confirmUserRemoval(usersToDelete); !confirmed {
+		if confirmed, _ := confirmUserRemoval(usersToDelete, int(totalUsers.Site.Users.TotalCount), *daysToDelete, *displayUsersToDelete); !confirmed {
 			fmt.Println("Aborting removal")
 			return nil
 		} else {
@@ -152,9 +189,9 @@ query getInactiveUsers {
 	})
 }
 
-// computes days since last usage from current day and time and UsageStatistics.LastActiveTime, uses time.Parse
+// computes days since last usage from current day and time and aggregated_user_statistics.lastActiveAt, uses time.Parse
 func computeDaysSinceLastUse(user SiteUser) (timeDiff int, hasLastActive bool, _ error) {
-	// handle for null LastActiveAt returned from
+	// handle for null LastActiveAt, users who have never been active
 	if user.LastActiveAt == "" {
 		hasLastActive = false
 		return 0, hasLastActive, nil
@@ -170,11 +207,7 @@ func computeDaysSinceLastUse(user SiteUser) (timeDiff int, hasLastActive bool, _
 
 // Issue graphQL api request to remove user
 func removeUser(user SiteUser, client api.Client, ctx context.Context) error {
-	query := `mutation DeleteUser($user: ID!) {
-  deleteUser(user: $user) {
-    alwaysNil
-  }
-}`
+	query := `mutation DeleteUser($user: ID!) { deleteUser(user: $user) { alwaysNil }}`
 	vars := map[string]interface{}{
 		"user": user.ID,
 	}
@@ -190,25 +223,27 @@ type UserToDelete struct {
 }
 
 // Verify user wants to remove users with table of users and a command prompt for [y/N]
-func confirmUserRemoval(usersToRemove []UserToDelete) (bool, error) {
-	fmt.Printf("Users to remove from instance at %s\n", cfg.Endpoint)
-	t := table.NewWriter()
-	t.SetOutputMirror(os.Stdout)
-	t.AppendHeader(table.Row{"Username", "Email", "Days Since Last Active"})
-	for _, user := range usersToRemove {
-		if user.User.Email != "" {
-			t.AppendRow([]interface{}{user.User.Username, user.User.Email, user.DaysSinceLastUse})
-			t.AppendSeparator()
-		} else {
-			t.AppendRow([]interface{}{user.User.Username, "", user.DaysSinceLastUse})
-			t.AppendSeparator()
+func confirmUserRemoval(usersToDelete []UserToDelete, totalUsers int, daysThreshold int, displayUsers bool) (bool, error) {
+	if displayUsers {
+		fmt.Printf("Users to remove from %s\n", cfg.Endpoint)
+		t := table.NewWriter()
+		t.SetOutputMirror(os.Stdout)
+		t.AppendHeader(table.Row{"Username", "Email", "Days Since Last Active"})
+		for _, user := range usersToDelete {
+			if user.User.Email != "" {
+				t.AppendRow([]interface{}{user.User.Username, user.User.Email, user.DaysSinceLastUse})
+				t.AppendSeparator()
+			} else {
+				t.AppendRow([]interface{}{user.User.Username, "", user.DaysSinceLastUse})
+				t.AppendSeparator()
+			}
 		}
+		t.SetStyle(table.StyleRounded)
+		t.Render()
 	}
-	t.SetStyle(table.StyleRounded)
-	t.Render()
 	input := ""
 	for strings.ToLower(input) != "y" && strings.ToLower(input) != "n" {
-		fmt.Printf("Do you  wish to proceed with user removal [y/N]: ")
+		fmt.Printf("%v users were inactive for more than %v days on %v.\nDo you  wish to proceed with user removal [y/N]: ", len(usersToDelete), daysThreshold, cfg.Endpoint)
 		if _, err := fmt.Scanln(&input); err != nil {
 			return false, err
 		}