Skip to content

Commit 38767e1

Browse files
committed
fix(dashboard): address bot review — 3 bugs
1. processPage: if bookmarked dashboard ID was deleted between requests, use findIndex + fallback to process entire page (no silent skip). 2. parseDashboardListArgs: filter out undefined/empty args that buildListCommand's interceptSubcommand injects. 3. Include titleFilter in pagination context key so different filter queries don't share cursors.
1 parent 25323f0 commit 38767e1

File tree

2 files changed

+26
-14
lines changed

2 files changed

+26
-14
lines changed

src/commands/dashboard/list.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,19 @@ function processPage(
230230
glob: InstanceType<typeof Bun.Glob> | undefined;
231231
}
232232
): PageResult {
233-
let skipping = !!opts.afterId;
234-
235-
for (const item of data) {
236-
if (skipping) {
237-
if (item.id === opts.afterId) {
238-
skipping = false;
239-
}
240-
continue;
233+
// When resuming mid-page, find the afterId and skip everything up to and
234+
// including it. If the afterId was deleted between requests, fall through
235+
// and process the entire page from the start (no results lost).
236+
let startIdx = 0;
237+
if (opts.afterId) {
238+
const afterPos = data.findIndex((d) => d.id === opts.afterId);
239+
if (afterPos !== -1) {
240+
startIdx = afterPos + 1;
241241
}
242+
}
243+
244+
for (let i = startIdx; i < data.length; i++) {
245+
const item = data[i] as DashboardListItem;
242246
if (!opts.glob || opts.glob.match(item.title.toLowerCase())) {
243247
results.push(item);
244248
if (results.length >= opts.limit) {
@@ -383,7 +387,9 @@ export const listCommand = buildListCommand("dashboard", {
383387
}
384388

385389
// Resolve pagination cursor (handles "last" magic value)
386-
const contextKey = buildPaginationContextKey("dashboard", orgSlug);
390+
const contextKey = buildPaginationContextKey("dashboard", orgSlug, {
391+
...(titleFilter && { q: titleFilter }),
392+
});
387393
const rawCursor = resolveOrgCursor(
388394
flags.cursor,
389395
PAGINATION_KEY,

src/commands/dashboard/resolve.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,23 +141,29 @@ export function parseDashboardListArgs(args: string[]): {
141141
targetArg: string | undefined;
142142
titleFilter: string | undefined;
143143
} {
144-
if (args.length === 0) {
144+
// buildListCommand's interceptSubcommand may replace args[0] with undefined
145+
// when the first positional matches a subcommand name (e.g. "view", "create").
146+
// Filter those out so we don't crash on .includes("/").
147+
const filtered = args.filter(
148+
(a): a is string => a !== null && a !== undefined && a !== ""
149+
);
150+
if (filtered.length === 0) {
145151
return { targetArg: undefined, titleFilter: undefined };
146152
}
147-
if (args.length >= 2) {
153+
if (filtered.length >= 2) {
148154
// First arg is the target, remaining args are joined as the filter.
149155
// This handles unquoted multi-word titles: `my-org/ CLI Health` arrives
150156
// as ["my-org/", "CLI", "Health"] and becomes filter "CLI Health".
151157
// Normalize bare org slug to org/ format so parseOrgProjectArg treats
152158
// it as org-all (dashboards are org-scoped, project is irrelevant).
153-
const raw = args[0] as string;
159+
const raw = filtered[0] as string;
154160
const target = raw.includes("/") ? raw : `${raw}/`;
155-
const titleFilter = args.slice(1).join(" ");
161+
const titleFilter = filtered.slice(1).join(" ");
156162
return { targetArg: target, titleFilter };
157163
}
158164
// 1 arg: if it contains "/" it may be a target, or an org/project/name combo.
159165
// Without "/" it's always a title filter (dashboards are org-scoped).
160-
const arg = args[0] as string;
166+
const arg = filtered[0] as string;
161167
if (arg.includes("/")) {
162168
return splitOrgProjectName(arg);
163169
}

0 commit comments

Comments
 (0)