Commit efb7212f authored by Nathan Friend's avatar Nathan Friend

Speed up initial page load on Releases page

This commit updates the Releases page to make its initial GraphQL
query earlier in the application lifecycle, allowing the content to
load more quickly.

Changelog: performance
parent 449671f2
<script> <script>
import { GlButton } from '@gitlab/ui'; import { GlButton } from '@gitlab/ui';
import allReleasesQuery from 'shared_queries/releases/all_releases.query.graphql';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { historyPushState, getParameterByName } from '~/lib/utils/common_utils'; import { historyPushState, getParameterByName } from '~/lib/utils/common_utils';
import { scrollUp } from '~/lib/utils/scroll_utils'; import { scrollUp } from '~/lib/utils/scroll_utils';
import { setUrlParams } from '~/lib/utils/url_utility'; import { setUrlParams } from '~/lib/utils/url_utility';
import { __ } from '~/locale'; import { __ } from '~/locale';
import { PAGE_SIZE, DEFAULT_SORT } from '~/releases/constants'; import { PAGE_SIZE, DEFAULT_SORT } from '~/releases/constants';
import allReleasesQuery from '~/releases/graphql/queries/all_releases.query.graphql';
import { convertAllReleasesGraphQLResponse } from '~/releases/util'; import { convertAllReleasesGraphQLResponse } from '~/releases/util';
import ReleaseBlock from './release_block.vue'; import ReleaseBlock from './release_block.vue';
import ReleaseSkeletonLoader from './release_skeleton_loader.vue'; import ReleaseSkeletonLoader from './release_skeleton_loader.vue';
......
fragment Release on Release { fragment Release on Release {
__typename
name name
tagName tagName
tagPath tagPath
...@@ -7,15 +8,20 @@ fragment Release on Release { ...@@ -7,15 +8,20 @@ fragment Release on Release {
createdAt createdAt
upcomingRelease upcomingRelease
assets { assets {
__typename
count count
sources { sources {
__typename
nodes { nodes {
__typename
format format
url url
} }
} }
links { links {
__typename
nodes { nodes {
__typename
id id
name name
url url
...@@ -26,13 +32,16 @@ fragment Release on Release { ...@@ -26,13 +32,16 @@ fragment Release on Release {
} }
} }
evidences { evidences {
__typename
nodes { nodes {
__typename
filepath filepath
collectedAt collectedAt
sha sha
} }
} }
links { links {
__typename
editUrl editUrl
selfUrl selfUrl
openedIssuesUrl openedIssuesUrl
...@@ -42,22 +51,27 @@ fragment Release on Release { ...@@ -42,22 +51,27 @@ fragment Release on Release {
closedMergeRequestsUrl closedMergeRequestsUrl
} }
commit { commit {
__typename
sha sha
webUrl webUrl
title title
} }
author { author {
__typename
webUrl webUrl
avatarUrl avatarUrl
username username
} }
milestones { milestones {
__typename
nodes { nodes {
__typename
id id
title title
description description
webPath webPath
stats { stats {
__typename
totalIssuesCount totalIssuesCount
closedIssuesCount closedIssuesCount
} }
......
#import "../fragments/release.fragment.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( query allReleases(
$fullPath: ID! $fullPath: ID!
$first: Int $first: Int
...@@ -9,11 +15,14 @@ query allReleases( ...@@ -9,11 +15,14 @@ query allReleases(
$sort: ReleaseSort $sort: ReleaseSort
) { ) {
project(fullPath: $fullPath) { project(fullPath: $fullPath) {
__typename
releases(first: $first, last: $last, before: $before, after: $after, sort: $sort) { releases(first: $first, last: $last, before: $before, after: $after, sort: $sort) {
__typename
nodes { nodes {
...Release ...Release
} }
pageInfo { pageInfo {
__typename
startCursor startCursor
hasPreviousPage hasPreviousPage
hasNextPage hasNextPage
......
# 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
}
}
}
}
...@@ -4,6 +4,10 @@ module ReleasesHelper ...@@ -4,6 +4,10 @@ module ReleasesHelper
IMAGE_PATH = 'illustrations/releases.svg' IMAGE_PATH = 'illustrations/releases.svg'
DOCUMENTATION_PATH = 'user/project/releases/index' 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 def illustration
image_path(IMAGE_PATH) image_path(IMAGE_PATH)
end end
...@@ -25,6 +29,19 @@ module ReleasesHelper ...@@ -25,6 +29,19 @@ module ReleasesHelper
end end
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 def data_for_show_page
{ {
project_id: @project.id, project_id: @project.id,
......
- page_title _('Releases') - 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 } #js-releases-page{ data: data_for_releases_page }
...@@ -14,9 +14,14 @@ RSpec.describe 'User views releases', :js do ...@@ -14,9 +14,14 @@ RSpec.describe 'User views releases', :js do
let_it_be(:maintainer) { create(:user) } let_it_be(:maintainer) { create(:user) }
let_it_be(:guest) { 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 before do
project.add_maintainer(maintainer) project.add_maintainer(maintainer)
project.add_guest(guest) project.add_guest(guest)
stub_default_url_options(host: 'localhost')
end end
shared_examples 'releases index page' do shared_examples 'releases index page' do
...@@ -25,6 +30,8 @@ RSpec.describe 'User views releases', :js do ...@@ -25,6 +30,8 @@ RSpec.describe 'User views releases', :js do
sign_in(maintainer) sign_in(maintainer)
visit project_releases_path(project) visit project_releases_path(project)
wait_for_requests
end end
it 'sees the release' do it 'sees the release' do
...@@ -35,38 +42,18 @@ RSpec.describe 'User views releases', :js do ...@@ -35,38 +42,18 @@ RSpec.describe 'User views releases', :js do
end end
end end
context 'when there is a link as an asset' do it 'renders the correct links', :aggregate_failures do
let!(:release_link) { create(:release_link, release: release_v1, url: url ) } page.within("##{release_v1.tag} .js-assets-list") do
let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" } external_link_indicator_selector = '[data-testid="external-link-indicator"]'
let(:direct_asset_link) { Gitlab::Routing.url_helpers.project_release_url(project, release_v1) << "/downloads#{release_link.filepath}" }
it 'sees the link' do expect(page).to have_link internal_link.name, href: internal_link.url
page.within("##{release_v1.tag} .js-assets-list") do expect(find_link(internal_link.name)).not_to have_css(external_link_indicator_selector)
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
context 'when there is a link redirect' do 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}"
let!(:release_link) { create(:release_link, release: release_v1, name: 'linux-amd64 binaries', filepath: '/binaries/linux-amd64', url: url) } expect(find_link(internal_link_with_redirect.name)).not_to have_css(external_link_indicator_selector)
let(:url) { "#{project.web_url}/-/jobs/1/artifacts/download" }
it 'sees the link', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/329301' do expect(page).to have_link external_link.name, href: external_link.url
page.within("##{release_v1.tag} .js-assets-list") do expect(find_link(external_link.name)).to have_css(external_link_indicator_selector)
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
end end
end end
......
...@@ -3,6 +3,7 @@ import Vue from 'vue'; ...@@ -3,6 +3,7 @@ import Vue from 'vue';
import VueApollo from 'vue-apollo'; import VueApollo from 'vue-apollo';
import createMockApollo from 'helpers/mock_apollo_helper'; import createMockApollo from 'helpers/mock_apollo_helper';
import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import allReleasesQuery from 'shared_queries/releases/all_releases.query.graphql';
import createFlash from '~/flash'; import createFlash from '~/flash';
import { historyPushState } from '~/lib/utils/common_utils'; import { historyPushState } from '~/lib/utils/common_utils';
import ReleasesIndexApolloClientApp from '~/releases/components/app_index_apollo_client.vue'; import ReleasesIndexApolloClientApp from '~/releases/components/app_index_apollo_client.vue';
...@@ -12,7 +13,6 @@ import ReleasesEmptyState from '~/releases/components/releases_empty_state.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 ReleasesPaginationApolloClient from '~/releases/components/releases_pagination_apollo_client.vue';
import ReleasesSortApolloClient from '~/releases/components/releases_sort_apollo_client.vue'; import ReleasesSortApolloClient from '~/releases/components/releases_sort_apollo_client.vue';
import { PAGE_SIZE, CREATED_ASC, DEFAULT_SORT } from '~/releases/constants'; import { PAGE_SIZE, CREATED_ASC, DEFAULT_SORT } from '~/releases/constants';
import allReleasesQuery from '~/releases/graphql/queries/all_releases.query.graphql';
Vue.use(VueApollo); Vue.use(VueApollo);
......
...@@ -97,4 +97,42 @@ RSpec.describe ReleasesHelper do ...@@ -97,4 +97,42 @@ RSpec.describe ReleasesHelper do
end end
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment