monolith/S05 virtual Step 8: Connect the virtual detail view to the schema

What this does

Step 8 of the monolith/S05 virtual repository detail plan turns out to be the test side of connecting the virtual detail page to the GraphQL schema, not the application side. The step was written on the assumption that the upstream query had been built against Apollo local state, so its scope was to drop an @client directive, the local type declarations, and a mock resolver. None of that was ever added. The upstream query was pointed at the real schema from the start, so the application code needed no change here. What is left is to stop the specs reading hand written mocks: a new JavaScript fixture definition runs the upstream document against the real schema and writes its answer to disk, the virtual detail and upstream table specs read that instead, and a routed :js example renders the upstream table in a real browser.

One behaviour change came out of review. A null upstreamRepositories used to replace the whole page with the not-found state; it now renders a main region alert and leaves the page standing. The feature stays dark behind the disabled artifact_registry_ui flag, so nothing reaches users.

Closes #628669 (closed)

Related to #628475 (closed)

Changes

  • Add ee/spec/frontend/fixtures/artifact_registry/repository_upstream_repositories.rb, which runs the upstream document against the real schema and writes two fixtures: a virtual Maven repository with three upstreams and a virtual Docker one with two.
  • Re-point repository_detail_spec.js and upstream_repositories_table_spec.js at those fixtures, keeping the same assertions.
  • Delete the detail spec's asSelected helper. It existed only to strip a format the document never selects, and the schema's own answer does not carry one.
  • Add a :js example to ee/spec/features/artifact_registry/repositories_spec.rb rendering a virtual repository's upstream table over a doubled ArtifactRegistry::Client, with the automated accessibility assertion.
  • Add a table case that feeds the container fixture rows back with a format attached, so the "renders neither format" guard can fail if the table ever starts rendering one.
  • Stop rendering the not-found state when upstreamRepositories resolves null. repository_detail.vue now renders the existing main region alert instead, with copy naming the list rather than the service, and keeps the heading, the actions and the sidebar. The VirtualUpstreamsNotFound story is renamed VirtualUpstreamsNotReturned, since the name described the old render.

Notes for the reviewer

This MR is stacked on Step 7. It targets 628668-upstream-row-actions (!255796 (merged)), because Step 7 is the only place upstream_repository_actions.vue and the table's actions column exist. Once !255796 (merged) merges, this needs a rebase onto master and a retarget.

Nothing from the plan's @client removal list applied. Confirmed on this branch: the upstream query carries no @client directive, typedefs.graphql and mock_resolvers.js hold no upstream entries, and mock_upstream_repositories.js does not exist. mock_resolvers_spec.js and queries_spec.js needed no edits either, since queries_spec.js already asserts the document goes to the server whole. Step 6 below is the runnable proof.

mockMixedContainerUpstreamRepositories stays in mock_data.js. It looked dead once both specs read fixtures, but repository_detail.stories.js imports it for the VirtualContainerRepository story. An earlier revision of this description said Storybook cannot resolve the test_fixtures alias, which is wrong: storybook/config/webpack.config.js:254 aliases it, and EE stories already import through it. The real reason is that generated fixture JSON is not committed, so a story reading one soft-fails to storybook/fixture_stub.js whenever the fixtures have not been generated. The other upstream builders stay for the same reason, or because the 20 row volume case is about length rather than schema shape.

A null upstream list no longer renders a 404. isNotFound treated upstreamRepositories === null as a missing repository and replaced the page. By that point the detail read has already returned the repository, so the page has confirmed it exists and that the viewer can see it, and hiding it discards a result the viewer is entitled to. A null now renders the same upstream-repositories-error alert a failed upstream read renders, with the heading, actions and sidebar left in place. The two causes keep separate copy: a query that threw still says the service is unavailable, which is what the spec's error row asks for, while a null says Failed to load upstream repositories., since AR answered. An empty array is untouched and still renders the empty state.

Why the spec says otherwise, and the third null cause it missed

docs/specs/monolith/S05-repository-detail.md in gitlab-org/ops/artifact-registry specifies the old rendering:

404 on the upstream list under a loaded repository. The route's kind gate answers 404 for a repository that is not virtual, and so does a repository deleted between the two reads. The response cannot distinguish them, so the field resolves null and the view re-renders the not-found state, the handling the artifact lists already have.

That spec line needs the matching amendment, which is handled separately in the artifact-registry repository.

