Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: [M3-7433] - Query Key Factory for Images #10302

Merged
merged 4 commits into from
Mar 26, 2024
Merged

refactor: [M3-7433] - Query Key Factory for Images #10302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e6f022
initial refactor
bnussman Mar 20, 2024
ed00a82
Added changeset: Use query key factory for image queries
bnussman Mar 20, 2024
733cbe6
fix e2e test
bnussman Mar 20, 2024
ccbdcba
update polling logic so that we poll for image upload to finish
bnussman Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-10302-tech-stories-1710961356698.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tech Stories
---

Use query key factory for image queries ([#10302](https://github.com/linode/manager/pull/10302))
8 changes: 5 additions & 3 deletions packages/manager/cypress/e2e/core/images/machine-image-upload.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { authenticate } from 'support/api/authentication';
import { fbtVisible, getClick } from 'support/helpers';
import {
mockDeleteImage,
mockGetAllImages,
mockGetCustomImages,
mockGetImage,
mockUpdateImage,
} from 'support/intercepts/images';
import { ui } from 'support/ui';
Expand Down Expand Up @@ -249,10 +249,12 @@ describe('machine image', () => {
cy.wait('@imageUpload').then((xhr) => {
const imageId = xhr.response?.body.image.id;
assertProcessing(label, imageId);
mockGetImage(label, imageId, 'available').as('getImage');
mockGetAllImages([
imageFactory.build({ label, id: imageId, status: 'available' }),
]).as('getImages');
eventIntercept(label, imageId, status);
ui.toast.assertMessage(uploadMessage);
cy.wait('@getImage');
cy.wait('@getImages');
ui.toast.assertMessage(availableMessage);
cy.get(`[data-qa-image-cell="${imageId}"]`).within(() => {
fbtVisible(label);
Expand Down
7 changes: 4 additions & 3 deletions packages/manager/src/components/Uploaders/ImageUploader/ImageUploader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
import { uploadImageFile } from 'src/features/Images/requests';
import { Dispatch } from 'src/hooks/types';
import { useCurrentToken } from 'src/hooks/useAuthentication';
import { queryKey, useUploadImageQuery } from 'src/queries/images';
import { imageQueries, useUploadImageMutation } from 'src/queries/images';
import { redirectToLogin } from 'src/session';
import { setPendingUpload } from 'src/store/pendingUpload';
import { sendImageUploadEvent } from 'src/utilities/analytics';
Expand Down Expand Up @@ -86,7 +86,7 @@ export const ImageUploader = React.memo((props: ImageUploaderProps) => {
const { enqueueSnackbar } = useSnackbar();
const [uploadToURL, setUploadToURL] = React.useState<string>('');
const queryClient = useQueryClient();
const { mutateAsync: uploadImage } = useUploadImageQuery({
const { mutateAsync: uploadImage } = useUploadImageMutation({
cloud_init: isCloudInit ? isCloudInit : undefined,
description: description ? description : undefined,
label,
Expand Down Expand Up @@ -234,7 +234,8 @@ export const ImageUploader = React.memo((props: ImageUploaderProps) => {
redirectToLogin('/images');
}, 3000);
} else {
queryClient.invalidateQueries([`${queryKey}-list`]);
queryClient.invalidateQueries(imageQueries.paginated._def);
queryClient.invalidateQueries(imageQueries.all._def);
history.push('/images');
}
};
Expand Down
33 changes: 14 additions & 19 deletions packages/manager/src/features/Images/ImagesLanding.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Event, Image, ImageStatus } from '@linode/api-v4';
import { APIError } from '@linode/api-v4/lib/types';
import { Theme } from '@mui/material/styles';
import { makeStyles } from 'tss-react/mui';
import { useQueryClient } from '@tanstack/react-query';
import produce from 'immer';
import { useSnackbar } from 'notistack';
import * as React from 'react';
import { useQueryClient } from '@tanstack/react-query';
import { useHistory } from 'react-router-dom';
import { makeStyles } from 'tss-react/mui';

import { ActionsPanel } from 'src/components/ActionsPanel/ActionsPanel';
import { CircleProgress } from 'src/components/CircleProgress';
Expand All @@ -29,17 +29,16 @@ import { Typography } from 'src/components/Typography';
import { useOrder } from 'src/hooks/useOrder';
import { usePagination } from 'src/hooks/usePagination';
import { listToItemsByID } from 'src/queries/base';
import {
isEventImageUpload,
isEventInProgressDiskImagize,
} from 'src/queries/events/event.helpers';
import { useEventsInfiniteQuery } from 'src/queries/events/events';
import {
queryKey,
removeImageFromCache,
imageQueries,
useDeleteImageMutation,
useImagesQuery,
} from 'src/queries/images';
import {
isEventImageUpload,
isEventInProgressDiskImagize,
} from 'src/queries/events/event.helpers';
import { getErrorStringOrDefault } from 'src/utilities/errorUtils';

import ImageRow, { ImageWithEvent } from './ImageRow';
Expand Down Expand Up @@ -105,12 +104,8 @@ export const ImagesLanding: React.FC<CombinedProps> = () => {

const queryClient = useQueryClient();

// Pagination, order, and query hooks for manual/custom images
const paginationForManualImages = usePagination(
1,
`${queryKey}-manual`,
'manual'
);
const paginationForManualImages = usePagination(1, 'images-manual', 'manual');

const {
handleOrderChange: handleManualImagesOrderChange,
order: manualImagesOrder,
Expand All @@ -120,7 +115,7 @@ export const ImagesLanding: React.FC<CombinedProps> = () => {
order: 'asc',
orderBy: 'label',
},
`${queryKey}-manual-order`,
'images-manual-order',
'manual'
);

Expand Down Expand Up @@ -148,7 +143,7 @@ export const ImagesLanding: React.FC<CombinedProps> = () => {
// Pagination, order, and query hooks for automatic/recovery images
const paginationForAutomaticImages = usePagination(
1,
`${queryKey}-automatic`,
'images-automatic',
'automatic'
);
const {
Expand All @@ -160,7 +155,7 @@ export const ImagesLanding: React.FC<CombinedProps> = () => {
order: 'asc',
orderBy: 'label',
},
`${queryKey}-automatic-order`,
'images-automatic-order',
'automatic'
);

Expand Down Expand Up @@ -284,15 +279,15 @@ export const ImagesLanding: React.FC<CombinedProps> = () => {
imageLabel: string,
imageDescription: string
) => {
removeImageFromCache(queryClient);
queryClient.invalidateQueries(imageQueries.paginated._def);
history.push('/images/create/upload', {
imageDescription,
imageLabel,
});
};

const onCancelFailedClick = () => {
removeImageFromCache(queryClient);
queryClient.invalidateQueries(imageQueries.paginated._def);
};

const openForEdit = (label: string, description: string, imageID: string) => {
Expand Down
12 changes: 3 additions & 9 deletions packages/manager/src/queries/events/event.helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
import { Event, EventAction, Filter } from '@linode/api-v4';

export const isLongPendingEvent = (event: Event): boolean => {
const { action, status } = event;
return status === 'scheduled' && action === 'image_upload';
};

export const isInProgressEvent = (event: Event) => {
const { percent_complete } = event;
if (percent_complete === null || isLongPendingEvent(event)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were not considering image_upload an in progress event, which caused us to not poll for it. We probably did this in the past to poll less when an upload was happening but we must this event so that an image upload can transition from Processing to Ready without the user needing to refresh the page.

The change basically removes the special logic for image_upload to improve UX

if (event.percent_complete === null) {
return false;
} else {
return percent_complete !== null && percent_complete < 100;
}

return event.percent_complete < 100;
};

export const isEventInProgressDiskImagize = (event: Event): boolean => {
Expand Down
Loading
Loading