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

[Feat] Audit wizard integration #552

Merged
merged 11 commits into from
Sep 18, 2023
Merged

[Feat] Audit wizard integration #552

merged 11 commits into from
Sep 18, 2023

Conversation

fonstack
Copy link
Member

@fonstack fonstack commented Sep 18, 2023

Summary by CodeRabbit

  • New Feature: Integrated Audit Wizard functionality into the submission form. This includes data population from Audit Wizard submissions, signature verification, and disabling of the form when an Audit Wizard submission is present.
  • New Feature: Added a clear submission button to the form for better user control.
  • New Feature: Enhanced the Submission Review component with a Markdown preview feature for better readability of submission messages.
  • Style: Improved the visual layout of the submission form with additional CSS styling for better user experience.

@fonstack fonstack requested a review from shayzluf September 18, 2023 20:26
@github-actions
Copy link

github-actions bot commented Sep 18, 2023

Image description CodeRabbit

Walkthrough

This pull request integrates the Audit Wizard functionality into the submission form. It introduces new state variables, functions for handling audit wizard submissions, and modifies UI components to accommodate these changes. The form inputs and buttons are disabled based on a new context variable, and the GitHub issue description is modified conditionally.

Changes

File(s) Summary
packages/web/src/constants/constants.ts Added a new constant auditWizardVerifyService.
packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionContactInfo/SubmissionContactInfo.tsx,
packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionDescriptions/SubmissionDescriptions.tsx,
packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionProject/SubmissionProject.tsx,
packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionDescriptions/utils.ts
Introduced changes to disable certain form inputs and buttons based on the value of allFormDisabled. Modified the generated GitHub issue description conditionally.
packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionProcessed/SubmissionProcessed.tsx Appended an ellipsis to the existing value obtained from the translation function.
packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/SubmissionReview.tsx,
packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/styles.ts
Imported MDEditor component and disallowedElementsMarkdown constant. Rendered a Markdown preview and displayed the submission message in a read-only textarea. Adjusted the visual layout of the buttons.
packages/web/src/pages/Submissions/SubmissionFormPage/SubmissionFormPage.tsx,
packages/web/src/pages/Submissions/SubmissionFormPage/store.ts,
packages/web/src/pages/Submissions/SubmissionFormPage/submissionsService.api.ts,
packages/web/src/pages/Submissions/SubmissionFormPage/types.ts
Added new state variables, functions for handling audit wizard integration, and modified useEffect hooks. Introduced a function to verify the signature sent by Audit Wizard. Updated interfaces to accommodate audit wizard data.
packages/web/src/pages/Submissions/SubmissionFormPage/styles.ts Added a new CSS class "top-controls" for styling an audit wizard submission form.

🐇

In the land of code, where logic intertwines,

A rabbit hops, leaving traces in lines.

With each leap forward, a feature takes shape,

An Audit Wizard in the digital landscape.

Forms are filled, submissions take flight,

Celebrating changes, in the soft moonlight. 🌙


Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between 720f89d and 2e212e7 commits.
Files selected (13)
  • packages/web/src/constants/constants.ts (1)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionContactInfo/SubmissionContactInfo.tsx (5)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionDescriptions/SubmissionDescriptions.tsx (8)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionDescriptions/utils.ts (1)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionProject/SubmissionProject.tsx (2)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionProcessed/SubmissionProcessed.tsx (1)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/SubmissionReview.tsx (2)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/styles.ts (1)
  • packages/web/src/pages/Submissions/SubmissionFormPage/SubmissionFormPage.tsx (6)
  • packages/web/src/pages/Submissions/SubmissionFormPage/store.ts (1)
  • packages/web/src/pages/Submissions/SubmissionFormPage/styles.ts (2)
  • packages/web/src/pages/Submissions/SubmissionFormPage/submissionsService.api.ts (3)
  • packages/web/src/pages/Submissions/SubmissionFormPage/types.ts (3)
Files ignored due to filter (2)
  • packages/web/src/assets/images/audit_wizard.png
  • packages/web/src/languages/en.json
