fix(ui): fix the all option in the workflow archive list (argoproj#…

…4486)

Signed-off-by: Daniel Herman <dherman@factset.com>
Signed-off-by: Alex Capras <alexcapras@gmail.com>

persist/sqldb/workflow_archive.go

@@ -130,6 +130,14 @@ func (r *workflowArchive) ListWorkflows(namespace string, minStartedAt, maxStart
 	if err != nil {
 		return nil, err
 	}
+
+	// If we were passed 0 as the limit, then we should load all available archived workflows
+	// to match the behavior of the `List` operations in the Kubernetes API
+	if limit == 0 {
+		limit = -1
+		offset = -1
+	}
+
 	err = r.session.
 		Select("name", "namespace", "uid", "phase", "startedat", "finishedat").
 		From(archiveTableName).

server/workflowarchive/archived_workflow_server.go

@@ -38,9 +38,7 @@ func (w *archivedWorkflowServer) ListArchivedWorkflows(ctx context.Context, req
 		options.Continue = "0"
 	}
 	limit := int(options.Limit)
-	if limit == 0 {
-		limit = 10
-	}
+
 	offset, err := strconv.Atoi(options.Continue)
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, "listOptions.continue must be int")
@@ -77,37 +75,38 @@ func (w *archivedWorkflowServer) ListArchivedWorkflows(ctx context.Context, req
 		return nil, err
 	}
 
-	items := make(wfv1.Workflows, 0)
 	allowed, err := auth.CanI(ctx, "list", workflow.WorkflowPlural, namespace, "")
 	if err != nil {
 		return nil, err
 	}
 	if !allowed {
 		return nil, status.Error(codes.PermissionDenied, "permission denied")
 	}
-	hasMore := true
-	// keep trying until we have enough
-	for len(items) < limit {
-		moreItems, err := w.wfArchive.ListWorkflows(namespace, minStartedAt, maxStartedAt, requirements, limit+1, offset)
-		if err != nil {
-			return nil, err
-		}
-		for index, wf := range moreItems {
-			if index == limit {
-				break
-			}
-			items = append(items, wf)
-		}
-		if len(moreItems) < limit+1 {
-			hasMore = false
-			break
-		}
-		offset = offset + limit
+
+	// When the zero value is passed, we should treat this as returning all results
+	// to align ourselves with the behavior of the `List` endpoints in the Kubernetes API
+	loadAll := limit == 0
+	limitWithMore := 0
+
+	if !loadAll {
+		// Attempt to load 1 more record than we actually need as an easy way to determine whether or not more
+		// records exist than we're currently requesting
+		limitWithMore = limit + 1
 	}
+
+	items, err := w.wfArchive.ListWorkflows(namespace, minStartedAt, maxStartedAt, requirements, limitWithMore, offset)
+
+	if err != nil {
+		return nil, err
+	}
+
 	meta := metav1.ListMeta{}
-	if hasMore {
-		meta.Continue = fmt.Sprintf("%v", offset)
+
+	if !loadAll && len(items) > limit {
+		items = items[0:limit]
+		meta.Continue = fmt.Sprintf("%v", offset+limit)
 	}
+
 	sort.Sort(items)
 	return &wfv1.WorkflowList{ListMeta: meta, Items: items}, nil
 }

ui/src/app/shared/components/pagination-panel.tsx

@@ -33,16 +33,23 @@ export class PaginationPanel extends React.Component<{pagination: Pagination; on
                 <small className='fa-pull-right'>
                     <select
                         className='small'
-                        onChange={e =>
-                            this.props.onChange({
-                                offset: this.props.pagination.offset,
-                                limit: parseLimit(e.target.value)
-                            })
-                        }
-                        value={this.props.pagination.limit || 'all'}>
-                        {[5, 10, 20, 50, 100, 500, 'all'].map(limit => (
+                        onChange={e => {
+                            const limit = parseLimit(e.target.value);
+                            const newValue: Pagination = {limit};
+
+                            // Only return the offset if we're actually going to be limiting
+                            // the results we're requesting.  If we're requesting all records,
+                            // we should not skip any by setting an offset.
+                            if (limit) {
+                                newValue.offset = this.props.pagination.offset;
+                            }
+
+                            this.props.onChange(newValue);
+                        }}
+                        value={this.props.pagination.limit || 0}>
+                        {[5, 10, 20, 50, 100, 500, 0].map(limit => (
                             <option key={limit} value={limit}>
-                                {limit}
+                                {limit === 0 ? 'all' : limit}
                             </option>
                         ))}
                     </select>{' '}