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

#130 - Add Support for Multiple User Emails #266

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gamesover
Copy link
Contributor

This PR introduces the ability for users to associate multiple email addresses with their account. It addresses issue #130, which highlighted the need for this feature to prevent duplicate accounts and improve user experience, particularly for users who sign up with different emails (e.g., work and personal).

Key Changes:

  • Model Changes:

    • Created a new Email model to store multiple email addresses associated with a user (belongs_to :user).
    • Modified the User model to has_many :emails (must be declared before devise multi_email_authenticatable).
  • Devise Integration:

    • Added the devise-multi_email gem to leverage its features for managing multiple emails within Devise.
  • Controller Changes:

    • Created a new My::EmailsController to handle:
      • Adding new email addresses.
      • Setting a primary email.
      • Deleting email addresses.
      • Sending confirmation emails upon adding a new email.
    • Updated ReactivationsController and MailingList::Sync to use the user's primary email from the Email model.
  • Mailer Changes:

    • Added a trigger_after_confirmation callback to the Email model to handle email updates after confirmation.
  • View Changes:

    • Added views for managing multiple emails in the my/emails directory.
    • Updated the my/details/show view to display the user's associated email addresses and their confirmation status.
    • Added links for setting a primary email and deleting email addresses (for confirmed emails only).
  • Migration:

    • Created a migration to add the emails table.
    • Created a migration to temporarily remove the null: false constraint from the users.email column.
  • Testing:

    • Added tests for the Email model, My::EmailsController, and other relevant components to ensure proper functionality.
    • Updated existing tests to work with the new multi-email setup.

Migration Strategy for Existing Users:

  1. The users.email column will temporarily allow null values.
  2. run User.without_emails.find_each(&:update_emails) in rails console
  3. After the data migration, the users.email column will be removed or repurposed.

Further Considerations (Out of Scope for this PR):

  • Implementing a "one-click" email association feature for a smoother user experience.
  • Updating other parts of the application that might rely on User.email (e.g., notifications, other mailers).
  • Adding a mechanism for users to choose which email to use for specific types of notifications.
  • Finalizing the handling of the original email column in the User model.

** UI changes
add new email
image

list emails
image

delete email
image

@@ -10,7 +10,8 @@
import 'core-js/stable'
import 'regenerator-runtime/runtime'

require("@rails/ujs").start()
import Rails from "@rails/ujs"
Rails.start()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my local and production, I can see below error in chrome console

image

using import to fix the error

@@ -17,15 +17,6 @@
</div>
<% end %>

<div class="field">
<div class="label">
<%= form.label :email, class: 'required' %>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the ability to edit the email field. Instead, users can add a new email or delete an existing one.

@gamesover gamesover marked this pull request as draft December 18, 2024 16:21
email.update!(primary: true)
end

flash[:notice] = "Your primary email has been ukpdated to #{email.email}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change 'ukpdated' to updated

def destroy
email = Email.find(params[:id])
if email.destroy
flash[:notice] = "Your email #{email.email} have been deleted."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change have to has

if email.destroy
flash[:notice] = "Your email #{email.email} have been deleted."
else
flash[:error] = "Your email #{email.email} is failed to be deleted."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove is

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.

2 participants