diff --git a/changelog/+diff-summary.fixed.md b/changelog/+diff-summary.fixed.md new file mode 100644 index 0000000000..830ccccf8a --- /dev/null +++ b/changelog/+diff-summary.fixed.md @@ -0,0 +1 @@ +Fixed incorrect data diff counter when viewing a branch or proposed changes diff --git a/frontend/app/src/entities/diff/api/get-diff-tree-summary-from-api.ts b/frontend/app/src/entities/diff/api/get-diff-tree-summary-from-api.ts new file mode 100644 index 0000000000..abe66ad7d0 --- /dev/null +++ b/frontend/app/src/entities/diff/api/get-diff-tree-summary-from-api.ts @@ -0,0 +1,30 @@ +import { gql } from "@apollo/client"; + +import type { + Get_Diff_Tree_SummaryQuery, + Get_Diff_Tree_SummaryQueryVariables, +} from "@/shared/api/graphql/generated/graphql"; +import graphqlClient from "@/shared/api/graphql/graphqlClientApollo"; + +export const GET_PROPOSED_CHANGES_DIFF_SUMMARY = gql` + query GET_DIFF_TREE_SUMMARY($branch: String, $filters: DiffTreeQueryFilters) { + DiffTreeSummary(branch: $branch, filters: $filters) { + num_added + num_updated + num_removed + num_conflicts + } + } +`; + +export interface GetDiffTreeSummaryFromApiParams extends Get_Diff_Tree_SummaryQueryVariables {} + +export function getDiffTreeSummaryFromApi(variables: Get_Diff_Tree_SummaryQueryVariables) { + return graphqlClient.query({ + query: GET_PROPOSED_CHANGES_DIFF_SUMMARY, + variables, + context: { + processErrorMessage: () => {}, + }, + }); +} diff --git a/frontend/app/src/entities/diff/domain/get-diff-summary.query.ts b/frontend/app/src/entities/diff/domain/get-diff-summary.query.ts new file mode 100644 index 0000000000..0163fda309 --- /dev/null +++ b/frontend/app/src/entities/diff/domain/get-diff-summary.query.ts @@ -0,0 +1,18 @@ +import { queryOptions, useQuery } from "@tanstack/react-query"; + +import type { QueryConfig } from "@/shared/api/types"; + +import { type GetDiffSummaryParams, getDiffSummary } from "@/entities/diff/domain/get-diff-summary"; + +export function getDiffSummaryQueryOptions({ branch, filters }: GetDiffSummaryParams) { + return queryOptions({ + queryKey: ["diff-summary", branch, filters], + queryFn: () => getDiffSummary({ branch, filters }), + }); +} + +export type UseGetDiffSummaryConfig = QueryConfig; + +export function useGetDiffSummary(params: GetDiffSummaryParams, config?: UseGetDiffSummaryConfig) { + return useQuery({ ...getDiffSummaryQueryOptions(params), ...config }); +} diff --git a/frontend/app/src/entities/diff/domain/get-diff-summary.ts b/frontend/app/src/entities/diff/domain/get-diff-summary.ts new file mode 100644 index 0000000000..6d59417e23 --- /dev/null +++ b/frontend/app/src/entities/diff/domain/get-diff-summary.ts @@ -0,0 +1,27 @@ +import { + type GetDiffTreeSummaryFromApiParams, + getDiffTreeSummaryFromApi, +} from "@/entities/diff/api/get-diff-tree-summary-from-api"; + +export type GetDiffSummaryParams = GetDiffTreeSummaryFromApiParams; + +export type GetDiffSummaryResponse = { + num_added: number; + num_updated: number; + num_removed: number; + num_conflicts: number; +}; + +export type GetDiffSummary = ( + params: GetDiffSummaryParams +) => Promise; + +export const getDiffSummary: GetDiffSummary = async (params) => { + const { data, errors } = await getDiffTreeSummaryFromApi(params); + + if (errors) { + throw new Error(errors.map((e) => e.message).join("; ")); + } + + return (data.DiffTreeSummary as GetDiffSummaryResponse) ?? null; +}; diff --git a/frontend/app/src/entities/diff/node-diff/index.tsx b/frontend/app/src/entities/diff/node-diff/index.tsx index 979929da09..2d8d8c95b8 100644 --- a/frontend/app/src/entities/diff/node-diff/index.tsx +++ b/frontend/app/src/entities/diff/node-diff/index.tsx @@ -9,7 +9,9 @@ import { DateDisplay } from "@/shared/components/display/date-display"; import ErrorScreen from "@/shared/components/errors/error-screen"; import { LoadingIndicator } from "@/shared/components/loading/loading-indicator"; +import type { GetDiffSummaryParams } from "@/entities/diff/domain/get-diff-summary"; import { useDiffTreeInfiniteQuery } from "@/entities/diff/domain/get-diff-tree"; +import { DiffNode } from "@/entities/diff/node-diff/node"; import { DIFF_STATUS, type 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"; @@ -19,29 +21,24 @@ 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 { proposedChangedState } from "@/entities/proposed-changes/stores/proposedChanges.atom"; - -import { type DiffFilter, ProposedChangeDiffFilter } from "../../proposed-changes/ui/diff-filter"; -import { DiffNode } from "./node"; +import { DiffFilter } from "@/entities/proposed-changes/ui/diff-filter"; export const DiffContext = createContext({}); -type NodeDiffProps = { - filters: DiffFilter; - branchName: string; -}; +type NodeDiffProps = GetDiffSummaryParams; -export const NodeDiff = ({ branchName, filters }: NodeDiffProps) => { +export const NodeDiff = ({ branch, filters }: NodeDiffProps) => { const [qspStatus] = useQueryState(QSP.STATUS); const proposedChangesDetails = useAtomValue(proposedChangedState); - const branch = proposedChangesDetails?.source_branch?.value || branchName; // Used in proposed changes view and branch view + const branchName: string = proposedChangesDetails?.source_branch?.value || branch; // Used in proposed changes view and branch view // Get filters merged with status filter const finalFilters = buildFilters(filters, qspStatus); const { data, isPending, error, hasNextPage, fetchNextPage, isFetchingNextPage } = useDiffTreeInfiniteQuery({ - branchName: branch, + branchName, filters: finalFilters, }); @@ -66,14 +63,14 @@ export const NodeDiff = ({ branchName, filters }: NodeDiffProps) => { if (!firstPageNodes) { return ( ); } if (!qspStatus && firstPageNodes.nodes?.length === 0) { - return ; + return ; } const nodes = @@ -89,12 +86,12 @@ export const NodeDiff = ({ branchName, filters }: NodeDiffProps) => { return (
- + Updated - - + +
diff --git a/frontend/app/src/entities/diff/node-diff/utils.tsx b/frontend/app/src/entities/diff/node-diff/utils.tsx index 3ced9e71f6..386ecd7fa8 100644 --- a/frontend/app/src/entities/diff/node-diff/utils.tsx +++ b/frontend/app/src/entities/diff/node-diff/utils.tsx @@ -6,8 +6,6 @@ import { classNames, warnUnexpectedType } from "@/shared/utils/common"; import { capitalizeFirstLetter } from "@/shared/utils/string"; import { DIFF_STATUS, type DiffProperty, type DiffStatus } from "@/entities/diff/node-diff/types"; -import type { DiffFilter } from "@/entities/proposed-changes/ui/diff-filter"; - import { BadgeAdded, BadgeConflict, @@ -16,7 +14,8 @@ import { BadgeUnchanged, BadgeUpdated, type DiffBadgeProps, -} from "../ui/diff-badge"; +} from "@/entities/diff/ui/diff-badge"; +import type { DiffFilter } from "@/entities/proposed-changes/ui/diff-filter"; export const diffBadges: { [key: string]: BadgeType } = { ADDED: BadgeAdded, diff --git a/frontend/app/src/entities/diff/ui/diff-badge.tsx b/frontend/app/src/entities/diff/ui/diff-badge.tsx index 167a660663..db3b38e92f 100644 --- a/frontend/app/src/entities/diff/ui/diff-badge.tsx +++ b/frontend/app/src/entities/diff/ui/diff-badge.tsx @@ -8,10 +8,6 @@ export interface DiffBadgeProps extends BadgeProps { icon?: string; } -type CloseBadgeProps = { - className?: string; -}; - export type BadgeType = | typeof BadgeAdded | typeof BadgeRemoved @@ -53,10 +49,6 @@ export const BadgeAdded = ({ className, ...props }: DiffBadgeProps) => { ); }; -export const CloseBadgeAdded = () => { - return ; -}; - export const BadgeRemoved = ({ className, ...props }: DiffBadgeProps) => { return ( { ); }; -export const CloseBadgeRemoved = () => { - return ; -}; - export const BadgeConflict = ({ className, ...props }: DiffBadgeProps) => { return ( { ); }; -export const CloseBadgeConflict = () => { - return ; -}; - export const BadgeUpdated = ({ className, ...props }: DiffBadgeProps) => { return ( { /> ); }; - -export const CloseBadgeUpdated = () => { - return ; -}; - -const CloseBadge = ({ className }: CloseBadgeProps) => { - return ( -
- -
- ); -}; diff --git a/frontend/app/src/entities/proposed-changes/api/getProposedChangesDiffSummary.ts b/frontend/app/src/entities/proposed-changes/api/getProposedChangesDiffSummary.ts deleted file mode 100644 index 21ef69a0c8..0000000000 --- a/frontend/app/src/entities/proposed-changes/api/getProposedChangesDiffSummary.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { gql } from "@apollo/client"; - -export const getProposedChangesDiffSummary = gql` - query GET_PROPOSED_CHANGES_DIFF_SUMMARY($branch: String) { - DiffTreeSummary(branch: $branch) { - num_added - num_updated - num_removed - num_conflicts - } - } -`; diff --git a/frontend/app/src/entities/proposed-changes/ui/diff-filter.tsx b/frontend/app/src/entities/proposed-changes/ui/diff-filter.tsx index c0a51de413..b8a3237664 100644 --- a/frontend/app/src/entities/proposed-changes/ui/diff-filter.tsx +++ b/frontend/app/src/entities/proposed-changes/ui/diff-filter.tsx @@ -1,62 +1,36 @@ -import { useQuery } from "@apollo/client"; import { parseAsString, useQueryState } from "nuqs"; -import { toast } from "react-toastify"; import { QSP } from "@/config/qsp"; -import { Button, type ButtonProps } from "@/shared/components/buttons/button-primitive"; import ErrorScreen from "@/shared/components/errors/error-screen"; -import { ALERT_TYPES, Alert } from "@/shared/components/ui/alert"; -import { classNames } from "@/shared/utils/common"; -import { DIFF_STATUS } from "@/entities/diff/node-diff/types"; -import { DiffBadge } from "@/entities/diff/node-diff/utils"; +import type { GetDiffSummaryParams } from "@/entities/diff/domain/get-diff-summary"; +import { useGetDiffSummary } from "@/entities/diff/domain/get-diff-summary.query"; +import { DIFF_STATUS, type DiffStatus } from "@/entities/diff/node-diff/types"; +import { DiffSummarySkeleton } from "@/entities/proposed-changes/ui/diff-summary/diff-summary-skeleton"; import { - CloseBadgeAdded, - CloseBadgeConflict, - CloseBadgeRemoved, - CloseBadgeUpdated, -} from "@/entities/diff/ui/diff-badge"; -import { getProposedChangesDiffSummary } from "@/entities/proposed-changes/api/getProposedChangesDiffSummary"; + DiffSummaryTag, + DiffSummaryTagGroup, +} from "@/entities/proposed-changes/ui/diff-summary/diff-summary-tag-group"; -export type DiffFilter = { - namespace?: { - excludes?: string[]; - includes?: string[]; - }; - status?: { - excludes?: string[]; - includes?: string[]; - }; -}; - -type ProposedChangeDiffFilterProps = { - branch: string; - filters?: DiffFilter; -}; - -export const ProposedChangeDiffFilter = ({ branch, filters }: ProposedChangeDiffFilterProps) => { - const [qsp, setQsp] = useQueryState(QSP.STATUS, parseAsString.withOptions({ shallow: false })); +type DiffFilterProps = GetDiffSummaryParams; - const { error, data = {} } = useQuery(getProposedChangesDiffSummary, { - skip: !branch, - variables: { branch, filters }, - context: { - processErrorMessage: (message: string) => { - // If the branch is not found, then do not display alert - if (message.includes("not found")) return; +export function DiffFilter({ branch, filters }: DiffFilterProps) { + const [statusFilterQSP, setQsp] = useQueryState( + QSP.STATUS, + parseAsString.withOptions({ shallow: false }) + ); - toast(, { - toastId: "alert-error", - }); - }, - }, - }); + const { error, data, isPending } = useGetDiffSummary({ branch, filters }); - const handleFilter = (value: string) => { - setQsp(value === qsp ? null : value); + const handleFilter = (value: DiffStatus) => { + setQsp(value === statusFilterQSP ? null : value); }; + if (isPending) { + return ; + } + if (error) { return ( - + handleFilter(DIFF_STATUS.ADDED)} /> - handleFilter(DIFF_STATUS.REMOVED)} /> - handleFilter(DIFF_STATUS.UPDATED)} /> - handleFilter(DIFF_STATUS.CONFLICT)} /> -
+ ); -}; - -interface FilterButtonProps extends ButtonProps { - status: string; - count: number; - currentFilter: string | null | undefined; - onFilter: (value: string) => void; } - -const FilterButton = ({ status, count, currentFilter, onFilter, ...props }: FilterButtonProps) => { - const isMuted = !!currentFilter && currentFilter !== status; - const isDisabled = !count && currentFilter !== status; - - const CloseBadge = - status === DIFF_STATUS.ADDED - ? CloseBadgeAdded - : status === DIFF_STATUS.REMOVED - ? CloseBadgeRemoved - : status === DIFF_STATUS.UPDATED - ? CloseBadgeUpdated - : status === DIFF_STATUS.CONFLICT - ? CloseBadgeConflict - : null; - - return ( - - ); -}; diff --git a/frontend/app/src/entities/proposed-changes/ui/diff-summary.tsx b/frontend/app/src/entities/proposed-changes/ui/diff-summary.tsx deleted file mode 100644 index 47e1683c1d..0000000000 --- a/frontend/app/src/entities/proposed-changes/ui/diff-summary.tsx +++ /dev/null @@ -1,120 +0,0 @@ -import { useQuery } from "@apollo/client"; -import type React from "react"; -import { Link } from "react-router"; -import { toast } from "react-toastify"; - -import { QSP } from "@/config/qsp"; - -import { constructPath } from "@/shared/api/rest/fetch"; -import ErrorScreen from "@/shared/components/errors/error-screen"; -import { ALERT_TYPES, Alert } from "@/shared/components/ui/alert"; - -import { DiffBadge } from "@/entities/diff/node-diff/utils"; -import { getProposedChangesDiffSummary } from "@/entities/proposed-changes/api/getProposedChangesDiffSummary"; - -import { DIFF_STATUS, type DiffStatus } from "../../diff/node-diff/types"; - -interface DiffTreeSummary { - num_added: number; - num_removed: number; - num_updated: number; - num_conflicts: number; -} - -interface ProposedChangeDiffSummaryProps { - branchName: string; - proposedChangeId: string; -} - -const BadgeLink: React.FC<{ - status: DiffStatus; - count: number | undefined; - proposedChangeId: string; -}> = ({ status, count, proposedChangeId }) => { - const proposedChangeDetailsPath = `/proposed-changes/${proposedChangeId}`; - - return ( - - {count} - - ); -}; - -export const ProposedChangeDiffSummary: React.FC = ({ - proposedChangeId, - branchName, -}) => { - const { error, data, loading } = useQuery<{ DiffTreeSummary: DiffTreeSummary }>( - getProposedChangesDiffSummary, - { - skip: !branchName, - variables: { branch: branchName }, - context: { - processErrorMessage: (message: string) => { - if (!message.includes("not found")) { - toast(, { - toastId: "alert-error", - }); - } - }, - }, - } - ); - - if (loading) { - return ; - } - - if (error) { - return ( - - ); - } - - const { DiffTreeSummary } = data || {}; - - return ( -
- - - - -
- ); -}; - -const DiffSummarySkeleton: React.FC = () => { - return ( -
- {[...Array(4)].map((_, index) => ( -
- ))} -
- ); -}; diff --git a/frontend/app/src/entities/proposed-changes/ui/diff-summary/diff-summary-skeleton.tsx b/frontend/app/src/entities/proposed-changes/ui/diff-summary/diff-summary-skeleton.tsx new file mode 100644 index 0000000000..c6e41cc598 --- /dev/null +++ b/frontend/app/src/entities/proposed-changes/ui/diff-summary/diff-summary-skeleton.tsx @@ -0,0 +1,11 @@ +import { Row } from "@/shared/components/container"; + +export function DiffSummarySkeleton() { + return ( + + {[...Array(4)].map((_, index) => ( +
+ ))} + + ); +} diff --git a/frontend/app/src/entities/proposed-changes/ui/diff-summary/diff-summary-tag-group.tsx b/frontend/app/src/entities/proposed-changes/ui/diff-summary/diff-summary-tag-group.tsx new file mode 100644 index 0000000000..7596d8558e --- /dev/null +++ b/frontend/app/src/entities/proposed-changes/ui/diff-summary/diff-summary-tag-group.tsx @@ -0,0 +1,129 @@ +import { Icon } from "@iconify-icon/react"; +import { cva, type VariantProps } from "class-variance-authority"; +import { CircleMinusIcon, CirclePlusIcon, RefreshCwIcon, TriangleAlertIcon } from "lucide-react"; +import { + Tag, + TagGroup, + type TagGroupProps, + TagList, + type TagListProps, + type TagProps, +} from "react-aria-components"; + +import { disabledStyle, focusVisibleStyle } from "@/shared/components/style-rac"; +import { classNames } from "@/shared/utils/common"; + +export interface DiffSummaryProps + extends Omit, + Pick, "items" | "children" | "renderEmptyState"> {} + +export function DiffSummaryTagGroup({ + items, + children, + renderEmptyState, + ...props +}: DiffSummaryProps) { + return ( + + + + {children} + + + + ); +} + +const diffSummaryTagStyles = cva( + [ + disabledStyle, + focusVisibleStyle, + "relative inline-flex cursor-pointer items-center gap-1 rounded-full border border-transparent p-1 text-xs", + ], + { + variants: { + variant: { + added: "bg-green-200 text-green-800", + removed: "bg-red-200 text-red-800", + updated: "bg-blue-200 text-blue-800", + conflicts: "bg-yellow-200 text-yellow-800", + }, + isMuted: { + true: "opacity-50", + }, + }, + } +); + +export interface DiffSummaryTagProps extends VariantProps, TagProps { + count: number; + isClosable?: boolean; +} + +export function DiffSummaryTag({ + count, + variant, + className, + isClosable, + isMuted, + children, + ...props +}: DiffSummaryTagProps) { + return ( + + + {count} + {isClosable && } + + ); +} + +export function DiffSummaryIcon({ + variant, + ...props +}: Pick, "variant">) { + const className = "size-3"; + + switch (variant) { + case "added": + return ; + case "removed": + return ; + case "updated": + return ; + case "conflicts": + return ; + default: + return null; + } +} + +const diffSummaryCloseStyles = cva( + "-top-2 -right-2 absolute flex items-center justify-center rounded-full border-2 border-white", + { + variants: { + variant: { + added: "bg-green-200 text-green-800", + removed: "bg-red-200 text-red-800", + updated: "bg-blue-200 text-blue-800", + conflicts: "bg-yellow-200 text-yellow-800", + }, + }, + } +); + +export interface DiffSummaryCloseProps + extends React.HTMLProps, + VariantProps {} + +export function DiffSummaryClose({ className, variant, ...props }: DiffSummaryCloseProps) { + return ( +
+ +
+ ); +} diff --git a/frontend/app/src/entities/proposed-changes/ui/diff-summary/proposed-change-diff-summary.tsx b/frontend/app/src/entities/proposed-changes/ui/diff-summary/proposed-change-diff-summary.tsx new file mode 100644 index 0000000000..069ba0f320 --- /dev/null +++ b/frontend/app/src/entities/proposed-changes/ui/diff-summary/proposed-change-diff-summary.tsx @@ -0,0 +1,81 @@ +import { QSP } from "@/config/qsp"; + +import { constructPath } from "@/shared/api/rest/fetch"; +import ErrorScreen from "@/shared/components/errors/error-screen"; + +import { useGetDiffSummary } from "@/entities/diff/domain/get-diff-summary.query"; +import { DIFF_STATUS } from "@/entities/diff/node-diff/types"; +import { DiffSummarySkeleton } from "@/entities/proposed-changes/ui/diff-summary/diff-summary-skeleton"; +import { + DiffSummaryTag, + DiffSummaryTagGroup, +} from "@/entities/proposed-changes/ui/diff-summary/diff-summary-tag-group"; + +interface ProposedChangeDiffSummaryProps { + branchName: string; + proposedChangeId: string; +} + +export function ProposedChangeDiffSummary({ + proposedChangeId, + branchName, +}: ProposedChangeDiffSummaryProps) { + const { error, data, isPending } = useGetDiffSummary({ branch: branchName }); + + if (isPending) { + return ; + } + + if (error) { + return ( + + ); + } + + if (!data) { + return null; + } + + const proposedChangeDetailsPath = `/proposed-changes/${proposedChangeId}`; + + return ( + + + + + + + ); +} diff --git a/frontend/app/src/entities/proposed-changes/ui/proposed-change-item.tsx b/frontend/app/src/entities/proposed-changes/ui/proposed-change-item.tsx index f6e9f8f867..2bb49e8953 100644 --- a/frontend/app/src/entities/proposed-changes/ui/proposed-change-item.tsx +++ b/frontend/app/src/entities/proposed-changes/ui/proposed-change-item.tsx @@ -12,7 +12,7 @@ import { classNames } from "@/shared/utils/common"; import { useObjectsCount } from "@/entities/nodes/object/domain/get-objects-count.query"; import { useObjectTableContext } from "@/entities/nodes/object/ui/object-table/object-table-context"; import type { ProposedChangeItem } from "@/entities/proposed-changes/domain/get-proposed-changes"; -import { ProposedChangeDiffSummary } from "@/entities/proposed-changes/ui/diff-summary"; +import { ProposedChangeDiffSummary } from "@/entities/proposed-changes/ui/diff-summary/proposed-change-diff-summary"; import { ProposedChangesActionCell } from "@/entities/proposed-changes/ui/proposed-changes-actions-cell"; import { useSchema } from "@/entities/schema/ui/hooks/useSchema"; diff --git a/frontend/app/src/pages/branches/details.tsx b/frontend/app/src/pages/branches/details.tsx index 747e94f853..d20cf4a4a8 100644 --- a/frontend/app/src/pages/branches/details.tsx +++ b/frontend/app/src/pages/branches/details.tsx @@ -99,7 +99,7 @@ const BranchContent = ({ branchName }: { branchName: string }) => { case DIFF_TABS.SCHEMA: { return ( { case DIFF_TABS.DATA: { return (