The spec reasons from two causes, the kind gate and the deletion race. There is a third. RendersErrors#render_artifact_registry_authorization_error returns nil for a query when AR answers 401 or 403 carrying a status, logging it rather than raising. So an authorization rejection scoped to the upstream listing resolves the field null too, under a repository the detail read just returned through a field that authorized the viewer. Existence hiding has nothing left to hide at that point.

The artifact connection arm of isNotFound is deliberately left alone. It has the same shape, but it is untouched by this MR and its own spec row reads the same way, so changing it belongs with the spec amendment rather than here.

The format guard is now split in two. Both old assertions guarded that an upstream summary carries no format, and against a fixture that never had the key they were vacuous. The fixture key-set assertion stays as the schema canary, and a second case feeds the same container rows back with DOCKER and OCI attached, so the "renders neither format" check now exercises the table rather than the fixture. Verified by rendering format in the name cell on a scratch edit: the new case fails, and it passes again once the edit is reverted.

How to set up and validate locally

Run from the repository root. No running Artifact Registry is needed: the fixture stubs the REST reads with WebMock, and the :js example doubles the client, as every example in that file already does.

Step 1: Generate the two JavaScript fixtures

bundle exec rspec ee/spec/frontend/fixtures/artifact_registry/repository_upstream_repositories.rb
Output (✅)
Finished in 9.83 seconds (files took 25.97 seconds to load)
2 examples, 0 failures

Both fixtures landed:

maven     | name=maven-virtual  | kind=VIRTUAL | upstream rows=3
container | name=docker-virtual | kind=VIRTUAL | upstream rows=2

The Maven fixture, which is the schema's own answer rather than a transcription of it:

{
  "data": {
    "organization": {
      "artifactRegistryRepository": {
        "name": "maven-virtual",
        "kind": "VIRTUAL",
        "upstreamRepositories": [
          {
            "id": "a1000000-0000-4000-8000-000000000001",
            "position": 1,
            "upstreamRepository": {
              "id": "d1000000-0000-4000-8000-000000000001",
              "name": "maven-central-proxy",
              "kind": "REMOTE",
              "__typename": "ArtifactRegistryUpstreamRepositorySummary"
            },
            "__typename": "ArtifactRegistryUpstreamRepositoryAssociation"
          },
          {
            "id": "a1000000-0000-4000-8000-000000000002",
            "position": 2,
            "upstreamRepository": {
              "id": "d1000000-0000-4000-8000-000000000002",
              "name": "payments-releases",
              "kind": "HOSTED",
              "__typename": "ArtifactRegistryUpstreamRepositorySummary"
            },
            "__typename": "ArtifactRegistryUpstreamRepositoryAssociation"
          },
          {
            "id": "a1000000-0000-4000-8000-000000000003",
            "position": 3,
            "upstreamRepository": {
              "id": "d1000000-0000-4000-8000-000000000003",
              "name": "platform-snapshots",
              "kind": "HOSTED",
              "__typename": "ArtifactRegistryUpstreamRepositorySummary"
            },
            "__typename": "ArtifactRegistryUpstreamRepositoryAssociation"
          }
        ],
        "__typename": "ArtifactRegistryRepositoryDetails"
      }
    }
  }
}

Each summary carries id, name, kind and __typename and no format, which is what makes the deleted asSelected helper redundant.

Step 2: Run the two re-pointed Jest specs

node_modules/.bin/jest --config jest.config.js \
  ee/spec/frontend/packages_and_registries/artifact_registry/repositories/detail/repository_detail_spec.js \
  ee/spec/frontend/packages_and_registries/artifact_registry/repositories/detail/upstream_repositories_table_spec.js
Output (✅)
Test Suites: 2 passed, 2 total
Tests:       158 passed, 158 total
Snapshots:   0 total
Time:        3.951 s

Step 3: Run the Jest specs that must not regress

node_modules/.bin/jest --config jest.config.js \
  ee/spec/frontend/packages_and_registries/artifact_registry/graphql/queries_spec.js \
  ee/spec/frontend/packages_and_registries/artifact_registry/graphql/cache_config_spec.js
Output (✅)
Test Suites: 2 passed, 2 total
Tests:       41 passed, 41 total
Time:        1.208 s

cache_config_spec.js still reads the hand written mockUpstreamRepositories, and queries_spec.js still asserts the document goes to the server whole. Neither needed an edit.

Step 4: Validate the upstream document against the schema

bundle exec rake "gitlab:graphql:validate[ee/app/assets/javascripts/packages_and_registries/artifact_registry/graphql/queries/get_repository_upstream_repositories.query.graphql]"
Output (✅)
Validation timeout set to 1 second(s)
OK    ee/app/assets/javascripts/packages_and_registries/artifact_registry/graphql/queries/get_repository_upstream_repositories.query.graphql

