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

[Feature] New submissions section on VaultDetailsPage #551

Merged
merged 9 commits into from
Sep 26, 2023
5 changes: 5 additions & 0 deletions packages/web/src/languages/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,11 @@
"projectNotAvailable": "Project not available",
"projectNotAvailableExplanation": "The project on which you are attempting to submit a vulnerability is no longer available. \n\n If you think this is an error, please contact Hats support.",
"understand": "Understand",
"thereIsNoPublicSubmission": "There are no public submission for this vault yet.",
"gettingSubmissions": "Getting submissions",
"seeSubmissionsOnGithub": "See submissions on GitHub",
"openGithub": "Open GitHub",
"doYouWantToSeeSubmissionsOnGithub": "Do you want to see the submissions on GitHub?",
"PGPTool": {
"title": "PGP tool",
"unlockPgpTool": "Unlock the PGP tool",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import TerminalIcon from "@mui/icons-material/Terminal";
import ContractsIcon from "@mui/icons-material/ViewInAr";
import ContractsTwoIcon from "@mui/icons-material/ViewWeekOutlined";
import MDEditor from "@uiw/react-md-editor";
import { Alert, Button, CopyToClipboard, Pill, WithTooltip } from "components";
import { Alert, Button, CopyToClipboard, Loading, Pill, WithTooltip } from "components";
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The import statement seems to be unchanged except for the addition of line numbers. There's no issue with this line.

- import { Alert, Button, CopyToClipboard, Pill, WithTooltip } from "components";
+ import { Alert, Button, CopyToClipboard, Loading, Pill, WithTooltip } from "components";

import { disallowedElementsMarkdown } from "constants/constants";
import { defaultAnchorProps } from "constants/defaultAnchorProps";
import { useEffect, useState } from "react";
Expand Down Expand Up @@ -50,7 +50,7 @@ export const InScopeSection = ({ vault }: InScopeSectionProps) => {
getRepoContractsList();
}, [vault.description?.scope?.reposInformation]);

if (!vault.description) return null;
if (!vault.description) return <Loading fixed extraText={`${t("loading")}...`} />;

Choose a reason for hiding this comment

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

Image description CodeRabbit

The loading state handling has been improved by adding a loading component with extra text instead of returning null. This provides better user feedback during data fetching.

- if (!vault.description) return null;
+ if (!vault.description) return <Loading fixed extraText={`${t("loading")}...`} />;


const contractsCovered = severitiesToContractsCoveredForm(vault.description?.severities);
const docsLink = vault.description.scope?.docsLink;
Expand Down Expand Up @@ -219,7 +219,9 @@ export const InScopeSection = ({ vault }: InScopeSectionProps) => {
</>
)
) : (
<Alert type="info">{t("loadingContractsOnRepo")}</Alert>
<Alert type="info" className="mb-4">
{t("loadingContractsOnRepo")}
</Alert>
Comment on lines +222 to +224

Choose a reason for hiding this comment

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

Image description CodeRabbit

The alert component now includes a className prop and the message has been moved to a new line. This should improve readability and maintainability of the code.

- <Alert type="info">{t("loadingContractsOnRepo")}</Alert>
+ <Alert type="info" className="mb-4">
+   {t("loadingContractsOnRepo")}
+ </Alert>

)}

{/* Contracts covered */}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { IVault, IVulnerabilitySeverity } from "@hats-finance/shared";
import MDEditor from "@uiw/react-md-editor";
import { Pill } from "components";
import { disallowedElementsMarkdown } from "constants/constants";
import { getSeveritiesColorsArray } from "hooks/severities/useSeverityRewardInfo";
import moment from "moment";
import { IGithubIssue } from "pages/Honeypots/VaultDetailsPage/types";
import { useState } from "react";
import { useTranslation } from "react-i18next";
import { StyledPublicSubmissionCard } from "./styles";

type PublicSubmissionCardProps = {
vault: IVault;
submission: IGithubIssue;
};

