Merge pull request #1666 from consul/users-without-email

Users without email

app/controllers/management/users_controller.rb

@@ -6,7 +6,13 @@ class Management::UsersController < Management::BaseController
 
   def create
     @user = User.new(user_params)
-    @user.skip_password_validation = true
+
+    if @user.email.blank?
+      user_without_email
+    else
+      user_with_email
+    end
+
     @user.terms_of_service = '1'
     @user.residence_verified_at = Time.current
     @user.verified_at = Time.current
@@ -40,4 +46,24 @@ class Management::UsersController < Management::BaseController
       session[:document_number] = nil
     end
 
+    def user_without_email
+      new_password = "aAbcdeEfghiJkmnpqrstuUvwxyz23456789$!".split('').sample(10).join('')
+      @user.password = new_password
+      @user.password_confirmation = new_password
+
+      @user.email = nil
+      @user.confirmed_at = Time.current
+
+      @user.newsletter = false
+      @user.email_on_proposal_notification = false
+      @user.email_digest = false
+      @user.email_on_direct_message = false
+      @user.email_on_comment = false
+      @user.email_on_comment_reply = false
+    end
+
+    def user_with_email
+      @user.skip_password_validation = true
+    end
+
 end

app/mailers/mailer.rb

@@ -1,4 +1,6 @@
 class Mailer < ApplicationMailer
+  after_action :prevent_delivery_to_users_without_email
+
   helper :text_with_links
   helper :mailer
   helper :users
@@ -6,8 +8,10 @@ class Mailer < ApplicationMailer
   def comment(comment)
     @comment = comment
     @commentable = comment.commentable
+    @email_to = @commentable.author.email
+
     with_user(@commentable.author) do
-      mail(to: @commentable.author.email, subject: t('mailers.comment.subject', commentable: t("activerecord.models.#{@commentable.class.name.underscore}", count: 1).downcase)) if @commentable.present? && @commentable.author.present?
+      mail(to: @email_to, subject: t('mailers.comment.subject', commentable: t("activerecord.models.#{@commentable.class.name.underscore}", count: 1).downcase)) if @commentable.present? && @commentable.author.present?
     end
   end
 
@@ -16,96 +20,108 @@ class Mailer < ApplicationMailer
     @commentable = @reply.commentable
     parent = Comment.find(@reply.parent_id)
     @recipient = parent.author
+    @email_to = @recipient.email
+
     with_user(@recipient) do
-      mail(to: @recipient.email, subject: t('mailers.reply.subject')) if @commentable.present? && @recipient.present?
+      mail(to: @email_to, subject: t('mailers.reply.subject')) if @commentable.present? && @recipient.present?
     end
   end
 
   def email_verification(user, recipient, token, document_type, document_number)
     @user = user
-    @recipient = recipient
+    @email_to = recipient
     @token = token
     @document_type = document_type
     @document_number = document_number
 
     with_user(user) do
-      mail(to: @recipient, subject: t('mailers.email_verification.subject'))
+      mail(to: @email_to, subject: t('mailers.email_verification.subject'))
     end
   end
 
   def unfeasible_spending_proposal(spending_proposal)
     @spending_proposal = spending_proposal
     @author = spending_proposal.author
+    @email_to = @author.email
 
     with_user(@author) do
-      mail(to: @author.email, subject: t('mailers.unfeasible_spending_proposal.subject', code: @spending_proposal.code))
+      mail(to: @email_to, subject: t('mailers.unfeasible_spending_proposal.subject', code: @spending_proposal.code))
     end
   end
 
   def direct_message_for_receiver(direct_message)
     @direct_message = direct_message
     @receiver = @direct_message.receiver
+    @email_to = @receiver.email
 
     with_user(@receiver) do
-      mail(to: @receiver.email, subject: t('mailers.direct_message_for_receiver.subject'))
+      mail(to: @email_to, subject: t('mailers.direct_message_for_receiver.subject'))
     end
   end
 
   def direct_message_for_sender(direct_message)
     @direct_message = direct_message
     @sender = @direct_message.sender
+    @email_to = @sender.email
 
     with_user(@sender) do
-      mail(to: @sender.email, subject: t('mailers.direct_message_for_sender.subject'))
+      mail(to: @email_to, subject: t('mailers.direct_message_for_sender.subject'))
     end
   end
 
   def proposal_notification_digest(user, notifications)
     @notifications = notifications
+    @email_to = user.email
 
     with_user(user) do
