Skip to content

Commit

Permalink
Incrementally load node diff in UI view (#5676)
Browse files Browse the repository at this point in the history
  • Loading branch information
bilalabbad authored Feb 5, 2025
1 parent d11b870 commit 1c4a0f9
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 60 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { DiffTreeQueryFilters } from "@/shared/api/graphql/generated/graphql";
import graphqlClient from "@/shared/api/graphql/graphqlClientApollo";
import { gql } from "@apollo/client";

export const getProposedChangesDiffTree = gql`
query GET_PROPOSED_CHANGES_DIFF_TREE($branch: String, $filters: DiffTreeQueryFilters) {
DiffTree(branch: $branch, filters: $filters, include_parents: true) {
export const DIFF_TREE_QUERY = gql`
query GET_DIFF_TREE($branchName: String, $filters: DiffTreeQueryFilters, $limit: Int, $offset: Int) {
DiffTree(branch: $branchName, filters: $filters, include_parents: true, limit: $limit, offset: $offset) {
nodes {
uuid
relationships {
Expand Down Expand Up @@ -124,3 +126,27 @@ export const getProposedChangesDiffTree = gql`
}
}
`;

export type GetDiffTreeFromApiParams = {
branchName: string;
filters?: DiffTreeQueryFilters;
limit?: number;
offset?: number;
};

export const getDiffTreeFromApi = async ({
branchName,
filters,
limit,
offset,
}: GetDiffTreeFromApiParams) => {
return graphqlClient.query({
query: DIFF_TREE_QUERY,
variables: {
branchName,
filters,
limit,
offset,
},
});
};
51 changes: 51 additions & 0 deletions frontend/app/src/entities/diff/domain/get-diff-tree.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { getDiffTreeFromApi } from "@/entities/diff/api/get-diff-tree-from-api";
import { DiffTree, DiffTreeQueryFilters } from "@/shared/api/graphql/generated/graphql";
import { infiniteQueryOptions, useInfiniteQuery } from "@tanstack/react-query";

export const DIFF_TREE_PER_PAGE = 30;

export type GetDiffTreeParams = {
branchName: string;
filters?: DiffTreeQueryFilters;
limit?: number;
offset: number;
};

export type GetDiffTree = (params: GetDiffTreeParams) => Promise<DiffTree>;

export const getDiffTree: GetDiffTree = async ({
branchName,
limit = DIFF_TREE_PER_PAGE,
offset,
filters,
}) => {
const { data } = await getDiffTreeFromApi({ branchName, limit, offset, filters });

return data.DiffTree;
};

export type GetDiffTreeInfiniteQueryOptionsParams = {
branchName: string;
filters?: DiffTreeQueryFilters;
};

export const getDiffTreeInfiniteQueryOptions = ({
branchName,
filters,
}: GetDiffTreeInfiniteQueryOptionsParams) => {
return infiniteQueryOptions({
queryKey: ["diff-tree", branchName, filters],
queryFn: ({ pageParam }) => getDiffTree({ branchName, filters, offset: pageParam }),
initialPageParam: 0,
getNextPageParam: (lastPage, _, lastPageParam) => {
if (lastPage === null || (lastPage?.nodes && lastPage.nodes.length < DIFF_TREE_PER_PAGE)) {
return undefined;
}
return lastPageParam + DIFF_TREE_PER_PAGE;
},
});
};

export const useDiffTreeInfiniteQuery = (params: GetDiffTreeInfiniteQueryOptionsParams) => {
return useInfiniteQuery(getDiffTreeInfiniteQueryOptions(params));
};
94 changes: 41 additions & 53 deletions frontend/app/src/entities/diff/node-diff/index.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
import { DEFAULT_BRANCH_NAME, PROPOSED_CHANGES_OBJECT_THREAD_OBJECT } from "@/config/constants";
import { DEFAULT_BRANCH_NAME } from "@/config/constants";
import { QSP } from "@/config/qsp";
import { useDiffTreeInfiniteQuery } from "@/entities/diff/domain/get-diff-tree";
import { DIFF_STATUS, DiffNode as DiffNodeType } from "@/entities/diff/node-diff/types";
import { buildFilters } from "@/entities/diff/node-diff/utils";
import { DiffComputing } from "@/entities/diff/ui/diff-computing";
import { DiffEmpty } from "@/entities/diff/ui/diff-empty";
import { DiffNoFound } from "@/entities/diff/ui/diff-no-found";
import { DiffRebaseButton } from "@/entities/diff/ui/diff-rebase-button";
import { DiffRefreshButton } from "@/entities/diff/ui/diff-refresh-button";
import DiffTree from "@/entities/diff/ui/diff-tree";
import { getProposedChangesDiffTree } from "@/entities/proposed-changes/api/getProposedChangesDiffTree";
import { proposedChangedState } from "@/entities/proposed-changes/stores/proposedChanges.atom";
import { schemaState } from "@/entities/schema/stores/schema.atom";
import useQuery from "@/shared/api/graphql/useQuery";
import { DateDisplay } from "@/shared/components/display/date-display";
import ErrorScreen from "@/shared/components/errors/error-screen";
import LoadingScreen from "@/shared/components/loading-screen";
import { NetworkStatus } from "@apollo/client";
import { Spinner } from "@/shared/components/ui/spinner";
import { useAtomValue } from "jotai";
import { createContext } from "react";
import { createContext, useEffect } from "react";
import { StringParam, useQueryParam } from "use-query-params";
import { DiffFilter, ProposedChangeDiffFilter } from "../../proposed-changes/ui/diff-filter";
import { DiffNode } from "./node";
Expand All @@ -28,51 +27,38 @@ type NodeDiffProps = {
branchName: string;
};

// Handle QSP to filter from the status
const buildFilters = (filters: DiffFilter, qsp?: string | null) => {
const statusFilter = {
...filters?.status,
includes: Array.from(
// CONFLICT should not be part of the status filters
new Set([...(filters?.status?.includes ?? []), qsp !== DIFF_STATUS.CONFLICT && qsp])
).filter((value) => !!value),
};

return {
...filters,
status: statusFilter,
};
};

export const NodeDiff = ({ branchName, filters }: NodeDiffProps) => {
const [qspStatus] = useQueryParam(QSP.STATUS, StringParam);
const proposedChangesDetails = useAtomValue(proposedChangedState);
const nodeSchemas = useAtomValue(schemaState);

const branch = proposedChangesDetails?.source_branch?.value || branchName; // Used in proposed changes view and branch view

const schemaData = nodeSchemas.find((s) => s.kind === PROPOSED_CHANGES_OBJECT_THREAD_OBJECT);

// Get filters merged with status filter
const finalFilters = buildFilters(filters, qspStatus);

const { networkStatus, data, previousData, error } = useQuery(getProposedChangesDiffTree, {
skip: !schemaData,
variables: { branch, filters: finalFilters },
notifyOnNetworkStatusChange: true,
});
const { data, isPending, error, hasNextPage, fetchNextPage, isFetchingNextPage } =
useDiffTreeInfiniteQuery({
branchName: branch,
filters: finalFilters,
});

if (networkStatus === NetworkStatus.loading) return <LoadingScreen message="Loading diff..." />;
useEffect(() => {
if (hasNextPage && !isFetchingNextPage) {
fetchNextPage();
}
}, [hasNextPage, isFetchingNextPage]);

if (isPending) return <LoadingScreen message="Loading diff..." />;

if (error) {
return <ErrorScreen message={error?.message} className="max-w-lg m-auto" />;
}

const diffTreeData = (data || previousData)?.DiffTree;
const firstPageNodes = data.pages[0];

// 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) {
if (!firstPageNodes) {
return (
<DiffComputing
sourceBranch={branch}
Expand All @@ -81,24 +67,26 @@ export const NodeDiff = ({ branchName, filters }: NodeDiffProps) => {
);
}

if (!qspStatus && diffTreeData.nodes.length === 0) {
return <DiffEmpty branchName={branch} lastRefreshedAt={diffTreeData.to_time} />;
if (!qspStatus && firstPageNodes.nodes?.length === 0) {
return <DiffEmpty branchName={branch} lastRefreshedAt={firstPageNodes.to_time} />;
}

// 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 === DIFF_STATUS.CONFLICT) return node.contains_conflict;

return true;
}) ?? [];
const nodes =
data.pages
.flatMap((page) => page?.nodes)
.flatMap((node) => {
if (!node || node.status === "UNCHANGED") return [];
// Manually filter conflicts items since it's not available yet in the backend filters
if (qspStatus === DIFF_STATUS.CONFLICT && !node.contains_conflict) return [];
return node as unknown as DiffNodeType;
}) ?? [];

return (
<div className="h-full overflow-hidden flex flex-col">
<header className="flex items-center px-4 py-2 border-b gap-2">
<ProposedChangeDiffFilter branch={branch} filters={filters} />
<span className="text-xs inline-flex gap-1 ml-auto">
Updated <DateDisplay date={diffTreeData?.to_time} />
Updated <DateDisplay date={firstPageNodes?.to_time} />
</span>
<DiffRefreshButton size="sm" variant="primary" branchName={branch} />
<DiffRebaseButton branchName={branch} />
Expand All @@ -111,19 +99,19 @@ export const NodeDiff = ({ branchName, filters }: NodeDiffProps) => {

<main className="space-y-4 p-4 col-start-2 col-end-5 overflow-auto bg-stone-100">
{nodes.length ? (
nodes
.filter(({ status }) => status !== "UNCHANGED")
.map((node) => (
<DiffNode
key={node.uuid}
node={node}
sourceBranch={diffTreeData?.base_branch}
destinationBranch={diffTreeData?.diff_branch}
/>
))
nodes.map((node) => (
<DiffNode
key={node.uuid}
node={node}
sourceBranch={firstPageNodes?.base_branch}
destinationBranch={firstPageNodes?.diff_branch}
/>
))
) : (
<DiffNoFound diffStatus={qspStatus as string} />
)}

{isFetchingNextPage && <Spinner className="flex justify-center" />}
</main>
</div>
</div>
Expand Down
20 changes: 19 additions & 1 deletion frontend/app/src/entities/diff/node-diff/utils.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { DiffProperty, DiffStatus } from "@/entities/diff/node-diff/types";
import { DIFF_STATUS, DiffProperty, DiffStatus } from "@/entities/diff/node-diff/types";
import { DiffFilter } from "@/entities/proposed-changes/ui/diff-filter";
import { DiffTreeQueryFilters } from "@/shared/api/graphql/generated/graphql";
import Accordion from "@/shared/components/display/accordion";
import { classNames, warnUnexpectedType } from "@/shared/utils/common";
import { capitalizeFirstLetter } from "@/shared/utils/string";
Expand Down Expand Up @@ -128,3 +130,19 @@ export const formatPropertyName = (name: DiffProperty["property_type"]) => {
}
}
};

// Handle QSP to filter from the status
export const buildFilters = (filters: DiffFilter, qsp?: string | null): DiffTreeQueryFilters => {
const statusFilter = {
...filters?.status,
includes: Array.from(
// CONFLICT should not be part of the status filters
new Set([...(filters?.status?.includes ?? []), qsp !== DIFF_STATUS.CONFLICT && qsp])
).filter((value) => !!value),
};

return {
...filters,
status: statusFilter,
} as DiffTreeQueryFilters;
};
8 changes: 5 additions & 3 deletions frontend/app/src/entities/diff/ui/diff-refresh-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
useUpdateDiffMutation,
} from "@/entities/diff/domain/update-diff.mutation";
import graphqlClient from "@/shared/api/graphql/graphqlClientApollo";
import { queryClient } from "@/shared/api/rest/client";
import { Button, ButtonProps } from "@/shared/components/buttons/button-primitive";
import { ALERT_TYPES, Alert } from "@/shared/components/ui/alert";
import { classNames } from "@/shared/utils/common";
Expand All @@ -25,9 +26,10 @@ export function DiffRefreshButton({ branchName, ...props }: DiffRefreshButtonPro

const handleRefreshDiff = async () => {
updateDiffMutation.mutate(branchName, {
onSuccess: async () => {
await graphqlClient.refetchQueries({
include: ["GET_PROPOSED_CHANGES_DIFF_TREE", "GET_PROPOSED_CHANGES_DIFF_SUMMARY"],
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ["diff-tree"] });
graphqlClient.refetchQueries({
include: ["GET_PROPOSED_CHANGES_DIFF_SUMMARY"],
});
toast(<Alert type={ALERT_TYPES.SUCCESS} message="Diff updated!" />);
},
Expand Down

0 comments on commit 1c4a0f9

Please sign in to comment.