Skip to content
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

Feat/issue 4476 #1779

Open
wants to merge 7 commits into
base: staging
Choose a base branch
from
Open

Feat/issue 4476 #1779

wants to merge 7 commits into from

Conversation

Reshzera
Copy link
Contributor

@Reshzera Reshzera commented Aug 23, 2024

Summary by CodeRabbit

  • New Features

    • Introduced email verification features, including sending confirmation codes and verifying email addresses.
    • Added new mutations to handle email confirmation workflows in user authentication.
    • New utility function for generating email verification codes.
  • Bug Fixes

    • Enhanced error messaging for email verification scenarios, providing clearer user feedback.
  • Documentation

    • Updated localization files to support new user-related messages in English and Spanish.
  • Tests

    • Expanded test coverage for new email verification functionalities to ensure robust performance.

@Reshzera Reshzera requested a review from RamRamez August 23, 2024 21:30
Copy link
Contributor

coderabbitai bot commented Aug 23, 2024

Walkthrough

This update introduces a comprehensive set of changes aimed at enhancing email verification processes within the application. Key modifications include the addition of email verification columns in the user database, implementation of new methods in notification adapters, and the introduction of mutations for sending and verifying email codes. Additional enhancements involve the expansion of error messages and localization support, along with testing improvements to ensure functionality related to user email verification.

Changes

File(s) Change Summary
migration/.../add_email_verification_columns.ts Added columns for email verification in the user table with methods for migration and rollback.
src/adapters/notifications/MockNotificationAdapter.ts, src/adapters/notifications/NotificationAdapterInterface.ts, src/adapters/notifications/NotificationCenterAdapter.ts Introduced a method for handling email confirmation codes across notification adapters.
src/analytics/analytics.ts Added a new enum value for tracking email confirmation events.
src/entities/user.ts Enhanced User entity with properties for email verification status and code.
src/resolvers/types/userResolver.ts Updated UserByAddressResponse class with new optional fields.
src/resolvers/userResolver.ts Added mutations for sending email confirmation codes, verifying codes, and checking email availability.
src/resolvers/userResolver.test.ts Implemented new test cases for email verification and availability functionalities.
src/utils/errorMessages.ts, src/utils/locales/en.json, src/utils/locales/es.json Added new error messages and corresponding localization entries for email verification processes.
src/utils/utils.ts Introduced a function to generate random email verification codes.
test/graphqlQueries.ts Added new GraphQL mutations for email confirmation and availability checks.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UserResolver
    participant NotificationCenterAdapter
    participant NotificationService

    User->>UserResolver: Request email confirmation code
    UserResolver->>NotificationCenterAdapter: Send email confirmation code
    NotificationCenterAdapter->>NotificationService: Trigger email sending
    NotificationService-->>NotificationCenterAdapter: Email sent confirmation
    NotificationCenterAdapter-->>UserResolver: Confirmation sent response
    UserResolver-->>User: Respond with success message
Loading

🐰 In the meadow so bright,
I hop with delight,
New codes for verification,
Make our system just right!
With messages clear and true,
Our users will know what to do,
Hooray for the changes anew! 🌼


Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0210f4c and fe94dc4.

Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
Files selected for processing (15)
  • migration/1723764281125-add_email_verification_columns.ts (1 hunks)
  • src/adapters/notifications/MockNotificationAdapter.ts (1 hunks)
  • src/adapters/notifications/NotificationAdapterInterface.ts (1 hunks)
  • src/adapters/notifications/NotificationCenterAdapter.ts (1 hunks)
  • src/analytics/analytics.ts (1 hunks)
  • src/entities/user.ts (2 hunks)
  • src/resolvers/types/userResolver.ts (1 hunks)
  • src/resolvers/userResolver.test.ts (3 hunks)
  • src/resolvers/userResolver.ts (3 hunks)
  • src/utils/errorMessages.ts (2 hunks)
  • src/utils/locales/en.json (1 hunks)
  • src/utils/locales/es.json (1 hunks)
  • src/utils/utils.ts (1 hunks)
  • src/utils/validators/commonValidators.ts (1 hunks)
  • test/graphqlQueries.ts (1 hunks)
Additional comments not posted (21)
src/utils/validators/commonValidators.ts (1)

2-5: Verify the impact of allowing empty strings in email validation.

The change to allow empty strings as valid input may affect parts of the codebase that rely on email validation. Ensure that this change aligns with the intended functionality and does not introduce unintended side effects.

Run the following script to identify where validateEmail is used and verify its impact:

src/resolvers/types/userResolver.ts (1)

8-11: Verify the integration of new fields in the GraphQL schema.

The newly added fields isEmailSent and useHasProject enhance the response structure. Ensure these fields are correctly integrated and utilized in the GraphQL schema and queries.

Run the following script to verify the integration of these new fields:

migration/1723764281125-add_email_verification_columns.ts (1)

6-20: Verify the alignment of migration with the database schema and application logic.

The migration script adds verificationCode and isEmailVerified columns. Ensure these changes align with the database schema and application logic, and that they are correctly utilized in the application.

Run the following script to verify the alignment of the migration with the database schema and application logic:

Verification successful

Migration aligns with database schema and application logic.

The verificationCode and isEmailVerified columns added in the migration script are well-integrated into the application logic and database schema. They are utilized in user-related operations, notifications, and are covered by tests, ensuring proper alignment and functionality.

  • src/resolvers/userResolver.ts: Handles operations using these fields.
  • src/entities/user.ts: Defines these fields in the user entity.
  • src/resolvers/userResolver.test.ts: Contains tests verifying the logic.
  • src/adapters/notifications/NotificationCenterAdapter.ts: Uses verificationCode for notifications.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the alignment of migration with the database schema and application logic.