-      mail(to: user.email, subject: t('mailers.proposal_notification_digest.title', org_name: Setting['org_name']))
+      mail(to: @email_to, subject: t('mailers.proposal_notification_digest.title', org_name: Setting['org_name']))
     end
   end
 
   def user_invite(email)
+    @email_to = email
+
     I18n.with_locale(I18n.default_locale) do
-      mail(to: email, subject: t('mailers.user_invite.subject', org_name: Setting["org_name"]))
+      mail(to: @email_to, subject: t('mailers.user_invite.subject', org_name: Setting["org_name"]))
     end
   end
 
   def budget_investment_created(investment)
     @investment = investment
+    @email_to = @investment.author.email
 
     with_user(@investment.author) do
-      mail(to: @investment.author.email, subject: t('mailers.budget_investment_created.subject'))
+      mail(to: @email_to, subject: t('mailers.budget_investment_created.subject'))
     end
   end
 
   def budget_investment_unfeasible(investment)
     @investment = investment
     @author = investment.author
+    @email_to = @author.email
 
     with_user(@author) do
-      mail(to: @author.email, subject: t('mailers.budget_investment_unfeasible.subject', code: @investment.code))
+      mail(to: @email_to, subject: t('mailers.budget_investment_unfeasible.subject', code: @investment.code))
     end
   end
 
   def budget_investment_selected(investment)
     @investment = investment
     @author = investment.author
+    @email_to = @author.email
 
     with_user(@author) do
-      mail(to: @author.email, subject: t('mailers.budget_investment_selected.subject', code: @investment.code))
+      mail(to: @email_to, subject: t('mailers.budget_investment_selected.subject', code: @investment.code))
     end
   end
 
   def budget_investment_unselected(investment)
     @investment = investment
     @author = investment.author
+    @email_to = @author.email
 
     with_user(@author) do
-      mail(to: @author.email, subject: t('mailers.budget_investment_unselected.subject', code: @investment.code))
+      mail(to: @email_to, subject: t('mailers.budget_investment_unselected.subject', code: @investment.code))
     end
   end
 
@@ -116,4 +132,11 @@ class Mailer < ApplicationMailer
       block.call
     end
   end
+
+  def prevent_delivery_to_users_without_email
+    if @email_to.blank?
+      mail.perform_deliveries = false
+    end
+  end
+
 end

app/models/user.rb

@@ -247,7 +247,7 @@ class User < ActiveRecord::Base
   end
 
   def email_required?
-    !erased?
+    !erased? && unverified?
   end
 
   def locale

app/views/account/show.html.erb

@@ -40,52 +40,54 @@
             <% end %>
           </div>
 
-          <h2><%= t("account.show.notifications")%></h2>
+          <% if @account.email.present? %>
+            <h2><%= t("account.show.notifications")%></h2>
 
-          <div>
+            <div>
-            <%= f.label :email_on_comment do %>
+              <%= f.label :email_on_comment do %>
-              <%= f.check_box :email_on_comment, title: t('account.show.email_on_comment_label'), label: false %>
+                <%= f.check_box :email_on_comment, title: t('account.show.email_on_comment_label'), label: false %>
-              <span class="checkbox">
+                <span class="checkbox">
-                <%= t("account.show.email_on_comment_label") %>
+                  <%= t("account.show.email_on_comment_label") %>
-              </span>
+                </span>
-            <% end %>
+              <% end %>
-          </div>
+            </div>
 
-          <div>
+            <div>
-            <%= f.label :email_on_comment_reply do %>
+              <%= f.label :email_on_comment_reply do %>
-              <%= f.check_box :email_on_comment_reply, title: t('account.show.email_on_comment_reply_label'), label: false %>
+                <%= f.check_box :email_on_comment_reply, title: t('account.show.email_on_comment_reply_label'), label: false %>
-              <span class="checkbox">
+                <span class="checkbox">
-                <%= t("account.show.email_on_comment_reply_label") %>
+                  <%= t("account.show.email_on_comment_reply_label") %>
-              </span>
+                </span>
-            <% end %>
+              <% end %>
-          </div>
+            </div>
 
