Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add facts modal/code viewer & detail drawer tab #1057

Merged
merged 7 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions client/src/app/api/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,3 +652,10 @@ export interface WaveWithStatus extends MigrationWave {
fullApplications: Application[];
allStakeholders: StakeholderWithRole[];
}
export type UnstructuredFact = any;

export type Fact = {
name: string;
//TODO: Address this when moving to structured facts api
data: any;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to suggest we replace any with unknown here to prevent any issues with code assuming it's a string (JSON.stringify won't care what it is), but since we're going to revisit this to support structured facts it's probably fine to leave it as-is (and it stands out as something to be fixed anyway which is maybe good)

};
13 changes: 13 additions & 0 deletions client/src/app/api/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ import {
Ref,
TrackerProject,
TrackerProjectIssuetype,
Fact,
UnstructuredFact,
} from "./models";
import { QueryKey } from "@tanstack/react-query";
import { serializeRequestParamsForHub } from "@app/shared/hooks/table-controls";
Expand Down Expand Up @@ -77,6 +79,7 @@ export const TRACKERS = HUB + "/trackers";
export const TRACKER_PROJECTS = "projects";
export const TRACKER_PROJECT_ISSUETYPES = "issuetypes";
export const TICKETS = HUB + "/tickets";
export const FACTS = HUB + "/facts";

export const RULESETS = HUB + "/rulesets";
export const FILES = HUB + "/files";
Expand Down Expand Up @@ -697,3 +700,13 @@ export const createTagCategory = (

export const updateTagCategory = (obj: TagCategory): Promise<TagCategory> =>
axios.put(`${TAG_CATEGORIES}/${obj.id}`, obj);

// Facts

export const getFacts = (
id: number | string | undefined
): Promise<UnstructuredFact> =>
//TODO: Address this when moving to structured facts api
id
? axios.get(`${APPLICATIONS}/${id}/facts`).then((response) => response.data)
: Promise.reject();
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ import {
ExclamationCircleIcon,
} from "@patternfly/react-icons";
import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
import { Identity, Task } from "@app/api/models";
import { Fact, Identity, Task } from "@app/api/models";
import { getKindIDByRef } from "@app/utils/model-utils";
import { useFetchIdentities } from "@app/queries/identities";
import {
ApplicationDetailDrawer,
IApplicationDetailDrawerProps,
} from "./application-detail-drawer";
import { EmptyTextMessage } from "@app/shared/components";
import { useFetchFacts } from "@app/queries/facts";
import { ApplicationFacts } from "./application-facts";

export interface IApplicationDetailDrawerAnalysisProps
extends Pick<
Expand All @@ -36,6 +38,7 @@ export const ApplicationDetailDrawerAnalysis: React.FC<
const { t } = useTranslation();

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

let matchingSourceCredsRef: Identity | undefined;
let matchingMavenCredsRef: Identity | undefined;
Expand Down Expand Up @@ -137,6 +140,7 @@ export const ApplicationDetailDrawerAnalysis: React.FC<
)}
</TextContent>
}
factsTabContent={!isFetching && <ApplicationFacts facts={facts} />}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ export interface IApplicationDetailDrawerProps
applications?: Application[];
detailsTabMainContent: React.ReactNode;
reportsTabContent?: React.ReactNode;
factsTabContent?: React.ReactNode;
}

enum TabKey {
Details = 0,
Tags,
Reports,
Facts,
}

export const ApplicationDetailDrawer: React.FC<
Expand All @@ -41,6 +43,7 @@ export const ApplicationDetailDrawer: React.FC<
task,
detailsTabMainContent,
reportsTabContent = null,
factsTabContent = null,
}) => {
const [activeTabKey, setActiveTabKey] = React.useState<TabKey>(
TabKey.Details
Expand Down Expand Up @@ -108,6 +111,14 @@ export const ApplicationDetailDrawer: React.FC<
)}
</Tab>
)}
{factsTabContent && (
<Tab
eventKey={TabKey.Facts}
title={<TabTitleText>Facts</TabTitleText>}
>
{factsTabContent}
</Tab>
)}
</Tabs>
</PageDrawerContent>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import {
FilterCategory,
FilterToolbar,
FilterType,
} from "@app/shared/components/FilterToolbar";
import {
Button,
Toolbar,
ToolbarContent,
ToolbarItem,
ToolbarToggleGroup,
} from "@patternfly/react-core";
import React from "react";
import FilterIcon from "@patternfly/react-icons/dist/esm/icons/filter-icon";
import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
import { useTranslation } from "react-i18next";
import { useLegacyFilterState } from "@app/shared/hooks/useLegacyFilterState";
import { Fact } from "@app/api/models";
import { FactDetailModal } from "./fact-detail-modal/fact-detail-modal";

export interface IApplicationRiskProps {
facts: Fact[];
}