function PublicSubmissionCard({ vault, submission }: PublicSubmissionCardProps) {
const { t } = useTranslation();

const [isOpen, setIsOpen] = useState(false);

const severityColors = getSeveritiesColorsArray(vault);
const severityIndex =
submission.severity &&
vault?.description?.severities.findIndex((sev: IVulnerabilitySeverity) =>
sev.name.toLowerCase().includes(submission.severity ?? "")
);
Comment on lines +20 to +27

Choose a reason for hiding this comment

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

Image description CodeRabbit

The code is trying to find the index of a severity level in the vault.description.severities array that matches the submission.severity. However, it's using toLowerCase() on the severity name but not on the submission.severity, which could lead to inconsistencies if the case doesn't match. I suggest converting both to lowercase for comparison.

- sev.name.toLowerCase().includes(submission.severity ?? "")
+ sev.name.toLowerCase().includes((submission.severity ?? "").toLowerCase())


return (
<StyledPublicSubmissionCard isOpen={isOpen}>
<div className="card-header" onClick={() => setIsOpen((prev) => !prev)}>
<div className="severity">
<Pill textColor={severityColors[severityIndex ?? 0]} isSeverity text={submission.severity ?? t("noSeverity")} />

Choose a reason for hiding this comment

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

Image description CodeRabbit

The code is defaulting to the first color in severityColors when severityIndex is undefined. This might be misleading if the first color does not represent "no severity". Consider adding a neutral color for "no severity" cases.

- <Pill textColor={severityColors[severityIndex ?? 0]} isSeverity text={submission.severity ?? t("noSeverity")} />
+ <Pill textColor={severityIndex !== undefined ? severityColors[severityIndex] : 'neutralColor'} isSeverity text={submission.severity ?? t("noSeverity")} />

</div>
<p className="date">{moment(submission.createdAt).format("Do MMM YYYY - hh:mma")}</p>
<p className="submission-title">{submission.issueData.issueTitle}</p>
</div>

<div className="card-content">
<MDEditor.Markdown
disallowedElements={disallowedElementsMarkdown}
className="submission-content"
source={submission.issueData.issueDescription
.replace("\n**Submission hash", "\\\n**Submission hash")
.replace("\n**Severity:**", "\\\n**Severity:**")}
/>
</div>
</StyledPublicSubmissionCard>
);
}

export default PublicSubmissionCard;
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import styled, { css } from "styled-components";
import { getSpacing } from "styles";