Files skipped from review due to trivial changes (5)
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionDescriptions/SubmissionDescriptions.tsx
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionProcessed/SubmissionProcessed.tsx
  • packages/web/src/pages/Submissions/SubmissionFormPage/FormSteps/SubmissionSubmit/components/SubmissionReview/styles.ts
  • packages/web/src/pages/Submissions/SubmissionFormPage/store.ts
  • packages/web/src/pages/Submissions/SubmissionFormPage/styles.ts
Review comments generated (27)
  • Review: 25
  • LGTM: 2

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

Comment on lines +91 to 94
return `${submissionData.ref === "audit-wizard" ? "***Submitted via auditwizard.io***\n" : ""}
**Github username:** ${submissionData.contact?.githubUsername ? `@${submissionData.contact?.githubUsername}` : "--"}
**Submission hash (on-chain):** ${submissionData.submissionResult?.transactionHash}
**Severity:** ${description.severity}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The changes made here are adding a conditional check to modify the generated GitHub issue description based on whether the submission is from the audit wizard or not. This seems like a reasonable change, but it would be better if the string "audit-wizard" was replaced with a constant to avoid potential typos and improve maintainability.

- return `${submissionData.ref === "audit-wizard" ? "***Submitted via auditwizard.io***\n" : ""}`
+ const AUDIT_WIZARD_REF = "audit-wizard";
+ return `${submissionData.ref === AUDIT_WIZARD_REF ? "***Submitted via auditwizard.io***\n" : ""}`

This way, if the reference for the audit wizard ever needs to be changed, it can be done in one place rather than having to find all instances of the string in the codebase.

@@ -26,6 +26,8 @@ export const MAX_NFT_TIER = 3;
export const stagingServiceUrl = "https://hats-backend-dev.herokuapp.com/v1";
export const prodServiceUrl = "https://hats-backend-prod.herokuapp.com/v1";

export const auditWizardVerifyService = "https://app.auditwizard.io:5000/submissions/finalizeHats";

Choose a reason for hiding this comment

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

Image description CodeRabbit

The URL for the auditWizardVerifyService is hardcoded. This could lead to issues when you need to switch between different environments (like staging, production, etc.). Consider using environment variables for such values.

- export const auditWizardVerifyService = "https://app.auditwizard.io:5000/submissions/finalizeHats";
+ export const auditWizardVerifyService = process.env.AUDIT_WIZARD_VERIFY_SERVICE || "https://app.auditwizard.io:5000/submissions/finalizeHats";

This way, you can set the AUDIT_WIZARD_VERIFY_SERVICE environment variable as needed per environment, and if it's not set, it will default to the provided URL.

@@ -9,7 +9,7 @@ import { StyledSubmissionProject } from "./styles";

export function SubmissionProject() {
const { t } = useTranslation();
const { submissionData, setSubmissionData } = useContext(SubmissionFormContext);
const { submissionData, setSubmissionData, allFormDisabled } = useContext(SubmissionFormContext);

Choose a reason for hiding this comment

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

Image description CodeRabbit

The new variable allFormDisabled is introduced from the context. It's good to see that you're using context to manage global state, but it would be better if you could add some error handling here. If for some reason the context does not provide this variable, it could lead to unexpected behavior or crashes. Consider providing a default value when destructuring the context.

- const { submissionData, setSubmissionData, allFormDisabled } = useContext(SubmissionFormContext);
+ const { submissionData, setSubmissionData, allFormDisabled = false } = useContext(SubmissionFormContext);

@@ -36,7 +36,7 @@ export function SubmissionProject() {
key={index}
vault={vault}
expanded={false}
onSelect={() => handleSelectedProject(vault)}
onSelect={!allFormDisabled ? () => handleSelectedProject(vault) : () => {}}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The ternary operator used here is a bit confusing and might lead to code readability issues. Instead of using an empty function as a fallback, consider moving the condition inside the handleSelectedProject function. This way, the onSelect prop always points to the same function, which internally decides whether to perform any action based on the allFormDisabled flag.

- onSelect={!allFormDisabled ? () => handleSelectedProject(vault) : () => {}}
+ onSelect={() => handleSelectedProject(vault)}

And then in your handleSelectedProject function:

- const handleSelectedProject = (vault) => {
-   // existing logic...
- }
+ const handleSelectedProject = (vault) => {
+   if (allFormDisabled) return;
+   // existing logic...
+ }

Comment on lines +26 to 45
import { submitVulnerabilitySubmission, verifyAuditWizardSignature } from "./submissionsService.api";
import {
IAuditWizardSubmissionData,
ISubmissionData,
SubmissionOpStatus,
SubmissionStep,
getCurrentAuditwizardSubmission,
} from "./types";

export const SubmissionFormPage = () => {
const { t } = useTranslation();
const confirm = useConfirm();

const [searchParams] = useSearchParams();
const { chain } = useNetwork();
const [currentStep, setCurrentStep] = useState<number>();
const [submissionData, setSubmissionData] = useState<ISubmissionData>();
const [allFormDisabled, setAllFormDisabled] = useState(false);
const [receivedSubmissionAuditwizard, setReceivedSubmissionAuditwizard] = useState<IAuditWizardSubmissionData | undefined>();

Choose a reason for hiding this comment

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

Image description CodeRabbit

The new state variables and functions introduced here seem to be necessary for the integration of the audit wizard functionality. However, it's important to ensure that these changes are properly tested, especially verifyAuditWizardSignature which is a critical security function.

@@ -44,7 +44,7 @@ export function SubmissionContactInfo() {
setSubmissionData((prev) => {
if (prev)
return {
...prev!,
...prev,

Choose a reason for hiding this comment

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

Image description CodeRabbit

The spread operator (...) is used to create a shallow copy of the prev object. This is a good practice when you want to update the state in React because it creates a new object with the same properties, which helps React optimize re-rendering.

-          ...prev!,
+          ...prev,
           contact: { ...contactData, verified: true },
           submissionsDescriptions: { ...prev.submissionsDescriptions, verified: false },
           submissionResult: undefined,

@@ -70,13 +70,15 @@ export function SubmissionContactInfo() {

<FormInput
{...register("beneficiary")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormInput component and its value is set to allFormDisabled. This will disable the input field if allFormDisabled is true.

-        {...register("beneficiary")}
+        {...register("beneficiary")}
+        disabled={allFormDisabled}
         label={`${t("beneficiaryWalletAddress")}`}
         placeholder={t("beneficiaryWalletAddressPlaceholder")}
         colorable

label={`${t("beneficiaryWalletAddress")}`}
placeholder={t("beneficiaryWalletAddressPlaceholder")}
colorable
/>

<FormRadioInput
{...register("communicationChannelType")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormRadioInput component and its value is set to allFormDisabled. This will disable the radio input if allFormDisabled is true.

-        {...register("communicationChannelType")}
+        {...register("communicationChannelType")}
+        disabled={allFormDisabled}
         label={t("Submissions.addPreferredCommunicationChannel")}
         radioOptions={[
           { label: t("discord"), value: "discord" },

@@ -87,6 +89,7 @@ export function SubmissionContactInfo() {

<FormInput
{...register("communicationChannel")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormInput component and its value is set to allFormDisabled. This will disable the input field if allFormDisabled is true.

-        {...register("communicationChannel")}
+        {...register("communicationChannel")}
+        disabled={allFormDisabled}
         label={communicationChannelLabel}
         placeholder={t("enterCommunicationChannel")}
         colorable

@@ -97,6 +100,7 @@ export function SubmissionContactInfo() {
<p className="mb-2">{t("Submissions.addGithubAccountConnectIssue")}</p>
<FormInput
{...register("githubUsername")}
disabled={allFormDisabled}

Choose a reason for hiding this comment

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

Image description CodeRabbit

The disabled attribute is added to the FormInput component and its value is set to allFormDisabled. This will disable the input field if allFormDisabled is true.

-            {...register("githubUsername")}
+            {...register("githubUsername")}
+            disabled={allFormDisabled}
             label={`${t("githubUsername")}`}
             placeholder={t("githubUsernamePlaceholder")}
             colorable

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2e212e7
Status: ✅  Deploy successful!
Preview URL: https://9475ec15.dapp-a9y.pages.dev
Branch Preview URL: https://develop.dapp-a9y.pages.dev

View logs

@fonstack fonstack merged commit 1ece132 into main Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants