diff --git a/hub/app/controllers/prototype_authors_controller.rb b/hub/app/controllers/prototype_authors_controller.rb new file mode 100644 index 00000000..08546f26 --- /dev/null +++ b/hub/app/controllers/prototype_authors_controller.rb @@ -0,0 +1,27 @@ +# Offers bounded registration-author choices only within an authorized repository. +class PrototypeAuthorsController < ApplicationController + include HasRepositoryWorkspace + + rescue_from GithubClient::Error, with: -> { head :service_unavailable } + + # @route GET /repositories/:repository_github_id/prototype_authors (repository_prototype_authors) + def index + load_repository_workspace + return if performed? + + page = @repository.prototype_listing.authors \ + user: Current.user, + query: params[:q], + cursor: params[:cursor] + response.headers["Cache-Control"] = "private, no-store" + render \ + json: { + authors: page.fetch(:authors).map do |author| + { id: author.id.to_s, login: author.login, name: author.name, avatarUrl: author.avatar_url } + end, + nextCursor: page.fetch(:next_cursor) + } + rescue Repository::PrototypeListing::InvalidQuery + head :bad_request + end +end diff --git a/hub/app/controllers/repositories_controller.rb b/hub/app/controllers/repositories_controller.rb index acd45e7a..1d6dbf25 100644 --- a/hub/app/controllers/repositories_controller.rb +++ b/hub/app/controllers/repositories_controller.rb @@ -23,8 +23,13 @@ def show load_repository_workspace return if performed? - @prototypes = @repository.reviewable_prototypes.active.registered - .includes(:author, :current_build).order(registered_at: :desc, id: :desc) + @listing = @repository.prototype_listing.page \ + user: Current.user, + query: params[:q], + author: params[:author], + page: params[:page] + @prototypes = @listing.prototypes + ActiveRecord::Associations::Preloader.new(records: @prototypes, associations: [ :author, :current_build ]).call prototype_ids = @prototypes.map(&:id) @latest_builds = @prototypes.filter_map(&:current_build).index_by(&:prototype_id) missing_prototype_ids = prototype_ids - @latest_builds.keys @@ -47,6 +52,8 @@ def show @candidate_check_failed = true end end + rescue Repository::PrototypeListing::InvalidQuery + head :bad_request end # @route POST /repositories/:github_id/poll (poll_repository) diff --git a/hub/app/javascript/application/repositories-show-view/index.test.tsx b/hub/app/javascript/application/repositories-show-view/index.test.tsx index 43948a3f..0e97bb98 100644 --- a/hub/app/javascript/application/repositories-show-view/index.test.tsx +++ b/hub/app/javascript/application/repositories-show-view/index.test.tsx @@ -16,6 +16,9 @@ const state = vi.hoisted(() => ({ vi.mock("@hooks/use-page-data", () => ({ usePageData: () => state.data, })) +vi.mock("@hooks/use-prototype-listing", () => ({ + usePrototypeListing: () => ({}), +})) vi.mock("@pages/repositories-show-page", () => ({ RepositoriesShowPage: ({ managingPrototypes }: { managingPrototypes: boolean }) => ( {managingPrototypes ? "Managing prototypes" : "Idle"} diff --git a/hub/app/javascript/features/prototype-author-filter/index.test.tsx b/hub/app/javascript/features/prototype-author-filter/index.test.tsx new file mode 100644 index 00000000..6ab935fa --- /dev/null +++ b/hub/app/javascript/features/prototype-author-filter/index.test.tsx @@ -0,0 +1,116 @@ +import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react" +import { describe, expect, it, vi } from "vitest" + +import { prototypeListing } from "@/storybook/fixtures/prototype-listing" +import type { PrototypeListingControls } from "@utils/prototype-listing/types" + +import { PrototypeAuthorFilter } from "." + +describe("PrototypeAuthorFilter()", () => { + it("pins Me first and supports selecting and clearing an author with the keyboard", () => { + const onChange = vi.fn() + const onSearch = vi.fn(() => Promise.resolve({ authors: [], nextCursor: null })) + const { rerender } = render( + , + ) + const trigger = screen.getByRole("button", { name: "Author: All authors" }) + fireEvent.click(trigger) + const menu = screen.getByRole("menu", { name: "Author" }) + expect(within(menu).getAllByRole("menuitemradio")[0]).toHaveTextContent("Me") + expect(within(menu).getByRole("searchbox")).toHaveFocus() + expect( + within(menu).getByRole("menuitemradio", { name: "All authors", checked: true }), + ).toBeInTheDocument() + fireEvent.keyDown(within(menu).getByRole("searchbox"), { key: "ArrowDown" }) + const me = within(menu).getByRole("menuitemradio", { name: "Me" }) + expect(me).toHaveFocus() + fireEvent.keyDown(me, { key: " " }) + expect(onChange).toHaveBeenCalledExactlyOnceWith("me") + expect(screen.queryByRole("menu")).not.toBeInTheDocument() + expect(trigger).toHaveFocus() + rerender( + , + ) + fireEvent.click(screen.getByRole("button", { name: "Author: Me" })) + const all = screen.getByRole("menuitemradio", { name: "All authors", checked: false }) + all.focus() + fireEvent.keyDown(all, { key: "Enter" }) + expect(onChange).toHaveBeenCalledTimes(2) + expect(onChange).toHaveBeenLastCalledWith("") + expect(screen.queryByRole("menu")).not.toBeInTheDocument() + expect(trigger).toHaveFocus() + }) + + it("replaces author pages and ignores stale results after the search changes", async () => { + let finishOldSearch!: ( + results: Awaited>, + ) => void + const oldSearch = new Promise>>( + (resolve) => { + finishOldSearch = resolve + }, + ) + const onSearch = vi + .fn(() => + Promise.resolve({ + authors: [{ id: "2", login: "mona", name: null, avatarUrl: null }], + nextCursor: "next", + }), + ) + .mockResolvedValueOnce({ + authors: [{ id: "1", login: "octocat", name: null, avatarUrl: null }], + nextCursor: "next", + }) + render( + , + ) + fireEvent.click(screen.getByRole("button", { name: "Author: All authors" })) + expect(await screen.findByRole("menuitemradio", { name: "@octocat" })).toBeInTheDocument() + fireEvent.click(screen.getByRole("button", { name: "More" })) + expect(await screen.findByRole("menuitemradio", { name: "@mona" })).toBeInTheDocument() + expect(screen.queryByRole("menuitemradio", { name: "@octocat" })).not.toBeInTheDocument() + onSearch.mockReturnValueOnce(oldSearch) + fireEvent.change(screen.getByRole("searchbox"), { target: { value: "old" } }) + await waitFor(() => + expect(onSearch).toHaveBeenLastCalledWith("old", null, expect.any(AbortSignal)), + ) + fireEvent.change(screen.getByRole("searchbox"), { target: { value: "mona" } }) + await waitFor(() => + expect(onSearch).toHaveBeenLastCalledWith("mona", null, expect.any(AbortSignal)), + ) + expect(await screen.findByRole("menuitemradio", { name: "@mona" })).toBeInTheDocument() + await act(async () => { + finishOldSearch({ + authors: [{ id: "3", login: "outdated", name: null, avatarUrl: null }], + nextCursor: null, + }) + await oldSearch + }) + expect(screen.queryByRole("menuitemradio", { name: "@outdated" })).not.toBeInTheDocument() + expect(screen.getByRole("menuitemradio", { name: "@mona" })).toBeInTheDocument() + fireEvent.keyDown(screen.getByRole("searchbox"), { key: "Escape" }) + expect(screen.queryByRole("menu")).not.toBeInTheDocument() + }) + + it("reports lookup failures and retries without losing the pinned Me option", async () => { + const onSearch = vi + .fn(() => Promise.resolve({ authors: [], nextCursor: null })) + .mockRejectedValueOnce(new Error("Unavailable")) + render( + , + ) + fireEvent.click(screen.getByRole("button", { name: "Author: All authors" })) + expect(await screen.findByRole("alert")).toHaveTextContent(prototypeListing.copy.authorsFailed) + expect(screen.getByRole("menuitemradio", { name: "Me" })).toBeInTheDocument() + fireEvent.click(screen.getByRole("button", { name: "Retry" })) + expect(await screen.findByRole("status")).toHaveTextContent(prototypeListing.copy.loading) + await waitFor(() => expect(screen.queryByRole("status")).not.toBeInTheDocument()) + expect(screen.getByRole("menuitemradio", { name: "Me" })).toBeInTheDocument() + fireEvent.change(screen.getByRole("searchbox"), { target: { value: "missing" } }) + await waitFor(() => expect(screen.getByRole("status")).toHaveTextContent("No matching authors")) + }) +}) diff --git a/hub/app/javascript/features/prototype-author-filter/index.tsx b/hub/app/javascript/features/prototype-author-filter/index.tsx new file mode 100644 index 00000000..064d035e --- /dev/null +++ b/hub/app/javascript/features/prototype-author-filter/index.tsx @@ -0,0 +1,39 @@ +import { ActionMenu } from "@primer/react-brand" + +import type { PrototypeAuthorResults, PrototypeListing } from "@utils/prototype-listing/types" + +import { PrototypeAuthorOptions } from "./options" + +import "./styles.css" + +/** Offers a searchable, single-selection menu of saved registration authors. */ +export function PrototypeAuthorFilter({ + listing, + onChange, + onSearch, +}: { + listing: PrototypeListing + onChange: (author: string) => void + onSearch: ( + query: string, + cursor: string | null, + signal: AbortSignal, + ) => Promise +}) { + const label = `${listing.copy.author}: ${listing.authorLabel}` + + return ( +
+ + + + {label} + + + + + + +
+ ) +} diff --git a/hub/app/javascript/features/prototype-author-filter/options/index.tsx b/hub/app/javascript/features/prototype-author-filter/options/index.tsx new file mode 100644 index 00000000..9cf601d9 --- /dev/null +++ b/hub/app/javascript/features/prototype-author-filter/options/index.tsx @@ -0,0 +1,188 @@ +import { useEffect, useRef, useState } from "react" +import type { KeyboardEvent } from "react" + +import { ActionMenu, Button, TextInput } from "@primer/react-brand" + +import { HubSpinner } from "@components/hub-spinner" +import type { PrototypeAuthorResults, PrototypeListing } from "@utils/prototype-listing/types" + +/** Loads author choices only while the menu is mounted and cancels stale searches. */ +export function PrototypeAuthorOptions({ + listing, + onSearch, + handler, +}: { + listing: PrototypeListing + onSearch: ( + query: string, + cursor: string | null, + signal: AbortSignal, + ) => Promise + handler?: (author: string) => void +}) { + const [query, setQuery] = useState("") + const [cursor, setCursor] = useState(null) + const [results, setResults] = useState(null) + const [failed, setFailed] = useState(false) + const [retry, setRetry] = useState(0) + const input = useRef(null) + const copy = listing.copy + + useEffect(() => { + input.current?.focus() + }, []) + + useEffect(() => { + const controller = new AbortController() + const timer = setTimeout(() => { + onSearch(query, cursor, controller.signal).then( + (value) => { + if (!controller.signal.aborted) setResults(value) + }, + () => { + if (!controller.signal.aborted) setFailed(true) + }, + ) + }, 200) + return () => { + clearTimeout(timer) + controller.abort() + } + }, [query, cursor, retry, onSearch]) + + function selectOnKeyDown(event: KeyboardEvent) { + if (event.key !== " " && event.key !== "Enter") return + + event.preventDefault() + event.stopPropagation() + event.currentTarget.click() + } + + const choiceProps = { handler, type: "single" as const, onKeyDownCapture: selectOnKeyDown } + + return ( + <> +
  • { + if (event.target === event.currentTarget) input.current?.focus() + }} + > + { + if (event.key === "Enter" || event.key === " ") event.stopPropagation() + if (event.key === "ArrowDown") { + event.preventDefault() + event.stopPropagation() + event.currentTarget + .closest('[role="menu"]') + ?.querySelector('[role="menuitemradio"]') + ?.focus() + } + }} + onChange={(event) => { + setQuery(event.target.value) + setCursor(null) + setResults(null) + setFailed(false) + }} + /> +
    • + + + {copy.me} + + + {copy.allAuthors} + + + {failed ? ( +
  • +
    +

    {copy.authorsFailed}

    + +
    +
    • + ) : !results ? ( +
  • +
    + + {copy.loading} +
    +
    • + ) : !results.authors.length && query.trim() ? ( +
  • +

    {copy.noAuthors}

    +
    • + ) : ( + results.authors.map((author) => ( + + + @{author.login} + {author.name ? ` (${author.name})` : ""} + + + )) + )} + + {results?.nextCursor || cursor ? ( +
  • + {results?.nextCursor ? ( + + ) : null} + {cursor ? ( + + ) : null} +
    • + ) : null} + + ) +} diff --git a/hub/app/javascript/features/prototype-author-filter/stories.tsx b/hub/app/javascript/features/prototype-author-filter/stories.tsx new file mode 100644 index 00000000..f592be90 --- /dev/null +++ b/hub/app/javascript/features/prototype-author-filter/stories.tsx @@ -0,0 +1,86 @@ +import type { Meta, StoryObj } from "@storybook/react-vite" +import { useState } from "react" +import { fn } from "storybook/test" + +import { prototypeListing } from "@/storybook/fixtures/prototype-listing" +import { mona } from "@/storybook/fixtures/users" + +import { PrototypeAuthorFilter } from "." + +const meta = { + title: "Features/Prototype Author Filter", + component: PrototypeAuthorFilter, + args: { + listing: prototypeListing, + onChange: fn(), + onSearch: fn(() => + Promise.resolve({ + authors: [{ id: "2", ...mona }], + nextCursor: null, + }), + ), + }, + render: function AuthorFilterStory(args) { + const [author, setAuthor] = useState(args.listing.author) + return ( + { + setAuthor(value) + args.onChange(value) + }} + /> + ) + }, +} satisfies Meta + +export default meta +type Story = StoryObj + +export const Empty: Story = { + args: { onSearch: fn(() => Promise.resolve({ authors: [], nextCursor: null })) }, +} + +export const Single: Story = {} + +export const Me: Story = { + args: { listing: { ...prototypeListing, author: "me", authorLabel: "Me" } }, +} + +export const LongAuthor: Story = { + args: { + listing: { + ...prototypeListing, + author: "2", + authorLabel: `${mona.name} (@${mona.login})`.repeat(5), + }, + onSearch: fn((_query: string, cursor: string | null) => + Promise.resolve({ + authors: [{ id: "2", ...mona, name: mona.name.repeat(10) }], + nextCursor: cursor ? null : "next", + }), + ), + }, +} + +export const Failed: Story = { + args: { + onSearch: fn(() => Promise.reject(new Error("Unavailable"))), + }, +} + +export const Loading: Story = { + args: { onSearch: fn(() => new Promise(() => {})) }, +} diff --git a/hub/app/javascript/features/prototype-author-filter/styles.css b/hub/app/javascript/features/prototype-author-filter/styles.css new file mode 100644 index 00000000..bced5040 --- /dev/null +++ b/hub/app/javascript/features/prototype-author-filter/styles.css @@ -0,0 +1,87 @@ +.prototype-author-filter { + width: 16rem; + max-width: 100%; +} + +.prototype-author-filter [aria-haspopup], +.prototype-author-filter [role="menu"] { + width: 100%; + min-width: 0; +} + +.prototype-author-filter [role="menuitemradio"] { + min-width: 0; + grid-template-columns: min-content minmax(0, 1fr); + padding-inline-end: var(--brand-control-small-paddingInline-condensed); +} + +.prototype-author-filter [aria-haspopup] { + justify-content: space-between; + text-align: start; + color: var(--hub-fg-default); +} + +.prototype-author-filter [aria-haspopup]:not(:hover):not(:active):not([aria-expanded="true"]) { + background-color: var(--hub-canvas-default); +} + +.prototype-author-filter [aria-haspopup] > span:first-child, +.prototype-author-filter [role="menuitemradio"] > span:last-child { + min-width: 0; + overflow: hidden; + flex: 1; + width: auto; +} + +.prototype-author-filter [aria-haspopup] > span:first-child > span { + min-width: 0; + width: 100%; + text-align: start; +} + +.prototype-author-filter [role="menuitemradio"] > span:last-child > span { + min-width: 0; +} + +.prototype-author-filter__label { + display: block; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.prototype-author-filter__search { + margin-bottom: var(--base-size-8); +} + +.prototype-author-filter__search, +.prototype-author-filter__status, +.prototype-author-filter__paging { + list-style: none; +} + +.prototype-author-filter__status { + padding: var(--base-size-8); +} + +.prototype-author-filter__loading { + display: flex; + align-items: center; + gap: var(--base-size-12); + padding: var(--base-size-8) var(--brand-control-small-paddingInline-condensed); + color: var(--hub-fg-muted); +} + +.prototype-author-filter__paging { + display: flex; + justify-content: center; + flex-wrap: wrap; + gap: var(--base-size-8); + margin-top: var(--base-size-8); +} + +@media (prefers-reduced-motion: reduce) { + .prototype-author-filter [role="menu"] { + animation: none; + } +} diff --git a/hub/app/javascript/features/prototype-index-header/index.tsx b/hub/app/javascript/features/prototype-index-header/index.tsx index 50b9931e..0b02ad5c 100644 --- a/hub/app/javascript/features/prototype-index-header/index.tsx +++ b/hub/app/javascript/features/prototype-index-header/index.tsx @@ -4,6 +4,8 @@ import type { ReactNode } from "react" import { XIcon } from "@primer/octicons-react" import { Heading, Text } from "@primer/react-brand" +import { HubIconButton } from "@components/hub-icon-button" + import "./styles.css" /** Heads the prototype collection and filters its contents. */ @@ -13,20 +15,26 @@ export function PrototypeIndexHeader({ heading, headingAs = "h1", onSearchChange, + onSearchSubmit, searchLabel, searchQuery, + searchMaxLength, searchShortcutEnabled = true, tools, + trailingTools, }: { action?: ReactNode clearSearchLabel: string heading: string headingAs?: "h1" | "h2" onSearchChange: (query: string) => void + onSearchSubmit?: () => void searchLabel: string searchQuery: string + searchMaxLength?: number searchShortcutEnabled?: boolean tools?: ReactNode + trailingTools?: ReactNode }) { const searchInputRef = useRef(null) @@ -67,19 +75,26 @@ export function PrototypeIndexHeader({ placeholder={searchLabel} value={searchQuery} onChange={(event) => onSearchChange(event.target.value)} + maxLength={searchMaxLength} + onKeyDown={(event) => { + if (event.key === "Enter" && !event.nativeEvent.isComposing) { + event.preventDefault() + onSearchSubmit?.() + } + }} /> {searchQuery ? ( - + /> ) : null} + + {trailingTools ? ( +
    {trailingTools}
    + ) : null} ) diff --git a/hub/app/javascript/features/prototype-index-header/styles.css b/hub/app/javascript/features/prototype-index-header/styles.css index 9c28002f..2220d775 100644 --- a/hub/app/javascript/features/prototype-index-header/styles.css +++ b/hub/app/javascript/features/prototype-index-header/styles.css @@ -34,6 +34,12 @@ height: var(--base-size-36); } +.prototype-index-header__trailing-tools { + display: flex; + align-items: center; + margin-inline-start: var(--base-size-8); +} + .prototype-index-header__search input { width: 100%; height: 100%; @@ -94,20 +100,23 @@ opacity: 0.82; } -.prototype-index-header__clear-search { +.prototype-index-header__clear-search:is(button) { position: absolute; - top: 0.25rem; + top: 50%; right: 2.9375rem; display: grid; - width: 1.75rem; - height: 1.75rem; + width: var(--base-size-28); + min-width: var(--base-size-28); + height: var(--base-size-28); + min-height: var(--base-size-28); padding: 0; place-items: center; border: 0; border-radius: var(--base-size-6); - color: var(--hub-fg-muted); + color: var(--hub-fg-default); background: transparent; cursor: pointer; + transform: translateY(-50%); } .prototype-index-header__clear-search:hover { @@ -132,4 +141,9 @@ .prototype-index-header__search { width: 100%; } + + .prototype-index-header__trailing-tools { + align-self: flex-end; + margin-inline-start: 0; + } } diff --git a/hub/app/javascript/hooks/use-prototype-listing/index.test.tsx b/hub/app/javascript/hooks/use-prototype-listing/index.test.tsx new file mode 100644 index 00000000..bae77c34 --- /dev/null +++ b/hub/app/javascript/hooks/use-prototype-listing/index.test.tsx @@ -0,0 +1,210 @@ +import { NavigationContext } from "@thoughtbot/superglue" +import { act, renderHook } from "@testing-library/react" +import type { ReactNode } from "react" +import { describe, expect, it, vi } from "vitest" + +import { prototypeListing } from "@/storybook/fixtures/prototype-listing" + +import { usePrototypeListing } from "." + +function navigation(visit = vi.fn(() => Promise.resolve(undefined))) { + return function Wrapper({ children }: { children: ReactNode }) { + return ( + + {children} + + ) + } +} + +describe("usePrototypeListing()", () => { + it("debounces search but immediately submits author changes and cancels pending edits on reset", async () => { + vi.useFakeTimers() + try { + const visit = vi.fn(() => Promise.resolve(undefined)) + const { result } = renderHook( + () => usePrototypeListing({ ...prototypeListing, author: "me" }), + { wrapper: navigation(visit) }, + ) + act(() => { + result.current.onQueryChange("nav") + result.current.onQueryChange(" navigation ") + }) + expect(visit).not.toHaveBeenCalled() + await act(async () => vi.advanceTimersByTimeAsync(300)) + expect(visit).toHaveBeenCalledExactlyOnceWith("/repositories/2001?q=navigation&author=me", { + dataset: { sgReplace: "true" }, + }) + expect(result.current.query).toBe("navigation") + act(() => { + result.current.onQueryChange("immediate") + result.current.onQuerySubmit() + }) + await act(async () => vi.advanceTimersByTimeAsync(500)) + expect(visit).toHaveBeenCalledTimes(2) + expect(visit).toHaveBeenLastCalledWith("/repositories/2001?q=immediate&author=me", { + dataset: {}, + }) + act(() => result.current.onQueryChange("checkout")) + await act(async () => { + result.current.onAuthorChange("2") + await Promise.resolve() + }) + expect(visit).toHaveBeenLastCalledWith("/repositories/2001?q=checkout&author=2", { + dataset: {}, + }) + act(() => result.current.onQueryChange("next draft")) + await act(async () => { + result.current.onQueryChange("") + await Promise.resolve() + }) + await act(async () => vi.advanceTimersByTimeAsync(500)) + expect(visit).toHaveBeenCalledTimes(4) + expect(visit).toHaveBeenLastCalledWith("/repositories/2001?author=2", { + dataset: { sgReplace: "true" }, + }) + expect(result.current.query).toBe("") + } finally { + vi.useRealTimers() + } + }) + + it("ignores superseded visit failures and retries the current failed request", async () => { + let rejectOld!: (error: Error) => void + const old = new Promise((_resolve, reject) => { + rejectOld = reject + }) + const failure = new Error("Repository unavailable") + const visit = vi + .fn(() => Promise.resolve(undefined)) + .mockReturnValueOnce(old) + .mockRejectedValueOnce(failure) + const { result } = renderHook(() => usePrototypeListing(prototypeListing), { + wrapper: navigation(visit), + }) + act(() => result.current.onNavigate("/repositories/2001?page=2")) + await act(async () => { + result.current.onAuthorChange("me") + await Promise.resolve() + }) + expect(result.current.error).toBe(true) + await act(async () => { + result.current.onRetry() + await Promise.resolve() + }) + expect(result.current.error).toBe(false) + expect(visit).toHaveBeenLastCalledWith("/repositories/2001?author=me", { dataset: {} }) + await act(async () => { + rejectOld(failure) + await Promise.resolve() + }) + expect(result.current.error).toBe(false) + expect(result.current.pending).toBe(false) + }) + + it("restores URL query state on browser history and cancels pending edits on unmount", async () => { + vi.useFakeTimers() + const originalUrl = window.location.href + try { + const visit = vi.fn(() => Promise.resolve(undefined)) + const { result, unmount } = renderHook(() => usePrototypeListing(prototypeListing), { + wrapper: navigation(visit), + }) + act(() => result.current.onQueryChange("unsent")) + act(() => { + window.history.replaceState({}, "", "/repositories/2001?q=restored&author=me") + window.dispatchEvent(new PopStateEvent("popstate")) + }) + expect(result.current.query).toBe("restored") + await act(async () => vi.advanceTimersByTimeAsync(500)) + expect(visit).not.toHaveBeenCalled() + act(() => result.current.onQueryChange("another unsent edit")) + unmount() + await act(async () => vi.advanceTimersByTimeAsync(500)) + expect(visit).not.toHaveBeenCalled() + } finally { + window.history.replaceState({}, "", originalUrl) + vi.useRealTimers() + } + }) + + it("loads validated author pages with same-origin credentials and the caller's cancellation signal", async () => { + const fetch = vi.fn(() => + Promise.resolve( + new Response( + JSON.stringify({ + authors: [{ id: "2", login: "mona", name: "Mona", avatarUrl: null }], + nextCursor: "next", + }), + ), + ), + ) + vi.stubGlobal("fetch", fetch) + const { result } = renderHook(() => usePrototypeListing(prototypeListing), { + wrapper: navigation(), + }) + const controller = new AbortController() + await expect( + result.current.loadAuthors(" mona ", "boundary", controller.signal), + ).resolves.toMatchObject({ authors: [{ id: "2", login: "mona" }], nextCursor: "next" }) + expect(fetch).toHaveBeenCalledWith( + new URL( + "/repositories/2001/prototype_authors?q=mona&cursor=boundary", + window.location.origin, + ), + { + headers: { Accept: "application/json" }, + credentials: "same-origin", + signal: controller.signal, + }, + ) + fetch.mockResolvedValueOnce(new Response("", { status: 403 })) + await expect(result.current.loadAuthors("", null, controller.signal)).rejects.toThrow() + fetch.mockResolvedValueOnce(new Response(JSON.stringify({ authors: "invalid" }))) + await expect(result.current.loadAuthors("", null, controller.signal)).rejects.toThrow() + }) + + it("discards old drafts, requests, and retry targets when switching repositories", async () => { + vi.useFakeTimers() + try { + let rejectOld!: (error: Error) => void + const visit = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectOld = reject + }), + ) + const { result, rerender } = renderHook((listing) => usePrototypeListing(listing), { + initialProps: prototypeListing, + wrapper: navigation(visit), + }) + act(() => result.current.onNavigate("/repositories/2001?page=2")) + act(() => result.current.onQueryChange("unsent old repository search")) + expect(result.current.pending).toBe(true) + rerender({ + ...prototypeListing, + path: "/repositories/2002", + authorsPath: "/repositories/2002/prototype_authors", + query: "new repository", + author: "me", + }) + expect(result.current.query).toBe("new repository") + expect(result.current.pending).toBe(false) + await act(async () => { + rejectOld(new Error("Old repository request failed")) + await vi.advanceTimersByTimeAsync(500) + }) + expect(result.current.error).toBe(false) + act(() => result.current.onRetry()) + expect(visit).toHaveBeenCalledTimes(1) + act(() => result.current.onQuerySubmit()) + expect(visit).toHaveBeenLastCalledWith("/repositories/2002?q=new+repository&author=me", { + dataset: {}, + }) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/hub/app/javascript/hooks/use-prototype-listing/index.ts b/hub/app/javascript/hooks/use-prototype-listing/index.ts new file mode 100644 index 00000000..a60abdcc --- /dev/null +++ b/hub/app/javascript/hooks/use-prototype-listing/index.ts @@ -0,0 +1,179 @@ +import { NavigationContext } from "@thoughtbot/superglue" +import { useCallback, useContext, useEffect, useRef, useState } from "react" +import { z } from "zod" + +import type { PrototypeAuthorResults, PrototypeListing } from "@utils/prototype-listing/types" + +/** Keeps collection navigation in Superglue and author lookups independently cancellable. */ +export function usePrototypeListing(listing: PrototypeListing) { + const { visit } = useContext(NavigationContext) + const [query, setQuery] = useState(listing.query) + const [pending, setPending] = useState(false) + const [error, setError] = useState(false) + const timer = useRef | null>(null) + const version = useRef(0) + const draft = useRef(listing.query) + const author = useRef(listing.author) + const source = useRef(listing.path) + const editing = useRef(false) + const retry = useRef<{ path: string; replace: boolean; focusResults: boolean } | null>(null) + + const cancelTimer = useCallback(() => { + if (timer.current) clearTimeout(timer.current) + timer.current = null + }, []) + + useEffect(() => { + if (source.current === listing.path) return + + source.current = listing.path + cancelTimer() + version.current += 1 + editing.current = false + draft.current = listing.query + author.current = listing.author + retry.current = null + setQuery(listing.query) + setPending(false) + setError(false) + }, [cancelTimer, listing.path, listing.query, listing.author]) + + useEffect(() => { + if (!editing.current) { + draft.current = listing.query + setQuery(listing.query) + } + author.current = listing.author + }, [listing.query, listing.author]) + + useEffect(() => { + function restoreHistory() { + cancelTimer() + version.current += 1 + editing.current = false + draft.current = new URLSearchParams(window.location.search).get("q") ?? "" + author.current = new URLSearchParams(window.location.search).get("author") ?? "" + setQuery(draft.current) + setPending(false) + setError(false) + } + window.addEventListener("popstate", restoreHistory) + return () => { + window.removeEventListener("popstate", restoreHistory) + cancelTimer() + version.current += 1 + } + }, [cancelTimer, listing.path]) + + const navigate = useCallback( + (path: string, replace = false, focusResults = false) => { + cancelTimer() + const currentVersion = ++version.current + retry.current = { path, replace, focusResults } + setPending(true) + setError(false) + const requestedQuery = new URL(path, window.location.origin).searchParams.get("q") ?? "" + void visit(path, { dataset: replace ? { sgReplace: "true" } : {} }).then( + () => { + if (version.current !== currentVersion) return + + setPending(false) + if (draft.current.trim() === requestedQuery) { + editing.current = false + draft.current = requestedQuery + setQuery(requestedQuery) + } + if (focusResults) { + window.requestAnimationFrame(() => { + if (version.current === currentVersion) { + document.querySelector("[data-prototype-results]")?.focus() + } + }) + } + }, + () => { + if (version.current !== currentVersion) return + + setPending(false) + setError(true) + }, + ) + }, + [cancelTimer, visit], + ) + + const applyFilters = useCallback( + (nextQuery: string, nextAuthor: string, replace = false) => { + const url = new URL(listing.path, window.location.origin) + if (nextQuery.trim()) url.searchParams.set("q", nextQuery.trim()) + if (nextAuthor) url.searchParams.set("author", nextAuthor) + navigate(`${url.pathname}${url.search}`, replace) + }, + [listing.path, navigate], + ) + + const loadAuthors = useCallback( + async ( + search: string, + cursor: string | null, + signal: AbortSignal, + ): Promise => { + const url = new URL(listing.authorsPath, window.location.origin) + if (search.trim()) url.searchParams.set("q", search.trim()) + if (cursor) url.searchParams.set("cursor", cursor) + const response = await fetch(url, { + headers: { Accept: "application/json" }, + credentials: "same-origin", + signal, + }) + if (!response.ok) throw new Error("Prototype author search failed") + + return z + .object({ + authors: z.array( + z.object({ + id: z.string(), + login: z.string(), + name: z.string().nullable(), + avatarUrl: z.string().nullable(), + }), + ), + nextCursor: z.string().nullable(), + }) + .parse(await response.json()) + }, + [listing.authorsPath], + ) + + return { + query, + pending, + error, + loadAuthors, + onQueryChange(value: string) { + cancelTimer() + editing.current = true + draft.current = value + setQuery(value) + if (!value) { + applyFilters("", author.current, true) + } else { + timer.current = setTimeout(() => applyFilters(value, author.current, true), 300) + } + }, + onQuerySubmit() { + applyFilters(draft.current, author.current) + }, + onAuthorChange(value: string) { + author.current = value + applyFilters(draft.current, value) + }, + onNavigate(path: string) { + navigate(path, false, true) + }, + onRetry() { + if (retry.current) + navigate(retry.current.path, retry.current.replace, retry.current.focusResults) + }, + } +} diff --git a/hub/app/javascript/pages/repositories-show-page/index.test.tsx b/hub/app/javascript/pages/repositories-show-page/index.test.tsx index 39808fa7..76f01a62 100644 --- a/hub/app/javascript/pages/repositories-show-page/index.test.tsx +++ b/hub/app/javascript/pages/repositories-show-page/index.test.tsx @@ -1,10 +1,15 @@ import { fireEvent, render, screen } from "@testing-library/react" -import { describe, it } from "vitest" +import type { ComponentProps } from "react" +import { describe, it, vi } from "vitest" -import { RepositoriesShowPage } from "./index" +import { prototypeListing } from "@/storybook/fixtures/prototype-listing" +import type { PrototypeListingControls } from "@utils/prototype-listing/types" + +import { RepositoriesShowPage as Page } from "./index" import type { RepositoriesShowPageData } from "./types" const PAGE_DATA: RepositoriesShowPageData = { + listing: prototypeListing, accountMenuLabel: "Account menu for @octocat", darkThemeLabel: "Dark", feedbackLabel: "Feedback", @@ -22,7 +27,7 @@ const PAGE_DATA: RepositoriesShowPageData = { galleryViewLabel: "Gallery view", listViewLabel: "List view", prototypeColumnLabel: "Prototype", - ownerColumnLabel: "Owner", + ownerColumnLabel: "Author", buildColumnLabel: "Build", passingLabel: "passing", notPassingLabel: "not passing", @@ -56,8 +61,6 @@ const PAGE_DATA: RepositoriesShowPageData = { }, ], prototypesCountLabel: "1 prototype", - prototypeCountUnitOne: "prototype", - prototypeCountUnitOther: "prototypes", prototypesHeading: "Prototypes", prototypesLabel: "Prototypes", prototypesPath: "/repositories/2001", @@ -81,7 +84,85 @@ const PAGE_DATA: RepositoriesShowPageData = { }, } +function RepositoriesShowPage({ + controls, + ...props +}: Omit, "controls"> & { controls?: PrototypeListingControls }) { + return ( + + ) +} + +function listingControls( + overrides: Partial = {}, +): PrototypeListingControls { + return { + query: "", + pending: false, + error: false, + onQueryChange: vi.fn(), + onQuerySubmit: vi.fn(), + onAuthorChange: vi.fn(), + onNavigate: vi.fn(), + onRetry: vi.fn(), + loadAuthors: vi.fn(() => Promise.resolve({ authors: [], nextCursor: null })), + ...overrides, + } +} + describe("RepositoriesShowPage()", () => { + it("navigates a page once while keeping ordinary links available for modified clicks", () => { + const controls = listingControls() + const delegatedClick = vi.fn() + render( +
    + +
    , + ) + const next = screen.getByRole("button", { name: "Page 3" }) + expect(screen.getByRole("button", { name: "Page 1" })).toHaveAttribute("aria-current", "page") + expect(next).toHaveAttribute("href", "/repositories/2001?q=design&author=me&page=3") + expect(screen.getByText("80 prototypes (1 shown)")).toBeInTheDocument() + fireEvent.click(next) + expect(controls.onNavigate).toHaveBeenCalledExactlyOnceWith( + "/repositories/2001?q=design&author=me&page=3", + ) + expect(delegatedClick).not.toHaveBeenCalled() + fireEvent.click(next, { ctrlKey: true }) + expect(controls.onNavigate).toHaveBeenCalledTimes(1) + expect(delegatedClick).toHaveBeenCalledTimes(1) + }) + + it("retains known cards during loading and request failure and exposes retry", () => { + const controls = listingControls({ pending: true }) + const { rerender } = render() + expect(screen.getByRole("region", { name: "Prototypes" })).toHaveAttribute("aria-busy", "true") + expect(screen.getByRole("status")).toHaveTextContent(prototypeListing.copy.updating) + expect(screen.getByRole("link", { name: "Welcome" })).toBeInTheDocument() + rerender( + , + ) + expect(screen.getByText(prototypeListing.copy.resultsFailed)).toBeInTheDocument() + expect(screen.getByRole("link", { name: "Welcome" })).toBeInTheDocument() + fireEvent.click(screen.getByRole("button", { name: "Retry" })) + expect(controls.onRetry).toHaveBeenCalledOnce() + }) + it("invites people to build their first prototype with a decorative branch icon", () => { const { container } = render( { "src", "/repositories/2001/prototypes/1/builds/1/screenshots/desktop", ) - expect(screen.getByRole("columnheader", { name: "Owner" })).toBeInTheDocument() + expect(screen.getByRole("columnheader", { name: "Author" })).toBeInTheDocument() expect(screen.getByRole("columnheader", { name: "Build" })).toBeInTheDocument() expect(screen.getByRole("cell", { name: /@octocat/ })).toBeInTheDocument() const authorLinks = screen.getAllByRole("link", { name: /@octocat/ }) @@ -253,7 +334,7 @@ describe("RepositoriesShowPage()", () => { ) }) - it("preserves newest-first server order in gallery, list, and search results", () => { + it("preserves server order in gallery and list views", () => { render( { "Z newest prototype", "A oldest prototype", ]) - - fireEvent.change(screen.getByRole("searchbox", { name: "Search prototypes" }), { - target: { value: "prototype" }, - }) - expect(prototypeLinks().map((link) => link.textContent)).toEqual([ - "Z newest prototype", - "A oldest prototype", - ]) - - fireEvent.click(screen.getByRole("radio", { name: "Gallery view" })) - expect(prototypeLinks().map((link) => link.textContent)).toEqual([ - "Z newest prototype", - "A oldest prototype", - ]) }) it("offers separate registration and personal dismissal forms for each recent branch", () => { @@ -446,56 +513,40 @@ describe("RepositoriesShowPage()", () => { ).toBeInTheDocument() }) - it("filters gallery and list by name, owner, description, and branch with a visible count", () => { - render( + it("delegates search without filtering server results again and renders empty results", () => { + const controls = listingControls({ query: "octocat" }) + const { rerender } = render( , ) + expect(screen.getByRole("link", { name: "Welcome" })).toBeInTheDocument() const search = screen.getByRole("searchbox", { name: "Search prototypes" }) - fireEvent.change(search, { target: { value: " MONA " } }) - expect(screen.getByText("1 prototype")).toBeInTheDocument() - expect(screen.getByRole("link", { name: "Navigation" })).toBeInTheDocument() - expect(screen.queryByRole("link", { name: "Welcome" })).not.toBeInTheDocument() + fireEvent.change(search, { target: { value: "missing" } }) + expect(controls.onQueryChange).toHaveBeenCalledWith("missing") + fireEvent.keyDown(search, { key: "Enter" }) + expect(controls.onQuerySubmit).toHaveBeenCalledOnce() - fireEvent.click(screen.getByRole("radio", { name: "List view" })) - expect(screen.getAllByRole("row")).toHaveLength(2) - expect(screen.getByRole("link", { name: "Navigation" })).toBeInTheDocument() - - fireEvent.change(search, { target: { value: "prototype/welcome" } }) - expect(screen.getByRole("link", { name: "Welcome" })).toBeInTheDocument() - fireEvent.change(search, { target: { value: "another layout" } }) - expect(screen.getByRole("link", { name: "Navigation" })).toBeInTheDocument() - fireEvent.change(search, { target: { value: "null" } }) + rerender( + , + ) expect(screen.getByText("0 prototypes")).toBeInTheDocument() expect(screen.getByText("No prototypes found")).toBeInTheDocument() expect(screen.queryByRole("link", { name: "Welcome" })).not.toBeInTheDocument() fireEvent.click(screen.getAllByRole("button", { name: "Clear search" })[0]!) - expect(search).toHaveValue("") + expect(controls.onQueryChange).toHaveBeenLastCalledWith("") expect(search).toHaveFocus() - expect(screen.getByText("2 prototypes")).toBeInTheDocument() }) it("focuses search with Command K and Control K", () => { diff --git a/hub/app/javascript/pages/repositories-show-page/index.tsx b/hub/app/javascript/pages/repositories-show-page/index.tsx index aa84a38f..d857f8df 100644 --- a/hub/app/javascript/pages/repositories-show-page/index.tsx +++ b/hub/app/javascript/pages/repositories-show-page/index.tsx @@ -1,7 +1,7 @@ import { useState } from "react" import { GearIcon, GitBranchIcon, SearchIcon } from "@primer/octicons-react" -import { Button, Text } from "@primer/react-brand" +import { Button, Pagination, Text } from "@primer/react-brand" import { HubAlert } from "@components/hub-alert" import { HubEmptyState } from "@components/hub-empty-state" @@ -10,8 +10,10 @@ import { PrototypeCard } from "@components/prototype-card" import { PrototypeViewModeControl } from "@components/prototype-view-mode-control" import type { PrototypeViewMode } from "@components/prototype-view-mode-control/types" import { PrototypeIndexHeader } from "@features/prototype-index-header" +import { PrototypeAuthorFilter } from "@features/prototype-author-filter" import { RepositoryWorkspace } from "@features/repository-workspace" import { WorkflowInstallation } from "@features/workflow-installation" +import type { PrototypeListingControls } from "@utils/prototype-listing/types" import type { RepositoriesShowPageData } from "./types" @@ -20,24 +22,21 @@ import "./styles.css" /** Shows branches after the server confirms repository access. */ export function RepositoriesShowPage({ data, + controls, managingPrototypes = false, }: { data: RepositoriesShowPageData + controls: PrototypeListingControls managingPrototypes?: boolean }) { const [view, setView] = useState("gallery") - const [searchQuery, setSearchQuery] = useState("") - const normalizedQuery = searchQuery.trim().toLowerCase() - const visiblePrototypes = data.prototypes.filter((prototype) => - `${prototype.name} ${prototype.description ?? ""} ${prototype.author.ownerLabel} ${prototype.branchName}` - .toLowerCase() - .includes(normalizedQuery), - ) - const countLabel = normalizedQuery - ? `${visiblePrototypes.length} ${ - visiblePrototypes.length === 1 ? data.prototypeCountUnitOne : data.prototypeCountUnitOther - }` - : data.prototypesCountLabel + const filtered = Boolean(data.listing.query || data.listing.author) + const visiblePrototypes = data.prototypes + function pageHref(page: number) { + const url = new URL(data.listing.pagePath, window.location.origin) + if (page > 1) url.searchParams.set("page", String(page)) + return `${url.pathname}${url.search}` + } const managePrototypesLabelIndex = data.managePrototypesLabel ? data.emptyLabel.indexOf(data.managePrototypesLabel) : -1 @@ -81,27 +80,59 @@ export function RepositoriesShowPage({ } clearSearchLabel={data.clearSearchLabel} heading={data.prototypesHeading} - onSearchChange={setSearchQuery} + onSearchChange={controls.onQueryChange} + onSearchSubmit={controls.onQuerySubmit} searchLabel={data.searchLabel} - searchQuery={searchQuery} + searchQuery={controls.query} + searchMaxLength={200} tools={
    - {countLabel} + {data.prototypesCountLabel} + {data.listing.matchingCount > data.prototypes.length + ? ` (${data.listing.copy.shown})` + : ""} -
    } + trailingTools={ + + } /> -
    +
    + {controls.pending ? {data.listing.copy.updating} : null} + {controls.error ? ( + + {data.listing.copy.retry} + + } + /> + ) : null} {data.alert ? : null} {data.notice ? : null} @@ -159,13 +190,11 @@ export function RepositoriesShowPage({ /> ) : null} - {normalizedQuery && visiblePrototypes.length === 0 ? ( + {filtered && visiblePrototypes.length === 0 ? ( } heading={data.noSearchResultsHeading} description={data.noSearchResultsDescription} - actionLabel={data.clearSearchLabel} - onAction={() => setSearchQuery("")} /> ) : data.prototypes.length === 0 ? ( data.newPrototypePath ? ( @@ -212,6 +241,36 @@ export function RepositoriesShowPage({ )} + + {data.listing.pageCount > 1 ? ( + ({ "data-sg-visit": "true" })} + labels={{ + prev: data.listing.copy.previous, + prevAriaLabel: data.listing.copy.previous, + next: data.listing.copy.next, + nextAriaLabel: data.listing.copy.next, + }} + onPageChange={(event, page) => { + if ( + event.button === 0 && + !event.metaKey && + !event.ctrlKey && + !event.shiftKey && + !event.altKey + ) { + event.preventDefault() + event.stopPropagation() + controls.onNavigate(pageHref(page)) + } + }} + /> + ) : null}
    diff --git a/hub/app/javascript/pages/repositories-show-page/stories.tsx b/hub/app/javascript/pages/repositories-show-page/stories.tsx index ea013f4a..5bcd881d 100644 --- a/hub/app/javascript/pages/repositories-show-page/stories.tsx +++ b/hub/app/javascript/pages/repositories-show-page/stories.tsx @@ -1,6 +1,8 @@ import type { Meta, StoryObj } from "@storybook/react-vite" +import { fn } from "storybook/test" import { authenticatedHeaderData } from "@/storybook/fixtures/authenticated-header" +import { prototypeListing } from "@/storybook/fixtures/prototype-listing" import { welcomePrototype } from "@/storybook/fixtures/prototypes" import { githubEmpty, githubPrototype } from "@/storybook/fixtures/repositories" @@ -8,6 +10,7 @@ import { RepositoriesShowPage } from "." import type { RepositoriesShowPageData } from "./types" const data: RepositoriesShowPageData = { + listing: { ...prototypeListing, matchingCount: 0 }, ...authenticatedHeaderData, alert: null, collapseNavigationLabel: "Collapse sidebar", @@ -21,7 +24,7 @@ const data: RepositoriesShowPageData = { managePrototypesLabel: "Manage prototypes", managePrototypesPath: "/repositories/2001/prototypes/manage", prototypeColumnLabel: "Prototype", - ownerColumnLabel: "Owner", + ownerColumnLabel: "Author", buildColumnLabel: "Build", passingLabel: "passing", notPassingLabel: "not passing", @@ -35,8 +38,6 @@ const data: RepositoriesShowPageData = { pageTitle: "github/prototype", prototypes: [], prototypesCountLabel: "0 prototypes", - prototypeCountUnitOne: "prototype", - prototypeCountUnitOther: "prototypes", prototypesHeading: "Prototypes", prototypesLabel: "Prototypes", prototypesPath: "/repositories/2001", @@ -58,7 +59,25 @@ const data: RepositoriesShowPageData = { const meta = { title: "Hub/Pages/Repositories/Show", component: RepositoriesShowPage, - args: { data }, + args: { + data, + controls: { + query: "", + pending: false, + error: false, + onQueryChange: fn(), + onQuerySubmit: fn(), + onAuthorChange: fn(), + onNavigate: fn(), + onRetry: fn(), + loadAuthors: fn(() => + Promise.resolve({ + authors: [], + nextCursor: null, + }), + ), + }, + }, parameters: { layout: "fullscreen" }, } satisfies Meta @@ -175,9 +194,78 @@ export const WideViewSwitching: Story = { } export const Searched: Story = { + args: { + controls: { ...meta.args.controls, query: "Navigation" }, + data: { + ...data, + listing: { ...prototypeListing, query: "Navigation" }, + prototypes: [ + { + ...welcomePrototype, + id: "2", + name: "Navigation", + path: "/repositories/2001/prototypes/2", + }, + ], + prototypesCountLabel: "1 prototype", + }, + }, +} + +export const MeSelected: Story = { + ...Single, + args: { + ...Single.args, + data: { + ...data, + listing: { ...prototypeListing, author: "me", authorLabel: "Me" }, + prototypes: [welcomePrototype], + prototypesCountLabel: "1 prototype", + }, + }, +} + +export const Paginated: Story = { ...Multiple, - play: async ({ canvas, userEvent }) => { - await userEvent.type(canvas.getByRole("searchbox", { name: "Search prototypes" }), "Navigation") + args: { + ...Multiple.args, + data: { + ...data, + ...Multiple.args?.data, + listing: { + ...prototypeListing, + matchingCount: 50, + pageCount: 3, + copy: { ...prototypeListing.copy, shown: "2 shown" }, + }, + prototypesCountLabel: "50 prototypes", + }, + }, +} + +export const NoMatches: Story = { + args: { + data: { + ...data, + listing: { ...prototypeListing, author: "me", authorLabel: "Me", matchingCount: 0 }, + workflowInstallation: undefined, + }, + }, +} + +export const Updating: Story = { + ...Single, + args: { + ...Single.args, + controls: { ...meta.args.controls, pending: true }, + }, +} + +export const RequestFailed: Story = { + ...Single, + args: { + ...Single.args, + controls: { ...meta.args.controls, error: true }, }, } diff --git a/hub/app/javascript/pages/repositories-show-page/styles.css b/hub/app/javascript/pages/repositories-show-page/styles.css index d825ba9b..31d4339e 100644 --- a/hub/app/javascript/pages/repositories-show-page/styles.css +++ b/hub/app/javascript/pages/repositories-show-page/styles.css @@ -9,6 +9,11 @@ min-height: 2.25rem; align-items: center; gap: 1rem; + flex-wrap: wrap; +} + +.repository-detail__pagination { + align-self: center; } .repository-detail__count { diff --git a/hub/app/javascript/pages/repositories-show-page/types.ts b/hub/app/javascript/pages/repositories-show-page/types.ts index b2c8200b..55d37b3d 100644 --- a/hub/app/javascript/pages/repositories-show-page/types.ts +++ b/hub/app/javascript/pages/repositories-show-page/types.ts @@ -2,8 +2,10 @@ import type { RepositoryWorkspaceData } from "@features/repository-workspace/typ import type { WorkflowInstallationData } from "@features/workflow-installation/types" import type { PrototypeCardData } from "@components/prototype-card/types" +import type { PrototypeListing } from "@utils/prototype-listing/types" export interface RepositoriesShowPageData extends RepositoryWorkspaceData { + listing: PrototypeListing alert: string | null candidateCheckFailed?: string clearSearchLabel: string @@ -36,8 +38,6 @@ export interface RepositoriesShowPageData extends RepositoryWorkspaceData { prototypes: PrototypeCardData[] prototypesCountLabel: string prototypesHeading: string - prototypeCountUnitOne: string - prototypeCountUnitOther: string searchLabel: string viewLabel: string workflowInstallation?: WorkflowInstallationData diff --git a/hub/app/javascript/storybook/fixtures/prototype-listing.ts b/hub/app/javascript/storybook/fixtures/prototype-listing.ts new file mode 100644 index 00000000..dfe30ea0 --- /dev/null +++ b/hub/app/javascript/storybook/fixtures/prototype-listing.ts @@ -0,0 +1,31 @@ +import type { PrototypeListing } from "@utils/prototype-listing/types" + +export const prototypeListing: PrototypeListing = { + query: "", + author: "", + authorLabel: "All authors", + matchingCount: 1, + page: 1, + pageCount: 1, + pagePath: "/repositories/2001", + authorsPath: "/repositories/2001/prototype_authors", + path: "/repositories/2001", + copy: { + author: "Author", + me: "Me", + allAuthors: "All authors", + searchAuthors: "Search authors", + moreAuthors: "More", + firstAuthors: "First authors", + noAuthors: "No matching authors", + loading: "Loading authors", + retry: "Retry", + authorsFailed: "Hub could not load authors. Try again.", + resultsFailed: "Hub could not load prototypes. Your previous results are still shown.", + previous: "Previous", + next: "Next", + pagination: "Prototype pages", + updating: "Updating prototypes", + shown: "1 shown", + }, +} diff --git a/hub/app/javascript/utils/prototype-listing/types.ts b/hub/app/javascript/utils/prototype-listing/types.ts new file mode 100644 index 00000000..0d0d8a80 --- /dev/null +++ b/hub/app/javascript/utils/prototype-listing/types.ts @@ -0,0 +1,57 @@ +export interface PrototypeAuthor { + id: string + login: string + name: string | null + avatarUrl: string | null +} + +export interface PrototypeAuthorResults { + authors: PrototypeAuthor[] + nextCursor: string | null +} + +export interface PrototypeListingControls { + query: string + pending: boolean + error: boolean + onQueryChange: (query: string) => void + onQuerySubmit: () => void + onAuthorChange: (author: string) => void + onNavigate: (path: string) => void + onRetry: () => void + loadAuthors: ( + query: string, + cursor: string | null, + signal: AbortSignal, + ) => Promise +} + +export interface PrototypeListing { + query: string + author: string + authorLabel: string + matchingCount: number + page: number + pageCount: number + pagePath: string + authorsPath: string + path: string + copy: { + author: string + me: string + allAuthors: string + searchAuthors: string + moreAuthors: string + firstAuthors: string + noAuthors: string + loading: string + retry: string + authorsFailed: string + resultsFailed: string + previous: string + next: string + pagination: string + updating: string + shown: string + } +} diff --git a/hub/app/models/prototype.rb b/hub/app/models/prototype.rb index c772389e..5ae9d66b 100644 --- a/hub/app/models/prototype.rb +++ b/hub/app/models/prototype.rb @@ -29,6 +29,8 @@ # index_prototypes_on_active_branch (repository_id,branch_name) UNIQUE WHERE branch_deleted_at IS NULL # index_prototypes_on_author_id (author_id) # index_prototypes_on_current_build_id (current_build_id) +# index_prototypes_on_gallery_author (repository_id,author_id,registered_at,id) WHERE branch_deleted_at IS NULL AND registered_at IS NOT NULL +# index_prototypes_on_gallery_order (repository_id,registered_at,id) WHERE branch_deleted_at IS NULL AND registered_at IS NOT NULL # index_prototypes_on_repository_id (repository_id) # # Foreign Keys diff --git a/hub/app/models/repository.rb b/hub/app/models/repository.rb index 103d72b2..fb2d0a5f 100644 --- a/hub/app/models/repository.rb +++ b/hub/app/models/repository.rb @@ -59,6 +59,7 @@ class Repository < ApplicationRecord has_object :branch_presence has_object :local_discovery has_object :prototype_candidates + has_object :prototype_listing has_object :workflow_installation has_object :workflow_run_discoveries diff --git a/hub/app/models/repository/prototype_listing.rb b/hub/app/models/repository/prototype_listing.rb new file mode 100644 index 00000000..300444a8 --- /dev/null +++ b/hub/app/models/repository/prototype_listing.rb @@ -0,0 +1,117 @@ +# Selects bounded gallery pages and registration authors within one repository. +class Repository::PrototypeListing < ActiveRecord::AssociatedObject + class InvalidQuery < StandardError; end + + PAGE_SIZE = 24 + AUTHORS_PAGE_SIZE = 20 + QUERY_MAXIMUM_LENGTH = 200 + Page = Data.define \ + :prototypes, :query, :author, :author_user, :matching_count, + :page, :page_count + + # Filters before paging, leaving Build hydration to the selected records. + def page(user:, query: nil, author: nil, page: nil) + query = normalized_query(query) + author = normalized_author(author) + author_id = author == "me" ? user.id : author.presence&.to_i + relation = base + relation = relation.where(author_id:) if author_id + if query.present? + relation = relation.left_joins(:author).where \ + "prototypes.name LIKE :query ESCAPE '\\' OR prototypes.description LIKE :query ESCAPE '\\' " \ + "OR prototypes.branch_name LIKE :query ESCAPE '\\' OR users.login LIKE :query ESCAPE '\\' " \ + "OR users.name LIKE :query ESCAPE '\\'", + query: "%#{Prototype.sanitize_sql_like(query)}%" + end + matching_count = relation.count + raise InvalidQuery if page && (!page.is_a?(String) || !page.match?(/\A[1-9]\d{0,8}\z/)) + + page_count = [ (matching_count.to_f / PAGE_SIZE).ceil, 1 ].max + page = [ page ? page.to_i : 1, page_count ].min + rows = relation.order(registered_at: :desc, id: :desc) + .offset((page - 1) * PAGE_SIZE).limit(PAGE_SIZE).to_a + + Page.new \ + prototypes: rows, + query:, + author:, + author_user: author_id && User.where(id: base.select(:author_id)).find_by(id: author_id), + matching_count:, + page:, + page_count: + end + + # Searches all eligible registration authors without enumerating the gallery. + def authors(user:, query: nil, cursor: nil) + query = normalized_query(query) + relation = User.where(id: base.select(:author_id)).where.not(id: user.id) + if query.present? + relation = relation.where \ + "login LIKE :query ESCAPE '\\' OR name LIKE :query ESCAPE '\\'", + query: "%#{User.sanitize_sql_like(query)}%" + end + scope = { "version" => 1, "repository" => record.id, "user" => user.id, "query" => query } + boundary = read_cursor(cursor, scope) + if boundary + raise InvalidQuery unless boundary["login"].is_a?(String) && boundary["login"].length <= 255 + + relation = relation.where \ + "login > :login OR (login = :login AND id > :id)", + login: boundary.fetch("login"), id: boundary.fetch("id") + end + rows = relation.order(:login, :id).limit(AUTHORS_PAGE_SIZE + 1).to_a + authors = rows.first(AUTHORS_PAGE_SIZE) + next_cursor = if rows.size > AUTHORS_PAGE_SIZE + write_cursor(scope, { "login" => authors.last.login, "id" => authors.last.id }) + end + { authors:, next_cursor: } + end + + private + + # Shares eligibility with the gallery, not all stored branch owners. + def base + record.reviewable_prototypes.active.registered + end + + # Rejects malformed or oversized input rather than silently changing its meaning. + def normalized_query(value) + return "" if value.nil? + raise InvalidQuery unless value.is_a?(String) && value.length <= QUERY_MAXIMUM_LENGTH + + value.strip + end + + # Named authors use immutable saved IDs; Me always resolves on the server. + def normalized_author(value) + return "" if value.nil? || value == "" + return value if value == "me" + raise InvalidQuery unless value.is_a?(String) && value.match?(/\A[1-9]\d{0,17}\z/) + + value + end + + # Validates only server-issued boundaries, never treating them as authorization. + def read_cursor(value, scope) + return if value.nil? + raise InvalidQuery unless value.is_a?(String) && value.bytesize <= 2_048 + + payload = verifier.verified(value) + unless payload.is_a?(Hash) && payload.slice(*scope.keys) == scope && + payload["id"].is_a?(Integer) && payload["id"].positive? + raise InvalidQuery + end + + payload + end + + # Adds the boundary to the exact source query. + def write_cursor(scope, boundary) + verifier.generate(scope.merge(boundary)) + end + + # Separates gallery tokens from other signed Hub values. + def verifier + Rails.application.message_verifier("prototype-listing") + end +end diff --git a/hub/app/views/repositories/show.json.props b/hub/app/views/repositories/show.json.props index 66b7fb04..9db67da7 100644 --- a/hub/app/views/repositories/show.json.props +++ b/hub/app/views/repositories/show.json.props @@ -39,9 +39,7 @@ end json.prototypesHeading t("repositories.show.prototypes_heading") json.alert flash[:alert] json.notice flash[:notice] -json.prototypesCountLabel t("repositories.show.prototype_count", count: @prototypes.length) -json.prototypeCountUnitOne t("repositories.show.prototype_count_unit_one") -json.prototypeCountUnitOther t("repositories.show.prototype_count_unit_other") +json.prototypesCountLabel t("repositories.show.prototype_count", count: @listing.matching_count) json.searchLabel t("repositories.show.search") json.clearSearchLabel t("repositories.show.clear_search") json.noSearchResultsHeading t("repositories.show.no_search_results_heading") @@ -59,6 +57,34 @@ json.emptyHeading t("repositories.show.empty_heading") json.emptyLabel t(@repository.local? ? "repositories.show.local_empty" : "repositories.show.empty") json.buildPrototypeLabel t("repositories.show.build_prototype") +listing_params = { q: @listing.query.presence, author: @listing.author.presence }.compact +json.listing do + json.query @listing.query + json.author @listing.author + author_label = if @listing.author == "me" + t("repositories.show.filters.me") + elsif @listing.author_user + "@#{@listing.author_user.login}" + elsif @listing.author.present? + t("repositories.show.unknown_author") + else + t("repositories.show.filters.all_authors") + end + json.authorLabel author_label + json.matchingCount @listing.matching_count + json.page @listing.page + json.pageCount @listing.page_count + json.pagePath repository_path(@repository.route_id, **listing_params) + json.authorsPath repository_prototype_authors_path(@repository.route_id) + json.path repository_path(@repository.route_id) + json.copy do + %w[author me all_authors search_authors more_authors first_authors no_authors loading retry + authors_failed results_failed previous next pagination updating shown].each do |key| + json.set! key.camelize(:lower), t("repositories.show.filters.#{key}", count: @prototypes.length) + end + end +end + if @repository_writable json.managePrototypesPath repository_prototype_management_path(@repository.route_id) json.managePrototypesLabel t("repositories.show.manage_prototypes") @@ -136,24 +162,19 @@ json.prototypes do json.ownerLabel "@#{Current.user.login}" json.url nil end - elsif latest_build&.commit_author_login + elsif prototype.author json.author do - json.avatarUrl latest_build.commit_author_avatar_url - json.label t("repositories.show.card_author", login: latest_build.commit_author_login) - json.ownerLabel "@#{latest_build.commit_author_login}" - json.url latest_build.commit_author.profile_url + json.avatarUrl prototype.author.avatar_url + json.label t("repositories.show.card_author", login: prototype.author.login) + json.ownerLabel "@#{prototype.author.login}" + json.url "#{Rails.configuration.x.github_url}/#{CGI.escapeURIComponent(prototype.author.login)}" end else json.author do json.avatarUrl nil - if latest_build&.commit_author_name - json.label t("repositories.show.card_author_name", name: latest_build.commit_author_name) - json.ownerLabel latest_build.commit_author_name - else - json.label t("repositories.show.unknown_author") - json.ownerLabel t("repositories.show.unknown_author") - end + json.label t("repositories.show.unknown_author") + json.ownerLabel t("repositories.show.unknown_author") json.url nil end diff --git a/hub/app/views/repositories/show.tsx b/hub/app/views/repositories/show.tsx index b6fd00e4..30ab24cf 100644 --- a/hub/app/views/repositories/show.tsx +++ b/hub/app/views/repositories/show.tsx @@ -3,14 +3,18 @@ import { useSelector } from "react-redux" import type { AppState } from "@application/store" import { usePageData } from "@hooks/use-page-data" import { useRepositoryInbox } from "@hooks/use-repository-inbox" +import { usePrototypeListing } from "@hooks/use-prototype-listing" import { RepositoriesShowPage } from "@pages/repositories-show-page" import type { RepositoriesShowPageData } from "@pages/repositories-show-page/types" export default function RepositoriesShowView() { const data = useRepositoryInbox(usePageData()) + const controls = usePrototypeListing(data.listing) const managingPrototypes = useSelector( (state: AppState) => state.navigationRequest.path === data.managePrototypesPath, ) - return + return ( + + ) } diff --git a/hub/config/locales/gallery.en.yml b/hub/config/locales/gallery.en.yml index 43fb5b03..52e736ca 100644 --- a/hub/config/locales/gallery.en.yml +++ b/hub/config/locales/gallery.en.yml @@ -18,17 +18,15 @@ en: prototype_count: one: 1 prototype other: "%{count} prototypes" - prototype_count_unit_one: prototype - prototype_count_unit_other: prototypes search: Search prototypes clear_search: Clear search no_search_results_heading: No prototypes found - no_search_results_description: Try a different search, or clear the current search to browse all prototypes. + no_search_results_description: Try a different search or author, or clear the filters to browse all prototypes. view: Prototype layout gallery_view: Gallery view list_view: List view prototype_column: Prototype - owner_column: Owner + owner_column: Author build_column: Build passing: passing not_passing: not passing @@ -49,6 +47,23 @@ en: unknown_author: Author unavailable screenshot_alt: "Desktop screenshot of %{branch}, build %{number}" preview_fallback: No successful preview is available. + filters: + author: Author + me: Me + all_authors: All authors + search_authors: Search authors + more_authors: More + first_authors: First authors + no_authors: No matching authors + loading: Loading authors + retry: Retry + authors_failed: Hub could not load authors. Try again. + results_failed: Hub could not load prototypes. Your previous results are still shown. + previous: Previous + next: Next + pagination: Prototype pages + updating: Updating prototypes + shown: "%{count} shown" workflow: heading: Prototype workflow different_heading: Replica workflow update available diff --git a/hub/config/routes.rb b/hub/config/routes.rb index 12a12633..9588de68 100644 --- a/hub/config/routes.rb +++ b/hub/config/routes.rb @@ -112,6 +112,7 @@ constraints: { memory_id: /mem_[0-9a-f]{24}/ } end get "new", to: "new_prototypes#show", as: :new_prototype + resources :prototype_authors, only: :index get "replica", to: "replica_browses#show", as: :replica_browse resources :templates, only: %i[index show], constraints: { id: /[a-z0-9]+(?:-[a-z0-9]+)*/ } get "templates/:template_id/builds/:build_id/screenshots/:size", to: "template_screenshots#show", diff --git a/hub/db/migrate/20261019000000_index_prototype_gallery.rb b/hub/db/migrate/20261019000000_index_prototype_gallery.rb new file mode 100644 index 00000000..e44d385d --- /dev/null +++ b/hub/db/migrate/20261019000000_index_prototype_gallery.rb @@ -0,0 +1,10 @@ +class IndexPrototypeGallery < ActiveRecord::Migration[8.1] + def change + add_index :prototypes, [ :repository_id, :registered_at, :id ], + name: "index_prototypes_on_gallery_order", + where: "branch_deleted_at IS NULL AND registered_at IS NOT NULL" + add_index :prototypes, [ :repository_id, :author_id, :registered_at, :id ], + name: "index_prototypes_on_gallery_author", + where: "branch_deleted_at IS NULL AND registered_at IS NOT NULL" + end +end diff --git a/hub/db/schema.rb b/hub/db/schema.rb index f73e261d..62f04922 100644 --- a/hub/db/schema.rb +++ b/hub/db/schema.rb @@ -461,8 +461,10 @@ t.string "retired_head_sha" t.index ["author_id"], name: "index_prototypes_on_author_id" t.index ["current_build_id"], name: "index_prototypes_on_current_build_id" + t.index ["repository_id", "author_id", "registered_at", "id"], name: "index_prototypes_on_gallery_author", where: "branch_deleted_at IS NULL AND registered_at IS NOT NULL" t.index ["repository_id", "branch_name", "branch_deleted_at"], name: "idx_on_repository_id_branch_name_branch_deleted_at_37960952d6" t.index ["repository_id", "branch_name"], name: "index_prototypes_on_active_branch", unique: true, where: "branch_deleted_at IS NULL" + t.index ["repository_id", "registered_at", "id"], name: "index_prototypes_on_gallery_order", where: "branch_deleted_at IS NULL AND registered_at IS NOT NULL" t.index ["repository_id"], name: "index_prototypes_on_repository_id" t.check_constraint "description IS NULL OR length(trim(description)) BETWEEN 1 AND 500", name: "prototypes_description" t.check_constraint "length(trim(branch_name)) BETWEEN 1 AND 255", name: "prototypes_branch_name" diff --git a/hub/docs/gallery.md b/hub/docs/gallery.md index 22f748da..bafda1a9 100644 --- a/hub/docs/gallery.md +++ b/hub/docs/gallery.md @@ -60,6 +60,18 @@ Operators configure the video through the [deployment settings](deployment.md#we ## Browse prototypes +Prototypes appear in pages of up to 24, newest registration first. Numbered navigation with **Previous** and **Next** uses the same Primer Brand pagination component as Manage prototypes and keeps the current search and author filter. Hub loads card and Build data only for the selected page. New registrations or removals can shift records between pages while browsing. + +The header groups the Author dropdown beside search, followed by the gallery/list controls. On narrow screens, these controls stack in that order, keeping filtering separate from presentation. + +Search matches prototype names, descriptions, branches, and registration authors across the full selected repository, not only the visible page. Search treats punctuation such as `%` and `_` literally. SQLite's built-in matching ignores ASCII letter case; non-ASCII letters retain SQLite's case behavior. The count shows all matches, with the number shown on the current page when fewer are displayed. + +The **Author** dropdown puts **Me** first, followed by **All authors** and searchable pages of the repository's registration authors. Long labels and options truncate with an ellipsis rather than widening the dropdown or scrolling horizontally; hovering reveals their full text. **Me** uses the signed-in person's saved identity. Cards and the list's **Author** column show the first person who added the prototype to Hub; later commits by another person do not change attribution or author-filter results. Prototypes without a saved author remain visible with no author filter and display **Author unavailable**. Local mode keeps its existing local-identity presentation and applies author filters to saved registration authors. + +Search and author filters combine. Applying either starts again at the newest matching page. Choose **All authors** to remove the author filter. The URL preserves applied search, author, and page state across reloads and browser history. Switching repositories starts without those filters. Author choices load in bounded pages and are not restricted to authors on the visible prototype page; **More** loads the next page of authors. Clearing or changing author-option search starts its choices from the beginning. An empty author collection leaves the pinned options available without an empty-state message; a search with no matches shows **No matching authors**. + +While Hub fetches results, it keeps the previous cards visible and announces the update. A failed request keeps those cards with an explicit error and **Retry**. No matches has its own empty state, separate from a repository with no prototypes. A page beyond the remaining results shows the last available page. + An empty Prototypes page shows a branch Octicon, a heading, and guidance. For GitHub repositories, **Build prototype** opens **Create**. Local repositories keep their branch-discovery guidance without offering the Copilot app flow. The **Create** sidebar link opens a centered brief composer for the selected repository, with **What should we create?** above it and the **Templates** catalog below. Choose a template inside the composer before **Add details**, or open a catalog card to preview it first. The picker starts with **Default**. Choosing another template includes its quoted route, query, and fragment in the prompt and instructs the agent to set it as the new prototype's landing page through `startPage` in `.prototype`. Choosing Default removes that extra context. Catalog search does not change the selected template, brief, or saved details. Unavailable or corrupt template metadata returns an explicit service failure rather than an incomplete collection. @@ -130,7 +142,7 @@ Local repositories omit workflow installation. Hub checks committed local branch The Prototypes page orders prototypes by when they were first added to Hub, newest first. New prototypes appear at the top left of the gallery and at the top of the list. Editing details or publishing another Build does not change that order. -The Prototypes page filters the current repository's gallery and list as you type in the header search. Search matches prototype names, descriptions, branch names, and owners; Command K (or Control K) focuses the field. The count above the collection reflects matching prototypes, and clearing the search restores the full list. +Command K (or Control K) focuses the Prototypes search field. Clearing search keeps the selected author. Opening a prototype from its card or list row uses in-app navigation rather than reloading the document. List links use normal text colors without underlines. Hovering or focusing an owner's profile link underlines the name; selecting it opens that profile instead of the prototype. The **passing** and **not passing** badges use lowercase text and have no icons. The review shell opens the latest successful Build. Its controls stay above the rounded preview. Gallery cards and Build details show the top of each full-height screenshot in a fixed preview. Hovering a preview scrolls through the remaining image; on touch devices or with reduced motion enabled, scroll the preview manually. In those modes, list screenshots remain manual scroll areas while the rest of each row opens the prototype. Older viewport-height screenshots stay at the top without scrolling. diff --git a/hub/test/integration/gallery_test.rb b/hub/test/integration/gallery_test.rb index 7a1c7d4d..3836f090 100644 --- a/hub/test/integration/gallery_test.rb +++ b/hub/test/integration/gallery_test.rb @@ -124,6 +124,13 @@ class GalleryTest < ActionDispatch::IntegrationTest assert_response :success data = JSON.parse(response.body).fetch("data") assert_empty data.fetch("prototypes") + assert_equal 0, data.fetch("listing").fetch("matchingCount") + assert_equal 1, data.fetch("listing").fetch("page") + assert_equal 1, data.fetch("listing").fetch("pageCount") + assert_equal repository_path(repository.route_id), data.fetch("listing").fetch("pagePath") + %w[previousPath nextPath newestPath].each do |key| + assert_not data.fetch("listing").key?(key) + end assert_equal "No prototypes yet", data.fetch("emptyHeading") assert_equal "Build prototype", data.fetch("buildPrototypeLabel") assert_equal repository_new_prototype_path(repository.github_id), data.fetch("newPrototypePath") @@ -135,7 +142,10 @@ class GalleryTest < ActionDispatch::IntegrationTest sign_in repository = Repository.find_by!(github_id: 2001) prototype = prototypes.create(repository:, branch_name: "prototype/welcome") - prototype.register!(name: "Welcome", description: "Try the new welcome flow.") + prototype.register! \ + name: "Welcome", + description: "Try the new welcome flow.", + author: User.find_by!(github_id: 1001) build_with_state(prototype, 1, "successful", "a", Time.current) latest = build_with_state(prototype, 2, "running", "b", Time.current) latest.commit_author.record!(commit_payload(latest.commit_sha)) diff --git a/hub/test/integration/learning_execution_test.rb b/hub/test/integration/learning_execution_test.rb index 9d69c362..ef58a1dc 100644 --- a/hub/test/integration/learning_execution_test.rb +++ b/hub/test/integration/learning_execution_test.rb @@ -165,7 +165,7 @@ def queue_adapter_for_test claim(Learning::Run::FindLearningsJob.name).perform assert_equal "queued", run.reload.state assert_nil run.started_at - travel 35.seconds + travel_to SolidQueue::ScheduledExecution.sole.scheduled_at + 1.second assert_equal 1, SolidQueue::ScheduledExecution.dispatch_next_batch(100) end assert_raises(Learning::ExecutionLock::Busy) { claim(Learning::Run::FindLearningsJob.name).perform } @@ -201,7 +201,7 @@ def queue_adapter_for_test (attempts - 1).times do claim(ReviewComment::RemoveFromLearningStoreJob.name).perform assert @comment.reload.learning_removal_due_at - travel 35.seconds + travel_to SolidQueue::ScheduledExecution.sole.scheduled_at + 1.second assert_equal 1, SolidQueue::ScheduledExecution.dispatch_next_batch(100) end assert_raises(Learning::ExecutionLock::Busy) { claim(ReviewComment::RemoveFromLearningStoreJob.name).perform } diff --git a/hub/test/integration/prototype_listing_test.rb b/hub/test/integration/prototype_listing_test.rb new file mode 100644 index 00000000..775625ba --- /dev/null +++ b/hub/test/integration/prototype_listing_test.rb @@ -0,0 +1,340 @@ +require "test_helper" + +class PrototypeListingTest < ActionDispatch::IntegrationTest + setup do + github_app_configurations.create_installed + end + + test "filters before numbered paging and preserves filters in the page path" do + repository, user = sign_in_repository + other = create_author("other-reviewer") + time = Time.current + matching = 49.times.map do |index| + registered_prototype(repository, author: user, name: "Needle #{index}", at: time - 1.day) + end + 25.times { registered_prototype(repository, author: other, name: "Needle", at: time) } + registered_prototype(repository, author: user, name: "Unrelated", at: time) + expected = matching.reverse.map { _1.id.to_s } + + data = listing(repository, q: "nEeDlE", author: user.id.to_s, page: 3) + [ 3, 2, 1 ].each do |page| + assert_equal expected.slice((page - 1) * 24, 24), data.fetch("prototypes").pluck("id") + pagination = data.fetch("listing") + assert_equal page, pagination.fetch("page") + assert_equal 3, pagination.fetch("pageCount") + assert_equal 49, pagination.fetch("matchingCount") + assert_equal repository_path(repository.route_id), pagination.fetch("path") + assert_equal authors_path(repository), pagination.fetch("authorsPath") + page_path = pagination.fetch("pagePath") + assert_equal repository_path(repository.route_id), URI.parse(page_path).path + assert_filters page_path, q: "nEeDlE", author: user.id.to_s + assert_equal %w[author q], Rack::Utils.parse_query(URI.parse(page_path).query).keys.sort + %w[previousPath nextPath newestPath].each { assert_not pagination.key?(_1) } + data = listing_at(page_path, params: { page: page - 1 }) if page > 1 + end + end + + test "selects only eligible prototypes and bounds card and Build hydration to 24 results" do + repository, user = sign_in_repository + time = Time.current + records = 60.times.map do |index| + prototype = registered_prototype(repository, author: user, at: time) + builds.create_successful(prototype:, number: 1) + latest = builds.create(prototype:, number: 2, commit_sha: "b" * 40) + prototype.update!(current_build: latest) if index.even? + prototype + end + prototypes.create(repository:, branch_name: "prototype/unregistered", author: user) + prototypes.create(repository:, branch_name: repository.default_branch) + prototypes.create(repository:, branch_name: "replica/install-workflow") + retired = registered_prototype(repository, author: user) + retired.update!(branch_deleted_at: Time.current) + foreign = repositories.create(github_id: 2002, name_with_owner: "github/other") + stub_github_permissions([ repository_payload(2002, "github/other") ], { 2002 => "pull" }) + registered_prototype(foreign, author: user) + instantiated = Hash.new(0) + subscriber = lambda do |*arguments| + payload = arguments.last + instantiated[payload.fetch(:class_name)] += payload.fetch(:record_count) + end + data = nil + + ActiveSupport::Notifications.subscribed(subscriber, "instantiation.active_record") do + data = listing(repository) + end + + cards = data.fetch("prototypes") + assert_equal records.reverse.first(24).map { _1.id.to_s }, cards.pluck("id") + assert_equal 1, data.fetch("listing").fetch("page") + assert_equal 60, data.fetch("listing").fetch("matchingCount") + cards.each do |card| + assert_equal "queued", card.fetch("latestBuild").fetch("state") + assert_equal 1, card.fetch("reviewBuild").fetch("number") + end + assert_operator instantiated.fetch("Prototype"), :<=, 24 + assert_operator instantiated.fetch("Build"), :<=, 24 * 2 + end + + test "searches names descriptions branches and registration author login and name case insensitively" do + repository, user = sign_in_repository + by_login = create_author("needle-owner", name: "Another person") + by_name = create_author("name-owner", name: "Needle Person") + matches = [ + registered_prototype(repository, author: user, name: "A NEEDLE design"), + registered_prototype(repository, author: user, description: "A Needle description"), + registered_prototype(repository, author: user, branch_name: "prototype/needle-branch"), + registered_prototype(repository, author: by_login), + registered_prototype(repository, author: by_name) + ] + unrelated = registered_prototype(repository, author: user) + build = builds.create(prototype: unrelated) + build.commit_author.record! \ + "sha" => build.commit_sha, + "commit" => { "author" => { "name" => "Needle Committer" } }, + "author" => { "id" => 9001, "login" => "needle-committer", "avatar_url" => nil } + + data = listing(repository, q: "nEeDlE") + + assert_equal matches.map { _1.id.to_s }.sort, data.fetch("prototypes").pluck("id").sort + end + + test "treats percent underscore and backslash as literal search characters" do + repository, user = sign_in_repository + literal = registered_prototype(repository, author: user, name: '100%_done\path') + registered_prototype(repository, author: user, name: "100XXdonepath") + + [ "%", "_", "\\" ].each do |query| + data = listing(repository, q: query) + assert_equal [ literal.id.to_s ], data.fetch("prototypes").pluck("id"), query + end + end + + test "author filters and card attribution use the saved registrant rather than the commit author" do + repository, user = sign_in_repository + other = create_author("registrant") + mine = registered_prototype(repository, author: user) + theirs = registered_prototype(repository, author: other) + unknown = registered_prototype(repository) + [ theirs, unknown ].each do |prototype| + build = builds.create(prototype:) + build.commit_author.record! \ + "sha" => build.commit_sha, + "commit" => { "author" => { "name" => "Octocat" } }, + "author" => { "id" => user.github_id, "login" => user.login, "avatar_url" => user.avatar_url } + end + + all = listing(repository) + cards = all.fetch("prototypes").index_by { _1.fetch("id") } + author = cards.fetch(theirs.id.to_s).fetch("author") + assert_equal "@registrant", author.fetch("ownerLabel") + assert_equal "https://github.com/registrant", author.fetch("url") + unknown_author = cards.fetch(unknown.id.to_s).fetch("author") + assert_equal I18n.t("repositories.show.unknown_author"), unknown_author.fetch("ownerLabel") + assert_nil unknown_author.fetch("url") + + me = listing(repository, author: "me") + assert_equal [ mine.id.to_s ], me.fetch("prototypes").pluck("id") + named = listing(repository, author: other.id.to_s) + assert_equal [ theirs.id.to_s ], named.fetch("prototypes").pluck("id") + assert_includes named.fetch("listing").fetch("authorLabel"), other.login + end + + test "clamps a stale page to the last filtered page after a deletion" do + repository, user = sign_in_repository + originals = 25.times.map { registered_prototype(repository, author: user, name: "Needle") } + second = listing(repository, q: "Needle", author: "me", page: 2) + assert_equal [ originals.first.id.to_s ], second.fetch("prototypes").pluck("id") + assert_equal 2, second.fetch("listing").fetch("pageCount") + originals.first.update!(branch_deleted_at: Time.current) + + stale = listing_at(second.fetch("listing").fetch("pagePath"), params: { page: 2 }) + + assert_equal originals.drop(1).reverse.map { _1.id.to_s }, stale.fetch("prototypes").pluck("id") + assert_equal 1, stale.fetch("listing").fetch("page") + assert_equal 1, stale.fetch("listing").fetch("pageCount") + assert_equal 24, stale.fetch("listing").fetch("matchingCount") + assert_filters stale.fetch("listing").fetch("pagePath"), q: "Needle", author: "me" + end + + test "rejects malformed filters and page numbers instead of silently defaulting them" do + repository, user = sign_in_repository + registered_prototype(repository, author: user) + + [ + { q: "x" * 201 }, { q: [ "query" ] }, { author: "0" }, + *[ "", "0", "-1", "1.5", "+1", "1e1", " 1", "1 ", "１", "1000000000", + [ "1" ], { value: "1" } ].map { { page: _1 } } + ].each do |params| + get repository_path(repository.github_id), params:, headers: json_headers + assert_response :bad_request, params.inspect + end + + accepted = listing(repository, q: "x" * 200) + assert_equal "x" * 200, accepted.fetch("listing").fetch("query") + assert_equal 1, accepted.fetch("listing").fetch("page") + last = listing(repository, page: "999999999") + assert_equal 1, last.fetch("listing").fetch("page") + assert_equal 1, last.fetch("listing").fetch("pageCount") + assert_equal 1, last.fetch("listing").fetch("matchingCount") + end + + test "author cursors reject tampering and changes to repository query or viewer" do + repository, = sign_in_repository + 25.times do |index| + registered_prototype(repository, author: create_author("needle-#{index}"), name: "Needle") + end + query = { "q" => "needle", "cursor" => author_lookup(repository, q: "needle").fetch("nextCursor") } + assert_not_nil query.fetch("cursor") + other_repository = repositories.create(github_id: 2002, name_with_owner: "github/other") + stub_github_permissions([ repository_payload(2002, "github/other") ], { 2002 => "pull" }) + [ query.merge("cursor" => "#{query.fetch('cursor')}tampered"), + query.merge("q" => "Different") ].each do |invalid| + get authors_path(repository), params: invalid, headers: json_headers + assert_response :bad_request + end + get authors_path(other_repository), params: query, headers: json_headers + assert_response :bad_request + + sign_in_as_another_reader + get authors_path(repository), params: query, headers: json_headers + assert_response :bad_request + end + + test "pages distinct registrants outside the loaded page without shifting for new authors" do + repository, user = sign_in_repository + time = Time.current + authors = 22.times.map do |index| + login = index < 2 ? "contributor-00" : "contributor-#{index.to_s.rjust(2, '0')}" + author = create_author(login, name: index.zero? ? nil : "Contributor #{index}") + registered_prototype(repository, author:, at: time - 1.day) + author + end + 25.times { registered_prototype(repository, author: authors.first, at: time) } + registered_prototype(repository, author: user, at: time) + registered_prototype(repository, at: time) + first = author_lookup(repository) + expected = authors.sort_by { [ _1.login, _1.id ] } + assert_equal expected.first(20).map { _1.id.to_s }, first.fetch("authors").pluck("id") + expected_author = { + "id" => authors.first.id.to_s, "login" => authors.first.login, "name" => nil, "avatarUrl" => nil + } + assert_equal expected_author, first.fetch("authors").first + registered_prototype(repository, author: create_author("aaa-new-owner")) + second = author_lookup(repository, cursor: first.fetch("nextCursor")) + assert_equal expected.last(2).map { _1.id.to_s }, second.fetch("authors").pluck("id") + assert_nil second.fetch("nextCursor") + end + + test "author lookup searches literal login and name substrings within eligible repository registrants only" do + repository, user = sign_in_repository + by_login = create_author("needle-login") + by_name = create_author("another-login", name: "A NEEDLE person") + literal = create_author("literal-owner", name: "100%_Team") + [ by_login, by_name, literal ].each { registered_prototype(repository, author: _1) } + registered_prototype(repository, author: user) + registered_prototype(repository) + hidden = create_author("needle-hidden") + prototypes.create(repository:, branch_name: "prototype/unregistered", author: hidden) + prototypes.create(repository:, branch_name: "replica/install-workflow", author: hidden) + prototypes.create(repository:, branch_name: repository.default_branch, author: hidden) + retired = registered_prototype(repository, author: hidden) + retired.update!(branch_deleted_at: Time.current) + foreign = repositories.create(github_id: 2002, name_with_owner: "github/other") + stub_github_permissions([ repository_payload(2002, "github/other") ], { 2002 => "pull" }) + registered_prototype(foreign, author: hidden) + distractor = create_author("distractor", name: "100XXTeam") + registered_prototype(repository, author: distractor) + + result = author_lookup(repository, q: "nEeDlE") + assert_equal [ by_login.id.to_s, by_name.id.to_s ].sort, result.fetch("authors").pluck("id").sort + [ "%", "_" ].each do |query| + assert_equal [ literal.id.to_s ], author_lookup(repository, q: query).fetch("authors").pluck("id") + end + assert_equal [ by_login, by_name, literal, distractor ].map { _1.id.to_s }.sort, + author_lookup(repository).fetch("authors").pluck("id").sort + end + + test "both listing endpoints require a signed in repository reader and fail closed on permission errors" do + visible = repository_payload(2001, "github/prototype") + denied = repository_payload(2002, "github/private") + failing = repository_payload(2003, "github/unavailable") + stub_github_sign_in \ + repositories: [ visible, denied, failing ], + permissions: { 2001 => "pull", 2002 => nil, 2003 => :failure } + sign_in_to_hub + [ [ 2002, :not_found ], [ 2003, :service_unavailable ] ].each do |github_id, status| + [ "/repositories/#{github_id}", "/repositories/#{github_id}/prototype_authors" ].each do |path| + get path, headers: json_headers + assert_response status + end + end + + delete session_path + [ "/repositories/2001", "/repositories/2001/prototype_authors" ].each do |path| + get path, headers: json_headers + assert_redirected_to root_path + end + end + + private + + def sign_in_repository + payload = repository_payload(2001, "github/prototype") + stub_github_sign_in(repositories: [ payload ], permissions: { 2001 => "pull" }) + sign_in_to_hub + [ Repository.find_by!(github_id: 2001), User.find_by!(github_id: 1001) ] + end + + def sign_in_as_another_reader + delete session_path + stub_github_oauth_user(github_id: 1002, login: "another-reader") + payloads = Repository.where.not(github_id: nil).map { repository_payload(_1.github_id, _1.name_with_owner) } + stub_github_permissions \ + payloads, + payloads.to_h { [ _1.fetch("id"), "pull" ] }, + github_id: 1002, + login: "another-reader" + sign_in_to_hub(github_id: 1002) + end + + def create_author(login, name: nil) + users.create(github_id: User.maximum(:github_id).to_i + 1, login:, name:, avatar_url: nil) + end + + def registered_prototype(repository, author: nil, name: "Prototype", description: nil, + branch_name: "prototype/#{SecureRandom.hex(6)}", at: Time.current) + prototype = prototypes.create(repository:, branch_name:) + travel_to(at) { prototype.register!(name:, description:, author:) } + prototype + end + + def listing(repository, **params) + listing_at(repository_path(repository.route_id), params:) + end + + def listing_at(path, params: {}) + get path, params:, headers: json_headers + assert_response :success + JSON.parse(response.body).fetch("data") + end + + def authors_path(repository) + "/repositories/#{repository.route_id}/prototype_authors" + end + + def author_lookup(repository, **params) + get authors_path(repository), params:, headers: json_headers + assert_response :success + JSON.parse(response.body) + end + + def assert_filters(path, q:, author:) + params = Rack::Utils.parse_query(URI.parse(path).query) + assert_equal q, params.fetch("q") + assert_equal author, params.fetch("author") + end + + def json_headers + { "Accept" => "application/json" } + end +end