export const ApplicationFacts: React.FC<IApplicationRiskProps> = ({
facts,
}) => {
const { t } = useTranslation();
const sources = new Set<string>();

// TODO: work through how to store sources for facts in the ui for sorting
// facts.forEach((fact) => sources.add(fact.source || ""));

const filterCategories: FilterCategory<Fact, "source">[] = [
{
key: "source",
title: t("terms.source"),
type: FilterType.multiselect,
placeholderText: t("terms.source"),
// getItemValue: (fact) => fact.source || "Source default name",
selectOptions: Array.from(sources)
//TODO: Custom sorting for facts may be required
// .sort(compareSources)
.map((source) => source || "Source default name")
.map((source) => ({ key: source, value: source })),
logicOperator: "OR",
},
];

const {
filterValues,
setFilterValues,
filteredItems: filteredFacts,
} = useLegacyFilterState(facts, filterCategories);

const [selectedFactForDetailModal, setSelectedFactForDetailModal] =
React.useState<Fact | null>(null);

return (
<>
<Toolbar
clearAllFilters={() => setFilterValues({})}
clearFiltersButtonText={t("actions.clearAllFilters")}
>
<ToolbarContent className={spacing.p_0}>
<ToolbarItem>Filter by:</ToolbarItem>
<ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">
<FilterToolbar
filterCategories={filterCategories}
filterValues={filterValues}
setFilterValues={setFilterValues}
showFiltersSideBySide
/>
</ToolbarToggleGroup>
</ToolbarContent>
</Toolbar>
{filteredFacts.map((fact) => {
return (
<div>
<Button
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flat list of link-buttons with no real layout aside from line breaks really bothers me for some reason, but that's a design issue. Maybe we can revisit this with Justin. I think even just a compact table with one unlabeled column, or maybe a DataList (https://www.patternfly.org/v4/components/data-list), something to make it look less plain. Outside the scope of this PR though, just thinking out loud.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue for this & linked in the description.

variant="link"
isInline
onClick={() => setSelectedFactForDetailModal(fact)}
>
{fact.name}
</Button>
</div>
);
})}
{selectedFactForDetailModal ? (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You use the form {renderCondition && (<element />)} form in other places, why ternary here?

Copy link
Member Author

@ibolton336 ibolton336 Jun 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to match the style we used in issue-affected-files-table for consistency's sake.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not consistent about it in this repo, but I've always tried to favor the ternary because && can fall back to falsy values that still render something (like 0). https://kentcdodds.com/blog/use-ternaries-rather-than-and-and-in-jsx

<FactDetailModal
fact={selectedFactForDetailModal}
onClose={() => setSelectedFactForDetailModal(null)}
/>
) : null}
</>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import * as React from "react";
import { CodeEditor, Language } from "@patternfly/react-code-editor";
import { Fact } from "@app/api/models";
import yaml from "js-yaml";
export interface IFactCodeSnipViewerProps {
fact: Fact;
}

export const FactCodeSnipViewer: React.FC<IFactCodeSnipViewerProps> = ({
fact,
}) => {
return (
<CodeEditor
isReadOnly
isDarkTheme
isLineNumbersVisible
language={Language.json}
height="450px"
code={yaml.dump(fact.data, { skipInvalid: true })}
/>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import * as React from "react";
import { Button, Modal } from "@patternfly/react-core";
import { FactCodeSnipViewer } from "./fact-code-snip-viewer";
import { Fact } from "@app/api/models";

export interface IFactDetailModalProps {
fact: Fact;
onClose: () => void;
}

export const FactDetailModal: React.FC<IFactDetailModalProps> = ({
fact,
onClose,
}) => {
return (
<Modal
title={fact.name}
variant="large"
isOpen
onClose={onClose}
actions={[
<Button key="close" variant="primary" onClick={onClose}>
Close
</Button>,
]}
>
<FactCodeSnipViewer fact={fact}></FactCodeSnipViewer>
</Modal>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./fact-detail-modal";
27 changes: 27 additions & 0 deletions client/src/app/queries/facts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { useQuery } from "@tanstack/react-query";

import { getFacts } from "@app/api/rest";
import { AxiosError } from "axios";
import { Fact } from "@app/api/models";

export const FactsQueryKey = "facts";

export const useFetchFacts = (applicationID: number | string | undefined) => {
const { data, isLoading, error, refetch } = useQuery(
[FactsQueryKey, applicationID],
{
queryFn: () => getFacts(applicationID),
enabled: !!applicationID,
onError: (error: AxiosError) => console.log("error, ", error),
select: (facts): Fact[] =>
Object.keys(facts).map((fact) => ({ name: fact, data: facts[fact] })),
}
);

return {
facts: data || [],
isFetching: isLoading,
fetchError: error,
refetch,
};
};