-          <div>
+            <div>
-            <%= f.label :email_newsletter_subscribed do %>
+              <%= f.label :email_newsletter_subscribed do %>
-              <%= f.check_box :newsletter, title: t('account.show.subscription_to_website_newsletter_label'), label: false %>
+                <%= f.check_box :newsletter, title: t('account.show.subscription_to_website_newsletter_label'), label: false %>
-              <span class="checkbox">
+                <span class="checkbox">
-                <%= t("account.show.subscription_to_website_newsletter_label") %>
+                  <%= t("account.show.subscription_to_website_newsletter_label") %>
-              </span>
+                </span>
-            <% end %>
+              <% end %>
-          </div>
+            </div>
 
-          <div>
+            <div>
-            <%= f.label :email_digest do %>
+              <%= f.label :email_digest do %>
-              <%= f.check_box :email_digest, title: t('account.show.email_digest_label'), label: false %>
+                <%= f.check_box :email_digest, title: t('account.show.email_digest_label'), label: false %>
-              <span class="checkbox">
+                <span class="checkbox">
-                <%= t("account.show.email_digest_label") %>
+                  <%= t("account.show.email_digest_label") %>
-              </span>
+                </span>
-            <% end %>
+              <% end %>
-          </div>
+            </div>
 
-          <div>
+            <div>
-            <%= f.label :email_on_direct_message do %>
+              <%= f.label :email_on_direct_message do %>
-              <%= f.check_box :email_on_direct_message, title: t('account.show.email_on_direct_message_label'), label: false %>
+                <%= f.check_box :email_on_direct_message, title: t('account.show.email_on_direct_message_label'), label: false %>
-              <span class="checkbox">
+                <span class="checkbox">
-                <%= t("account.show.email_on_direct_message_label") %>
+                  <%= t("account.show.email_on_direct_message_label") %>
-              </span>
+                </span>
-            <% end %>
+              <% end %>
-          </div>
+            </div>
+          <% end %>
 
           <% if @account.official_level == 1 %>
             <div>

app/views/management/users/new.html.erb

@@ -13,7 +13,7 @@
           label: t('management.username_label'),
           placeholder: t('management.username_label') %>
       <%= f.text_field :email,
-          label: t('management.email_label'),
+          label: t('management.users.email_optional_label'),
           placeholder: t('management.email_label') %>
       <div class="date-of-birth">
         <%= f.label t("management.date_of_birth") %>

app/views/management/users/show.html.erb

@@ -1,4 +1,8 @@
-<p><%= t("management.users.create_user_success_html", email: @user.email) %></p>
+<% if @user.email.blank? %>
+  <p><%= t("management.users.autogenerated_password_html", password: @user.password) %></p>
+<% else %>
+  <p><%= t("management.users.create_user_success_html", email: @user.email) %></p>
+<% end %>
 
 <%= render 'management/user_permissions',
       message: t("management.document_verifications.in_census_has_following_permissions"),

config/locales/management.en.yml

@@ -114,6 +114,8 @@ en:
       create_user_info: 'We will create an account with the following data:'
       create_user_submit: Create user
       create_user_success_html: We have sent an email to the email address <b>%{email}</b> in order to verify that it belongs to this user. It contains a link they have to click. Then they will have to set their access password before being able to log in to the website
+      autogenerated_password_html: "Autogenerated password is <b>%{password}</b>, you can change it in the 'My account' section of the web"
+      email_optional_label: Email (optional)
       erased_notice: User account deleted.
       erased_by_manager: "Deleted by manager: %{manager}"
       erase_account_link: Delete user

config/locales/management.es.yml

@@ -114,6 +114,8 @@ es:
       create_user_info: 'Procedemos a crear un usuario con la siguiente información:'
       create_user_submit: Crear usuario
       create_user_success_html: Hemos enviado un correo electrónico a <b>%{email}</b> para verificar que es suya. El correo enviado contiene un link que el usuario deberá pulsar. Entonces podrá seleccionar una clave de acceso, y entrar en la web de participación.
+      autogenerated_password_html: "Se ha asignado la contraseña <b>%{password}</b> a este usuario. Puede modificarla desde el apartado 'Mi cuenta' de la web."
+      email_optional_label: "Email (recomendado pero opcional)"
       erased_notice: Cuenta de usuario borrada.
       erased_by_manager: "Borrada por el manager: %{manager}"
       erase_account_link: Borrar cuenta

spec/factories.rb

@@ -42,6 +42,9 @@ FactoryGirl.define do
       confirmed_hide_at Time.current
     end
 
+    trait :verified do
+      verified_at Time.current
+    end
   end
 
   factory :identity do

spec/features/emails_spec.rb

@@ -370,4 +370,15 @@ feature 'Emails' do
     end
 
   end
