Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

EVG-16587 Bump Apollo and graphql deps #1365

Merged
merged 12 commits into from
Jul 7, 2022
6 changes: 2 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"extends": "react-app"
},
"dependencies": {
"@apollo/client": "3.3.7",
"@apollo/client": "3.6.9",
"@bugsnag/js": "7.16.0",
"@bugsnag/plugin-react": "7.16.0",
"@emotion/css": "^11.9.0",
Expand Down Expand Up @@ -104,7 +104,7 @@
"date-fns-tz": "^1.1.2",
"deep-object-diff": "1.1.0",
"env-cmd": "10.1.0",
"graphql": "15.5.0",
"graphql": "16.5.0",
"html-react-parser": "1.2.4",
"js-cookie": "2.2.1",
"linkifyjs": "2.1.9",
Expand Down Expand Up @@ -151,7 +151,6 @@
"@testing-library/react": "12.1.5",
"@testing-library/react-hooks": "8.0.0",
"@testing-library/user-event": "12.5.0",
"@types/graphql": "^14.5.0",
"@types/jest": "^27.5.1",
"@types/js-cookie": "^2.2.7",
"@types/lodash.debounce": "4.0.6",
Expand Down Expand Up @@ -189,7 +188,6 @@
"eslint-plugin-storybook": "0.5.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

(placement of comment is random)

Could we possibly remove @types/graphql from devDependencies?

Screen Shot 2022-07-06 at 3 56 58 PM

"eslint-plugin-testing-library": "^3.9.2",
"eslint-webpack-plugin": "^2.1.0",
"graphql-tag": "^2.11.0",
"http-proxy": "^1.18.1",
"husky": "4.3.0",
"identity-obj-proxy": "3.0.0",
Expand Down
3 changes: 3 additions & 0 deletions src/components/Table/TasksTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ interface TasksTableProps {
tasks: TaskTableInfo[];
taskNameInputProps?: InputFilterProps;
variantInputProps?: InputFilterProps;
loading?: boolean;
}

export const TasksTable: React.VFC<TasksTableProps> = ({
Expand All @@ -59,11 +60,13 @@ export const TasksTable: React.VFC<TasksTableProps> = ({
tableChangeHandler,
tasks,
variantInputProps,
loading = false,
}) => (
<Table
data-cy="tasks-table"
rowKey={rowKey}
pagination={false}
loading={loading}
columns={
sorts
? getColumnDefsWithSort({
Expand Down
32 changes: 17 additions & 15 deletions src/pages/Version.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ export const VersionPage: React.VFC = () => {
const [redirectURL, setRedirectURL] = useState(undefined);
const [isLoadingData, setIsLoadingData] = useState(true);

const [
getVersion,
Comment on lines +49 to +50
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this above its usage on line 73 due to it becoming undefined. This was likely a result of one of the various useLazyQuery refactors that happened between our updates.

{ data, error: versionError, refetch, startPolling, stopPolling },
] = useLazyQuery<VersionQuery, VersionQueryVariables>(GET_VERSION, {
variables: { id },
pollInterval,
fetchPolicy: "cache-and-network",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed unnecessary fetchNextPolicy thats behavior was updated in apollographql/apollo-client#8465

onError: (e) => {
dispatchToast.error(
`There was an error loading the version: ${e.message}`
);
setIsLoadingData(false);
},
});

const { error: hasVersionError } = useQuery<
GetHasVersionQuery,
GetHasVersionQueryVariables
Expand Down Expand Up @@ -87,21 +102,6 @@ export const VersionPage: React.VFC = () => {
},
});

const [
getVersion,
{ data, error: versionError, refetch, startPolling, stopPolling },
] = useLazyQuery<VersionQuery, VersionQueryVariables>(GET_VERSION, {
variables: { id },
pollInterval,
fetchPolicy: "network-only",
nextFetchPolicy: "cache-and-network",
onError: (e) => {
dispatchToast.error(
`There was an error loading the version: ${e.message}`
);
setIsLoadingData(false);
},
});
usePolling(startPolling, stopPolling, refetch, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move usePolling up as well? The hook is always called right after query definitions, and it would be good to be consistent


// Decide where to redirect the user based off of whether or not the patch has been activated
Expand All @@ -117,6 +117,8 @@ export const VersionPage: React.VFC = () => {
setRedirectURL(getCommitQueueRoute(projectID));
setIsLoadingData(false);
} else {
console.log("Calling getVersion");
console.log({ getVersion });
Copy link
Contributor

Choose a reason for hiding this comment

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

remove console.log

getVersion({ variables: { id } });
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ const attachProjectToRepoMock = {
},
result: {
data: {
id: "evergreen",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These mocks were invalid and were fixed.

attachProjectToRepo: {
id: "evergreen",
},
},
},
};
Expand All @@ -322,7 +324,9 @@ const detachProjectFromRepoMock = {
},
result: {
data: {
id: "evergreen",
detachProjectFromRepo: {
id: "evergreen",
},
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,7 @@ describe("spawnVolumeModal", () => {
<SpawnVolumeModal visible onCancel={() => {}} />
);
const { queryByText, queryByDataCy } = render(
<MockedProvider
addTypename={false}
mocks={[...baseMocks, spawnVolumeMutation]}
>
<MockedProvider mocks={[...baseMocks, spawnVolumeMutation]}>
<Component />
</MockedProvider>
);
Expand All @@ -130,7 +127,6 @@ describe("spawnVolumeModal", () => {
fireEvent.click(queryByDataCy("i-00b212e96b3f91079-option"));
fireEvent.click(queryByText("Spawn"));
await waitFor(() => expect(dispatchToast.success).toHaveBeenCalledTimes(1));
await waitFor(() => expect(dispatchToast.error).toHaveBeenCalledTimes(0));
});
});

Expand Down Expand Up @@ -292,6 +288,7 @@ const userMock = {
user: {
userId: "a",
displayName: "A",
emailAddress: "[email protected]",
},
},
},
Expand Down
23 changes: 10 additions & 13 deletions src/pages/version/Tasks.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useEffect } from "react";
import { useQuery } from "@apollo/client";
import { Skeleton } from "antd";
import { useParams, useLocation } from "react-router-dom";
import { pollInterval } from "constants/index";
import { useToastContext } from "context/toast";
Expand Down Expand Up @@ -28,7 +27,7 @@ export const Tasks: React.VFC<Props> = ({ taskCount }) => {
const updateQueryParams = useUpdateURLQueryParams();

const queryVariables = useQueryVariables(search, id);
const noQueryVariables = !Object.keys(parseQueryString(search)).length;
const hasQueryVariables = Object.keys(parseQueryString(search)).length > 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

might be worth also changing the one in TaskDuration.tsx to look like this

const { sorts, limit, page } = queryVariables;

useEffect(() => {
Expand All @@ -50,23 +49,21 @@ export const Tasks: React.VFC<Props> = ({ taskCount }) => {
});
};

const { data, refetch, startPolling, stopPolling } = useQuery<
const { data, loading, refetch, startPolling, stopPolling } = useQuery<
PatchTasksQuery,
PatchTasksQueryVariables
>(GET_PATCH_TASKS, {
variables: queryVariables,
skip: noQueryVariables,
pollInterval,
fetchPolicy: "network-only",
nextFetchPolicy: "cache-and-network",
skip: !hasQueryVariables,
fetchPolicy: "cache-and-network",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nextFetchPolicy had inconsistent behavior which was fixed in a subsequent update. nextFetchPolicy would revert to the original fetchPolicy after it is called. I updated the fetchPolicy as well as updated the loading state of the table. Imo it works better than before. and is more in line with the loading behavior on the LG table

onError: (err) => {
dispatchToast.error(`Error fetching patch tasks ${err}`);
},
});
usePolling(startPolling, stopPolling, refetch);
const { patchTasks } = data || {};
const { tasks } = patchTasks || {};

const { tasks } = patchTasks || { tasks: [] };
Copy link
Contributor

Choose a reason for hiding this comment

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

alternatively const { tasks =[] } = patchTasks || {}, which would be parallel to what we have in TaskDuration.tsx

return (
<>
<TableControl
Expand All @@ -76,11 +73,11 @@ export const Tasks: React.VFC<Props> = ({ taskCount }) => {
page={page}
onClear={clearQueryParams}
/>
{!data ? (
<Skeleton active title={false} paragraph={{ rows: 8 }} />
) : (
<PatchTasksTable sorts={sorts} tasks={tasks} />
)}
<PatchTasksTable
sorts={sorts}
tasks={tasks}
loading={tasks.length === 0 && loading}
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

/>
</>
);
};
8 changes: 7 additions & 1 deletion src/pages/version/tasks/PatchTasksTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,14 @@ const { toSortString } = queryString;
interface Props {
tasks: PatchTasksQuery["patchTasks"]["tasks"];
sorts: SortOrder[];
loading: boolean;
}

export const PatchTasksTable: React.VFC<Props> = ({ tasks, sorts }) => {
export const PatchTasksTable: React.VFC<Props> = ({
tasks = [],
Copy link
Contributor

Choose a reason for hiding this comment

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

most likely not necessary?

sorts,
loading,
}) => {
const { id: versionId } = useParams<{ id: string }>();
const updateQueryParams = useUpdateURLQueryParams();
const { sendEvent } = useVersionAnalytics(versionId);
Expand Down Expand Up @@ -84,6 +89,7 @@ export const PatchTasksTable: React.VFC<Props> = ({ tasks, sorts }) => {
sorts={sorts}
tableChangeHandler={tableChangeHandler}
tasks={tasks}
loading={loading}
onExpand={(expanded) => {
sendEvent({
name: "Toggle Display Task Dropdown",
Expand Down
Loading