feat: restrict email address display to owner and admins (Closes #840) #871
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Removed email display from issue sidebar and timeline. - Removed email from assignee picker and search filter. - Removed email from user menu header. - Removed email from public reporting form status message. - Added read-only email field to profile settings page. - Updated Drizzle queries to stop fetching emails where no longer needed. - Updated E2E and unit tests to reflect privacy changes.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR successfully restricts email address displays throughout the application to only the owner's profile page and admin's user management page, improving data privacy for all members and guests.
Changes:
- Removed email addresses from issue reporter/author displays in sidebars and timelines
- Removed email addresses from assignee picker dropdowns and search filters
- Removed email address from user menu profile dropdown
- Simplified reporting form status message from "Logged in as [email]" to "Logged in"
- Added read-only email field to profile settings page for users to view their own email
- Updated TypeScript types and Drizzle queries to stop fetching email data where not displayed
- Updated E2E and unit tests to reflect the new privacy behavior
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/types/issue.ts |
Removed email field from IssueCommentWithAuthor, IssueListItem, and IssueWithAllRelations types |
src/lib/machines/queries.ts |
Removed email fetching from getMachineOwner query and simplified return objects |
src/lib/issues/utils.ts |
Removed email field from IssueReporterInfo interface and resolveIssueReporter function |
src/lib/issues/utils.test.ts |
Updated unit tests to match removed email fields from resolved reporter objects |
src/components/layout/user-menu-client.tsx |
Removed email prop and email display from user menu header |
src/components/layout/MainLayout.tsx |
Removed email prop when rendering UserMenu component |
src/components/issues/SidebarActions.tsx |
Updated allUsers type to remove email field |
src/components/issues/IssueTimeline.tsx |
Removed email display from timeline author information |
src/components/issues/IssueSidebar.tsx |
Removed email display from sidebar reporter section and updated interface |
src/components/issues/IssueCard.tsx |
Updated type definitions to remove email from reporter user objects |
src/components/issues/AssigneePicker.tsx |
Removed email from picker interface, display, and search filter logic |
src/components/issues/AssigneePicker.test.tsx |
Updated mock data to remove email fields |
src/app/report/unified-report-form.tsx |
Changed logged-in status message from showing email to simple "Logged in" |
src/app/(app)/settings/profile-form.tsx |
Added read-only email field to profile form |
src/app/(app)/settings/page.tsx |
Passed email prop to ProfileForm component |
src/app/(app)/m/new/page.tsx |
Changed getUnifiedUsers call to use includeEmails: false |
src/app/(app)/m/[initials]/page.tsx |
Removed conditional email fetching from machine owner queries and changed to includeEmails: false |
src/app/(app)/m/[initials]/i/[issueNumber]/page.tsx |
Removed admin role check and conditional email fetching from all user-related queries |
src/app/(app)/m/[initials]/i/[issueNumber]/assign-issue-form.tsx |
Updated users prop type to remove email field |
src/app/(app)/issues/page.tsx |
Removed conditional email fetching from issue list query |
src/app/(app)/dashboard/page.tsx |
Removed isAdmin parameter and conditional email fetching from dashboard data query |
e2e/smoke/reporter-variations.spec.ts |
Updated test to verify admins also do NOT see emails in sidebars anymore |
| email: isAdmin | ||
| ? sql<string | null>`COALESCE(${authUsers.email}, null)` | ||
| : sql<null>`null`, | ||
| email: sql<null>`null`, |
There was a problem hiding this comment.
The email: sql<null>null`` field in this query is unnecessary. Since the components now expect only { id: string; name: string }[], the email field should be removed entirely from the select statement for cleaner code. The query can simply select only `id` and `name` fields without explicitly setting email to null.
- Remove email visibility assertion from invite-signup E2E test (user menu no longer displays email) - Remove unnecessary `email: sql<null>` field from assignee query in issue detail page - Addresses GitHub Copilot review feedback Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes email address displays from all public and member-facing parts of the application, strictly restricting them to the owner's profile page and the admin's user management page.
Key Changes:
Filed follow-up issue #870 for thorough E2E assertion cleanup.