Skip to content

Davidgu/email sender and domain table - #85

Open
shaply wants to merge 8 commits into
mainfrom
davidgu/EmailSenderAndDomainTable
Open

Davidgu/email sender and domain table#85
shaply wants to merge 8 commits into
mainfrom
davidgu/EmailSenderAndDomainTable

Conversation

@shaply

@shaply shaply commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

No description provided.

shaply and others added 4 commits April 3, 2026 21:22
* added proper routing for user types, read only mode for users, error card on login if users have no projects, and server side admin enforcement for project actions

* linting

* no magic numbers, restricted project routing, read only mode default true
* Available user types filtered based on user role

* Forgot to do linting

* fix: format

---------

Co-authored-by: Long Lam <llam36@gatech.edu>
Co-authored-by: Long Lam <lamhoanglong.3001@gmail.com>
@shaply
shaply requested a review from llam36 April 6, 2026 03:43
@netlify

netlify Bot commented Apr 6, 2026

Copy link
Copy Markdown

Deploy Preview for juno-dashboard failed. Why did it fail? →

Name Link
🔨 Latest commit 64a7c60
🔍 Latest deploy log https://app.netlify.com/projects/juno-dashboard/deploys/69d6f485cd7251000827ebe0

@greptile-apps

greptile-apps Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds email sender and domain management tables, matching forms, new server-action utilities, and a new configurations page under the email service route. The table/form components (EmailDomainTable, EmailSenderTable, AddEmailDomainForm, AddEmailSenderForm) follow existing React Query patterns correctly.

The main blocking issue is that registerJunoDomain and registerJunoSenderAddress in sdkUtils.ts do not forward the user JWT to the Juno SDK — unlike every other call in the file — so domain and sender registration will fail with authentication errors in production. Additionally, a toast.error is called directly in the EmailConfigurationsPage render body rather than inside a useEffect, causing repeated notifications on every re-render when an error is present.

Confidence Score: 4/5

Not safe to merge — missing JWT in new SDK mutation calls will cause production auth failures

Two clear P1 defects on the new code path: SDK calls without authentication in sdkUtils.ts and a toast fired in the render body. The table and form components themselves are well-structured and follow existing patterns correctly.

src/lib/sdkUtils.ts (missing credentials in both new functions), src/app/(dashboard)/projects/[projectId]/services/email/configurations/page.tsx (toast.error in render body)

Important Files Changed

Filename Overview
src/lib/sdkUtils.ts New domain/sender registration functions missing user JWT credentials in SDK calls
src/app/(dashboard)/projects/[projectId]/services/email/configurations/page.tsx toast.error called in render body; uses manual useState/useEffect instead of React Query for email config check
src/lib/settings.ts New getEmailSenders/getEmailDomains server actions; contains console.error calls to remove
src/components/sidebar/project-sidebar.tsx Adds email collapsible submenu correctly; has a redundant nested async function
src/components/emailDomainTable/emailDomainTable.tsx New domain table with correct React Query mutation and cache invalidation
src/components/emailSenderTable/emailSenderTable.tsx New sender table with correct React Query mutation and cache invalidation
src/components/forms/AddEmailDomainForm.tsx Clean zod-validated form for domain registration
src/components/forms/AddEmailSenderForm.tsx Well-structured zod-validated form with email format enforcement
src/components/baseTable.tsx Minor read-only mode integration; logic is correct

Sequence Diagram

sequenceDiagram
    participant UI as EmailConfigurationsPage
    participant ST as EmailSenderTable/DomainTable
    participant SA as settings.ts
    participant SU as sdkUtils.ts
    participant SDK as Juno SDK

    UI->>SA: getEmailConfig(projectId)
    SA->>SDK: email.getEmailConfig(projectId, {userJwt, projectId})
    SDK-->>SA: EmailConfigResponse
    SA-->>UI: config or null

    ST->>SA: getEmailSenders / getEmailDomains
    SA->>SDK: email.getSenders/getDomains({userJwt, projectId})
    SDK-->>SA: senders / domains
    SA-->>ST: {success, data}

    ST->>SU: registerJunoDomain / registerJunoSenderAddress
    Note over SU,SDK: ❌ Missing credentials (userJwt)
    SU->>SDK: email.registerDomain({domain, subdomain})
    SDK-->>SU: auth error or result
    SU-->>ST: {success, message}
Loading

Comments Outside Diff (3)

  1. src/lib/sdkUtils.ts, line 62-70 (link)

    P1 Missing credentials in SDK call

    registerJunoDomain calls juno.email.registerDomain({ domain, subdomain }) without passing credentials. Every other SDK call in this file — e.g. setupJunoEmail (line 29) — passes { userJwt: session.jwt, projectId: Number(projectId) } as a second argument. The session is fetched and the admin check runs, but the JWT is never forwarded, so this call will run unauthenticated and fail in production. Additionally, the function signature doesn't accept a projectId parameter at all, so even the project scope is missing.

  2. src/lib/sdkUtils.ts, line 97-109 (link)

    P1 Missing credentials in SDK call

    registerJunoSenderAddress calls juno.email.registerSenderAddress(...) without a credentials object. The session variable is retrieved and the admin check runs, but the JWT is discarded — mirroring the same issue as registerJunoDomain above. Both new mutation functions need to forward { userJwt: session.jwt } to the SDK.

  3. src/components/sidebar/project-sidebar.tsx, line 64-78 (link)

    P2 Redundant nested async function

    getAllProjects is defined inside fetchProjects (line 66) but is only called once in the else branch (line 100). Its body can be inlined directly, eliminating an extra layer of indirection and the duplicate getProjects() call pattern.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "Prettier" | Re-trigger Greptile

Comment thread src/app/(dashboard)/projects/[projectId]/services/email/configurations/page.tsx Outdated
Comment thread src/lib/settings.ts
shaply and others added 3 commits April 7, 2026 22:52
…urations/page.tsx

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@kworathur
kworathur self-requested a review April 12, 2026 18:16

@kworathur kworathur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

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.

4 participants