Skip to content
Snippets Groups Projects
Commit 00335da7 authored by Rahul Chanila's avatar Rahul Chanila :two: Committed by Jose Ivan Vargas
Browse files

Defers loading of package assets on package details page

Changelog: changed
parent 0f06af58
No related branches found
No related tags found
1 merge request!120828Defers loading of package assets on package details page
Showing
with 236 additions and 145 deletions
<script>
import { GlLink, GlTable, GlDropdownItem, GlDropdown, GlButton, GlFormCheckbox } from '@gitlab/ui';
import {
GlAlert,
GlLink,
GlTable,
GlDropdownItem,
GlDropdown,
GlButton,
GlFormCheckbox,
GlLoadingIcon,
} from '@gitlab/ui';
import { last } from 'lodash';
import { numberToHumanSize } from '~/lib/utils/number_utils';
import { __, s__ } from '~/locale';
......@@ -9,21 +18,26 @@ import { packageTypeToTrackCategory } from '~/packages_and_registries/package_re
import FileIcon from '~/vue_shared/components/file_icon.vue';
import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
import {
FETCH_PACKAGE_FILES_ERROR_MESSAGE,
GRAPHQL_PACKAGE_FILES_PAGE_SIZE,
REQUEST_DELETE_SELECTED_PACKAGE_FILE_TRACKING_ACTION,
SELECT_PACKAGE_FILE_TRACKING_ACTION,
TRACKING_LABEL_PACKAGE_ASSET,
TRACKING_ACTION_EXPAND_PACKAGE_ASSET,
} from '~/packages_and_registries/package_registry/constants';
import getPackageFilesQuery from '~/packages_and_registries/package_registry/graphql/queries/get_package_files.query.graphql';
export default {
name: 'PackageFiles',
components: {
GlAlert,
GlLink,
GlTable,
GlDropdown,
GlDropdownItem,
GlFormCheckbox,
GlButton,
GlLoadingIcon,
FileIcon,
TimeAgoTooltip,
FileSha,
......@@ -40,14 +54,36 @@ export default {
required: false,
default: false,
},
packageId: {
type: String,
required: true,
},
packageType: {
type: String,
required: true,
},
},
apollo: {
packageFiles: {
type: Array,
required: false,
default: () => [],
query: getPackageFilesQuery,
context: {
isSingleRequest: true,
},
variables() {
return this.queryVariables;
},
update(data) {
return data.package?.packageFiles?.nodes || [];
},
error() {
this.fetchPackageFilesError = true;
},
},
},
data() {
return {
fetchPackageFilesError: false,
packageFiles: [],
selectedReferences: [],
};
},
......@@ -56,7 +92,7 @@ export default {
return this.selectedReferences.length > 0;
},
areAllFilesSelected() {
return this.packageFiles.every(this.isSelected);
return this.packageFiles.length > 0 && this.packageFiles.every(this.isSelected);
},
filesTableRows() {
return this.packageFiles.map((pf) => ({
......@@ -68,10 +104,8 @@ export default {
hasSelectedSomeFiles() {
return this.areFilesSelected && !this.areAllFilesSelected;
},
showCommitColumn() {
// note that this is always false for now since we do not return
// pipelines associated to files for performance concerns
return this.filesTableRows.some((row) => Boolean(row.pipeline?.id));
loading() {
return this.$apollo.queries.packageFiles.loading || this.isLoading;
},
filesTableHeaderFields() {
return [
......@@ -85,11 +119,6 @@ export default {
key: 'name',
label: __('Name'),
},
{
key: 'commit',
label: __('Commit'),
hide: !this.showCommitColumn,
},
{
key: 'size',
label: __('Size'),
......@@ -108,6 +137,12 @@ export default {
},
].filter((c) => !c.hide);
},
queryVariables() {
return {
id: this.packageId,
first: GRAPHQL_PACKAGE_FILES_PAGE_SIZE,
};
},
tracking() {
return {
category: packageTypeToTrackCategory(this.packageType),
......@@ -142,6 +177,7 @@ export default {
deleteFile: __('Delete asset'),
deleteSelected: s__('PackageRegistry|Delete selected'),
moreActionsText: __('More actions'),
fetchPackageFilesErrorMessage: FETCH_PACKAGE_FILES_ERROR_MESSAGE,
},
};
</script>
......@@ -151,8 +187,8 @@ export default {
<div class="gl-display-flex gl-align-items-center gl-justify-content-space-between">
<h3 class="gl-font-lg gl-mt-5">{{ __('Assets') }}</h3>
<gl-button
v-if="canDelete"
:disabled="isLoading || !areFilesSelected"
v-if="!fetchPackageFilesError && canDelete"
:disabled="loading || !areFilesSelected"
category="secondary"
variant="danger"
data-testid="delete-selected"
......@@ -161,7 +197,16 @@ export default {
{{ $options.i18n.deleteSelected }}
</gl-button>
</div>
<gl-alert
v-if="fetchPackageFilesError"
variant="danger"
@dismiss="fetchPackageFilesError = false"
>
{{ $options.i18n.fetchPackageFilesErrorMessage }}
</gl-alert>
<gl-table
v-else
:busy="loading"
:fields="filesTableHeaderFields"
:items="filesTableRows"
show-empty
......@@ -171,6 +216,9 @@ export default {
:tbody-tr-attr="{ 'data-testid': 'file-row' }"
@row-selected="updateSelectedReferences"
>
<template #table-busy>
<gl-loading-icon size="lg" class="gl-my-5" />
</template>
<template #head(checkbox)="{ selectAllRows, clearSelected }">
<gl-form-checkbox
v-if="canDelete"
......@@ -218,16 +266,6 @@ export default {
</gl-link>
</template>
<template #cell(commit)="{ item }">
<gl-link
v-if="item.pipeline && item.pipeline"
:href="item.pipeline.commitPath"
class="gl-text-gray-500"
data-testid="commit-link"
>{{ item.pipeline.sha }}
</gl-link>
</template>
<template #cell(created)="{ item }">
<time-ago-tooltip :time="item.createdAt" />
</template>
......
......@@ -102,6 +102,9 @@ export const FETCH_PACKAGE_PIPELINES_ERROR_MESSAGE = s__(
export const FETCH_PACKAGE_METADATA_ERROR_MESSAGE = s__(
'PackageRegistry|Something went wrong while fetching the package metadata.',
);
export const FETCH_PACKAGE_FILES_ERROR_MESSAGE = s__(
'PackageRegistry|Something went wrong while fetching package assets.',
);
export const DELETE_PACKAGES_TRACKING_ACTION = 'delete_packages';
export const REQUEST_DELETE_PACKAGES_TRACKING_ACTION = 'request_delete_packages';
......@@ -232,3 +235,4 @@ export const REQUEST_FORWARDING_HELP_PAGE_PATH = helpPagePath(
);
export const GRAPHQL_PACKAGE_PIPELINES_PAGE_SIZE = 10;
export const GRAPHQL_PACKAGE_FILES_PAGE_SIZE = 100;
......@@ -21,6 +21,9 @@ export const apolloProvider = new VueApollo({
keyArgs: false,
merge: mergeVariables,
},
packageFiles: {
merge: mergeVariables,
},
},
},
},
......
......@@ -52,13 +52,7 @@ query getPackageDetails($id: PackagesPackageID!) {
}
nodes {
id
fileMd5
fileName
fileSha1
fileSha256
size
createdAt
downloadPath
}
}
versions {
......
query getPackageFiles($id: PackagesPackageID!, $first: Int) {
package(id: $id) {
id
packageFiles(first: $first) {
nodes {
id
fileMd5
fileName
fileSha1
fileSha256
size
createdAt
downloadPath
}
}
}
}
......@@ -158,8 +158,8 @@ export default {
isLoading() {
return this.$apollo.queries.packageEntity.loading;
},
packageFilesLoading() {
return this.isLoading || this.mutationLoading;
packageFilesMutationLoading() {
return this.mutationLoading;
},
isValidPackage() {
return this.isLoading || Boolean(this.packageEntity.name);
......@@ -360,7 +360,7 @@ export default {
<gl-tabs>
<gl-tab :title="__('Detail')">
<div v-if="!isLoading" data-qa-selector="package_information_content">
<div data-qa-selector="package_information_content">
<package-history :package-entity="packageEntity" :project-name="projectName" />
<installation-commands :package-entity="packageEntity" />
......@@ -370,16 +370,17 @@ export default {
:package-id="packageEntity.id"
:package-type="packageType"
/>
</div>
<package-files
v-if="showFiles"
:can-delete="packageEntity.canDestroy"
:is-loading="packageFilesLoading"
:package-files="packageFiles"
@download-file="track($options.trackingActions.DOWNLOAD_PACKAGE_ASSET_TRACKING_ACTION)"
@delete-files="handleFileDelete"
/>
<package-files
v-if="showFiles"
:can-delete="packageEntity.canDestroy"
:is-loading="packageFilesMutationLoading"
:package-id="packageEntity.id"
:package-type="packageType"
@download-file="track($options.trackingActions.DOWNLOAD_PACKAGE_ASSET_TRACKING_ACTION)"
@delete-files="handleFileDelete"
/>
</div>
</gl-tab>
<gl-tab v-if="showDependencies">
......
......@@ -32141,6 +32141,9 @@ msgstr ""
msgid "PackageRegistry|Something went wrong while deleting the package."
msgstr ""
 
msgid "PackageRegistry|Something went wrong while fetching package assets."
msgstr ""
msgid "PackageRegistry|Something went wrong while fetching the package history."
msgstr ""
 
import { GlDropdown, GlButton, GlFormCheckbox } from '@gitlab/ui';
import { nextTick } from 'vue';
import { GlAlert, GlDropdown, GlButton, GlFormCheckbox, GlLoadingIcon } from '@gitlab/ui';
import Vue, { nextTick } from 'vue';
import VueApollo from 'vue-apollo';
import { stubComponent } from 'helpers/stub_component';
import { mountExtended, extendedWrapper } from 'helpers/vue_test_utils_helper';
import { packageFiles as packageFilesMock } from 'jest/packages_and_registries/package_registry/mock_data';
import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises';
import { s__ } from '~/locale';
import {
packageFiles as packageFilesMock,
packageFilesQuery,
} from 'jest/packages_and_registries/package_registry/mock_data';
import PackageFiles from '~/packages_and_registries/package_registry/components/details/package_files.vue';
import FileIcon from '~/vue_shared/components/file_icon.vue';
import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
import getPackageFiles from '~/packages_and_registries/package_registry/graphql/queries/get_package_files.query.graphql';
Vue.use(VueApollo);
describe('Package Files', () => {
let wrapper;
let apolloProvider;
const findAllRows = () => wrapper.findAllByTestId('file-row');
const findDeleteSelectedButton = () => wrapper.findByTestId('delete-selected');
const findFirstRow = () => extendedWrapper(findAllRows().at(0));
const findSecondRow = () => extendedWrapper(findAllRows().at(1));
const findPackageFilesAlert = () => wrapper.findComponent(GlAlert);
const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
const findFirstRowDownloadLink = () => findFirstRow().findByTestId('download-link');
const findFirstRowCommitLink = () => findFirstRow().findByTestId('commit-link');
const findSecondRowCommitLink = () => findSecondRow().findByTestId('commit-link');
const findFirstRowFileIcon = () => findFirstRow().findComponent(FileIcon);
const findFirstRowCreatedAt = () => findFirstRow().findComponent(TimeAgoTooltip);
const findFirstActionMenu = () => extendedWrapper(findFirstRow().findComponent(GlDropdown));
......@@ -30,16 +42,23 @@ describe('Package Files', () => {
const [file] = files;
const createComponent = ({
packageFiles = [file],
packageId = '1',
packageType = 'NPM',
isLoading = false,
canDelete = true,
stubs,
resolver = jest.fn().mockResolvedValue(packageFilesQuery([file])),
} = {}) => {
const requestHandlers = [[getPackageFiles, resolver]];
apolloProvider = createMockApollo(requestHandlers);
wrapper = mountExtended(PackageFiles, {
apolloProvider,
propsData: {
canDelete,
isLoading,
packageFiles,
packageId,
packageType,
},
stubs: {
GlTable: false,
......@@ -49,35 +68,61 @@ describe('Package Files', () => {
};
describe('rows', () => {
it('renders a single file for an npm package', () => {
it('do not get rendered when query is loading', () => {
createComponent();
expect(findLoadingIcon().exists()).toBe(true);
expect(findDeleteSelectedButton().props('disabled')).toBe(true);
});
it('renders a single file for an npm package', async () => {
createComponent();
await waitForPromises();
expect(findAllRows()).toHaveLength(1);
expect(findLoadingIcon().exists()).toBe(false);
});
it('renders multiple files for a package that contains more than one file', () => {
createComponent({ packageFiles: files });
it('renders multiple files for a package that contains more than one file', async () => {
createComponent({ resolver: jest.fn().mockResolvedValue(packageFilesQuery()) });
await waitForPromises();
expect(findAllRows()).toHaveLength(2);
});
it('does not render gl-alert', async () => {
createComponent();
await waitForPromises();
expect(findPackageFilesAlert().exists()).toBe(false);
});
it('renders gl-alert if load fails', async () => {
createComponent({ resolver: jest.fn().mockRejectedValue() });
await waitForPromises();
expect(findPackageFilesAlert().exists()).toBe(true);
expect(findPackageFilesAlert().text()).toBe(
s__('PackageRegistry|Something went wrong while fetching package assets.'),
);
});
});
describe('link', () => {
it('exists', () => {
beforeEach(async () => {
createComponent();
await waitForPromises();
});
it('exists', () => {
expect(findFirstRowDownloadLink().exists()).toBe(true);
});
it('has the correct attrs bound', () => {
createComponent();
expect(findFirstRowDownloadLink().attributes('href')).toBe(file.downloadPath);
});
it('emits "download-file" event on click', () => {
createComponent();
findFirstRowDownloadLink().vm.$emit('click');
expect(wrapper.emitted('download-file')).toEqual([[]]);
......@@ -85,90 +130,43 @@ describe('Package Files', () => {
});
describe('file-icon', () => {
it('exists', () => {
beforeEach(async () => {
createComponent();
await waitForPromises();
});
it('exists', () => {
expect(findFirstRowFileIcon().exists()).toBe(true);
});
it('has the correct props bound', () => {
createComponent();
expect(findFirstRowFileIcon().props('fileName')).toBe(file.fileName);
});
});
describe('time-ago tooltip', () => {
it('exists', () => {
beforeEach(async () => {
createComponent();
await waitForPromises();
});
it('exists', () => {
expect(findFirstRowCreatedAt().exists()).toBe(true);
});
it('has the correct props bound', () => {
createComponent();
expect(findFirstRowCreatedAt().props('time')).toBe(file.createdAt);
});
});
describe('commit', () => {
const withPipeline = {
...file,
pipelines: [
{
sha: 'sha',
id: 1,
commitPath: 'commitPath',
},
],
};
describe('when package file has a pipeline associated', () => {
it('exists', () => {
createComponent({ packageFiles: [withPipeline] });
expect(findFirstRowCommitLink().exists()).toBe(true);
});
it('the link points to the commit path', () => {
createComponent({ packageFiles: [withPipeline] });
expect(findFirstRowCommitLink().attributes('href')).toBe(
withPipeline.pipelines[0].commitPath,
);
});
it('the text is the pipeline sha', () => {
createComponent({ packageFiles: [withPipeline] });
expect(findFirstRowCommitLink().text()).toBe(withPipeline.pipelines[0].sha);
});
});
describe('when package file has no pipeline associated', () => {
it('does not exist', () => {
createComponent();
expect(findFirstRowCommitLink().exists()).toBe(false);
});
});
describe('when only one file lacks an associated pipeline', () => {
it('renders the commit when it exists and not otherwise', () => {
createComponent({ packageFiles: [withPipeline, file] });
expect(findFirstRowCommitLink().exists()).toBe(true);
expect(findSecondRowCommitLink().exists()).toBe(false);
});
});
});
describe('action menu', () => {
describe('when the user can delete', () => {
it('exists', () => {
beforeEach(async () => {
createComponent();
await waitForPromises();
});
it('exists', () => {
expect(findFirstActionMenu().exists()).toBe(true);
expect(findFirstActionMenu().props('icon')).toBe('ellipsis_v');
expect(findFirstActionMenu().props('textSrOnly')).toBe(true);
......@@ -178,14 +176,10 @@ describe('Package Files', () => {
describe('menu items', () => {
describe('delete file', () => {
it('exists', () => {
createComponent();
expect(findActionMenuDelete().exists()).toBe(true);
});
it('emits a delete event when clicked', async () => {
createComponent();
await findActionMenuDelete().trigger('click');
const [[items]] = wrapper.emitted('delete-files');
......@@ -199,8 +193,9 @@ describe('Package Files', () => {
describe('when the user can not delete', () => {
const canDelete = false;
it('does not exist', () => {
it('does not exist', async () => {
createComponent({ canDelete });
await waitForPromises();
expect(findFirstActionMenu().exists()).toBe(false);
});
......@@ -209,22 +204,33 @@ describe('Package Files', () => {
describe('multi select', () => {
describe('when user can delete', () => {
it('delete selected button exists & is disabled', () => {
it('delete selected button exists & is disabled', async () => {
createComponent();
await waitForPromises();
expect(findDeleteSelectedButton().exists()).toBe(true);
expect(findDeleteSelectedButton().text()).toMatchInterpolatedText('Delete selected');
expect(findDeleteSelectedButton().props('disabled')).toBe(true);
});
it('delete selected button exists & is disabled when isLoading prop is true', () => {
createComponent({ isLoading: true });
it('delete selected button exists & is disabled when isLoading prop is true', async () => {
createComponent();
await waitForPromises();
const first = findAllRowCheckboxes().at(0);
await first.setChecked(true);
expect(findDeleteSelectedButton().props('disabled')).toBe(false);
await wrapper.setProps({ isLoading: true });
expect(findDeleteSelectedButton().props('disabled')).toBe(true);
expect(findLoadingIcon().exists()).toBe(true);
});
it('checkboxes to select file are visible', () => {
createComponent({ packageFiles: files });
it('checkboxes to select file are visible', async () => {
createComponent({ resolver: jest.fn().mockResolvedValue(packageFilesQuery()) });
await waitForPromises();
expect(findCheckAllCheckbox().exists()).toBe(true);
expect(findAllRowCheckboxes()).toHaveLength(2);
......@@ -232,6 +238,7 @@ describe('Package Files', () => {
it('selecting a checkbox enables delete selected button', async () => {
createComponent();
await waitForPromises();
const first = findAllRowCheckboxes().at(0);
......@@ -244,7 +251,8 @@ describe('Package Files', () => {
it('will toggle between selecting all and deselecting all files', async () => {
const getChecked = () => findAllRowCheckboxes().filter((x) => x.element.checked === true);
createComponent({ packageFiles: files });
createComponent({ resolver: jest.fn().mockResolvedValue(packageFilesQuery()) });
await waitForPromises();
expect(getChecked()).toHaveLength(0);
......@@ -262,9 +270,10 @@ describe('Package Files', () => {
expect(findCheckAllCheckbox().props('indeterminate')).toBe(state);
createComponent({
packageFiles: files,
resolver: jest.fn().mockResolvedValue(packageFilesQuery()),
stubs: { GlFormCheckbox: stubComponent(GlFormCheckbox, { props: ['indeterminate'] }) },
});
await waitForPromises();
expectIndeterminateState(false);
......@@ -288,6 +297,7 @@ describe('Package Files', () => {
it('emits a delete event when selected', async () => {
createComponent();
await waitForPromises();
const first = findAllRowCheckboxes().at(0);
......@@ -301,7 +311,8 @@ describe('Package Files', () => {
});
it('emits delete event with both items when all are selected', async () => {
createComponent({ packageFiles: files });
createComponent({ resolver: jest.fn().mockResolvedValue(packageFilesQuery()) });
await waitForPromises();
await findCheckAllCheckbox().setChecked(true);
......@@ -315,14 +326,16 @@ describe('Package Files', () => {
describe('when user cannot delete', () => {
const canDelete = false;
it('delete selected button does not exist', () => {
it('delete selected button does not exist', async () => {
createComponent({ canDelete });
await waitForPromises();
expect(findDeleteSelectedButton().exists()).toBe(false);
});
it('checkboxes to select file are not visible', () => {
createComponent({ packageFiles: files, canDelete });
it('checkboxes to select file are not visible', async () => {
createComponent({ resolver: jest.fn().mockResolvedValue(packageFilesQuery()), canDelete });
await waitForPromises();
expect(findCheckAllCheckbox().exists()).toBe(false);
expect(findAllRowCheckboxes()).toHaveLength(0);
......@@ -332,24 +345,27 @@ describe('Package Files', () => {
describe('additional details', () => {
describe('details toggle button', () => {
it('exists', () => {
it('exists', async () => {
createComponent();
await waitForPromises();
expect(findFirstToggleDetailsButton().exists()).toBe(true);
});
it('is hidden when no details is present', () => {
it('is hidden when no details is present', async () => {
const { ...noShaFile } = file;
noShaFile.fileSha256 = null;
noShaFile.fileMd5 = null;
noShaFile.fileSha1 = null;
createComponent({ packageFiles: [noShaFile] });
createComponent({ resolver: jest.fn().mockResolvedValue(packageFilesQuery([noShaFile])) });
await waitForPromises();
expect(findFirstToggleDetailsButton().exists()).toBe(false);
});
it('toggles the details row', async () => {
createComponent();
await waitForPromises();
expect(findFirstToggleDetailsButton().props('icon')).toBe('chevron-down');
......@@ -380,6 +396,7 @@ describe('Package Files', () => {
${'sha-1'} | ${'SHA-1'} | ${'be93151dc23ac34a82752444556fe79b32c7a1ad'}
`('has a $title row', async ({ selector, title, sha }) => {
createComponent();
await waitForPromises();
await showShaFiles();
......@@ -393,7 +410,8 @@ describe('Package Files', () => {
const { ...missingMd5 } = file;
missingMd5.fileMd5 = null;
createComponent({ packageFiles: [missingMd5] });
createComponent({ resolver: jest.fn().mockResolvedValue(packageFilesQuery([missingMd5])) });
await waitForPromises();
await showShaFiles();
......
......@@ -257,7 +257,7 @@ export const packageDetailsQuery = ({
pageInfo: {
hasNextPage: true,
},
nodes: packageFiles(),
nodes: packageFiles().map(({ id, size }) => ({ id, size })),
__typename: 'PackageFileConnection',
},
versions: {
......@@ -285,6 +285,19 @@ export const packagePipelinesQuery = (pipelines = packagePipelines()) => ({
},
});
export const packageFilesQuery = (files = packageFiles()) => ({
data: {
package: {
id: 'gid://gitlab/Packages::Package/111',
packageFiles: {
nodes: files,
__typename: 'PackageFileConnection',
},
__typename: 'PackageDetailsType',
},
},
});
export const emptyPackageDetailsQuery = () => ({
data: {
package: {
......
......@@ -328,18 +328,18 @@ describe('PackagesApp', () => {
describe('package files', () => {
it('renders the package files component and has the right props', async () => {
const expectedFile = { ...packageFiles()[0] };
// eslint-disable-next-line no-underscore-dangle
delete expectedFile.__typename;
createComponent();
await waitForPromises();
expect(findPackageFiles().exists()).toBe(true);
expect(findPackageFiles().props('packageFiles')[0]).toMatchObject(expectedFile);
expect(findPackageFiles().props('canDelete')).toBe(packageData().canDestroy);
expect(findPackageFiles().props('isLoading')).toEqual(false);
expect(findPackageFiles().props()).toMatchObject({
canDelete: packageData().canDestroy,
isLoading: false,
packageId: packageData().id,
packageType: packageData().packageType,
});
});
it('does not render the package files table when the package is composer', async () => {
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment