Skip to content

Commit

Permalink
🐛 Remove unnecessary query invalidations for reviews (#1735)
Browse files Browse the repository at this point in the history
https://issues.redhat.com/browse/MTA-2359

- Removes query invalidation & additional archetypes / reviews api calls
inside each instance of a rendered (but hidden) drawer. This was causing
a huge performance hit.

 - Adds margin at top of reviews tab
<img width="540" alt="Screenshot 2024-03-04 at 2 46 40 PM"
src="https://github.com/konveyor/tackle2-ui/assets/11218376/9c6c8a8f-f8c6-45c9-8609-86a30e206a99">

Signed-off-by: Ian Bolton <[email protected]>
  • Loading branch information
ibolton336 authored Mar 5, 2024
1 parent 4cdf7ca commit 6bbca51
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ import {
useDeleteAssessmentMutation,
useFetchAssessments,
} from "@app/queries/assessments";
import { useDeleteReviewMutation } from "@app/queries/reviews";
import { useDeleteReviewMutation, useFetchReviews } from "@app/queries/reviews";
import { useFetchIdentities } from "@app/queries/identities";
import { useFetchTagsWithTagItems } from "@app/queries/tags";

Expand Down Expand Up @@ -117,6 +117,7 @@ export const ApplicationsTable: React.FC = () => {
const { pushNotification } = React.useContext(NotificationsContext);

const { identities } = useFetchIdentities();
const { reviews, isFetching: isFetchingReviews } = useFetchReviews();

const [saveApplicationModalState, setSaveApplicationModalState] =
React.useState<"create" | Application | null>(null);
Expand Down Expand Up @@ -1078,6 +1079,8 @@ export const ApplicationsTable: React.FC = () => {
application={activeItem}
applications={applications}
assessments={assessments}
archetypes={archetypes}
reviews={reviews}
onCloseClick={clearActiveItem}
onEditClick={() => setSaveApplicationModalState(activeItem)}
task={activeItem ? getTask(activeItem) : null}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
Ref,
Archetype,
AssessmentWithSectionOrder,
Review,
} from "@app/api/models";
import {
IPageDrawerContentProps,
Expand All @@ -57,7 +58,6 @@ import { ReviewFields } from "./review-fields";
import { LabelsFromItems } from "@app/components/labels/labels-from-items/labels-from-items";
import { RiskLabel } from "@app/components/RiskLabel";
import { ApplicationDetailFields } from "./application-detail-fields";
import { useFetchArchetypes } from "@app/queries/archetypes";
import { AssessedArchetypes } from "./components/assessed-archetypes";

export interface IApplicationDetailDrawerProps
Expand All @@ -66,6 +66,8 @@ export interface IApplicationDetailDrawerProps
task: Task | undefined | null;
applications?: Application[];
assessments?: AssessmentWithSectionOrder[];
reviews?: Review[];
archetypes?: Archetype[];
onEditClick: () => void;
}

Expand All @@ -79,7 +81,15 @@ enum TabKey {

export const ApplicationDetailDrawer: React.FC<
IApplicationDetailDrawerProps
> = ({ onCloseClick, application, assessments, task, onEditClick }) => {
> = ({
onCloseClick,
application,
assessments,
reviews,
archetypes,
task,
onEditClick,
}) => {
const { t } = useTranslation();
const [activeTabKey, setActiveTabKey] = React.useState<TabKey>(
TabKey.Details
Expand All @@ -88,7 +98,6 @@ export const ApplicationDetailDrawer: React.FC<
const isTaskRunning = task?.state === "Running";

const { identities } = useFetchIdentities();
const { archetypes } = useFetchArchetypes();
const { facts, isFetching } = useFetchFacts(application?.id);

const [taskIdToView, setTaskIdToView] = React.useState<number>();
Expand All @@ -106,8 +115,9 @@ export const ApplicationDetailDrawer: React.FC<

const reviewedArchetypes =
application?.archetypes
?.map((archetypeRef) =>
archetypes.find((archetype) => archetype.id === archetypeRef.id)
?.map(
(archetypeRef) =>
archetypes?.find((archetype) => archetype.id === archetypeRef.id)
)
.filter((fullArchetype) => fullArchetype?.review)
.filter(Boolean) || [];
Expand Down Expand Up @@ -445,7 +455,7 @@ export const ApplicationDetailDrawer: React.FC<
eventKey={TabKey.Reviews}
title={<TabTitleText>{t("terms.review")}</TabTitleText>}
>
<ReviewFields application={application} />
<ReviewFields application={application} reviews={reviews} />
</Tab>
</Tabs>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import {
DescriptionListDescription,
} from "@patternfly/react-core";
import { Application, Archetype, Review } from "@app/api/models";
import { useFetchReviewById, useFetchReviews } from "@app/queries/reviews";
import { useFetchReviewById } from "@app/queries/reviews";
import { useFetchArchetypes } from "@app/queries/archetypes";
import { EmptyTextMessage } from "@app/components/EmptyTextMessage";
import { PROPOSED_ACTION_LIST, EFFORT_ESTIMATE_LIST } from "@app/Constants";
import { ReviewLabel } from "./review-label";
import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";

export type ReviewDrawerLabelItem = {
review?: Review | null;
Expand All @@ -21,9 +22,9 @@ export type ReviewDrawerLabelItem = {
export const ReviewFields: React.FC<{
application?: Application | null;
archetype?: Archetype | null;
}> = ({ application, archetype }) => {
reviews?: Review[];
}> = ({ application, archetype, reviews }) => {
const { archetypes } = useFetchArchetypes();
const { reviews } = useFetchReviews();
const { t } = useTranslation();

const { review: appReview } = useFetchReviewById(application?.review?.id);
Expand All @@ -41,7 +42,7 @@ export const ReviewFields: React.FC<{

const matchedArchetypeReviews: Review[] = (applicationArchetypes || [])
.map((archetype) => {
return reviews.find((review) => review.id === archetype?.review?.id);
return reviews?.find((review) => review.id === archetype?.review?.id);
})
.filter(Boolean);

Expand Down Expand Up @@ -71,7 +72,7 @@ export const ReviewFields: React.FC<{
].filter((item) => item.review?.proposedAction);

return (
<>
<div className={spacing.mtMd}>
<DescriptionListGroup>
<DescriptionListTerm>{t("terms.proposedAction")}</DescriptionListTerm>
<DescriptionListDescription cy-data="proposed-action">
Expand Down Expand Up @@ -150,6 +151,6 @@ export const ReviewFields: React.FC<{
})}
</DescriptionListDescription>
</DescriptionListGroup>
</>
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from "@patternfly/react-core";
import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";

import { Archetype, Ref, Tag, TagRef } from "@app/api/models";
import { Archetype, Ref, Review, Tag, TagRef } from "@app/api/models";
import { EmptyTextMessage } from "@app/components/EmptyTextMessage";
import { PageDrawerContent } from "@app/components/PageDrawerContext";

Expand All @@ -34,6 +34,7 @@ import { Link } from "react-router-dom";
export interface IArchetypeDetailDrawerProps {
onCloseClick: () => void;
archetype: Archetype | null;
reviews?: Review[];
}

enum TabKey {
Expand All @@ -44,6 +45,7 @@ enum TabKey {
const ArchetypeDetailDrawer: React.FC<IArchetypeDetailDrawerProps> = ({
onCloseClick,
archetype,
reviews,
}) => {
const { t } = useTranslation();

Expand Down Expand Up @@ -226,7 +228,7 @@ const ArchetypeDetailDrawer: React.FC<IArchetypeDetailDrawerProps> = ({
eventKey={TabKey.Reviews}
title={<TabTitleText>{t("terms.review")}</TabTitleText>}
>
<ReviewFields archetype={archetype} />
<ReviewFields archetype={archetype} reviews={reviews} />
</Tab>
</Tabs>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export const ReviewForm: React.FC<IReviewFormProps> = ({
application,
review,
}) => {
console.log("existing review", review);
const { t } = useTranslation();
const history = useHistory();
const { pushNotification } = React.useContext(NotificationsContext);
Expand Down
2 changes: 0 additions & 2 deletions client/src/app/queries/applications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
updateAllApplications,
updateApplication,
} from "@app/api/rest";
import { reviewsQueryKey } from "./reviews";
import { assessmentsByItemIdQueryKey } from "./assessments";
import saveAs from "file-saver";

Expand All @@ -41,7 +40,6 @@ export const useFetchApplications = (refetchDisabled: boolean = false) => {
queryFn: getApplications,
refetchInterval: !refetchDisabled ? 5000 : false,
onSuccess: () => {
queryClient.invalidateQueries([reviewsQueryKey]);
queryClient.invalidateQueries([assessmentsByItemIdQueryKey]);
},
onError: (error: AxiosError) => console.log(error),
Expand Down
2 changes: 0 additions & 2 deletions client/src/app/queries/archetypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
updateArchetype,
} from "@app/api/rest";
import { assessmentsByItemIdQueryKey } from "./assessments";
import { reviewsQueryKey } from "./reviews";
import { useState } from "react";

export const ARCHETYPES_QUERY_KEY = "archetypes";
Expand Down Expand Up @@ -39,7 +38,6 @@ export const useFetchArchetypes = (forApplication?: Application | null) => {
setFilteredArchetypes([]);
}

queryClient.invalidateQueries([reviewsQueryKey]);
queryClient.invalidateQueries([assessmentsByItemIdQueryKey]);
},
onError: (error: AxiosError) => console.log(error),
Expand Down

0 comments on commit 6bbca51

Please sign in to comment.