Improve beta request emails#1953
Conversation
📝 WalkthroughWalkthroughBackend adjusts JoinFwLiteBetaEmail recipient logic to fall back to username when email is missing. Frontend updates approval URL construction by pre-encoding the email and adding memberSearch alongside userSearch, removing the extra slash before query and reusing the encoded value. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
UI unit Tests 1 files ± 0 3 suites - 37 0s ⏱️ -23s Results for commit 83871e5. ± Comparison against base commit c6b5cc8. This pull request removes 82 and adds 10 tests. Note that renamed tests count towards both. |
There was a problem hiding this comment.
Pull Request Overview
This PR improves the beta request email functionality by adding a memberSearch parameter to admin URLs and providing a fallback for users without email addresses.
- Adds memberSearch parameter to admin URLs for more convenient user lookup
- Provides username fallback when email is not available for beta request emails
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/lib/email/JoinFwLiteBetaRequest.svelte | Adds memberSearch parameter to admin URL and extracts encoded email to a variable |
| backend/LexBoxApi/Services/EmailService.cs | Adds username fallback when user email is null in beta request emails |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
backend/LexBoxApi/Services/EmailService.cs (2)
228-228: Fallback to username is reasonable, but tighten semantics and whitespace handlingThe change aligns with the PR goal of making approvals searchable even when a user lacks an email. Two nits:
- Trim the chosen identifier to avoid accidental leading/trailing whitespace.
- Use a local variable to clarify intent (“identifier” rather than “email”), since this value may be a username.
Apply:
- await RenderEmail(email, new JoinFwLiteBetaEmail(user.Name, user.Email ?? user.Username ?? ""), user.LocalizationCode); + var identifier = string.IsNullOrWhiteSpace(user.Email) ? (user.Username ?? "") : user.Email; + await RenderEmail(email, new JoinFwLiteBetaEmail(user.Name, identifier.Trim()), user.LocalizationCode);Also confirm the email template’s body copy doesn’t literally label this field as an email when it might be a username. If it does, consider updating the wording to “email/username” in translations.
227-227: Keep the support address hardcoded; remove the TODO to prevent driftPer prior decision (learned from PR #1650), this address should remain hardcoded. To avoid future churn, replace the TODO with an explanatory comment.
Using retrieved learnings: The support address was intentionally hardcoded; keeping it that way is desired.
- var email = StartUserEmail("Lexbox Support", "lexbox_support@groups.sil.org"); // TODO: Get from environment + var email = StartUserEmail("Lexbox Support", "lexbox_support@groups.sil.org"); // Intentionally hardcoded per team decision (see PR #1650)frontend/src/lib/email/JoinFwLiteBetaRequest.svelte (1)
8-9: Use URLSearchParams to build the admin-link
Switching to URLSearchParams makes the intent clearer and offloads encoding to the API.Locations:
- frontend/src/lib/email/JoinFwLiteBetaRequest.svelte (around lines 8–9)
Suggested change:
- const encodedEmail = encodeURIComponent(email); - let approveUrl = new URL(`/admin?userSearch=${encodedEmail}&memberSearch=${encodedEmail}`, baseUrl); + // Build query params declaratively and let URLSearchParams handle encoding + const params = new URLSearchParams({ + userSearch: email, + memberSearch: email + }); + const approveUrl = new URL(`/admin?${params}`, baseUrl);Note: In this component
memberSearch. In the admin UI:
userSearchdrives a “contains” filter over name, email, and username.memberSearchapplies an exact-match filter on project membership by email or username.
This refactor preserves the existing behavior and improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backend/LexBoxApi/Services/EmailService.cs(1 hunks)frontend/src/lib/email/JoinFwLiteBetaRequest.svelte(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-13T09:48:47.276Z
Learnt from: rmunn
PR: sillsdev/languageforge-lexbox#1650
File: backend/LexBoxApi/Services/EmailService.cs:0-0
Timestamp: 2025-05-13T09:48:47.276Z
Learning: The email address "lexbox_supportgroups.sil.org" in the SendJoinFWLiteBetaEmail method should remain hardcoded as per decision in meetings with Tim, despite the TODO comment.
Applied to files:
backend/LexBoxApi/Services/EmailService.cs
📚 Learning: 2025-05-13T09:48:02.112Z
Learnt from: rmunn
PR: sillsdev/languageforge-lexbox#1650
File: backend/LexBoxApi/Services/Email/IEmailService.cs:0-0
Timestamp: 2025-05-13T09:48:02.112Z
Learning: XML documentation for the `SendJoinFWLiteBetaEmail` method in `IEmailService` interface is not needed according to the project owner.
Applied to files:
backend/LexBoxApi/Services/EmailService.cs
⏰ 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). (4)
- GitHub Check: Build UI / publish-ui
- GitHub Check: Build API / publish-api
- GitHub Check: check-and-lint
- GitHub Check: Analyze (csharp)
Adding
memberSearch=${encodedEmail}will be quite convenient. I basically always do that manually atm.I think there's only been one request from a user with no email address, so that's not as critical, but it's still an improvement.