OK rather than SKIP ... client query is the point of the step. The task walks the document against the real schema, which it could only do if no @client directive were shielding a field from it.

Step 5: Render the virtual upstream table in a browser

bundle exec rspec ee/spec/features/artifact_registry/repositories_spec.rb \
  -e 'renders the upstream table and passes axe automated accessibility testing'
Output (✅)
Finished in 40.2 seconds (files took 43.34 seconds to load)
1 example, 0 failures

The whole file, to show the new example sits beside the existing ones without disturbing them:

bundle exec rspec ee/spec/features/artifact_registry/repositories_spec.rb

10 examples, 0 failures

Step 6: Confirm no local GraphQL scaffolding remains for the upstream surface

grep -rn "@client" ee/app/assets/javascripts/packages_and_registries/artifact_registry/graphql/queries/get_repository_upstream_repositories.query.graphql
grep -rin "upstream" ee/app/assets/javascripts/packages_and_registries/artifact_registry/graphql/typedefs.graphql \
  ee/app/assets/javascripts/packages_and_registries/artifact_registry/graphql/mock_resolvers.js
ls ee/app/assets/javascripts/packages_and_registries/artifact_registry/graphql/mock_upstream_repositories.js
Output (✅)
# @client in the upstream document
(no matches, exit 1)

# upstream in typedefs.graphql or mock_resolvers.js
(no matches, exit 1)

# mock_upstream_repositories.js
ls: .../graphql/mock_upstream_repositories.js: No such file or directory

For contrast, the one @client directive left in the whole query directory is the unrelated manifest lookup, which the plan says stays while its backend is unmerged:

queries/get_manifest.query.graphql:19:      manifest(digest: $digest, artifactId: $artifactId) @client {

Evidence

Screen recordings

Recorded on this branch at 3795a62, driving the real page in the GDK at 1440x900. Real AR cannot serve either state, since it answers 422 to a virtual create and its listUpstreamRepositories route is still contract only, so the monolith was pointed at a small HTTP stub standing in for AR's REST boundary. Everything above it is real: the GraphQL resolver, the ArtifactRegistry::Client, Apollo, and the Vue page.

A virtual npm repository with upstreams

npm-virtual-with-upstreams

The table renders the three associations in the order AR returned them, storefront-releases and design-system-npm as Hosted and npmjs-proxy as Remote, with the order badges, the row actions column, the sidebar count reading 3 Repositories, and the Setup instructions button.

A virtual npm repository with no upstreams

npm-virtual-no-upstreams

An empty array renders the illustrated empty state in place of the table. The sidebar stats and the Setup instructions button both drop, which is the same treatment an empty main region gets on every kind.

Both clips were re-recorded without the scroll pass an earlier revision had, which moved the heading up behind the sticky breadcrumb and back and read as a rendering fault that is not there.

Asserted in the same run, on visible nodes only: upstream names storefront-releases, design-system-npm, npmjs-proxy; kinds Hosted, Hosted, Remote; order badges 1, 2, 3; the empty state present on the second repository and absent on the first; no error alert and no not-found state on either. The only console errors are the twelve /-/collect_events CSP rejections every GDK page logs when it is served from :3333 while the configured external URL is :3000.

A virtual npm repository whose upstream list resolves null

This is the state the MR changes, so it is recorded on both sides. The stub answers 404 on the upstream listing for npm-virtual-upstreams-404 while serving the repository itself normally, which is what makes ArtifactRegistry::Client#upstream_repositories return nil through nil_on_missing and the field resolve null.

Before, on master:

npm-virtual-upstreams-null-before

After, on this branch at 3795a62:

npm-virtual-upstreams-null-after

Both runs open the same repository from the same listing against the same stub. On master the page is replaced by Page not found: no heading, no sidebar, nothing naming the repository the detail read had already returned. On this branch the alert reads Failed to load upstream repositories. in the main region, and the heading, the description, the Virtual badge, Setup instructions, the actions menu and the sidebar all stay rendered. Asserted in the same runs: notFound true and no alert before, no notFound and the alert text after, sidebar absent before and present after, and no table or empty state in either.

The routed :js example in Step 5 above is the other browser level evidence: it drives the virtual detail page in a headless browser, asserts the position badges, upstream names, and kind badges the table renders, and runs the automated accessibility check on the result.

Edited by Fiona McCawley

Merge request reports

Loading
Loading