diff --git a/app/controllers/management/users_controller.rb b/app/controllers/management/users_controller.rb index 8d3d148fa..d46360d89 100644 --- a/app/controllers/management/users_controller.rb +++ b/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 diff --git a/app/mailers/mailer.rb b/app/mailers/mailer.rb index 7e8970c61..c12f12087 100644 --- a/app/mailers/mailer.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index 95b915b02..1b4ed81f1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -247,7 +247,7 @@ class User < ActiveRecord::Base end def email_required? - !erased? + !erased? && unverified? end def locale diff --git a/app/views/account/show.html.erb b/app/views/account/show.html.erb index 247aa6024..a0d9f1693 100644 --- a/app/views/account/show.html.erb +++ b/app/views/account/show.html.erb @@ -40,52 +40,54 @@ <% end %> -

<%= t("account.show.notifications")%>

+ <% if @account.email.present? %> +

<%= t("account.show.notifications")%>

-
- <%= f.label :email_on_comment do %> - <%= f.check_box :email_on_comment, title: t('account.show.email_on_comment_label'), label: false %> - - <%= t("account.show.email_on_comment_label") %> - - <% end %> -
+
+ <%= f.label :email_on_comment do %> + <%= f.check_box :email_on_comment, title: t('account.show.email_on_comment_label'), label: false %> + + <%= t("account.show.email_on_comment_label") %> + + <% end %> +
-
- <%= 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 %> - - <%= t("account.show.email_on_comment_reply_label") %> - - <% end %> -
+
+ <%= 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 %> + + <%= t("account.show.email_on_comment_reply_label") %> + + <% end %> +
-
- <%= f.label :email_newsletter_subscribed do %> - <%= f.check_box :newsletter, title: t('account.show.subscription_to_website_newsletter_label'), label: false %> - - <%= t("account.show.subscription_to_website_newsletter_label") %> - - <% end %> -
+
+ <%= f.label :email_newsletter_subscribed do %> + <%= f.check_box :newsletter, title: t('account.show.subscription_to_website_newsletter_label'), label: false %> + + <%= t("account.show.subscription_to_website_newsletter_label") %> + + <% end %> +
-
- <%= f.label :email_digest do %> - <%= f.check_box :email_digest, title: t('account.show.email_digest_label'), label: false %> - - <%= t("account.show.email_digest_label") %> - - <% end %> -
+
+ <%= f.label :email_digest do %> + <%= f.check_box :email_digest, title: t('account.show.email_digest_label'), label: false %> + + <%= t("account.show.email_digest_label") %> + + <% end %> +
-
- <%= 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 %> - - <%= t("account.show.email_on_direct_message_label") %> - - <% end %> -
+
+ <%= 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 %> + + <%= t("account.show.email_on_direct_message_label") %> + + <% end %> +
+ <% end %> <% if @account.official_level == 1 %>
diff --git a/app/views/management/users/new.html.erb b/app/views/management/users/new.html.erb index d52597590..fd28a1248 100644 --- a/app/views/management/users/new.html.erb +++ b/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') %>
<%= f.label t("management.date_of_birth") %> diff --git a/app/views/management/users/show.html.erb b/app/views/management/users/show.html.erb index ed78d1aea..ae29d9d73 100644 --- a/app/views/management/users/show.html.erb +++ b/app/views/management/users/show.html.erb @@ -1,4 +1,8 @@ -

<%= t("management.users.create_user_success_html", email: @user.email) %>

+<% if @user.email.blank? %> +

<%= t("management.users.autogenerated_password_html", password: @user.password) %>

+<% else %> +

<%= t("management.users.create_user_success_html", email: @user.email) %>

+<% end %> <%= render 'management/user_permissions', message: t("management.document_verifications.in_census_has_following_permissions"), diff --git a/config/locales/management.en.yml b/config/locales/management.en.yml index d8aaf826c..d2524990a 100644 --- a/config/locales/management.en.yml +++ b/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 %{email} 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 %{password}, 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 diff --git a/config/locales/management.es.yml b/config/locales/management.es.yml index 10543798a..5ef4eee19 100644 --- a/config/locales/management.es.yml +++ b/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 %{email} 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 %{password} 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 diff --git a/spec/features/emails_spec.rb b/spec/features/emails_spec.rb index 47d0e7f1d..dd0d71f2c 100644 --- a/spec/features/emails_spec.rb +++ b/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 diff --git a/spec/features/management/managed_users_spec.rb b/spec/features/management/managed_users_spec.rb index 311b8f5cd..d35dc35e1 100644 --- a/spec/features/management/managed_users_spec.rb +++ b/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 diff --git a/spec/features/management/users_spec.rb b/spec/features/management/users_spec.rb index 5d08ce711..e7c43f4a9 100644 --- a/spec/features/management/users_spec.rb +++ b/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") diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 283a01c96..2bf09f378 100644 --- a/spec/models/user_spec.rb +++ b/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