#142 if user is verified after registration put the changes to the database#162
Conversation
📝 Walkthrough## Walkthrough
This PR replaces the Resend email service with nodemailer across the application. The API endpoint and its tests have been updated to use a nodemailer transport and mock, respectively, for sending emails. The package dependencies have been adjusted accordingly. In the frontend, the EmailVerification and RegisterForm components now pass the user's email along with the verification code, integrating GraphQL queries and mutations to handle user verification. Additionally, a GraphQL constant has been renamed, and the Keystone user model’s access control for the verification field has been broadened. The login flow is enhanced to include email verification for unverified users, with toast notifications added for user feedback.
## Changes
| File(s) | Change Summary |
|---------|----------------|
| `veterans/app/api/sendEmail/route.ts`, `veterans/app/api/sendEmail/route.test.ts`, `veterans/package.json` | Replaced Resend with nodemailer: updated email transport instantiation and email content in the API, integrated a nodemailer mock in tests, and updated dependencies by removing `resend` and adding `nodemailer` with its types. Added `react-hot-toast` dependency. |
| `veterans/components/common/authForm/EmailVerification/EmailVerification.tsx`, `veterans/components/common/authForm/RegisterForm.tsx` | Enhanced verification: added an email reference/prop to pass user email, integrated Apollo Client GraphQL queries and mutations for user verification, updated error handling, and added redirection logic. |
| `veterans/constants/graphql.ts`, `veterans/keystone/models/User.ts` | Updated backend logic: renamed mutation constant from `UPDATE_USER_VERIFICATION` to `VERIFICATION_MUTATION`, added `isVerified` field to `LOGIN_MUTATION`, and changed `isVerified` field access control to allow both admins and users to update it. |
| `veterans/app/layout.tsx` | Added `Toaster` component from `react-hot-toast` for toast notifications in the layout. |
| `veterans/components/common/authForm/LoginForm.tsx` | Introduced new login flow with verification step for unverified users, including state management for verification code and step tracking, integration of email sending helper, and conditional rendering of verification component. |
| `veterans/helpers/handleSendEmail.ts` | Replaced alert notification with toast notification for email verification success feedback. |
| `veterans/components/common/authForm/LoginForm.test.tsx`, `veterans/components/common/authForm/EmailVerification/EmailVerification.test.tsx` | Updated tests: restructured mocks in `LoginForm` tests, added `isVerified` field in mocks, added new test suite for `EmailVerification` covering various scenarios including error handling and user interactions. |
| `veterans/components/common/authForm/RegisterForm.test.tsx` | Extended tests to include login error handling and network error scenarios; updated placeholder text for verification code input. |
| `veterans/auth.ts` | Updated session cookie settings to use `sameSite` and `secure` attributes conditionally based on environment (production vs non-production). |
## Possibly related PRs
- **boarlabsxyz/razom#147**: Involves similar changes in email sending functionality by transitioning from Resend to nodemailer and updating tests to simulate email delivery.
- **boarlabsxyz/razom#155**: Related to the introduction and handling of the `isVerified` field in user verification flows, including GraphQL mutations and access control.
- **boarlabsxyz/razom#90**: Introduces the `isAdminOrSameUser` access control function used here to broaden update permissions on the `isVerified` field.
## Suggested reviewers
- yuramax
- VitalyyP
- Anton-88Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
veterans/app/api/sendEmail/route.ts (1)
23-28: Improve the email sender display name.Currently, the
fromfield uses just the email address from environment variables. For better user experience, consider adding a friendly display name.- from: process.env.GMAIL_USER, + from: `Razom Veterans <${process.env.GMAIL_USER}>`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
veterans/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
veterans/app/api/sendEmail/route.test.ts(2 hunks)veterans/app/api/sendEmail/route.ts(2 hunks)veterans/helpers/handleSendEmail.ts(1 hunks)veterans/package.json(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
veterans/app/api/sendEmail/route.test.ts (1)
veterans/app/api/sendEmail/route.ts (1)
POST(4-45)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: e2e-tests
- GitHub Check: unit-tests
🔇 Additional comments (8)
veterans/package.json (2)
40-40: Dependencies updated appropriately.The addition of the
nodemailerpackage aligns with the PR objective of replacing Resend with a Gmail-based email solution. The version chosen (^6.10.0) is appropriate.
64-64: Type definitions properly added.Adding
@types/nodemailerto devDependencies is the correct approach for TypeScript support. The version chosen (^6.4.17) matches the nodemailer package requirement.veterans/app/api/sendEmail/route.test.ts (4)
1-5: Nodemailer mock implementation looks good.The mock appropriately simulates the nodemailer transport and sendMail method, returning a test message ID that can be verified in the tests.
7-16: NextResponse mock correctly updated.The mock now returns a proper Response object with the correct status and content type headers, which better simulates the actual Next.js response behavior.
49-50: Test assertion properly checks message ID.The test correctly verifies that the response includes the message ID from the successful email send operation.
53-71: Error handling test looks good.The test properly overrides the sendMail method to simulate an error condition and verifies the correct error response is returned.
veterans/app/api/sendEmail/route.ts (2)
2-2: Nodemailer import looks good.The import correctly brings in the nodemailer package that was added to the dependencies.
30-30: Email sending implementation is correct.The implementation correctly awaits the result of
transporter.sendMail()and passes it in the response, which is the expected pattern when using nodemailer.
…ut-the-changes-to-the-database
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
veterans/app/api/sendEmail/route.ts(2 hunks)veterans/components/common/authForm/EmailVerification/EmailVerification.tsx(1 hunks)veterans/components/common/authForm/RegisterForm.tsx(3 hunks)veterans/constants/graphql.ts(1 hunks)veterans/keystone/models/User.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- veterans/app/api/sendEmail/route.ts
🧰 Additional context used
🧬 Code Definitions (2)
veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (1)
veterans/constants/graphql.ts (2)
VERIFICATION_MUTATION(83-90)CHECK_USER_QUERY(73-81)
veterans/components/common/authForm/RegisterForm.tsx (1)
veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (1)
EmailVerification(12-74)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: sonarqube
- GitHub Check: e2e-tests
🔇 Additional comments (8)
veterans/constants/graphql.ts (1)
83-90: LGTM! The constant has been renamed to better reflect its purpose.The renaming of the GraphQL mutation constant from
UPDATE_USER_VERIFICATIONtoVERIFICATION_MUTATIONimproves code readability by better indicating its purpose in the verification flow. The implementation is consistent with other GraphQL queries and mutations in the file.veterans/components/common/authForm/RegisterForm.tsx (3)
40-40: LGTM! The email reference is correctly initialized.The
emailRefprovides a way to persist the email value across component renderings, which is necessary for the verification flow.
79-79: LGTM! Email is correctly stored for verification.The user's email is properly stored in the ref after successful registration, making it available for the verification process.
205-208: LGTM! Email is correctly passed to the EmailVerification component.The EmailVerification component now receives both the verification code and the email address, enabling it to perform the verification process correctly.
veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (4)
2-5: LGTM! Required imports for GraphQL functionality.The imports for Apollo Client's
useMutationanduseQueryhooks, along with the GraphQL constants, are correctly added to support the new verification functionality.
9-9: LGTM! Interface properly updated to include email property.The
EmailVerificationPropsinterface has been correctly updated to include the
14-14: LGTM! Component correctly accepts the email prop.The component function signature has been updated to destructure and use the
17-21: LGTM! GraphQL hooks correctly implemented.The implementation of the GraphQL hooks to query user data and update verification status is done correctly.
…ut-the-changes-to-the-database
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (1)
30-30:⚠️ Potential issueNull-check on
userData.user
In caseuserDataoruserData.userisnull, accessing.idhere can throw an error. Please add a null/undefined check before usinguserData.user.id.Here’s a quick fix suggestion:
- const userId = userData.user.id; + if (!userData?.user?.id) { + toast.error('Cannot find user data. Please refresh and try again.'); + return; + } + const userId = userData.user.id;
🧹 Nitpick comments (4)
veterans/app/layout.tsx (1)
55-55: Positioning the Toaster
Placing theToasterattop-centeris a straightforward way to ensure any notifications appear prominently. If you plan to customize the style or behavior of these toasts, consider passing additional props to match your design specs. Otherwise, this is fine as-is.veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (3)
18-18: Use ofuseMutation
Storing the mutation function in a constant is clean and follows best practices. Make sure to handle potential errors or loading states if you plan to display UI feedback during the request.
20-22: Fetching user data
Querying the user by
32-39: Performing the mutation
Good structure withupdateUserVerificationto setisVerified: true. You may wish to handle partial failures more explicitly (e.g., network or server errors) and display specific feedback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
veterans/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
veterans/app/layout.tsx(2 hunks)veterans/components/common/authForm/EmailVerification/EmailVerification.tsx(1 hunks)veterans/components/common/authForm/LoginForm.tsx(5 hunks)veterans/components/common/authForm/RegisterForm.tsx(5 hunks)veterans/constants/graphql.ts(2 hunks)veterans/helpers/handleSendEmail.ts(2 hunks)veterans/keystone/models/User.ts(1 hunks)veterans/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- veterans/package.json
- veterans/constants/graphql.ts
- veterans/keystone/models/User.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (1)
veterans/constants/graphql.ts (2)
VERIFICATION_MUTATION(84-91)CHECK_USER_QUERY(74-82)
veterans/components/common/authForm/RegisterForm.tsx (3)
veterans/constants/graphql.ts (2)
LOGIN_MUTATION(26-43)CURRENT_USER_QUERY(3-14)veterans/helpers/handleSendEmail.ts (1)
handleSendEmail(4-29)veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (1)
EmailVerification(13-74)
veterans/components/common/authForm/LoginForm.tsx (2)
veterans/helpers/handleSendEmail.ts (1)
handleSendEmail(4-29)veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (1)
EmailVerification(13-74)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e-tests
🔇 Additional comments (22)
veterans/app/layout.tsx (1)
2-2: Use ofToasterimport
Thanks for introducingToasterfromreact-hot-toast; this setup looks good for supporting toast notifications across the app.veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (7)
2-3: Introduced toast notifications
Great usage ofreact-hot-toastfor non-blocking error handling and user feedback. This improves the overall user experience compared to persistent alerts.
6-6: Centralized GraphQL imports
Good practice to import bothVERIFICATION_MUTATIONandCHECK_USER_QUERYfrom a single constants file for maintainability.
10-10: New
AddingEmailVerificationPropsmakes the component more robust by using the passed-in email for queries and mutation variables.
15-15: Destructured
Well-integrated into the component signature to facilitate the verification flow.
24-28: Code mismatch check
If the user’s confirmed code doesn’t match the actual verification code, displaying an error toast and returning early is clear and user-friendly. No concerns here.
40-44: Conditional post-mutation logic
Immediately navigating the user upon successful verification is a sensible flow. The fallback toast for failure is also appropriate.
45-51: Exception handling with toast
Your use oftoast.error()here ensures users receive real-time feedback if an unexpected error occurs, which is a big UX improvement.veterans/helpers/handleSendEmail.ts (2)
2-2: Swapping alerts for toast
Importingtoastfromreact-hot-toastis consistent with your shift towards non-blocking feedback.
19-19: Success notification
Usingtoast.success()to confirm email dispatch is a clear improvement over alerts, ensuring the user stays in context.veterans/components/common/authForm/RegisterForm.tsx (7)
9-9: Import update to support user authentication flowThe
useApolloClienthook has been added which is necessary for writing the authenticated user data to the Apollo cache after registration.
12-16: Improved GraphQL imports organizationThe imports are properly organized to include the
CURRENT_USER_QUERYwhich is needed for updating the authentication state in the Apollo cache.
44-45: New refs added to manage user authenticationThese refs are essential additions:
emailRefstores the user's email for verification purposesclientprovides access to the Apollo client for cache updatesBoth are necessary for the verification flow implementation.
65-82: Well-implemented login mutation with proper error handlingThe login mutation implementation is robust with:
- Proper handling of authentication success by updating the Apollo cache
- Clear error handling to provide feedback to the user
- Type checking with the 'item' property to ensure type safety
This implementation ensures a smooth user experience during the registration-to-login flow.
103-103: User email storage for verificationStoring the email in
emailRef.currentafter successful registration is necessary for the verification process.
107-112: Auto-login after registrationAutomatically logging in the user after successful registration is a good UX improvement, eliminating the need for the user to manually log in after registration.
235-238: Updated EmailVerification component usageThe
veterans/components/common/authForm/LoginForm.tsx (5)
20-21: Added verification-related importsThese imports are necessary for implementing the email verification flow when unverified users attempt to log in.
35-35: Added getValues to access form dataAdding
getValuesto the destructured form hook values is necessary to retrieve the email value later in the code.
44-46: Added state for verification flowThese additions are well-structured:
verificationCodestate stores the code sent to the userstepReftracks whether the user is in login or verification modeemailRefretains the email for verification purposesAll of these are necessary for implementing the verification workflow.
131-153: Enhanced login flow with verification checkThis implementation properly:
- Checks if the user is verified after successful login
- Guides unverified users through the verification process
- Redirects verified users to the home page
The code correctly retrieves and stores the user's email, sends a verification code, and manages the UI flow based on verification status.
165-251: Implemented conditional rendering for login/verification stepsThis implementation provides a smooth user experience by:
- Showing the login form during the initial login step
- Switching to the verification component when needed
- Properly passing both verification code and email to the verification component
The structure is clean and maintains all the original login form functionality.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
veterans/components/common/authForm/LoginForm.test.tsx (1)
113-236: 🛠️ Refactor suggestionConsider adding tests for unverified user flow.
The test suite thoroughly covers successful login with verified users but lacks tests for the new unverified user flow. Since the component now handles a verification step for unverified users, additional tests should be added to ensure this path works correctly.
// Add this test after line 211 + it('should show verification form for unverified users', async () => { + const unverifiedUserMock = [{ + request: { + query: LOGIN_MUTATION, + variables: { + email: 'unverified@example.com', + password: 'Password123', + }, + }, + result: { + data: { + authenticateUserWithPassword: { + item: { + id: '2', + email: 'unverified@example.com', + role: 'USER', + name: 'Unverified User', + isVerified: false, + }, + }, + }, + }, + }]; + + customRender(<LoginForm />, unverifiedUserMock); + fireEvent.change(screen.getByPlaceholderText('Email'), { + target: { value: 'unverified@example.com' }, + }); + fireEvent.change(screen.getByPlaceholderText('Password'), { + target: { value: 'Password123' }, + }); + fireEvent.click(screen.getByRole('button', { name: /sign in/i })); + + await waitFor(() => { + expect(screen.getByTestId('email-verification')).toBeInTheDocument(); + }); + });
🧹 Nitpick comments (3)
veterans/components/common/authForm/LoginForm.tsx (3)
129-151: Implemented conditional verification flow.The implementation correctly checks if a user is verified and either redirects to the homepage or initiates the email verification process.
However, there are a few improvements that could be made to enhance the user experience:
- Add loading state during email sending
- Provide more informative error messages
- Add ability to retry sending verification email
try { const { data } = await login({ variables: formData }); const authResult = data?.authenticateUserWithPassword; if (authResult && 'item' in authResult) { const isVerified = authResult.item?.isVerified; const userEmail = getValues('email'); emailRef.current = userEmail; if (!isVerified) { stepRef.current = 'verify'; + setIsSubmitting(true); const { success, code } = await handleSendEmail(userEmail); if (success && code) { setVerificationCode(code); } else { - setError('Failed to send verification email'); + setError('Failed to send verification email. Please check your internet connection and try again.'); + stepRef.current = 'login'; } + setIsSubmitting(false); } else { router.push('/'); } }
163-249: Implemented conditional rendering based on verification step.The component now correctly renders either the login form or the verification component based on the current step. This structure works well, but some improvements could be made for user experience.
Consider adding a back button in the verification component to allow users to return to the login screen if needed:
<EmailVerification verificationCode={verificationCode} email={emailRef.current} + onBack={() => { + stepRef.current = 'login'; + setError(null); + setSubmitError(null); + // Force a re-render + setIsSubmitting(false); + }} />
31-159: Consider refactoring for separation of concerns.The LoginForm component is handling multiple responsibilities (login, OAuth, verification flow). Consider extracting some of this logic into separate components or custom hooks to improve maintainability.
For example, you could create:
- A custom hook for the login logic
- A custom hook for the OAuth flow
- Move the verification step into its own component
This would make the code more modular and easier to test.
// Example hook extraction function useLoginForm() { const [isSubmitting, setIsSubmitting] = useState(false); const [submitError, setSubmitError] = useState(null); const [error, setError] = useState(null); const [verificationCode, setVerificationCode] = useState(''); const stepRef = useRef('login'); const emailRef = useRef(''); // Login mutation and form handling logic return { isSubmitting, submitError, error, verificationCode, stepRef, emailRef, handleLogin, // other methods and state }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
veterans/components/common/authForm/LoginForm.test.tsx(1 hunks)veterans/components/common/authForm/LoginForm.tsx(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unit-tests
- GitHub Check: e2e-tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
veterans/components/common/authForm/LoginForm.test.tsx (1)
38-38: Update verified user mock for login test consistency.The added
isVerified: trueproperty in the login mock response aligns with the new verification functionality implemented in the LoginForm component. This ensures that tests for successful login and redirection work correctly with the updated GraphQL schema.veterans/components/common/authForm/LoginForm.tsx (2)
20-21: Import of new verification components.The component now correctly imports the required helper and component for the email verification flow.
44-46: Added state management for verification flow.These state variables and refs effectively manage the verification step and user email persistence.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
veterans/components/common/authForm/EmailVerification/EmailVerification.test.tsx (1)
1-133: Consider enhancing accessibility in your tests.While the current tests are comprehensive, consider using more accessibility-focused queries like
getByRoleorgetByLabelTextinstead ofgetByPlaceholderTextwhen possible, as they better represent how users (including those using assistive technologies) interact with your application.- screen.getByPlaceholderText('Email verification') + screen.getByRole('textbox', { name: /email verification/i })This would require adding an appropriate aria-label or label element in the component, which would improve both testability and accessibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
veterans/components/common/authForm/EmailVerification/EmailVerification.test.tsx(1 hunks)veterans/components/common/authForm/LoginForm.test.tsx(2 hunks)veterans/components/common/authForm/LoginForm.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- veterans/components/common/authForm/LoginForm.test.tsx
- veterans/components/common/authForm/LoginForm.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
veterans/components/common/authForm/EmailVerification/EmailVerification.test.tsx (1)
veterans/components/common/authForm/EmailVerification/EmailVerification.tsx (1)
EmailVerification(13-74)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: unit-tests
- GitHub Check: e2e-tests
🔇 Additional comments (7)
veterans/components/common/authForm/EmailVerification/EmailVerification.test.tsx (7)
1-14: Good test setup with proper imports and mock configurations.The imports and mock configurations are well-structured, covering all the necessary dependencies for thorough testing of the EmailVerification component.
16-33: Well-organized test suite setup.Great job setting up the test suite with proper mocks for router and Apollo Client hooks. The beforeEach cleanup ensures tests don't influence each other.
35-44: Component rendering test looks good.The rendering test appropriately verifies that key UI elements are present in the document.
46-60: Thorough validation of error handling for invalid codes.Good test case that ensures the component correctly identifies and reports invalid verification codes.
62-87: Comprehensive happy path testing.The test correctly verifies the successful email verification flow, including:
- Mutation being called with correct parameters
- User being redirected after successful verification
89-110: Good coverage of backend verification failure.This test properly handles the scenario where the backend returns a false verification status, ensuring the appropriate error message is displayed.
112-131: Effective testing of network error scenarios.The test correctly verifies error handling when a network error occurs during the mutation.
IhorMasechko
force-pushed
the
142-if-user-is-verified-after-registration-put-the-changes-to-the-database
branch
from
2b9516b to
4e3df4a
Compare
IhorMasechko
force-pushed
the
142-if-user-is-verified-after-registration-put-the-changes-to-the-database
branch
from
253c610 to
4e3df4a
Compare
…ut-the-changes-to-the-database
|
2 participants
Added Gmail messenger and set the flag to true after successful verification
Summary by CodeRabbit
New Features
nodemailer, enhancing the verification experience with updated messaging.Toastercomponent for displaying notifications in the app layout.Tests
EmailVerificationcomponent, ensuring comprehensive coverage of various scenarios including success and error handling.RegisterFormtests to simulate network errors during login.Chores
sameSite='none'andsecure=trueattributes.