# Test: Search for the usage of `verificationCode` and `isEmailVerified`. Expect: Review the context of each usage to ensure alignment with the schema and logic.
rg --type ts -A 5 $'verificationCode|isEmailVerified'

Length of output: 7796

src/analytics/analytics.ts (1)

52-52: Addition of SEND_EMAIL_CONFIRMATION_CODE_FLOW to NOTIFICATIONS_EVENT_NAMES.

The addition of this enum value is appropriate and aligns with the objective of enhancing email verification processes.

src/adapters/notifications/NotificationAdapterInterface.ts (1)

66-69: Addition of sendEmailConfirmationCodeFlow method to NotificationAdapterInterface.

The introduction of this method is well-aligned with the goal of improving email confirmation processes.

src/entities/user.ts (3)

37-37: Addition of user.isEmailVerified to publicSelectionFields.

This addition is appropriate for exposing the email verification status in public queries.


160-162: Addition of isEmailVerified field to User class.

The field is correctly annotated and aligns with the objective of managing email verification states.


164-165: Addition of verificationCode field to User class.

The field is correctly annotated and supports the email verification process.

src/utils/locales/en.json (1)

121-123: Localization entries look good.

The new entries for "USER_ALREADY_VERIFIED", "INVALID_EMAIL_CODE", and "EMAIL_ALREADY_USED" are well-structured and enhance user feedback during email verification.

src/resolvers/userResolver.ts (1)

238-269: Ensure email uniqueness in sendUserEmailConfirmationCodeFlow.

The mutation checks if the email is already used and verified, but it should also ensure that the email is unique across all users before proceeding.

Consider adding a check to ensure the email is not associated with any other user, verified or not.

src/utils/locales/es.json (1)

110-112: Translations for new email verification messages look good.

The new entries "USER_ALREADY_VERIFIED", "INVALID_EMAIL_CODE", and "EMAIL_ALREADY_USED" are correctly translated and consistent with the existing format.

src/utils/utils.ts (1)

470-472: Implementation of generateEmailVerificationCode is correct.

The function generates a random six-digit number using Math.random(), which is suitable for email verification codes.

src/utils/errorMessages.ts (2)

208-210: New error messages for email verification are correctly added.

The entries USER_ALREADY_VERIFIED, INVALID_EMAIL_CODE, and EMAIL_ALREADY_USED are consistent with the existing format in errorMessages.


385-387: New translation keys for email verification are correctly added.

The entries USER_ALREADY_VERIFIED, INVALID_EMAIL_CODE, and EMAIL_ALREADY_USED are consistent with the existing format in translationErrorMessagesKeys.

src/resolvers/userResolver.test.ts (3)

941-966: LGTM! Comprehensive test coverage for email confirmation code sending.

The test cases cover successful email sending, failure when not logged in, and handling already verified users.


1045-1090: LGTM! Comprehensive test coverage for email code verification.

The test cases cover successful verification, failure when not logged in, and handling incorrect codes.


1159-1209: LGTM! Test coverage for email availability checks is adequate.

The test cases cover both available and unavailable email scenarios.

src/adapters/notifications/NotificationCenterAdapter.ts (1)

97-116: LGTM! New method sendEmailConfirmationCodeFlow is well-implemented.

The method aligns with the existing pattern for sending notifications and includes appropriate error logging.

test/graphqlQueries.ts (3)

2588-2592: LGTM: sendCodeToConfirmEmail Mutation.

The mutation is well-defined and correctly implemented. Ensure that the backend resolver handles the mutation as expected.


2594-2598: LGTM: verifyUserEmailCode Mutation.

The mutation is well-defined and correctly implemented. Ensure that the backend resolver handles the mutation as expected.


2600-2604: LGTM: checkEmailAvailability Mutation.

The mutation is well-defined and correctly implemented. Ensure that the backend resolver handles the mutation as expected.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Reshzera
Copy link
Contributor Author

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Outside diff range, codebase verification and nitpick comments (4)
src/adapters/notifications/MockNotificationAdapter.ts (1)

38-44: Consider implementing the sendEmailConfirmationCodeFlow method.

The method currently only logs the invocation and resolves with undefined, which is expected for a mock adapter. However, consider implementing actual logic or simulating behavior for testing purposes.

src/resolvers/userResolver.ts (2)

271-290: Consider logging successful email verification in verifyUserEmailCode.

Adding a log statement after successful email verification can help in tracking user actions and debugging.

    user.verificationCode = null;

+   logger.info(`User ${user.id} successfully verified their email.`);
    await user.save();

292-309: Consider improving error messages in checkEmailAvailability.

The error message for an already used email could be more descriptive, indicating the email is already verified and cannot be reused.

    if (!!isEmailAlreadyUsed && isEmailAlreadyUsed.isEmailVerified)
-     throw new Error(i18n.__(translationErrorMessagesKeys.EMAIL_ALREADY_USED));
+     throw new Error(i18n.__('The email is already verified and cannot be reused.'));
src/resolvers/userResolver.test.ts (1)

1015-1042: Clarify test description for already verified email.

The test description "should fail send the email when email is already verified" could be more descriptive. Consider specifying that the test checks for an error message when attempting to resend a confirmation code to an already verified email.

-  it('should fail send the email when email is already verified', async () => {
+  it('should return an error when trying to resend confirmation code to an already verified email', async () => {

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.

1 participant