-
Notifications
You must be signed in to change notification settings - Fork 0
feat(permissions): add email-to-username lookup for users #115
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
Conversation
Enable adding users to projects via email address in addition to username. When an email is provided, the system automatically lookups the username via NATS request/reply pattern and adds the user to the project. Backend changes: - Add EMAIL_TO_USERNAME NATS subject enum - Implement resolveEmailToUsername() in ProjectService with NATS integration - Enhance addUserToProjectPermissions() to detect and resolve emails - Add EmailToUsernameErrorResponse interface for type-safe errors Frontend changes: - Update user form to accept "Username or Email" - Enhance error handling for user-friendly 404 messages - Update labels and placeholders to indicate email support Technical details: - Email detection via @ symbol check - Email normalization (trim, lowercase) - Graceful 404 handling when email not found - Backward compatible with existing username flow - Uses req.log for proper request context Generated with [Claude Code](https://claude.ai/code) JIRA: LFXV2-640 Signed-off-by: Asitha de Silva <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUI labels changed to accept "Username or Email". Client-side 404 handling now customizes messages for email vs username. Server controller detects email input and resolves it to a username via a new ProjectService.resolveEmailToUsername (NATS). Shared NATS subject and an error interface were added. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend (User Form)
participant C as ProjectController
participant S as ProjectService
participant N as NATS
participant A as Auth Service
UI->>C: Add user to project (usernameOrEmail)
alt Input contains "@"
C->>S: resolveEmailToUsername(email)
S->>N: Request EMAIL_TO_USERNAME (email)
N->>A: Deliver request
A-->>N: Reply (username or {success:false,error})
N-->>S: Response
alt Username resolved
S-->>C: username
C->>C: Use resolved username
else Not found / timeout
S-->>C: Error (not found)
C-->>UI: 404 Not Found
end
else Username provided
C->>C: Use provided username
end
opt If username available
C->>C: updateProjectPermissions(username)
C-->>UI: Success
end
note over UI,C: UI shows specific "not found" message for emails on 404
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the ability to add users to projects using email addresses in addition to usernames, with automatic email-to-username resolution via NATS messaging.
- Added NATS-based email-to-username lookup functionality
- Enhanced user form to accept both usernames and email addresses
- Improved error handling with user-friendly messages for email not found scenarios
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/auth.interface.ts | Added EmailToUsernameErrorResponse interface for type-safe NATS error handling |
| packages/shared/src/enums/nats.enum.ts | Added EMAIL_TO_USERNAME NATS subject enum |
| apps/lfx-one/src/server/services/project.service.ts | Implemented resolveEmailToUsername method with NATS integration and error handling |
| apps/lfx-one/src/server/controllers/project.controller.ts | Added email detection logic and username resolution in addUserToProjectPermissions |
| apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts | Enhanced error handling for email not found scenarios |
| apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.html | Updated labels and placeholders to indicate email support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts (1)
95-95: Type the error parameter for better type safety.The error parameter is currently typed as
any, which loses TypeScript's type-checking benefits. Consider usingHttpErrorResponsefrom@angular/common/httpor defining a specific interface for the expected error structure.Apply this diff to improve type safety:
+import { HttpErrorResponse } from '@angular/common/http';- error: (error: any) => { + error: (error: HttpErrorResponse) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.html(1 hunks)apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts(1 hunks)apps/lfx-one/src/server/controllers/project.controller.ts(1 hunks)apps/lfx-one/src/server/services/project.service.ts(2 hunks)packages/shared/src/enums/nats.enum.ts(1 hunks)packages/shared/src/interfaces/auth.interface.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.tspackages/shared/src/interfaces/auth.interface.tspackages/shared/src/enums/nats.enum.tsapps/lfx-one/src/server/services/project.service.tsapps/lfx-one/src/server/controllers/project.controller.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.tsapps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.htmlpackages/shared/src/interfaces/auth.interface.tspackages/shared/src/enums/nats.enum.tsapps/lfx-one/src/server/services/project.service.tsapps/lfx-one/src/server/controllers/project.controller.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.tspackages/shared/src/interfaces/auth.interface.tspackages/shared/src/enums/nats.enum.tsapps/lfx-one/src/server/services/project.service.tsapps/lfx-one/src/server/controllers/project.controller.ts
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.html
packages/shared/src/interfaces/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all TypeScript interfaces in the shared package at packages/shared/src/interfaces
Files:
packages/shared/src/interfaces/auth.interface.ts
packages/shared/src/enums/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place all enums in the shared package at packages/shared/src/enums
Files:
packages/shared/src/enums/nats.enum.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/server/services/project.service.ts (3)
packages/shared/src/constants/api.constants.ts (1)
NATS_CONFIG(28-43)packages/shared/src/interfaces/auth.interface.ts (1)
EmailToUsernameErrorResponse(169-174)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (6)
packages/shared/src/enums/nats.enum.ts (1)
10-10: Enum update looks good.Nice addition of the EMAIL_TO_USERNAME subject; aligns with the new lookup flow and keeps shared messaging definitions centralized.
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.html (1)
7-20: Copy update aligns with new capability.Label, placeholder, and validation text now correctly reflect the username-or-email flow while keeping existing test hooks intact. Looks solid.
packages/shared/src/interfaces/auth.interface.ts (1)
165-174: Typed error contract looks good.
EmailToUsernameErrorResponsecaptures the NATS lookup failure shape cleanly and keeps shared typing consistent.apps/lfx-one/src/server/controllers/project.controller.ts (1)
262-295: Controller wiring looks solid.Email detection, resolution, and logging integrate cleanly, and the downstream permission update now uses the resolved username. Nicely done.
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts (2)
98-109: Email detection is adequate for UI messaging purposes.The simple
includes('@')check on Line 101 is sufficient for differentiating error messages between email and username inputs. While it doesn't validate email format, that validation is appropriately handled server-side.The error handling correctly uses optional chaining and provides clear, actionable messages to users.
However, consider adding a defensive check for
formValue.usernameto guard against potential null/undefined values:const usernameValue = formValue.username || '';While the form validator requires username, this defensive practice prevents potential runtime errors if validation logic changes in the future.
110-119: LGTM! Proper error handling with state management.The fallback error handling appropriately covers all non-404 error cases with optional chaining and sensible default messages. The
submittingstate is correctly reset (Line 118) to allow form retry, while the success path appropriately closes the dialog without needing to reset state.
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully. Test Configuration
|
Parse NATS response once and branch based on result shape to properly
handle JSON success responses ({"username":"jdoe"}), JSON strings ("jdoe"),
and plain text (jdoe). Prevents storing malformed usernames with quotes
or braces in project settings.
LFXV2-640
Signed-off-by: Asitha de Silva <[email protected]>
Signed-off-by: Asitha de Silva <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/lfx-one/src/server/services/project.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/server/services/project.service.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/server/services/project.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/server/services/project.service.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/server/services/project.service.ts (2)
packages/shared/src/constants/api.constants.ts (1)
NATS_CONFIG(28-43)apps/lfx-one/src/server/errors/index.ts (1)
ResourceNotFoundError(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Playwright E2E Tests
| // Extract username from JSON success response or JSON string | ||
| username = typeof parsed === 'string' ? parsed : parsed.username; | ||
| } catch (parseError) { | ||
| // Re-throw ResourceNotFoundError as-is | ||
| if (parseError instanceof ResourceNotFoundError) { | ||
| throw parseError; | ||
| } | ||
|
|
||
| // JSON parsing failed - use raw text as username | ||
| username = responseText; | ||
| } | ||
|
|
||
| // Trim and validate username | ||
| username = username.trim(); | ||
|
|
||
| if (!username || username === '') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against non-string usernames before trimming.
When parsed is an object but does not expose a string username field (e.g., { success: true } or nested payloads), parsed.username becomes undefined. The subsequent username.trim() call throws a runtime TypeError, aborting the flow instead of returning a clean ResourceNotFoundError. Please gate the value before trimming and fall back gracefully.
- username = typeof parsed === 'string' ? parsed : parsed.username;
+ if (typeof parsed === 'string') {
+ username = parsed;
+ } else if (parsed && typeof parsed === 'object' && 'username' in parsed && typeof (parsed as { username: unknown }).username === 'string') {
+ username = (parsed as { username: string }).username;
+ }
} catch (parseError) {
// Re-throw ResourceNotFoundError as-is
if (parseError instanceof ResourceNotFoundError) {
throw parseError;
}
// JSON parsing failed - use raw text as username
username = responseText;
}
- // Trim and validate username
- username = username.trim();
+ // Trim and validate username
+ if (typeof username !== 'string') {
+ req.log.info({ email: normalizedEmail }, 'Empty username returned from NATS');
+ throw new ResourceNotFoundError('User', normalizedEmail, {
+ operation: 'resolve_email_to_username',
+ service: 'project_service',
+ path: '/nats/email-to-username',
+ });
+ }
+
+ username = username.trim();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Extract username from JSON success response or JSON string | |
| username = typeof parsed === 'string' ? parsed : parsed.username; | |
| } catch (parseError) { | |
| // Re-throw ResourceNotFoundError as-is | |
| if (parseError instanceof ResourceNotFoundError) { | |
| throw parseError; | |
| } | |
| // JSON parsing failed - use raw text as username | |
| username = responseText; | |
| } | |
| // Trim and validate username | |
| username = username.trim(); | |
| if (!username || username === '') { | |
| // Extract username from JSON success response or JSON string | |
| if (typeof parsed === 'string') { | |
| username = parsed; | |
| } else if ( | |
| parsed && | |
| typeof parsed === 'object' && | |
| 'username' in parsed && | |
| typeof (parsed as { username: unknown }).username === 'string' | |
| ) { | |
| username = (parsed as { username: string }).username; | |
| } | |
| } catch (parseError) { | |
| // Re-throw ResourceNotFoundError as-is | |
| if (parseError instanceof ResourceNotFoundError) { | |
| throw parseError; | |
| } | |
| // JSON parsing failed - use raw text as username | |
| username = responseText; | |
| } | |
| // Trim and validate username | |
| if (typeof username !== 'string') { | |
| req.log.info({ email: normalizedEmail }, 'Empty username returned from NATS'); | |
| throw new ResourceNotFoundError('User', normalizedEmail, { | |
| operation: 'resolve_email_to_username', | |
| service: 'project_service', | |
| path: '/nats/email-to-username', | |
| }); | |
| } | |
| username = username.trim(); | |
| if (!username || username === '') { |
🤖 Prompt for AI Agents
In apps/lfx-one/src/server/services/project.service.ts around lines 246 to 261,
the code assumes parsed.username is a string and calls username.trim(), which
will throw if parsed.username is undefined or non-string; instead, guard and
coerce before trimming: check if parsed is a string and use it, otherwise if
parsed.username is a string use that, else fall back to responseText (or an
empty string), then call trim() on the confirmed string and proceed to the empty
check/ResourceNotFoundError path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts (1)
96-117: Consider normalizing the username value before email detection.The email detection logic checks for
@to determine if the input is an email. According to the PR objectives, the backend normalizes email input (trim, lowercase) before lookup. For consistency and to improve the accuracy of the email detection, consider normalizing theusernameValuebefore checking.Apply this diff to normalize the username value:
error: (error: HttpErrorResponse) => { console.error('Error saving user:', error); // Check if it's a 404 error for email not found if (error.status === 404 && error.error?.code === 'NOT_FOUND') { - const usernameValue = formValue.username; + const usernameValue = formValue.username?.trim().toLowerCase() || ''; const isEmail = usernameValue.includes('@'); this.messageService.add({ severity: 'error', summary: 'User Not Found', detail: isEmail ? `No user found with email address "${usernameValue}". Please verify the email address and try again.` : error.error?.message || 'User not found', }); } else { this.messageService.add({ severity: 'error', summary: 'Error', detail: error.error?.message || `Failed to ${this.isEditing() ? 'update' : 'add'} user. Please try again.`, }); } this.submitting.set(false); },Note: If you normalize here for display purposes, you may want to preserve the original case for the error message. In that case, normalize only for the
isEmailcheck:error: (error: HttpErrorResponse) => { console.error('Error saving user:', error); // Check if it's a 404 error for email not found if (error.status === 404 && error.error?.code === 'NOT_FOUND') { const usernameValue = formValue.username; - const isEmail = usernameValue.includes('@'); + const isEmail = usernameValue?.trim().toLowerCase().includes('@') || false; this.messageService.add({ severity: 'error', summary: 'User Not Found', detail: isEmail ? `No user found with email address "${usernameValue}". Please verify the email address and try again.` : error.error?.message || 'User not found', }); } else { this.messageService.add({ severity: 'error', summary: 'Error', detail: error.error?.message || `Failed to ${this.isEditing() ? 'update' : 'add'} user. Please try again.`, }); } this.submitting.set(false); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces
Files:
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts
🔇 Additional comments (1)
apps/lfx-one/src/app/modules/project/settings/components/user-form/user-form.component.ts (1)
4-4: LGTM!The import of
HttpErrorResponseis correctly added to support enhanced error handling.
Summary
Enable adding users to projects via email address in addition to username. When an email is provided, the system automatically lookups the username via NATS request/reply pattern and adds the user to the project.
Backend Changes
EMAIL_TO_USERNAMENATS subject enumresolveEmailToUsername()in ProjectService with NATS integrationaddUserToProjectPermissions()to detect and resolve emailsEmailToUsernameErrorResponseinterface for type-safe errorsFrontend Changes
Technical Details
@symbol checkreq.logfor proper request context trackingFiles Modified
packages/shared/src/enums/nats.enum.ts- Added EMAIL_TO_USERNAME subjectpackages/shared/src/interfaces/auth.interface.ts- Added error response interfaceapps/lfx-one/src/server/services/project.service.ts- Email resolution logicapps/lfx-one/src/server/controllers/project.controller.ts- Email detectionapps/lfx-one/src/app/modules/project/settings/components/user-form/*- UI updatesTesting
🤖 Generated with Claude Code
JIRA: LFXV2-640