Web UI: Docker group pages label a proxy member's times Cached, not Pushed

docs/changes/2.2.0/562.fixed.md

@@ -0,0 +1,6 @@
+A Docker group's pages labelled the times of manifests served by a proxy
+member "Pushed", though they are when Forge cached them from the upstream.
+An image's manifests now say "Cached" or "Pushed" by the member each comes
+from, and "Pushed / cached" with a label on each row where the members mix;
+a group's image list, which names no member, heads its column "Last pushed /
+cached".

web/src/components/ImageTables.tsx

@@ -1,4 +1,7 @@
+import { useQuery } from '@tanstack/react-query'
+
 import type { ImageManifest, ImageTag } from '~/api/images'
+import { browseReposQueryOptions } from '~/api/packages'
 import {
   formatBytes,
   formatDateTime,
@@ -37,6 +40,14 @@ import {
  * Forge first cached it from the upstream, not when it was pushed there, so
  * the column is labelled "Cached" for a proxy repository (as package
  * versions are, #110).
+ *
+ * A group's manifests come from its members (`source`), and each row's time
+ * is what its member's is: cached for a proxy member, pushed for a hosted one
+ * (#562). The members' kinds come from the readable repositories
+ * (`/browse/repos`), which list every member a group serves the reader. When
+ * every row has the same kind the column is labelled for it; when the rows
+ * mix kinds, or a row's member is not known, the column is "Pushed / cached"
+ * and each row of a known member says which its time is.
  */
 
 /** A short digest with its full value as tooltip, plus a copy button. */
@@ -136,6 +147,37 @@ export function TagTable({
   )
 }
 
+/** What a manifest's `created_at` is: when it was pushed, or first cached. */
+type TimeKind = 'pushed' | 'cached'
+
+/** The time kind of a repository (or a group's member) of `kind`. */
+function timeKindOf(kind: string | undefined): TimeKind | undefined {
+  if (kind === 'proxy') return 'cached'
+  if (kind === 'hosted') return 'pushed'
+  return undefined
+}
+
+const TIME_HEADERS: Record<TimeKind | 'mixed', { label: string; hint?: string }> = {
+  pushed: { label: 'Pushed' },
+  cached: {
+    label: 'Cached',
+    hint: 'When Forge first cached this manifest from the upstream',
+  },
+  mixed: {
+    label: 'Pushed / cached',
+    hint: 'When the manifest was pushed to a hosted member, or first cached from the upstream by a proxy member',
+  },
+}
+
+/**
+ * The kind of the member each manifest of a group comes from, by member
+ * name; empty for any other repository (no request is made).
+ */
+function useMemberKinds(repoKind: string): Map<string, string> {
+  const repos = useQuery({ ...browseReposQueryOptions, enabled: repoKind === 'group' })
+  return new Map((repoKind === 'group' ? (repos.data ?? []) : []).map((r) => [r.name, r.kind]))
+}
+
 const KIND_ICONS: Record<ManifestKind, IconName> = {
   Image: 'Package',
   Index: 'Layers',
@@ -160,6 +202,18 @@ export function ManifestTable({
   // Referrers name their subject by digest; show the subject's tags too.
   const tagsByDigest = new Map(manifests.map((m) => [m.digest, m.tags]))
   const showSource = repoKind === 'group'
+  const memberKinds = useMemberKinds(repoKind)
+  const rowTimeKind = (m: ImageManifest): TimeKind | undefined =>
+    timeKindOf(showSource ? m.source && memberKinds.get(m.source) : repoKind)
+  const rowKinds = new Set(manifests.map(rowTimeKind))
+  // One kind for every row labels the column for it; a mix (or a member of
+  // unknown kind) gets the neutral heading and a per-row label.
+  const single: TimeKind | undefined = showSource
+    ? rowKinds.size === 1
+      ? [...rowKinds][0]
+      : undefined
+    : (timeKindOf(repoKind) ?? 'pushed')
+  const timeHeader = TIME_HEADERS[single ?? 'mixed']
 
   return (
     <Table caption="Manifests">
@@ -169,13 +223,7 @@ export function ManifestTable({
         <TableHeader align="right">Size</TableHeader>
         <TableHeader>Platforms</TableHeader>
         <TableHeader>Tags</TableHeader>
-        {repoKind === 'proxy' ? (
-          <TableHeader hint="When Forge first cached this manifest from the upstream">
-            Cached
-          </TableHeader>
-        ) : (
-          <TableHeader>Pushed</TableHeader>
-        )}
+        <TableHeader hint={timeHeader.hint}>{timeHeader.label}</TableHeader>
         <TableHeader>Last pulled</TableHeader>
         {showSource && <TableHeader>Source</TableHeader>}
         {onDelete && <TableHeader align="right">Actions</TableHeader>}
@@ -199,7 +247,7 @@ export function ManifestTable({
               <Chips values={m.tags} />
             </TableCell>
             <TableCell nowrap title={m.created_at}>
-              {formatDateTime(m.created_at)}
+              <TimeCell manifest={m} label={single ? undefined : rowTimeKind(m)} />
             </TableCell>
             <TableCell nowrap title={m.last_pulled_at ?? undefined}>
               {formatDateTime(m.last_pulled_at)}
@@ -223,6 +271,20 @@ export function ManifestTable({
   )
 }
 
+/**
+ * A manifest's `created_at`; in a column of mixed kinds, with whether the
+ * row's time is a push or a cache.
+ */
+function TimeCell({ manifest, label }: { manifest: ImageManifest; label?: TimeKind }) {
+  if (!label) return <>{formatDateTime(manifest.created_at)}</>
+  return (
+    <span className="flex flex-col items-start gap-0.5">
+      <span>{formatDateTime(manifest.created_at)}</span>
+      <span className="fg-cell-sub">{label === 'cached' ? 'Cached' : 'Pushed'}</span>
+    </span>
+  )
+}
+
 /**
  * The manifest's kind badge (media / artifact type as tooltip); an index
  * adds how many manifests it lists, a referrer (signature, attestation, SBOM,

web/src/routes/Browse.test.tsx

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
-import { render, screen, waitFor, within } from '@testing-library/react'
+import { cleanup, render, screen, waitFor, within } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
 import {
@@ -115,6 +115,7 @@ function rowOf(element: HTMLElement): HTMLElement {
 function mockApi(
   browse: (url: URL) => Response,
   images: (url: URL) => Response = () => jsonResponse(imagePage(0, [])),
+  repoList: BrowseRepo[] = repos,
 ) {
   fetchMock.mockImplementation((input: RequestInfo | URL) => {
     const raw = typeof input === 'string' ? input : input.toString()
@@ -123,7 +124,7 @@ function mockApi(
       return Promise.resolve(jsonResponse(readerMe))
     }
     if (url.pathname.endsWith('/browse/repos')) {
-      return Promise.resolve(jsonResponse(repos))
+      return Promise.resolve(jsonResponse(repoList))
     }
     if (url.pathname.endsWith('/browse/packages')) {
       return Promise.resolve(browse(url))
@@ -606,6 +607,37 @@ describe('RepoBrowse of a docker repository (images)', () => {
     }
   })
 
+  it('labels a proxy\'s and a group\'s image times by what they are (#248, #562)', async () => {
+    const dockerRepos: BrowseRepo[] = [
+      ...repos,
+      { id: 6, name: 'docker-hub', kind: 'proxy', online: true, format: 'docker' },
+      { id: 7, name: 'docker-all', kind: 'group', online: true, format: 'docker' },
+    ]
+    const images = () => jsonResponse(imagePage(1, ['team/app']))
+    mockApi(() => jsonResponse(page(0, [])), images, dockerRepos)
+
+    await renderBrowse('/repos/docker-hub')
+
+    await screen.findByRole('link', { name: /team\/app/ })
+    expect(screen.getByRole('columnheader', { name: 'Last cached' })).toBeInTheDocument()
+    expect(screen.queryByRole('columnheader', { name: /pushed/i })).toBeNull()
+
+    cleanup()
+    await renderBrowse('/repos/docker-all')
+
+    // A group's image rows merge its hosted and proxy members' and name
+    // none of them: the column says both.
+    await screen.findByRole('link', { name: /team\/app/ })
+    expect(
+      screen.getByRole('columnheader', { name: 'Last pushed / cached' }),
+    ).toHaveAttribute(
+      'title',
+      'When a manifest of the image was last pushed to a hosted member, or cached from the upstream by a proxy member',
+    )
+    expect(screen.queryByRole('columnheader', { name: 'Last pushed' })).toBeNull()
+    expect(screen.queryByRole('columnheader', { name: 'Last cached' })).toBeNull()
+  })
+
   it('says a docker repository is offline on its 503', async () => {
     mockApi(
       () => jsonResponse(page(0, [])),

web/src/routes/ImageDetail.test.tsx

@@ -11,6 +11,7 @@ import {
 import { clearSession } from '~/lib/auth'
 import type { Me } from '~/api/me'
 import type { ChartDetail, ImageDetail } from '~/api/images'
+import type { BrowseRepo } from '~/api/packages'
 import { textContent } from '~/test/text'
 import { routeTree } from '~/routeTree.gen'
 import { searchOptions } from '~/lib/search'
@@ -121,6 +122,7 @@ const fetchMock = vi.fn()
 
 /**
  * Route the mock: `/admin/me` -> identity (`me`, a reader by default),
+ * `/browse/repos` -> `repos`,
  * `/browse/images/{repo}/{image…}`, `/browse/chart` -> `chart` (404 without
  * one), and a `DELETE /admin/repositories/{repo}/images` -> `deleteImage`.
  */
@@ -130,9 +132,12 @@ function mockApi(
   {
     me = readerMe,
     deleteImage,
+    repos,
   }: {
     me?: Me
     deleteImage?: (repo: string, query: URLSearchParams) => Response | Promise<Response>
+    /** `/browse/repos` (404 without). */
+    repos?: BrowseRepo[]
   } = {},
 ) {
   fetchMock.mockImplementation((input: RequestInfo | URL, init?: RequestInit) => {
@@ -145,6 +150,9 @@ function mockApi(
     if (del && init?.method === 'DELETE' && deleteImage) {
       return Promise.resolve(deleteImage(decodeURIComponent(del[1]), url.searchParams))
     }
+    if (url.pathname.endsWith('/browse/repos') && repos) {
+      return Promise.resolve(jsonResponse(repos))
+    }
     if (url.pathname.endsWith('/browse/chart')) {
       return Promise.resolve(
         chart ? chart(url.searchParams) : new Response('chart not found', { status: 404 }),
@@ -483,6 +491,106 @@ describe('ImageDetail', () => {
     expect(within(manifests).getAllByText('docker-hosted')).toHaveLength(4)
   })
 
+  describe('labels a group\'s manifest times by the member they come from (#562)', () => {
+    const members: BrowseRepo[] = [
+      { id: 1, name: 'docker-hosted', kind: 'hosted', online: true, format: 'docker' },
+      { id: 2, name: 'docker-hub', kind: 'proxy', online: true, format: 'docker' },
+      { id: 3, name: 'docker-group', kind: 'group', online: true, format: 'docker' },
+    ]
+    const groupDetail = (sources: string[]) => ({
+      ...hostedDetail,
+      repo: 'docker-group',
+      repo_kind: 'group',
+      pull_name: 'forge.example.com/docker-group/team/app',
+      manifests: hostedDetail.manifests.map((m, i) => ({ ...m, source: sources[i] })),
+    })
+    /** The time cell of the manifest row whose digest starts `short`. */
+    const timeOf = (manifests: HTMLElement, short: string) => {
+      const header = within(manifests).getByRole('columnheader', { name: /Pushed|Cached/ })
+      const column = Array.from(header.parentElement!.children).indexOf(header)
+      const row = within(manifests)
+        .getByText(short, { selector: '.fg-digest-short' })
+        .closest('tr') as HTMLElement
+      return within(row).getAllByRole('cell')[column]
+    }
+
+    it('says per row whether a mixed group\'s time is a push or a cache', async () => {
+      // The index and its amd64 image from the proxy, the rest pushed.
+      mockApi(
+        () =>
+          jsonResponse(
+            groupDetail(['docker-hub', 'docker-hub', 'docker-hosted', 'docker-hosted']),
+          ),
+        undefined,
+        { repos: members },
+      )
+
+      await renderDetail('/images/docker-group/team/app')
+
+      const manifests = await screen.findByRole('region', { name: 'Manifests' })
+      await waitFor(() =>
+        expect(within(manifests).getAllByText('Cached', { selector: '.fg-cell-sub' })).toHaveLength(2),
+      )
+      expect(
+        within(manifests).getByRole('columnheader', { name: 'Pushed / cached' }),
+      ).toHaveAttribute(
+        'title',
+        'When the manifest was pushed to a hosted member, or first cached from the upstream by a proxy member',
+      )
+      expect(within(manifests).queryByRole('columnheader', { name: 'Pushed' })).toBeNull()
+      expect(within(manifests).queryByRole('columnheader', { name: 'Cached' })).toBeNull()
+      expect(timeOf(manifests, '1111111111aa')).toHaveTextContent(/Cached$/)
+      expect(timeOf(manifests, '2222222222bb')).toHaveTextContent(/Cached$/)
+      expect(timeOf(manifests, '4444444444dd')).toHaveTextContent(/Pushed$/)
+      expect(
+        within(manifests).getAllByText('Pushed', { selector: '.fg-cell-sub' }),
+      ).toHaveLength(2)
+    })
+
+    it('labels the column "Cached" when every manifest comes from a proxy member', async () => {
+      mockApi(() => jsonResponse(groupDetail(Array(4).fill('docker-hub'))), undefined, {
+        repos: members,
+      })
+
+      await renderDetail('/images/docker-group/team/app')
+
+      const manifests = await screen.findByRole('region', { name: 'Manifests' })
+      expect(
+        await within(manifests).findByRole('columnheader', { name: 'Cached' }),
+      ).toHaveAttribute('title', 'When Forge first cached this manifest from the upstream')
+      expect(within(manifests).queryByRole('columnheader', { name: /Pushed/ })).toBeNull()
+      // One kind for the column: no per-row label.
+      expect(within(manifests).queryByText('Cached', { selector: '.fg-cell-sub' })).toBeNull()
+    })
+
+    it('labels the column "Pushed" when every manifest comes from a hosted member', async () => {
+      mockApi(() => jsonResponse(groupDetail(Array(4).fill('docker-hosted'))), undefined, {
+        repos: members,
+      })
+
+      await renderDetail('/images/docker-group/team/app')
+
+      const manifests = await screen.findByRole('region', { name: 'Manifests' })
+      expect(
+        await within(manifests).findByRole('columnheader', { name: 'Pushed' }),
+      ).toBeInTheDocument()
+      expect(within(manifests).queryByRole('columnheader', { name: /Cached/ })).toBeNull()
+    })
+
+    it('keeps the neutral heading while a member\'s kind is not known', async () => {
+      // No `/browse/repos`: the members' kinds are unknown.
+      mockApi(() => jsonResponse(groupDetail(Array(4).fill('docker-hub'))))
+
+      await renderDetail('/images/docker-group/team/app')
+
+      const manifests = await screen.findByRole('region', { name: 'Manifests' })
+      expect(
+        within(manifests).getByRole('columnheader', { name: 'Pushed / cached' }),
+      ).toBeInTheDocument()
+      expect(within(manifests).queryByText(/^(Pushed|Cached)$/, { selector: '.fg-cell-sub' })).toBeNull()
+    })
+  })
+
   it('pulls an untagged image by the digest of its top-level manifest', async () => {
     mockApi(() =>
       jsonResponse({

web/src/routes/RepoBrowse.tsx

@@ -571,14 +571,23 @@ function ImageBrowseResults({
           <TableHeader align="right" width="110px">
             Manifests
           </TableHeader>
-          {/* A proxy's images carry their cache time, not a push (#248). */}
+          {/* A proxy's images carry their cache time, not a push (#248).
+              A group's merge its members' and name none of them, so its
+              column says both (#562). */}
           {repoKind === 'proxy' ? (
             <TableHeader
               width="170px"
               hint="When Forge last cached a manifest of the image from the upstream"
             >
               Last cached
             </TableHeader>
+          ) : repoKind === 'group' ? (
+            <TableHeader
+              width="170px"
+              hint="When a manifest of the image was last pushed to a hosted member, or cached from the upstream by a proxy member"
+            >
+              Last pushed / cached
+            </TableHeader>
           ) : (
             <TableHeader width="170px">Last pushed</TableHeader>
           )}