Skip to content

Commit

Permalink
Differentiate when diff is empty vs when diff is being computed (#4247)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bilalabbad
bilalabbad authored Sep 3, 2024
1 parent f3beb83 commit 611b185
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 59 deletions.
6 changes: 2 additions & 4 deletions frontend/app/src/screens/diff/diff-tree.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@ import { useLocation, useNavigate } from "react-router-dom";
import { TREE_ROOT_ID } from "@/screens/ipam/constants";
import { useSchema } from "@/hooks/useSchema";
import { Icon } from "@iconify-icon/react";
import NoDataFound from "../errors/no-data-found";
import { Tooltip } from "@/components/ui/tooltip";

interface DiffTreeProps extends Omit<TreeProps, "data"> {
nodes: Array<DiffNode>;
emptyMessage?: string;
}

export default function DiffTree({ nodes, loading, emptyMessage, ...props }: DiffTreeProps) {
export default function DiffTree({ nodes, loading, ...props }: DiffTreeProps) {
const [treeData, setTreeData] = useState<TreeProps["data"]>(EMPTY_TREE);
const location = useLocation();
const navigate = useNavigate();
Expand All @@ -25,7 +23,7 @@ export default function DiffTree({ nodes, loading, emptyMessage, ...props }: Dif
setTreeData(addItemsToTree(EMPTY_TREE, generateRootCategoryNodeForDiffTree(formattedNodes)));
}, [nodes]);

if (treeData.length <= 1) return <NoDataFound message={emptyMessage ?? "No data to display."} />;
if (treeData.length <= 1) return null;

return (
<Tree
Expand Down
158 changes: 112 additions & 46 deletions frontend/app/src/screens/diff/node-diff/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@ import { getProposedChangesDiffTree } from "@/graphql/queries/proposed-changes/g
import { DiffNode } from "./node";
import { StringParam, useQueryParam } from "use-query-params";
import { QSP } from "@/config/qsp";
import NoDataFound from "@/screens/errors/no-data-found";
import { Card } from "@/components/ui/card";
import DiffTree from "@/screens/diff/diff-tree";
import type { DiffNode as DiffNodeType } from "@/screens/diff/node-diff/types";
import { Button } from "@/components/buttons/button-primitive";
import { Button, ButtonProps } from "@/components/buttons/button-primitive";
import { rebaseBranch } from "@/graphql/mutations/branches/rebaseBranch";
import { objectToString } from "@/utils/common";
import { classNames, objectToString } from "@/utils/common";
import graphqlClient from "@/graphql/graphqlClientApollo";
import { datetimeAtom } from "@/state/atoms/time.atom";
import { gql, useMutation } from "@apollo/client";
Expand All @@ -30,6 +28,12 @@ import { Alert, ALERT_TYPES } from "@/components/ui/alert";
import { DIFF_UPDATE } from "@/graphql/mutations/proposed-changes/diff/diff-update";
import { useAuth } from "@/hooks/useAuth";
import { DateDisplay } from "@/components/display/date-display";
import { Icon } from "@iconify-icon/react";
import type { DiffNode as DiffNodeType } from "@/screens/diff/node-diff/types";
import { formatFullDate, formatRelativeTimeFromNow } from "@/utils/date";
import { Tooltip } from "@/components/ui/tooltip";
import { DiffBadge } from "@/screens/diff/node-diff/utils";
import { Badge } from "@/components/ui/badge";

export const DiffContext = createContext({});

Expand Down Expand Up @@ -73,18 +77,13 @@ export const NodeDiff = ({ filters }: NodeDiffProps) => {
// Get filters merged with status filter
const finalFilters = buildFilters(filters, qspStatus);

const { loading, data } = useQuery(getProposedChangesDiffTree, {
const { loading, called, data, previousData } = useQuery(getProposedChangesDiffTree, {
skip: !schemaData,
variables: { branch, filters: finalFilters },
notifyOnNetworkStatusChange: true,
});

// Manually filter conflicts items since it's not available yet in the backend filters
const nodes: Array<DiffNodeType> =
data?.DiffTree?.nodes.filter((node: DiffNodeType) => {
if (qspStatus === diffActions.CONFLICT) return node.contains_conflict;

return true;
}) ?? [];
if (!called && loading) return <LoadingScreen message="Loading diff..." />;

const handleRefresh = async () => {
setIsLoadingUpdate(true);
Expand Down Expand Up @@ -134,20 +133,92 @@ export const NodeDiff = ({ filters }: NodeDiffProps) => {
setIsLoadingUpdate(false);
};

const diffTreeData = (data || previousData)?.DiffTree;

// When a proposed change is created, there is also a job that compute the diff that is triggered.
// If DiffTree is null, it means that diff is still being computed.
if (!diffTreeData) {
return (
<div className="flex flex-col items-center mt-10 gap-5">
<LoadingScreen hideText />

<h1 className="font-semibold">
We are computing the diff between{" "}
<Badge variant="blue">
<Icon icon={"mdi:layers-triple"} className="mr-1" />{" "}
{proposedChangesDetails.source_branch?.value}
</Badge>{" "}
and{" "}
<Badge variant="green">
<Icon icon={"mdi:layers-triple"} className="mr-1" />{" "}
{proposedChangesDetails.destination_branch?.value}
</Badge>
</h1>

<div className="text-center">
<p>This process may take a few seconds to a few minutes.</p>
<p>Once completed, you&apos;ll be able to view the detailed changes.</p>
</div>

<RefreshButton
onClick={handleRefresh}
disabled={!auth?.permissions?.write || isLoadingUpdate}
isLoading={isLoadingUpdate}
/>
</div>
);
}

if (!qspStatus && diffTreeData.nodes.length === 0) {
return (
<div className="flex flex-col items-center mt-10 gap-5">
<div className="p-3 rounded-full bg-white inline-flex">
<Icon icon="mdi:circle-off-outline" className="text-2xl text-custom-blue-800" />
</div>

<h1 className="font-semibold text-lg">No changes detected</h1>
<div className="text-center">
<p>
The last comparison was made{" "}
<Tooltip enabled content={formatFullDate(diffTreeData.to_time)}>
<span className="font-semibold">
{formatRelativeTimeFromNow(diffTreeData.to_time)}
</span>
</Tooltip>
.
</p>
<p>If you have made any changes, please refresh the diff:</p>
</div>

<RefreshButton
onClick={handleRefresh}
disabled={!auth?.permissions?.write || isLoadingUpdate}
isLoading={isLoadingUpdate}
/>
</div>
);
}

// Manually filter conflicts items since it's not available yet in the backend filters
const nodes: Array<DiffNodeType> =
diffTreeData.nodes.filter((node: DiffNodeType) => {
if (qspStatus === diffActions.CONFLICT) return node.contains_conflict;

return true;
}) ?? [];

return (
<div className="h-full overflow-hidden flex flex-col">
<div className="flex items-center p-2 bg-custom-white">
<div className="mr-2">
<ProposedChangesDiffSummary branch={branch} filters={filters} />
</div>
<ProposedChangesDiffSummary branch={branch} filters={filters} />

<div className="flex flex-1 items-center gap-2 justify-end pr-2">
{isLoadingUpdate && <LoadingScreen size={22} hideText />}

<div className="flex items-center">
<div className="flex items-center text-xs mr-2">
<span className="mr-1">Updated</span>
<DateDisplay date={data?.DiffTree?.to_time} />
<DateDisplay date={diffTreeData?.to_time} />
</div>

<Button
Expand All @@ -171,52 +242,47 @@ export const NodeDiff = ({ filters }: NodeDiffProps) => {

<div className="flex-grow grid grid-cols-4 overflow-hidden">
<Card className="col-span-1 my-2.5 ml-2.5 overflow-auto">
<DiffTree
nodes={nodes}
className="p-2 w-full"
loading={loading}
emptyMessage="No tree view available for the diff."
/>
<DiffTree nodes={nodes} className="p-2 w-full" />
</Card>

<div className="space-y-4 p-2.5 col-start-2 col-end-5 overflow-auto">
{loading && <LoadingScreen message="Loading diff..." />}

{!loading && !nodes.length && qspStatus && (
<div className="flex flex-col items-center justify-center">
<NoDataFound
message={`No diff matches the status: ${qspStatus}. Please adjust your filter settings.`}
/>
</div>
)}

{!loading && !nodes.length && !qspStatus && (
<div className="flex flex-col items-center justify-center">
<NoDataFound message="No diff to display. Try to manually load the latest changes." />
<Button
variant="primary-outline"
onClick={handleRefresh}
isLoading={isLoadingUpdate}
disabled={!auth?.permissions?.write || isLoadingUpdate}>
{isLoadingUpdate ? "Refreshing diff..." : "Refresh diff"}
</Button>
{nodes.length === 0 && qspStatus && (
<div className="flex flex-col items-center mt-10 gap-5">
<div className="p-3 rounded-full bg-white inline-flex">
<Icon icon="mdi:circle-off-outline" className="text-2xl text-custom-blue-800" />
</div>

<div className="text-center">
<h1 className="font-semibold">
No matches found for the status <DiffBadge status={qspStatus} />
</h1>
<p>Try adjusting the filter settings to include more results.</p>
</div>
</div>
)}

{!loading &&
!!nodes.length &&
{!!nodes.length &&
nodes
.filter(({ status }) => status !== "UNCHANGED")
.map((node) => (
<DiffNode
key={node.uuid}
node={node}
sourceBranch={data?.DiffTree?.base_branch}
destinationBranch={data?.DiffTree?.diff_branch}
sourceBranch={diffTreeData?.base_branch}
destinationBranch={diffTreeData?.diff_branch}
/>
))}
</div>
</div>
</div>
);
};

const RefreshButton = ({ isLoading, ...props }: ButtonProps) => {
return (
<Button variant="primary-outline" {...props}>
<Icon icon="mdi:reload" className={classNames("mr-1", isLoading && "animate-spin")} />
{isLoading ? "Refreshing..." : "Refresh"}
</Button>
);
};
2 changes: 2 additions & 0 deletions frontend/app/src/screens/diff/node-diff/utils.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
BadgeAdded,
BadgeConflict,
BadgeRemoved,
BadgeType,
BadgeUnchanged,
Expand All @@ -17,6 +18,7 @@ export const diffBadges: { [key: string]: BadgeType } = {
UPDATED: BadgeUpdated,
REMOVED: BadgeRemoved,
UNCHANGED: BadgeUnchanged,
CONFLICT: BadgeConflict,
};

export const DiffBadge = ({
Expand Down
8 changes: 4 additions & 4 deletions frontend/app/src/screens/proposed-changes/diff-summary.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export const ProposedChangesDiffSummary = ({ branch, filters }: tProposedChanges
<FilterButton
onClick={() => handleFilter(diffActions.ADDED)}
muted={!!qsp && qsp !== diffActions.ADDED}
disabled={!data?.DiffTreeSummary?.num_added}
disabled={!data?.DiffTreeSummary?.num_added && qsp !== diffActions.ADDED}
className="relative">
<BadgeAdded>{data?.DiffTreeSummary?.num_added}</BadgeAdded>
{qsp === diffActions.ADDED && <CloseBadgeAdded />}
Expand All @@ -93,7 +93,7 @@ export const ProposedChangesDiffSummary = ({ branch, filters }: tProposedChanges
<FilterButton
onClick={() => handleFilter(diffActions.REMOVED)}
muted={!!qsp && qsp !== diffActions.REMOVED}
disabled={!data?.DiffTreeSummary?.num_removed}
disabled={!data?.DiffTreeSummary?.num_removed && qsp !== diffActions.REMOVED}
className="relative">
<BadgeRemoved>{data?.DiffTreeSummary?.num_removed}</BadgeRemoved>
{qsp === diffActions.REMOVED && <CloseBadgeRemoved />}
Expand All @@ -102,7 +102,7 @@ export const ProposedChangesDiffSummary = ({ branch, filters }: tProposedChanges
<FilterButton
onClick={() => handleFilter(diffActions.UPDATED)}
muted={!!qsp && qsp !== diffActions.UPDATED}
disabled={!data?.DiffTreeSummary?.num_updated}
disabled={!data?.DiffTreeSummary?.num_updated && qsp !== diffActions.UPDATED}
className="relative">
<BadgeUpdated>{data?.DiffTreeSummary?.num_updated}</BadgeUpdated>
{qsp === diffActions.UPDATED && <CloseBadgeUpdated />}
Expand All @@ -111,7 +111,7 @@ export const ProposedChangesDiffSummary = ({ branch, filters }: tProposedChanges
<FilterButton
onClick={() => handleFilter(diffActions.CONFLICT)}
muted={!!qsp && qsp !== diffActions.CONFLICT}
disabled={!data?.DiffTreeSummary?.num_conflicts}
disabled={!data?.DiffTreeSummary?.num_conflicts && qsp !== diffActions.CONFLICT}
className="relative">
<BadgeConflict>{data?.DiffTreeSummary?.num_conflicts}</BadgeConflict>
{qsp === diffActions.CONFLICT && <CloseBadgeConflict />}
Expand Down
10 changes: 5 additions & 5 deletions frontend/app/tests/e2e/proposed-changes/proposed-changes_diff.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ test.describe("/proposed-changes diff data", () => {
});

await test.step("trigger the diff update", async () => {
await expect(page.getByRole("button", { name: "Refresh diff" })).toBeVisible();
await expect(page.getByText("No diff to display. Try to")).toBeVisible();
await page.getByRole("button", { name: "Refresh diff" }).first().click();
await expect(page.getByText("We are computing the diff")).toBeVisible();
await page.getByRole("button", { name: "Refresh" }).click();
await expect(page.getByText("Diff updated!")).toBeVisible();
});

Expand Down Expand Up @@ -90,13 +89,14 @@ test.describe("/proposed-changes diff data", () => {
});

test("should comment a proposed changes", async ({ page }) => {
await test.step("access proposed changes", async () => {
await test.step("access proposed change diff tab", async () => {
await page.goto("/proposed-changes");
await page.getByRole("link", { name: "conflict-test" }).click();
await page.getByText("Data").click();
await expect(
page.locator("header").filter({ hasText: "Proposed changesconflict-test" })
).toBeVisible();
await page.getByText("Data").click();
await expect(page.getByRole("button", { name: "Refresh diff" })).toBeVisible();
});

await test.step("comment proposed changes", async () => {
Expand Down

0 comments on commit 611b185

Please sign in to comment.