Skip to content

Equity grant test#1

Open
Adarsh9977 wants to merge 21 commits intomainfrom
equity-grant-test
Open

Equity grant test#1
Adarsh9977 wants to merge 21 commits intomainfrom
equity-grant-test

Conversation

@Adarsh9977
Copy link
Owner

@Adarsh9977 Adarsh9977 commented Aug 2, 2025

Summary by cubic

Removed legacy onboarding logic and pages, updated onboarding to use a checklist for investors and workers, and added a new modal-based equity grant flow.

  • New Features

    • Added onboarding checklist support for investors and combined roles.
    • Introduced a modal for issuing equity grants.
  • Refactors

    • Removed backend onboarding controllers, services, policies, and related tests.
    • Cleaned up routes and frontend onboarding components.
    • Updated checklist logic and sidebar display to use new onboarding approach.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

cubic analysis

7 issues found across 76 files • Review in cubic

React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.

error_message: z.string().optional(),
error: z.string().optional(),
});
const errorData = errorSchema.parse(await response.json().catch(() => ({})));
Copy link

Choose a reason for hiding this comment

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

Using errorSchema.parse here will throw a ZodError if the response shape deviates (for example if types differ), which then surfaces an unfriendly technical message to the user. Consider safeParse and falling back to a default error string instead. (Based on your team's feedback about providing user-friendly error handling.)

Prompt for AI agents
Address the following comment on frontend/app/settings/page.tsx at line 130:

<comment>Using errorSchema.parse here will throw a ZodError if the response shape deviates (for example if types differ), which then surfaces an unfriendly technical message to the user. Consider safeParse and falling back to a default error string instead. (Based on your team&#39;s feedback about providing user-friendly error handling.)</comment>

<file context>
@@ -123,11 +123,16 @@ const LeaveWorkspaceSection = () =&gt; {
       });
 
       if (!response.ok) {
-        const errorData = await response.json().catch(() =&gt; null);
-        throw new Error(errorData?.error_message || errorData?.error || &quot;Failed to leave workspace&quot;);
+        const errorSchema = z.object({
+          error_message: z.string().optional(),
+          error: z.string().optional(),
+        });
</file context>

zip_code: data.zip_code ?? "",
street_address: data.street_address ?? "",
birth_date: data.birth_date ? parseDate(data.birth_date) : null,
citizenship_country_code: data.citizenship_country_code ?? "",
Copy link

Choose a reason for hiding this comment

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

Defaulting citizenship_country_code to an empty string treats the user as a foreign taxpayer and changes validation / UI flows even when their citizenship is actually unknown.

Prompt for AI agents
Address the following comment on frontend/app/settings/tax/page.tsx at line 117:

<comment>Defaulting citizenship_country_code to an empty string treats the user as a foreign taxpayer and changes validation / UI flows even when their citizenship is actually unknown.</comment>

<file context>
@@ -114,6 +114,9 @@ export default function TaxPage() {
       zip_code: data.zip_code ?? &quot;&quot;,
       street_address: data.street_address ?? &quot;&quot;,
       birth_date: data.birth_date ? parseDate(data.birth_date) : null,
+      citizenship_country_code: data.citizenship_country_code ?? &quot;&quot;,
+      legal_name: data.legal_name ?? &quot;&quot;,
+      country_code: data.country_code ?? &quot;&quot;,
</file context>
Suggested change
citizenship_country_code: data.citizenship_country_code ?? "",
citizenship_country_code: data.citizenship_country_code ?? "US",

<ComboBox
{...field}
options={data.workers
.sort((a, b) => a.user.name.localeCompare(b.user.name))
Copy link

Choose a reason for hiding this comment

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

Sorting mutates the data.workers array in place, potentially corrupting the TRPC cache and causing side-effects elsewhere. Use a copied array before sort to keep fetched data immutable. (Based on your team's feedback about avoiding in-place mutation of shared data.)

Prompt for AI agents
Address the following comment on frontend/app/(dashboard)/equity/grants/NewEquityGrantModal.tsx at line 233:

<comment>Sorting mutates the data.workers array in place, potentially corrupting the TRPC cache and causing side-effects elsewhere. Use a copied array before sort to keep fetched data immutable. (Based on your team&#39;s feedback about avoiding in-place mutation of shared data.)</comment>

<file context>
@@ -0,0 +1,606 @@
+&quot;use client&quot;;
+
+import { zodResolver } from &quot;@hookform/resolvers/zod&quot;;
+import { CalendarDate, getLocalTimeZone, today } from &quot;@internationalized/date&quot;;
+import { ChevronDown, ChevronRight } from &quot;lucide-react&quot;;
+import { useEffect, useState } from &quot;react&quot;;
+import { useForm } from &quot;react-hook-form&quot;;
+import { z } from &quot;zod&quot;;
+import TemplateSelector from &quot;@/app/(dashboard)/document_templates/TemplateSelector&quot;;
</file context>
Suggested change
.sort((a, b) => a.user.name.localeCompare(b.user.name))
.slice().sort((a, b) => a.user.name.localeCompare(b.user.name))

}),
});

const errorInfo = errorInfoSchema.parse(JSON.parse(error.message));
Copy link

Choose a reason for hiding this comment

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

Assumes error.message is always valid JSON; if it is not, JSON.parse will throw and mask the original error, breaking the error handler. Wrap parsing in try/catch or safely parse.

Prompt for AI agents
Address the following comment on frontend/app/(dashboard)/equity/grants/NewEquityGrantModal.tsx at line 159:

<comment>Assumes error.message is always valid JSON; if it is not, JSON.parse will throw and mask the original error, breaking the error handler. Wrap parsing in try/catch or safely parse.</comment>

<file context>
@@ -0,0 +1,606 @@
+&quot;use client&quot;;
+
+import { zodResolver } from &quot;@hookform/resolvers/zod&quot;;
+import { CalendarDate, getLocalTimeZone, today } from &quot;@internationalized/date&quot;;
+import { ChevronDown, ChevronRight } from &quot;lucide-react&quot;;
+import { useEffect, useState } from &quot;react&quot;;
+import { useForm } from &quot;react-hook-form&quot;;
+import { z } from &quot;zod&quot;;
+import TemplateSelector from &quot;@/app/(dashboard)/document_templates/TemplateSelector&quot;;
</file context>

? (Number(data.sharePriceUsd) * numberOfShares).toFixed(2)
: null;

const isFormValid = form.formState.isValid;
Copy link

Choose a reason for hiding this comment

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

isValid only updates after a validation trigger; without specifying mode:"onChange" or manually calling trigger, the button will remain disabled until the user submits once, leading to confusing UX.

Prompt for AI agents
Address the following comment on frontend/app/(dashboard)/equity/grants/NewEquityGrantModal.tsx at line 110:

<comment>isValid only updates after a validation trigger; without specifying mode:&quot;onChange&quot; or manually calling trigger, the button will remain disabled until the user submits once, leading to confusing UX.</comment>

<file context>
@@ -0,0 +1,606 @@
+&quot;use client&quot;;
+
+import { zodResolver } from &quot;@hookform/resolvers/zod&quot;;
+import { CalendarDate, getLocalTimeZone, today } from &quot;@internationalized/date&quot;;
+import { ChevronDown, ChevronRight } from &quot;lucide-react&quot;;
+import { useEffect, useState } from &quot;react&quot;;
+import { useForm } from &quot;react-hook-form&quot;;
+import { z } from &quot;zod&quot;;
+import TemplateSelector from &quot;@/app/(dashboard)/document_templates/TemplateSelector&quot;;
</file context>

</DialogContent>
</Dialog>

<NewEquityGrantModal open={showNewGrantModal} onOpenChange={setShowNewGrantModal} />
Copy link

Choose a reason for hiding this comment

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

NewEquityGrantModal is rendered unconditionally, so its heavy data-fetching hooks run even when the modal is closed, causing an unnecessary network request and wasted work (Based on your team's feedback about avoiding unnecessary queries on initial render).

Prompt for AI agents
Address the following comment on frontend/app/(dashboard)/equity/grants/page.tsx at line 169:

<comment>NewEquityGrantModal is rendered unconditionally, so its heavy data-fetching hooks run even when the modal is closed, causing an unnecessary network request and wasted work (Based on your team&#39;s feedback about avoiding unnecessary queries on initial render).</comment>

<file context>
@@ -182,6 +165,8 @@ export default function GrantsPage() {
           ) : null}
         &lt;/DialogContent&gt;
       &lt;/Dialog&gt;
+
+      &lt;NewEquityGrantModal open={showNewGrantModal} onOpenChange={setShowNewGrantModal} /&gt;
     &lt;/&gt;
   );
</file context>
Suggested change
<NewEquityGrantModal open={showNewGrantModal} onOpenChange={setShowNewGrantModal} />
{showNewGrantModal && (
<NewEquityGrantModal open={showNewGrantModal} onOpenChange={setShowNewGrantModal} />
)}

{user.roles.worker ? (
!hasLegalDetails ? (
<Alert>
<Info />
Copy link

Choose a reason for hiding this comment

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

Icon size is missing; omit of "className="size-5"" makes this Lucide icon larger than the rest of the alert icons, breaking visual consistency.

Prompt for AI agents
Address the following comment on frontend/app/(dashboard)/invoices/page.tsx at line 326:

<comment>Icon size is missing; omit of &quot;className=\&quot;size-5\&quot;&quot; makes this Lucide icon larger than the rest of the alert icons, breaking visual consistency.</comment>

<file context>
@@ -347,11 +320,45 @@ export default function InvoicesPage() {
       /&gt;
 
       &lt;div className=&quot;grid gap-4&quot;&gt;
-        {workerNotice ? (
-          &lt;Alert&gt;
-            &lt;Info className=&quot;size-5&quot; /&gt;
-            &lt;AlertDescription&gt;{workerNotice}&lt;/AlertDescription&gt;
-          &lt;/Alert&gt;
+        {user.roles.worker ? (
</file context>
Suggested change
<Info />
<Info className="size-5" />

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.

1 participant

Comments