Merge branch 'nfriend-implement-startup-queries-for-releases-page' into 'master'

Update the Releases page to make use of startup queries

See merge request gitlab-org/gitlab!63914

app/assets/javascripts/releases/components/app_index_apollo_client.vue

 <script>
 import { GlButton } from '@gitlab/ui';
+import allReleasesQuery from 'shared_queries/releases/all_releases.query.graphql';
 import createFlash from '~/flash';
 import { historyPushState, getParameterByName } from '~/lib/utils/common_utils';
 import { scrollUp } from '~/lib/utils/scroll_utils';
 import { setUrlParams } from '~/lib/utils/url_utility';
 import { __ } from '~/locale';
 import { PAGE_SIZE, DEFAULT_SORT } from '~/releases/constants';
-import allReleasesQuery from '~/releases/graphql/queries/all_releases.query.graphql';
 import { convertAllReleasesGraphQLResponse } from '~/releases/util';
 import ReleaseBlock from './release_block.vue';
 import ReleaseSkeletonLoader from './release_skeleton_loader.vue';

app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql

 fragment Release on Release {
+  __typename
   name
   tagName
   tagPath
@@ -7,15 +8,20 @@ fragment Release on Release {
   createdAt
   upcomingRelease
   assets {
+    __typename
     count
     sources {
+      __typename
       nodes {
+        __typename
         format
         url
       }
     }
     links {
+      __typename
       nodes {
+        __typename
         id
         name
         url
@@ -26,13 +32,16 @@ fragment Release on Release {
     }
   }
   evidences {
+    __typename
     nodes {
+      __typename
       filepath
       collectedAt
       sha
     }
   }
   links {
+    __typename
     editUrl
     selfUrl
     openedIssuesUrl
@@ -42,22 +51,27 @@ fragment Release on Release {
     closedMergeRequestsUrl
   }
   commit {
+    __typename
     sha
     webUrl
     title
   }
   author {
+    __typename
     webUrl
     avatarUrl
     username
   }
   milestones {
+    __typename
     nodes {
+      __typename
       id
       title
       description
       webPath
       stats {
+        __typename
         totalIssuesCount
         closedIssuesCount
       }

app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql

 #import "../fragments/release.fragment.graphql"
 
+# This query is identical to
+# `app/graphql/queries/releases/all_releases.query.graphql`.
+# These two queries should be kept in sync.
+# When the `releases_index_apollo_client` feature flag is
+# removed, this query should be removed entirely.
+
 query allReleases(
   $fullPath: ID!
   $first: Int
@@ -9,11 +15,14 @@ query allReleases(
   $sort: ReleaseSort
 ) {
   project(fullPath: $fullPath) {
+    __typename
     releases(first: $first, last: $last, before: $before, after: $after, sort: $sort) {
+      __typename
       nodes {
         ...Release
       }
       pageInfo {
+        __typename
         startCursor
         hasPreviousPage
         hasNextPage

app/graphql/queries/releases/all_releases.query.graphql

+# This query is identical to
+# `app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql`.
+# These two queries should be kept in sync.
+query allReleases(
+  $fullPath: ID!
+  $first: Int
+  $last: Int
+  $before: String
+  $after: String
+  $sort: ReleaseSort
+) {
+  project(fullPath: $fullPath) {
+    __typename
+    releases(first: $first, last: $last, before: $before, after: $after, sort: $sort) {
+      __typename
+      nodes {
+        __typename
+        name
+        tagName
+        tagPath
+        descriptionHtml
+        releasedAt
+        createdAt
+        upcomingRelease
+        assets {
+          __typename
+          count
+          sources {
+            __typename
+            nodes {
+              __typename
+              format
+              url
+            }
+          }
+          links {
+            __typename
+            nodes {
+              __typename
+              id
+              name
+              url
+              directAssetUrl
+              linkType
+              external
+            }
+          }
+        }
+        evidences {
+          __typename
+          nodes {
+            __typename
+            filepath
+            collectedAt
+            sha
+          }
+        }
+        links {
+          __typename
+          editUrl
+          selfUrl
+          openedIssuesUrl
+          closedIssuesUrl
+          openedMergeRequestsUrl
+          mergedMergeRequestsUrl
+          closedMergeRequestsUrl
+        }
+        commit {
+          __typename
+          sha
+          webUrl
+          title
+        }
+        author {
+          __typename
+          webUrl
+          avatarUrl
+          username
+        }
+        milestones {
+          __typename
+          nodes {
+            __typename
+            id
+            title
+            description
+            webPath
+            stats {
+              __typename
+              totalIssuesCount
+              closedIssuesCount
+            }
+          }
+        }
+      }
+      pageInfo {
+        __typename
+        startCursor
+        hasPreviousPage
+        hasNextPage
+        endCursor
+      }
+    }
+  }
+}

app/helpers/releases_helper.rb

@@ -4,6 +4,10 @@ module ReleasesHelper
   IMAGE_PATH = 'illustrations/releases.svg'
   DOCUMENTATION_PATH = 'user/project/releases/index'
 
+  # This needs to be kept in sync with the constant in
+  # app/assets/javascripts/releases/constants.js
+  DEFAULT_SORT = 'RELEASED_AT_DESC'
+
   def illustration
     image_path(IMAGE_PATH)
   end
@@ -25,6 +29,19 @@ module ReleasesHelper
     end
   end
 
+  # For simplicity, only optimize non-paginated requests
+  def use_startup_query_for_index_page?
+    params[:before].nil? && params[:after].nil?
+  end
+
+  def index_page_startup_query_variables
+    {
+      fullPath: @project.full_path,
+      sort: DEFAULT_SORT,
+      first: 1
+    }
+  end
+
   def data_for_show_page
     {
       project_id: @project.id,

app/views/projects/releases/index.html.haml

 - page_title _('Releases')
+- if use_startup_query_for_index_page?
+  - add_page_startup_graphql_call('releases/all_releases', index_page_startup_query_variables)
 
 #js-releases-page{ data: data_for_releases_page }

spec/features/projects/releases/user_views_releases_spec.rb

@@ -14,9 +14,14 @@ RSpec.describe 'User views releases', :js do
   let_it_be(:maintainer) { create(:user) }
   let_it_be(:guest) { create(:user) }
 
+  let_it_be(:internal_link) { create(:release_link, release: release_v1, name: 'An internal link', url: "#{project.web_url}/-/jobs/1/artifacts/download", filepath: nil) }
+  let_it_be(:internal_link_with_redirect) { create(:release_link, release: release_v1, name: 'An internal link with a redirect', url: "#{project.web_url}/-/jobs/2/artifacts/download", filepath: '/binaries/linux-amd64' ) }
+  let_it_be(:external_link) { create(:release_link, release: release_v1, name: 'An external link', url: "https://example.com/an/external/link", filepath: nil) }
+
   before do
     project.add_maintainer(maintainer)
     project.add_guest(guest)
+    stub_default_url_options(host: 'localhost')
   end
 
   shared_examples 'releases index page' do
@@ -25,6 +30,8 @@ RSpec.describe 'User views releases', :js do
         sign_in(maintainer)
 
         visit project_releases_path(project)
+
+        wait_for_requests
       end
 
       it 'sees the release' do
@@ -35,38 +42,18 @@ RSpec.describe 'User views releases', :js do
         end
       end
 
-      context 'when there is a link as an asset' do
-        let!(:release_link) { create(:release_link, release: release_v1, url: url ) }
-        let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" }
-        let(:direct_asset_link) { Gitlab::Routing.url_helpers.project_release_url(project, release_v1) << "/downloads#{release_link.filepath}" }
+      it 'renders the correct links', :aggregate_failures do
+        page.within("##{release_v1.tag} .js-assets-list") do
+          external_link_indicator_selector = '[data-testid="external-link-indicator"]'
 
-        it 'sees the link' do
-          page.within("##{release_v1.tag} .js-assets-list") do
-            expect(page).to have_link release_link.name, href: direct_asset_link
-            expect(page).not_to have_css('[data-testid="external-link-indicator"]')
-          end
-        end
+          expect(page).to have_link internal_link.name, href: internal_link.url
+          expect(find_link(internal_link.name)).not_to have_css(external_link_indicator_selector)
 
-        context 'when there is a link redirect' do
-          let!(:release_link) { create(:release_link, release: release_v1, name: 'linux-amd64 binaries', filepath: '/binaries/linux-amd64', url: url) }
-          let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" }
+          expect(page).to have_link internal_link_with_redirect.name, href: Gitlab::Routing.url_helpers.project_release_url(project, release_v1) << "/downloads#{internal_link_with_redirect.filepath}"
+          expect(find_link(internal_link_with_redirect.name)).not_to have_css(external_link_indicator_selector)
 
-          it 'sees the link', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/329301' do
-            page.within("##{release_v1.tag} .js-assets-list") do
-              expect(page).to have_link release_link.name, href: direct_asset_link
-              expect(page).not_to have_css('[data-testid="external-link-indicator"]')
-            end
-          end
-        end
-
-        context 'when url points to external resource' do
-          let(:url) { 'http://google.com/download' }
-
-          it 'sees that the link is external resource', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/329302' do
-            page.within("##{release_v1.tag} .js-assets-list") do
-              expect(page).to have_css('[data-testid="external-link-indicator"]')
-            end
-          end
+          expect(page).to have_link external_link.name, href: external_link.url
+          expect(find_link(external_link.name)).to have_css(external_link_indicator_selector)
         end
       end
 

spec/frontend/releases/components/app_index_apollo_client_spec.js

@@ -3,6 +3,7 @@ import Vue from 'vue';
 import VueApollo from 'vue-apollo';
 import createMockApollo from 'helpers/mock_apollo_helper';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
+import allReleasesQuery from 'shared_queries/releases/all_releases.query.graphql';
 import createFlash from '~/flash';
 import { historyPushState } from '~/lib/utils/common_utils';
 import ReleasesIndexApolloClientApp from '~/releases/components/app_index_apollo_client.vue';
@@ -12,7 +13,6 @@ import ReleasesEmptyState from '~/releases/components/releases_empty_state.vue';
 import ReleasesPaginationApolloClient from '~/releases/components/releases_pagination_apollo_client.vue';
 import ReleasesSortApolloClient from '~/releases/components/releases_sort_apollo_client.vue';
 import { PAGE_SIZE, CREATED_ASC, DEFAULT_SORT } from '~/releases/constants';
-import allReleasesQuery from '~/releases/graphql/queries/all_releases.query.graphql';
 
 Vue.use(VueApollo);
 

spec/helpers/releases_helper_spec.rb

@@ -97,4 +97,42 @@ RSpec.describe ReleasesHelper do
       end
     end
   end
+
+  describe 'startup queries' do
+    describe 'use_startup_query_for_index_page?' do
+      it 'allows startup queries for non-paginated requests' do
+        allow(helper).to receive(:params).and_return({ unrelated_query_param: 'value' })
+
+        expect(helper.use_startup_query_for_index_page?).to be(true)
+      end
+
+      it 'disallows startup queries for requests paginated with a "before" cursor' do
+        allow(helper).to receive(:params).and_return({ unrelated_query_param: 'value', before: 'cursor' })
+
+        expect(helper.use_startup_query_for_index_page?).to be(false)
+      end
+
+      it 'disallows startup queries for requests paginated with an "after" cursor' do
+        allow(helper).to receive(:params).and_return({ unrelated_query_param: 'value', after: 'cursor' })
+
+        expect(helper.use_startup_query_for_index_page?).to be(false)
+      end
+    end
+
+    describe '#index_page_startup_query_variables' do
+      let_it_be(:project) { build(:project, namespace: create(:group)) }
+
+      before do
+        helper.instance_variable_set(:@project, project)
+      end
+
+      it 'returns the correct GraphQL variables for the startup query' do
+        expect(helper.index_page_startup_query_variables).to eq({
+          fullPath: project.full_path,
+          sort: 'RELEASED_AT_DESC',
+          first: 1
+        })
+      end
+    end
+  end
 end