Skip to content

Commit

Permalink
🐛 Remove unnecessary query invalidations for reviews (#1735) (#1741)
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">

<!--
## PR Title Prefix

Every **PR Title** should be prefixed with :text: to indicate its type.

- Breaking change: ⚠️ (`⚠️`)
- Non-breaking feature: ✨ (`✨`)
- Patch fix: 🐛 (`🐛`)
- Docs: 📖 (`📖`)
- Infra/Tests/Other: 🌱 (`🌱`)
- No release note: 👻 (`👻`)

For example, a pull request containing breaking changes might look like
`⚠️ My pull request contains breaking changes`.

Since GitHub supports emoji aliases (ie. `👻`), there is no need to
include
the emoji directly in the PR title -- **please use the alias**. It used
to be
the case that projects using emojis for PR typing had to include the
emoji
directly because GitHub didn't render the alias. Given that `⚠️`
is
easy enough to read as text, easy to parse in release tooling, and
rendered in
GitHub well, we prefer to standardize on the alias.

For more information, please see the Konveyor
[Versioning
Doc](https://github.com/konveyor/release-tools/blob/main/VERSIONING.md).
-->

Signed-off-by: Ian Bolton <[email protected]>
  • Loading branch information
ibolton336 authored Mar 5, 2024
1 parent 5fbf087 commit 6fd0c67
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,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 @@ -119,6 +119,9 @@ export const ApplicationsTable: React.FC = () => {
const { pushNotification } = React.useContext(NotificationsContext);

const { identities } = useFetchIdentities();

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

const [isToolbarKebabOpen, setIsToolbarKebabOpen] =
React.useState<boolean>(false);

Expand Down Expand Up @@ -1099,6 +1102,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 6fd0c67

Please sign in to comment.