export const StyledPublicSubmissionCard = styled.div<{ isOpen: boolean }>(
({ isOpen }) => css`
position: relative;

.card-header {
position: relative;
display: flex;
flex-direction: column;
gap: ${getSpacing(1.5)};
border: 1px solid var(--primary-light);
padding: ${getSpacing(3)} ${getSpacing(4)};
background: var(--background-2);
cursor: pointer;
transition: 0.2s;

&:hover {
border-color: var(--primary);
}

.date {
position: absolute;
top: ${getSpacing(3)};
right: ${getSpacing(4)};
}

.submission-title {
font-size: var(--small);
padding-left: ${getSpacing(1)};
font-weight: 700;
}
}

.card-content {
overflow: hidden;
height: 0;

${isOpen &&
css`
height: auto;
`}

.submission-content {
white-space: normal;
font-size: var(--xsmall);
background: var(--background-3);
padding: ${getSpacing(3)} ${getSpacing(4)};
color: var(--white);
border: 1px solid var(--primary-light);
border-top: none;
}
}
`
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { IVault } from "@hats-finance/shared";
import OpenIcon from "@mui/icons-material/OpenInNewOutlined";
import { Alert, Button, HatSpinner } from "components";
import useConfirm from "hooks/useConfirm";
import { useTranslation } from "react-i18next";
import { useSavedSubmissions, useVaultRepoName } from "../../hooks";
import PublicSubmissionCard from "./PublicSubmissionCard/PublicSubmissionCard";
import { StyledSubmissionsSection } from "./styles";

type VaultSubmissionsSectionProps = {
vault: IVault;
noDeployed?: boolean;
};

export const VaultSubmissionsSection = ({ vault }: VaultSubmissionsSectionProps) => {
const { t } = useTranslation();
const confirm = useConfirm();

const { data: savedSubmissions, isLoading } = useSavedSubmissions(vault);
const { data: repoName } = useVaultRepoName(vault);

const goToGithubIssues = async () => {
if (!repoName) return;

const githubLink = `https://github.com/hats-finance/${repoName}/issues`;

const wantToGo = await confirm({
title: t("openGithub"),
titleIcon: <OpenIcon className="mr-2" fontSize="large" />,
description: t("doYouWantToSeeSubmissionsOnGithub"),
cancelText: t("no"),
confirmText: t("yesGo"),
});

if (!wantToGo) return;
window.open(githubLink, "_blank");
};

return (
<StyledSubmissionsSection>
<h2>
{t("submissions")}

{repoName && (
<Button className="ml-3" styleType="text" textColor="secondary" onClick={goToGithubIssues}>
{t("seeSubmissionsOnGithub")}
</Button>
)}
</h2>

{savedSubmissions?.length === 0 && (
<Alert className="mt-5 mb-5" type="info">
{t("thereIsNoPublicSubmission")}
</Alert>
)}

{isLoading && <HatSpinner text={`${t("gettingSubmissions")}...`} />}

{savedSubmissions && savedSubmissions?.length > 0 && (
<div className="public-submissions">
{savedSubmissions.map((submission) => (
<PublicSubmissionCard key={submission._id} vault={vault} submission={submission} />
))}
</div>
)}
</StyledSubmissionsSection>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import styled from "styled-components";
import { getSpacing } from "styles";

export const StyledSubmissionsSection = styled.div`
padding-bottom: ${getSpacing(10)};

.public-submissions {
margin-top: ${getSpacing(3)};
display: flex;
flex-direction: column;
gap: ${getSpacing(3)};
}
`;
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { VaultRewardsSection } from "./VaultRewardsSection/VaultRewardsSection";
export { VaultScopeSection } from "./VaultScopeSection/VaultScopeSection";
export { VaultDepositsSection } from "./VaultDepositsSection/VaultDepositsSection";
export { VaultSubmissionsSection } from "./VaultSubmissionsSection/VaultSubmissionsSection";
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { IVault } from "@hats-finance/shared";
import { Alert, Loading, Seo, VaultCard } from "components";
import { useVaults } from "hooks/subgraph/vaults/useVaults";
import { useState } from "react";
import { useMemo, useState } from "react";
import { useTranslation } from "react-i18next";
import { redirect, useNavigate, useParams } from "react-router-dom";
import { useNavigate, useParams } from "react-router-dom";
import { HoneypotsRoutePaths } from "../router";
import { VaultDepositsSection, VaultRewardsSection, VaultScopeSection } from "./Sections";
import { VaultDepositsSection, VaultRewardsSection, VaultScopeSection, VaultSubmissionsSection } from "./Sections";
import { useSavedSubmissions } from "./hooks";
import { StyledSectionTab, StyledVaultDetailsPage } from "./styles";

const DETAILS_SECTIONS = [
Expand All @@ -21,6 +22,10 @@ const DETAILS_SECTIONS = [
title: "deposits",
component: VaultDepositsSection,
},
{
title: "submissions",
component: VaultSubmissionsSection,
},
];

type VaultDetailsPageProps = {
Expand All @@ -37,17 +42,28 @@ export const VaultDetailsPage = ({ vaultToUse, noActions = false, noDeployed = f
const { vaultSlug, sectionId } = useParams();
const vaultId = vaultSlug?.split("-").pop();
const vault = vaultToUse ?? allVaults?.find((vault) => vault.id === vaultId);
const isAudit = vault?.description?.["project-metadata"].type === "audit";

const { data: savedSubmissions } = useSavedSubmissions(vault);

const [openSectionId, setOpenSectionId] = useState(sectionId ?? DETAILS_SECTIONS[0].title);

const DETAILS_SECTIONS_TO_SHOW = useMemo(
() =>
DETAILS_SECTIONS.filter((section) => {
if (section.title === "deposits" && noActions) return false;
if (section.title === "submissions" && (!isAudit || savedSubmissions?.length === 0)) return false;
return true;
}),
[noActions, isAudit, savedSubmissions]
);

if (allVaults?.length === 0) return <Loading extraText={`${t("loadingVaultDetails")}...`} />;
if (!vault || !vault.description) {
redirect("/");
return null;
return <Loading extraText={`${t("loadingVaultDetails")}...`} />;
}

const activeClaim = vault.activeClaim;
Comment on lines +45 to 66
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The logic for determining whether to show the "submissions" section is a bit complex and could be simplified. Instead of filtering DETAILS_SECTIONS every time, you could create a separate function that checks if a section should be displayed based on its title and other conditions. This would make the code more readable and maintainable.

- const DETAILS_SECTIONS_TO_SHOW = useMemo(
-   () =>
-     DETAILS_SECTIONS.filter((section) => {
-       if (section.title === "deposits" && noActions) return false;
-       if (section.title === "submissions" && (!isAudit || savedSubmissions?.length === 0)) return false;
-       return true;
-     }),
-   [noActions, isAudit, savedSubmissions]
- );

+ const shouldShowSection = (sectionTitle: string): boolean => {
+   switch(sectionTitle) {
+     case 'deposits':
+       return !noActions;
+     case 'submissions':
+       return isAudit && savedSubmissions?.length > 0;
+     default:
+       return true;
+   }
+ }

+ const DETAILS_SECTIONS_TO_SHOW = useMemo(() => 
+   DETAILS_SECTIONS.filter(({ title }) => shouldShowSection(title)),
+   [noActions, isAudit, savedSubmissions]
+ );

const isAudit = vault.description["project-metadata"].type === "audit";
const vaultName = vault.description["project-metadata"].name;

const navigateToType = () => {
Expand All @@ -70,7 +86,7 @@ export const VaultDetailsPage = ({ vaultToUse, noActions = false, noDeployed = f
return (
<>
<Seo title={`${vaultName} ${isAudit ? t("auditCompetition") : t("bugBounty")}`} />
<StyledVaultDetailsPage className="content-wrapper" isAudit={isAudit}>
<StyledVaultDetailsPage className="content-wrapper" isAudit={isAudit} tabsNumber={DETAILS_SECTIONS_TO_SHOW.length}>
{!noActions && (
<div className="breadcrumb">
<span className="type" onClick={navigateToType}>
Expand All @@ -91,7 +107,7 @@ export const VaultDetailsPage = ({ vaultToUse, noActions = false, noDeployed = f
</div>

<div className="sections-tabs">
{DETAILS_SECTIONS.filter((section) => (noActions ? section.title !== "deposits" : true)).map((section) => (
{DETAILS_SECTIONS_TO_SHOW.map((section) => (
<StyledSectionTab
onClick={() => changeDetailsSection(section.title)}
active={openSectionId === section.title}
Expand Down
28 changes: 28 additions & 0 deletions packages/web/src/pages/Honeypots/VaultDetailsPage/hooks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { IVault } from "@hats-finance/shared";
import { UseQueryResult, useQuery } from "@tanstack/react-query";
import * as savedSubmissionsService from "./savedSubmissionsService";
import { IGithubIssue } from "./types";

/**
* Returns all saved submissions for a vault
*/
export const useSavedSubmissions = (vault: IVault | undefined): UseQueryResult<IGithubIssue[]> => {
return useQuery({
queryKey: ["github-issues", vault?.id],
queryFn: () => savedSubmissionsService.getSavedSubmissions(vault?.id),
refetchOnWindowFocus: false,
enabled: !!vault,
});
};

/**
* Returns the repo name created for a vault
*/
export const useVaultRepoName = (vault: IVault | undefined): UseQueryResult<string> => {
return useQuery({
queryKey: ["github-repo-name", vault?.id],
queryFn: () => savedSubmissionsService.getVaultRepoName(vault?.id),
refetchOnWindowFocus: false,
enabled: !!vault,
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { axiosClient } from "config/axiosClient";
import { BASE_SERVICE_URL } from "settings";
import { IGithubIssue } from "./types";

/**
* Get all saved github issues for a vault
*/
export async function getSavedSubmissions(vaultId: string | undefined): Promise<IGithubIssue[]> {
if (!vaultId) return [];

try {
const response = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/issues/${vaultId}`);
const githubIssues = response.data.githubIssues as IGithubIssue[];
const githubIssuesWithSeverity = githubIssues.map((issue) => {
const severity = issue.issueData.issueDescription.match(/(\*\*Severity:\*\* (.*)\n)/)?.[2];
return {
...issue,
severity,
};
});

return githubIssuesWithSeverity;
} catch (error) {
return [];
}
Comment on lines +8 to +25
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The function getSavedSubmissions does not handle errors properly. It simply returns an empty array when an error occurs, which might be misleading for the caller of this function. Instead, it would be better to throw the error and let the caller decide how to handle it.

-    } catch (error) {
-      return [];
-    }
+    } catch (error) {
+      throw error;
+    }

}

/**
* Get the repo name created for a vault
*/
export async function getVaultRepoName(vaultId: string | undefined): Promise<string | undefined> {
if (!vaultId) return undefined;

try {
const response = await axiosClient.get(`${BASE_SERVICE_URL}/github-repos/repo/${vaultId}`);
const repoName = response.data.repoName as string | undefined;
console.log(response);

return repoName;
} catch (error) {
return undefined;
}
Comment on lines +31 to +42
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

Similar to the previous comment, the function getVaultRepoName should throw the error instead of returning undefined. Also, there is a console.log statement on line 37 that should be removed as it could potentially leak sensitive information.

-    } catch (error) {
-      return undefined;
-    }
+    } catch (error) {
+      throw error;
+    }
-    console.log(response);

}
Loading