+
+  context "Users without email" do
+    scenario "should not receive emails", :js do
+      user = create(:user, :verified, email_on_comment: true)
+      proposal = create(:proposal, author: user)
+      user.update(email: nil)
+      comment_on(proposal)
+
+      expect { open_last_email }.to raise_error "No email has been sent!"
+    end
+  end
 end

spec/features/management/managed_users_spec.rb

@@ -84,7 +84,7 @@ feature 'Managed User' do
       end
     end
 
-    scenario "User is created as level three from scratch" do
+    scenario "User is created with email as level three from scratch" do
       login_as_manager
 
       visit management_document_verifications_path
@@ -101,6 +101,7 @@ feature 'Managed User' do
       click_button 'Create user'
 
       expect(page).to have_content "We have sent an email"
+      expect(page).to_not have_content "Autogenerated password is"
 
       user = User.last
       within(".account-info") do
@@ -110,6 +111,33 @@ feature 'Managed User' do
         expect(page).to have_content "#{user.document_number}"
       end
     end
+
+    scenario "User is created without email as level three from scratch" do
+      login_as_manager
+
+      visit management_document_verifications_path
+      fill_in 'document_verification_document_number', with: '12345678Z'
+      click_button 'Check'
+
+      expect(page).to have_content "Please introduce the email used on the account"
+
+      click_link 'Create a new account'
+
+      fill_in 'user_username', with: 'peppa'
+      fill_in 'user_email', with: ''
+
+      click_button 'Create user'
+
+      expect(page).to_not have_content "We have sent an email"
+      expect(page).to have_content "Autogenerated password is"
+
+      user = User.last
+      within(".account-info") do
+        expect(page).to have_content "Identified as"
+        expect(page).to have_content "#{user.username}"
+        expect(page).to have_content "#{user.document_number}"
+      end
+    end
   end
 
   scenario "Close the currently managed user session" do

spec/features/management/users_spec.rb

@@ -6,8 +6,7 @@ feature 'Users' do
     login_as_manager
   end
 
-  scenario 'Create a level 3 user from scratch' do
+  scenario 'Create a level 3 user with email from scratch' do
-
     visit management_document_verifications_path
     fill_in 'document_verification_document_number', with: '12345678Z'
     click_button 'Check'
@@ -23,6 +22,7 @@ feature 'Users' do
     click_button 'Create user'
 
     expect(page).to have_content "We have sent an email"
+    expect(page).to_not have_content "Autogenerated password is"
 
     user = User.find_by_email('pepe@gmail.com')
 
@@ -46,6 +46,32 @@ feature 'Users' do
     expect(page).to have_content "Your account has been confirmed."
   end
 
+  scenario 'Create a level 3 user without email from scratch' do
+    visit management_document_verifications_path
+    fill_in 'document_verification_document_number', with: '12345678Z'
+    click_button 'Check'
+
+    expect(page).to have_content "Please introduce the email used on the account"
+
+    click_link 'Create a new account'
+
+    fill_in 'user_username', with: 'Kelly Sue'
+    fill_in 'user_email', with: ''
+    select_date '31-December-1980', from: 'user_date_of_birth'
+
+    click_button 'Create user'
+
+    expect(page).to_not have_content "We have sent an email"
+    expect(page).to have_content "Autogenerated password is"
+
+    user = User.find_by_username('Kelly Sue')
+
+    expect(user).to be_level_three_verified
+    expect(user).to be_residence_verified
+    expect(user).to be_confirmed
+    expect(user.date_of_birth).to have_content (Date.new(1980,12,31))
+  end
+
   scenario 'Delete a level 2 user account from document verification page', :js do
     level_2_user = create(:user, :level_two, document_number: "12345678Z")
 

spec/models/user_spec.rb

@@ -632,4 +632,29 @@ describe User do
 
   end
 
+  describe "email_required?" do
+    it "is true for regular users" do
+      expect(subject.email_required?).to eq(true)
+      expect(create(:user, :hidden).email_required?).to eq(true)
+    end
+
+    it "is false for erased users" do
+      user = create(:user)
+      user.erase
+      user.reload
+
+      expect(user.email_required?).to eq(false)
+    end
+
+    it "is false for verified users with no email" do
+      user = create(:user,
+                     username: "Lois",
+                     email: "",
+                     verified_at: Time.current)
+
+      expect(user).to be_valid
+      expect(user.email_required?).to eq(false)
+    end
+  end
+
 end