From 646f3b95c8a542c489b290902528154c3c89c286 Mon Sep 17 00:00:00 2001 From: "tembo[bot]" <208362400+tembo[bot]@users.noreply.github.com> Date: Tue, 21 Apr 2026 18:15:13 +0000 Subject: [PATCH] refactor(filters): unify filter helpers and streamline status filters and sidebar lookups --- .../src/components/filters/filter-helpers.ts | 64 +++++ .../src/components/filters/issue-filters.ts | 211 +++++------------ .../src/components/filters/pull-filters.ts | 221 ++++++------------ .../pulls/detail/pull-detail-sidebar.tsx | 15 +- 4 files changed, 209 insertions(+), 302 deletions(-) create mode 100644 apps/dashboard/src/components/filters/filter-helpers.ts diff --git a/apps/dashboard/src/components/filters/filter-helpers.ts b/apps/dashboard/src/components/filters/filter-helpers.ts new file mode 100644 index 0000000..382041c --- /dev/null +++ b/apps/dashboard/src/components/filters/filter-helpers.ts @@ -0,0 +1,64 @@ +import { FolderLibraryIcon, UserCircleIcon } from "@diffkit/icons"; +import { createElement } from "react"; +import type { + FilterableItem, + FilterDefinition, + FilterOption, +} from "./use-list-filters"; + +/** + * Build a sorted list of unique repository options from a list of items. + */ +export function extractRepoOptions(items: FilterableItem[]): FilterOption[] { + const repos = new Map(); + for (const item of items) { + const name = item.repository.fullName; + if (!repos.has(name)) repos.set(name, name); + } + return [...repos.entries()] + .sort(([a], [b]) => a.localeCompare(b)) + .map(([value, label]) => ({ value, label })); +} + +/** + * Build a sorted list of unique author options (with avatar icons) from a + * list of items. Items without an author are ignored. + */ +export function extractAuthorOptions(items: FilterableItem[]): FilterOption[] { + const authors = new Map(); + for (const item of items) { + if (item.author && !authors.has(item.author.login)) { + authors.set(item.author.login, item.author); + } + } + return [...authors.entries()] + .sort(([a], [b]) => a.localeCompare(b)) + .map(([login, author]) => ({ + value: login, + label: login, + icon: createElement("img", { + src: author.avatarUrl, + alt: login, + className: "size-4 rounded-full", + }), + })); +} + +/** Reusable repository filter definition (for global/cross-repo lists). */ +export const repoFilterDef: FilterDefinition = { + id: "repo", + label: "Repository", + icon: FolderLibraryIcon, + extractOptions: extractRepoOptions, + match: (item, values) => values.has(item.repository.fullName), +}; + +/** Reusable author filter definition. */ +export const authorFilterDef: FilterDefinition = { + id: "author", + label: "Author", + icon: UserCircleIcon, + extractOptions: extractAuthorOptions, + match: (item, values) => + item.author ? values.has(item.author.login) : false, +}; diff --git a/apps/dashboard/src/components/filters/issue-filters.ts b/apps/dashboard/src/components/filters/issue-filters.ts index 64abe44..55d44fa 100644 --- a/apps/dashboard/src/components/filters/issue-filters.ts +++ b/apps/dashboard/src/components/filters/issue-filters.ts @@ -1,13 +1,10 @@ -import { - CircleIcon, - FolderLibraryIcon, - IssuesIcon, - UserCircleIcon, -} from "@diffkit/icons"; +import { CircleIcon, IssuesIcon } from "@diffkit/icons"; import { createElement } from "react"; +import { authorFilterDef, repoFilterDef } from "./filter-helpers"; import type { FilterableItem, FilterDefinition, + FilterOption, SortOption, } from "./use-list-filters"; @@ -19,159 +16,73 @@ function asIssue(item: FilterableItem): IssueFilterableItem { return item as IssueFilterableItem; } -export const issueFilterDefs: FilterDefinition[] = [ +type IssueStatus = "open" | "completed" | "not_planned"; + +function issueStatus(item: IssueFilterableItem): IssueStatus { + if (item.state !== "closed") return "open"; + return item.stateReason === "not_planned" ? "not_planned" : "completed"; +} + +const ISSUE_STATUS_META: { + value: IssueStatus; + label: string; + colorClass: string; +}[] = [ + { value: "open", label: "Open", colorClass: "text-green-500" }, { - id: "repo", - label: "Repository", - icon: FolderLibraryIcon, - extractOptions: (items) => { - const repos = new Map(); - for (const item of items) { - const name = item.repository.fullName; - if (!repos.has(name)) repos.set(name, name); - } - return [...repos.entries()] - .sort(([a], [b]) => a.localeCompare(b)) - .map(([value, label]) => ({ value, label })); - }, - match: (item, values) => values.has(item.repository.fullName), + value: "completed", + label: "Closed (completed)", + colorClass: "text-purple-500", }, { - id: "author", - label: "Author", - icon: UserCircleIcon, - extractOptions: (items) => { - const authors = new Map(); - for (const item of items) { - if (item.author && !authors.has(item.author.login)) { - authors.set(item.author.login, item.author); - } - } - return [...authors.entries()] - .sort(([a], [b]) => a.localeCompare(b)) - .map(([login, author]) => ({ - value: login, - label: login, - icon: createElement("img", { - src: author.avatarUrl, - alt: login, - className: "size-4 rounded-full", - }), - })); - }, - match: (item, values) => - item.author ? values.has(item.author.login) : false, + value: "not_planned", + label: "Closed (not planned)", + colorClass: "text-muted-foreground", }, - { - id: "status", - label: "Status", - icon: CircleIcon, - extractOptions: (items) => { - const statuses = new Set(); - for (const item of items) { - const issue = asIssue(item); - if (issue.state === "closed") { - statuses.add( - issue.stateReason === "not_planned" ? "not_planned" : "completed", - ); - } else { - statuses.add("open"); - } - } - const all = [ - { value: "open", label: "Open" }, - { value: "completed", label: "Closed (completed)" }, - { value: "not_planned", label: "Closed (not planned)" }, - ]; - const colorMap: Record = { - open: "text-green-500", - completed: "text-purple-500", - not_planned: "text-muted-foreground", - }; - return all - .filter((s) => statuses.has(s.value)) - .map((s) => ({ - value: s.value, - label: s.label, - icon: createElement(IssuesIcon, { - size: 14, - className: colorMap[s.value], - }), - })); - }, - match: (item, values) => { - const issue = asIssue(item); - if (issue.state === "closed") { - return issue.stateReason === "not_planned" - ? values.has("not_planned") - : values.has("completed"); - } - return values.has("open"); - }, +]; + +function toStatusOption( + meta: (typeof ISSUE_STATUS_META)[number], +): FilterOption { + return { + value: meta.value, + label: meta.label, + icon: createElement(IssuesIcon, { + size: 14, + className: meta.colorClass, + }), + }; +} + +const issueStatusFilterDef: FilterDefinition = { + id: "status", + label: "Status", + icon: CircleIcon, + extractOptions: (items) => { + const present = new Set(); + for (const item of items) present.add(issueStatus(asIssue(item))); + return ISSUE_STATUS_META.filter((m) => present.has(m.value)).map( + toStatusOption, + ); }, + match: (item, values) => values.has(issueStatus(asIssue(item))), +}; + +const repoIssueStatusFilterDef: FilterDefinition = { + ...issueStatusFilterDef, + extractOptions: () => ISSUE_STATUS_META.map(toStatusOption), +}; + +export const issueFilterDefs: FilterDefinition[] = [ + repoFilterDef, + authorFilterDef, + issueStatusFilterDef, ]; /** Filter defs for repo-scoped issue lists — static options, no repository filter. */ export const repoIssueFilterDefs: FilterDefinition[] = [ - { - id: "status", - label: "Status", - icon: CircleIcon, - extractOptions: () => { - const colorMap: Record = { - open: "text-green-500", - completed: "text-purple-500", - not_planned: "text-muted-foreground", - }; - return [ - { value: "open", label: "Open" }, - { value: "completed", label: "Closed (completed)" }, - { value: "not_planned", label: "Closed (not planned)" }, - ].map((s) => ({ - value: s.value, - label: s.label, - icon: createElement(IssuesIcon, { - size: 14, - className: colorMap[s.value], - }), - })); - }, - match: (item, values) => { - const issue = asIssue(item); - if (issue.state === "closed") { - return issue.stateReason === "not_planned" - ? values.has("not_planned") - : values.has("completed"); - } - return values.has("open"); - }, - }, - { - id: "author", - label: "Author", - icon: UserCircleIcon, - extractOptions: (items) => { - const authors = new Map(); - for (const item of items) { - if (item.author && !authors.has(item.author.login)) { - authors.set(item.author.login, item.author); - } - } - return [...authors.entries()] - .sort(([a], [b]) => a.localeCompare(b)) - .map(([login, author]) => ({ - value: login, - label: login, - icon: createElement("img", { - src: author.avatarUrl, - alt: login, - className: "size-4 rounded-full", - }), - })); - }, - match: (item, values) => - item.author ? values.has(item.author.login) : false, - }, + repoIssueStatusFilterDef, + authorFilterDef, ]; export const issueSortOptions: SortOption[] = [ diff --git a/apps/dashboard/src/components/filters/pull-filters.ts b/apps/dashboard/src/components/filters/pull-filters.ts index 116e7d2..0520d7a 100644 --- a/apps/dashboard/src/components/filters/pull-filters.ts +++ b/apps/dashboard/src/components/filters/pull-filters.ts @@ -1,16 +1,16 @@ import { CircleIcon, - FolderLibraryIcon, GitMergeIcon, GitPullRequestClosedIcon, GitPullRequestDraftIcon, GitPullRequestIcon, - UserCircleIcon, } from "@diffkit/icons"; import { createElement } from "react"; +import { authorFilterDef, repoFilterDef } from "./filter-helpers"; import type { FilterableItem, FilterDefinition, + FilterOption, SortOption, } from "./use-list-filters"; @@ -23,162 +23,87 @@ function asPull(item: FilterableItem): PullFilterableItem { return item as PullFilterableItem; } -export const pullFilterDefs: FilterDefinition[] = [ +type PullStatus = "open" | "draft" | "merged" | "closed"; + +function pullStatus(item: PullFilterableItem): PullStatus { + if (item.mergedAt) return "merged"; + if (item.state === "closed") return "closed"; + if (item.isDraft) return "draft"; + return "open"; +} + +const PULL_STATUS_META: { + value: PullStatus; + label: string; + icon: React.ComponentType<{ size?: number; className?: string }>; + colorClass: string; +}[] = [ + { + value: "open", + label: "Open", + icon: GitPullRequestIcon, + colorClass: "text-green-500", + }, { - id: "repo", - label: "Repository", - icon: FolderLibraryIcon, - extractOptions: (items) => { - const repos = new Map(); - for (const item of items) { - const name = item.repository.fullName; - if (!repos.has(name)) repos.set(name, name); - } - return [...repos.entries()] - .sort(([a], [b]) => a.localeCompare(b)) - .map(([value, label]) => ({ value, label })); - }, - match: (item, values) => values.has(item.repository.fullName), + value: "draft", + label: "Draft", + icon: GitPullRequestDraftIcon, + colorClass: "text-muted-foreground", }, { - id: "author", - label: "Author", - icon: UserCircleIcon, - extractOptions: (items) => { - const authors = new Map(); - for (const item of items) { - if (item.author && !authors.has(item.author.login)) { - authors.set(item.author.login, item.author); - } - } - return [...authors.entries()] - .sort(([a], [b]) => a.localeCompare(b)) - .map(([login, author]) => ({ - value: login, - label: login, - icon: createElement("img", { - src: author.avatarUrl, - alt: login, - className: "size-4 rounded-full", - }), - })); - }, - match: (item, values) => - item.author ? values.has(item.author.login) : false, + value: "merged", + label: "Merged", + icon: GitMergeIcon, + colorClass: "text-purple-500", }, { - id: "status", - label: "Status", - icon: CircleIcon, - extractOptions: (items) => { - const statuses = new Set(); - for (const item of items) { - const pull = asPull(item); - if (pull.mergedAt) statuses.add("merged"); - else if (pull.state === "closed") statuses.add("closed"); - else if (pull.isDraft) statuses.add("draft"); - else statuses.add("open"); - } - const all: { - value: string; - label: string; - icon: React.ComponentType<{ size?: number; className?: string }>; - }[] = [ - { value: "open", label: "Open", icon: GitPullRequestIcon }, - { value: "draft", label: "Draft", icon: GitPullRequestDraftIcon }, - { value: "merged", label: "Merged", icon: GitMergeIcon }, - { value: "closed", label: "Closed", icon: GitPullRequestClosedIcon }, - ]; - const colorMap: Record = { - open: "text-green-500", - draft: "text-muted-foreground", - merged: "text-purple-500", - closed: "text-red-500", - }; - return all - .filter((s) => statuses.has(s.value)) - .map((s) => ({ - value: s.value, - label: s.label, - icon: createElement(s.icon, { - size: 14, - className: colorMap[s.value], - }), - })); - }, - match: (item, values) => { - const pull = asPull(item); - if (pull.mergedAt) return values.has("merged"); - if (pull.state === "closed") return values.has("closed"); - if (pull.isDraft) return values.has("draft"); - return values.has("open"); - }, + value: "closed", + label: "Closed", + icon: GitPullRequestClosedIcon, + colorClass: "text-red-500", + }, +]; + +function toStatusOption(meta: (typeof PULL_STATUS_META)[number]): FilterOption { + return { + value: meta.value, + label: meta.label, + icon: createElement(meta.icon, { + size: 14, + className: meta.colorClass, + }), + }; +} + +const pullStatusFilterDef: FilterDefinition = { + id: "status", + label: "Status", + icon: CircleIcon, + extractOptions: (items) => { + const present = new Set(); + for (const item of items) present.add(pullStatus(asPull(item))); + return PULL_STATUS_META.filter((m) => present.has(m.value)).map( + toStatusOption, + ); }, + match: (item, values) => values.has(pullStatus(asPull(item))), +}; + +const repoPullStatusFilterDef: FilterDefinition = { + ...pullStatusFilterDef, + extractOptions: () => PULL_STATUS_META.map(toStatusOption), +}; + +export const pullFilterDefs: FilterDefinition[] = [ + repoFilterDef, + authorFilterDef, + pullStatusFilterDef, ]; /** Filter defs for repo-scoped pull lists — static options, no repository filter. */ export const repoPullFilterDefs: FilterDefinition[] = [ - { - id: "status", - label: "Status", - icon: CircleIcon, - extractOptions: () => { - const colorMap: Record = { - open: "text-green-500", - draft: "text-muted-foreground", - merged: "text-purple-500", - closed: "text-red-500", - }; - return ( - [ - { value: "open", label: "Open", icon: GitPullRequestIcon }, - { value: "draft", label: "Draft", icon: GitPullRequestDraftIcon }, - { value: "merged", label: "Merged", icon: GitMergeIcon }, - { value: "closed", label: "Closed", icon: GitPullRequestClosedIcon }, - ] as const - ).map((s) => ({ - value: s.value, - label: s.label, - icon: createElement(s.icon, { - size: 14, - className: colorMap[s.value], - }), - })); - }, - match: (item, values) => { - const pull = asPull(item); - if (pull.mergedAt) return values.has("merged"); - if (pull.state === "closed") return values.has("closed"); - if (pull.isDraft) return values.has("draft"); - return values.has("open"); - }, - }, - { - id: "author", - label: "Author", - icon: UserCircleIcon, - extractOptions: (items) => { - const authors = new Map(); - for (const item of items) { - if (item.author && !authors.has(item.author.login)) { - authors.set(item.author.login, item.author); - } - } - return [...authors.entries()] - .sort(([a], [b]) => a.localeCompare(b)) - .map(([login, author]) => ({ - value: login, - label: login, - icon: createElement("img", { - src: author.avatarUrl, - alt: login, - className: "size-4 rounded-full", - }), - })); - }, - match: (item, values) => - item.author ? values.has(item.author.login) : false, - }, + repoPullStatusFilterDef, + authorFilterDef, ]; export const pullSortOptions: SortOption[] = [ diff --git a/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx b/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx index 51a6547..ea1fe9f 100644 --- a/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx +++ b/apps/dashboard/src/components/pulls/detail/pull-detail-sidebar.tsx @@ -217,6 +217,15 @@ function ReviewersSection({ ); }, [teams, search]); + const collaboratorByLogin = useMemo( + () => new Map(collaborators.map((c) => [c.login, c])), + [collaborators], + ); + const teamBySlug = useMemo( + () => new Map(teams.map((t) => [t.slug, t])), + [teams], + ); + const pageQueryKey = githubQueryKeys.pulls.page(scope, { owner, repo, @@ -225,9 +234,7 @@ function ReviewersSection({ const toggleReviewer = (login: string) => { const isRequested = requestedLogins.has(login); - const collaborator = collaborators.find( - (candidate) => candidate.login === login, - ); + const collaborator = collaboratorByLogin.get(login); mutate({ mutationFn: () => @@ -271,7 +278,7 @@ function ReviewersSection({ const toggleTeam = (slug: string) => { const isRequested = requestedTeamSlugs.has(slug); - const team = teams.find((candidate) => candidate.slug === slug); + const team = teamBySlug.get(slug); mutate({ mutationFn: () =>