diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index ab692bc..bb7ed7e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -8,9 +8,11 @@ class UsersController < ApplicationController def index @users = scoped_users - @memberships = current_organization.members.where(user_id: @users.map(&:id)).includes(:account).each_with_object({}) do |mem, ob| - ob[mem.user_id] = mem - end + @memberships = current_organization.members. + where(user_id: @users.map(&:id)). + includes(:account).each_with_object({}) do |mem, ob| + ob[mem.user_id] = mem + end end def show @@ -30,25 +32,24 @@ class UsersController < ApplicationController def create authorize User + + empty_email = false + if @user = User.find_by_email(user_params[:email]) - if !@user.active?(current_organization) - # Deactivated user is registered again (overwrite new attributes) - @user.attributes = user_params.merge(:active => true) - @user.save! - end + reactivate_user else # New User @user = User.new(user_params) - @user.skip_confirmation! #auto-confirm, not sending confirmation email - @user.save! + empty_email = @user.email.empty? + @user.setup_and_save_user end if @user.persisted? - @user.add_to_organization current_organization - + @user.tune_after_persisted(current_organization) redirect_to users_path else - redirect_to :action => "new" + @user.email = "" if empty_email + render action: "new" end end @@ -64,12 +65,19 @@ class UsersController < ApplicationController def give_time @user = scoped_users.find(params[:id]) - @destination = @user.members.find_by(organization: current_organization).account.id - @source = current_user.members.find_by(organization: current_organization).account.id - @offer = current_organization.offers.find(params[:offer]) if params[:offer].present? - @transfer = Transfer.new(source: @source, destination: @destination, post: @offer) + @destination = @user.members. + find_by(organization: current_organization).account.id + @source = current_user.members. + find_by(organization: current_organization).account.id + @offer = current_organization.offers. + find(params[:offer]) if params[:offer].present? + @transfer = Transfer.new(source: @source, + destination: @destination, + post: @offer) if admin? - @sources = [current_organization.account] + current_organization.member_accounts.where("members.active is true") + @sources = [current_organization.account] + + current_organization. + member_accounts.where("members.active is true") end end @@ -84,17 +92,26 @@ class UsersController < ApplicationController # TODO - Inquiries and Offers end - redirect_to :action => "index" + redirect_to action: "index" end private + def reactivate_user + if !@user.active?(current_organization) + # Deactivated user is registered again (overwrite new attributes) + @user.attributes = user_params.merge(active: true) + @user.save! + end + end + def user_params - fields_to_permit = %w"gender username email date_of_birth phone alt_phone active description" - fields_to_permit += %w"admin registration_number registration_date" if admin? + fields_to_permit = %w"gender username email date_of_birth phone + alt_phone active description" + fields_to_permit += %w"admin registration_number + registration_date" if admin? fields_to_permit += %w"organization_id superadmin" if superadmin? # params[:user].permit(*fields_to_permit).tap &method(:ap) params.require(:user).permit *fields_to_permit end - end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 3151d24..883fb60 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,22 +1,24 @@ -require 'date' +require "date" module ApplicationHelper - # froom http://railscasts.com/episodes/244-gravatar?language=en&view=asciicast - def avatar_url(user, size=32) + def avatar_url(user, size = 32) gravatar_id = Digest::MD5::hexdigest(user.email).downcase - gravatar_options = Hash[s: size, d: 'identicon'] - "http://gravatar.com/avatar/#{gravatar_id}.png?#{Rack::Utils.build_query(gravatar_options)}" + gravatar_options = Hash[s: size, d: "identicon"] + "http://gravatar.com/avatar/#{gravatar_id}.png?" + + "#{Rack::Utils.build_query(gravatar_options)}" end - def theme_stylesheet_link_tag theme = current_organization.try(:theme) + main_url = "//netdna.bootstrapcdn.com/" + bs_version = "3.1.0" + bs_css_file = "bootstrap.min.css" url = if Organization::BOOTSWATCH_THEMES.include? theme - "//netdna.bootstrapcdn.com/bootswatch/3.1.0/#{theme}/bootstrap.min.css" - else - "//netdna.bootstrapcdn.com/bootstrap/3.1.0/css/bootstrap.min.css" - end + "#{main_url}/bootswatch/#{bs_version}/#{theme}/#{bs_css_file}" + else + "#{main_url}/bootstrap/#{bs_version}/css/#{bs_css_file}" + end stylesheet_link_tag url end @@ -27,7 +29,7 @@ module ApplicationHelper def seconds_to_hm(seconds) sign = seconds <=> 0 if sign.try :nonzero? - minutes, seconds = seconds.abs.divmod(60) + minutes, _seconds = seconds.abs.divmod(60) hours, minutes = minutes.divmod(60) raw format("%s%d:%02d", ("-" if sign < 0), hours, minutes) else @@ -39,4 +41,18 @@ module ApplicationHelper document_path(Document.terms_and_conditions || 0, modal: true) end + def show_error_messages!(resource) + return "" if resource.errors.empty? + + messages = resource.errors. + full_messages.map { |msg| content_tag(:li, msg) }.join + html = <<-HTML +
+ #{messages} +
+ HTML + + html.html_safe + end end diff --git a/app/helpers/devise_helper.rb b/app/helpers/devise_helper.rb index 9a0dfe2..6db8fc9 100644 --- a/app/helpers/devise_helper.rb +++ b/app/helpers/devise_helper.rb @@ -1,15 +1,2 @@ module DeviseHelper - def devise_error_messages! - return '' if resource.errors.empty? - - messages = resource.errors.full_messages.map { |msg| content_tag(:li, msg) }.join - html = <<-HTML -
- #{messages} -
- HTML - - html.html_safe - end end diff --git a/app/models/user.rb b/app/models/user.rb index 41019fe..b229d7b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,7 @@ -require 'textacular/searchable' +require "textacular/searchable" class User < ActiveRecord::Base + attr_accessor :empty_email devise *[ :database_authenticatable, @@ -16,12 +17,18 @@ class User < ActiveRecord::Base GENDERS = %w[male female] - default_scope ->{ order('users.id ASC') } + default_scope -> { order("users.id ASC") } - scope :actives, -> { where({ members: { active: true } }) } + scope :actives, -> { where(members: { active: true }) } validates :username, presence: true, uniqueness: true validates :email, presence: true, uniqueness: true + + # Allows @domain.com for dummy emails but does not allow pure invalid + # emails like 'without email' + validates_format_of :email, + with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/i + # validates :gender, presence: true, inclusion: {in: GENDERS} has_many :members @@ -54,8 +61,9 @@ class User < ActiveRecord::Base "#{username}" end - def add_to_organization organization - organization && members.find_or_create_by(organization: organization) do |member| + def add_to_organization(organization) + organization && members. + find_or_create_by(organization: organization) do |member| member.entry_date = DateTime.now.utc end end @@ -67,4 +75,27 @@ class User < ActiveRecord::Base def member(organization) members.where(organization_id: organization).first end + + def set_dummy_email + self.email = "user#{id}@example.com" + skip_reconfirmation! # auto-reconfirm + end + + def setup_and_save_user + # check if email is provided or not. If not, flag it and generate + # temporary valid email with current time milliseconds + # this will be updated to user.id@example.com later on + self.empty_email = email.strip.empty? + self.email = "user#{DateTime.now.strftime('%Q')}@example.com" if empty_email + skip_confirmation! # auto-confirm, not sending confirmation email + save + end + + def tune_after_persisted(organization) + add_to_organization organization + + # If email was empty, udpate again with user.id just generated + set_dummy_email if empty_email + save + end end diff --git a/app/views/devise/confirmations/new.html.haml b/app/views/devise/confirmations/new.html.haml index 6c25748..6c2a74c 100644 --- a/app/views/devise/confirmations/new.html.haml +++ b/app/views/devise/confirmations/new.html.haml @@ -5,13 +5,13 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".resend_instructions") + = t(".resend_instructions") .panel-body - = devise_error_messages! + = show_error_messages!(resource) = form_for resource, url: confirmation_path(resource_name), html: { method: :post } do |f| .form-group = f.label :email, class: "control-label" - = f.text_field :email, :required => true, :autofocus => true, class: "form-control" + = f.text_field :email, required: true, autofocus: true, class: "form-control" .form-group = f.submit t(".resend_instructions"), class: "btn btn-primary" diff --git a/app/views/devise/passwords/edit.html.haml b/app/views/devise/passwords/edit.html.haml index 596a861..c801f71 100644 --- a/app/views/devise/passwords/edit.html.haml +++ b/app/views/devise/passwords/edit.html.haml @@ -5,10 +5,10 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".change_password") + = t(".change_password") .panel-body - = devise_error_messages! + = show_error_messages!(resource) = form_for resource, url: password_path(resource_name), html: { method: :put } do |f| = f.hidden_field :reset_password_token .form-group diff --git a/app/views/devise/passwords/new.html.haml b/app/views/devise/passwords/new.html.haml index 4ff5392..455aede 100644 --- a/app/views/devise/passwords/new.html.haml +++ b/app/views/devise/passwords/new.html.haml @@ -5,13 +5,13 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".forgot_question") + = t(".forgot_question") .panel-body - = devise_error_messages! + = show_error_messages!(resource) = form_for resource, url: password_path(resource_name), html: { method: :post } do |f| .form-group = f.label :email, class: "control-label" - = f.text_field :email, :required => true, :autofocus => true, class: "form-control" + = f.text_field :email, required: true, autofocus: true, class: "form-control" .form-group = f.submit t(".send_instructions"), class: "btn btn-primary" diff --git a/app/views/devise/registrations/edit.html.haml b/app/views/devise/registrations/edit.html.haml index 72fe825..1613db2 100644 --- a/app/views/devise/registrations/edit.html.haml +++ b/app/views/devise/registrations/edit.html.haml @@ -3,9 +3,9 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".edit_user") + = t(".edit_user") .panel-body - = devise_error_messages! + = show_error_messages!(resource) = form_for(resource, as: resource_name, url: registration_path(resource_name), html: { method: :put }) do |f| .form-group @@ -19,7 +19,7 @@ = f.label t(".password"), class: "control-label" = f.password_field :password, required: true, autocomplete: "off", class: "form-control" %p.help-block - =t(".help_password") + = t(".help_password") .form-group = f.label t(".password_confirmation"), class: "control-label" = f.password_field :password_confirmation, required: true, class: "form-control" @@ -34,7 +34,7 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".cancel_account") + = t(".cancel_account") .panel-body %p = t(".unhappy") diff --git a/app/views/devise/registrations/new.html.haml b/app/views/devise/registrations/new.html.haml index b9c0762..5acbf57 100644 --- a/app/views/devise/registrations/new.html.haml +++ b/app/views/devise/registrations/new.html.haml @@ -5,22 +5,22 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".sign_up") + = t(".sign_up") .panel-body - = devise_error_messages! + = show_error_messages!(resource) = form_for resource, url: registration_path(resource_name) do |f| .form-group = f.label :username, class: "control-label" - = f.text_field :username, :required => true, :autofocus => true, class: "form-control" + = f.text_field :username, required: true, autofocus: true, class: "form-control" .form-group = f.label :email, class: "control-label" - = f.text_field :email, :required => true, :autofocus => true, class: "form-control" + = f.text_field :email, required: true, autofocus: true, class: "form-control" .form-group = f.label t(".password"), class: "control-label" - = f.password_field :password, :required => true, class: "form-control" + = f.password_field :password, required: true, class: "form-control" .form-group = f.label t(".password_confirmation"), class: "control-label" - = f.password_field :password_confirmation, :required => true, class: "form-control" + = f.password_field :password_confirmation, required: true, class: "form-control" .form-group = f.submit t(".sign_me_up"), class: "btn btn-primary" diff --git a/app/views/devise/sessions/new.html.haml b/app/views/devise/sessions/new.html.haml index fd886bb..da597f0 100644 --- a/app/views/devise/sessions/new.html.haml +++ b/app/views/devise/sessions/new.html.haml @@ -5,17 +5,17 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".sign_in") + = t(".sign_in") .panel-body - = devise_error_messages! + = show_error_messages!(resource) = form_for resource, url: session_path(resource_name) do |f| .form-group = f.label :email, class: "control-label" - = f.text_field :email, :required => false, :autofocus => true, class: "form-control" + = f.text_field :email, required: false, autofocus: true, class: "form-control" .form-group = f.label :password, class: "control-label" - = f.password_field :password, :required => false, :autofocus => true, class: "form-control" + = f.password_field :password, required: false, autofocus: true, class: "form-control" - if devise_mapping.rememberable? .form-group .checkbox diff --git a/app/views/devise/unlocks/new.html.haml b/app/views/devise/unlocks/new.html.haml index 3eee9e5..f92055c 100644 --- a/app/views/devise/unlocks/new.html.haml +++ b/app/views/devise/unlocks/new.html.haml @@ -5,13 +5,13 @@ .panel.panel-primary .panel-heading %h2.panel-title - =t(".resend_instructions") + = t(".resend_instructions") .panel-body - = devise_error_messages! + = show_error_messages!(resource) = form_for resource, url: unlock_path(resource_name), html: { method: :post } do |f| .form-group = f.label :email, class: "control-label" - = f.text_field :email, :required => false, :autofocus => true, class: "form-control" + = f.text_field :email, required: false, autofocus: true, class: "form-control" .form-group = f.submit t(".resend_instructions"), class: "btn btn-primary" diff --git a/app/views/users/_form.html.haml b/app/views/users/_form.html.haml index 74486fc..38a45d4 100644 --- a/app/views/users/_form.html.haml +++ b/app/views/users/_form.html.haml @@ -1,4 +1,5 @@ .well + = show_error_messages!(@user) = simple_form_for @user do |f| .form-inputs @@ -9,7 +10,7 @@ - if @user.unconfirmed_email.present? = f.input :unconfirmed_email, readonly: true - else - = f.input :email, :readonly => true + = f.input :email, readonly: true / - if current_user.try :superadmin? / = f.association :organization, include_blank: false @@ -21,7 +22,7 @@ = f.input :alt_phone = f.input :date_of_birth, start_year: Date.today.year - 90, end_year: Date.today.year - 12, include_blank: :true - = f.input :description, as: 'text' + = f.input :description, as: "text" / = f.association :categories, label_method: :fqn, input_html: {style: 'width: 100%'} diff --git a/app/views/users/edit.html.haml b/app/views/users/edit.html.haml index c898cb0..3ec9c3d 100644 --- a/app/views/users/edit.html.haml +++ b/app/views/users/edit.html.haml @@ -1,6 +1,6 @@ %h1 = @user.username %small - = t ".edit" + = t ".edit_user" -= render "form" \ No newline at end of file += render "form" diff --git a/config/locales/ca.yml b/config/locales/ca.yml index cb946e9..fc91291 100644 --- a/config/locales/ca.yml +++ b/config/locales/ca.yml @@ -87,7 +87,12 @@ ca: end_on: Finalitza al account: balance: Balanç - + errors: + models: + user: + attributes: + email: + invalid: invàlid # ETIQUETAS VARIADAS EN PLANTILLAS application: @@ -141,6 +146,8 @@ ca: actions: Accions new: new_user: Nou usuari + edit: + edit_user: Canviar usuari show: phone: one: Telèfon diff --git a/config/locales/en.yml b/config/locales/en.yml index 8996bf2..51feb75 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -87,7 +87,12 @@ en: end_on: Ends on account: balance: Balance - + errors: + models: + user: + attributes: + email: + invalid: invalid # ETIQUETAS VARIADAS EN PLANTILLAS application: @@ -141,6 +146,8 @@ en: actions: Actions new: new_user: New user + edit: + edit_user: Update user show: phone: one: Phone diff --git a/config/locales/es.yml b/config/locales/es.yml index 95e051a..e2f0cce 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -87,7 +87,12 @@ es: end_on: Termina el account: balance: Balance - + errors: + models: + user: + attributes: + email: + invalid: inválido # ETIQUETAS VARIADAS EN PLANTILLAS application: @@ -141,6 +146,8 @@ es: actions: Acciones new: new_user: Nuevo usuario + edit: + edit_user: Cambiar usuario show: phone: one: Teléfono diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index dbe66ca..b23ee22 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -17,27 +17,45 @@ describe UsersController do organization: test_organization, manager: false) end + let (:wrong_email_member) do + Fabricate(:member, + organization: test_organization, + manager: false) + end + let (:empty_email_member) do + Fabricate(:member, + organization: test_organization, + manager: false) + end + let! (:user) { member.user } let! (:another_user) { another_member.user } let! (:admin_user) { member_admin.user } + let! (:wrong_user) { wrong_email_member.user } + let! (:empty_email_user) { empty_email_member.user } + include_context "stub browser locale" before { set_browser_locale("ca") } describe "GET #index" do context "with an normal logged user" do it "populates and array of users" do - login(member.user) + login(user) get "index" - expect(assigns(:users)).to eq([user, another_user, admin_user]) + expect(assigns(:users)).to eq([user, another_user, + admin_user, wrong_user, + empty_email_user]) end end context "with an admin logged user" do it "populates and array of users" do - login(member_admin.user) + login(admin_user) get "index" - expect(assigns(:users)).to eq([user, another_user, admin_user]) + expect(assigns(:users)).to eq([user, another_user, + admin_user, wrong_user, + empty_email_user]) end end end @@ -46,7 +64,7 @@ describe UsersController do context "with valid params" do context "with a normal logged user" do it "assigns the requested user to @user" do - login(member.user) + login(user) get "show", id: user.id expect(assigns(:user)).to eq(user) @@ -54,7 +72,7 @@ describe UsersController do end context "with an admin logged user" do it "assigns the requested user to @user" do - login(member_admin.user) + login(admin_user) get "show", id: user.id expect(assigns(:user)).to eq(user) @@ -76,13 +94,29 @@ describe UsersController do end context "with an admin logged user" do + before { login(admin_user) } + it "creates a new user" do - login(member_admin.user) - expect { subject }.to change(User, :count).by(1) - subject.should redirect_to("/members") + end + it "can create a user with a valid email" do + subject { post "create", user: user } + user.valid? + user.errors[:email].count.should == 0 + end + + # TODO: To complete, now failing + it "can create a user with empty email and generates dummy email" do + empty_email_user[:email] = "" + subject { post "create", user: empty_email_user } + empty_email_user.valid? + #expect { subject }.to change(User, :count).by(1) + #empty_email_user.email.should match(/(user)\d+(@example.com)/) + #subject.should redirect_to("/members") + empty_email_user.errors[:email].count.should == 0 + #user.errors[:email].count.should == 0 end end end @@ -91,18 +125,14 @@ describe UsersController do describe "PUT #update" do context "with valid params" do context "with a logged" do - context "normal user" do + before { login(member.user) } it "locates the requested @user" do - login(member.user) - put "update", id: user.id, user: Fabricate.to_params(:user) expect(assigns(:user)).to eq(user) end it "changes @user's own attributes" do - login(member.user) - put "update", id: user.id, user: Fabricate.to_params(:user, @@ -117,8 +147,6 @@ describe UsersController do end it "cannot change another user's attributes" do - login(member.user) - put "update", id: another_user.id, user: Fabricate.to_params(:user, @@ -134,16 +162,14 @@ describe UsersController do end context "admin user" do - it "locates the requested @user" do - login(member_admin.user) + before { login(admin_user) } + it "locates the requested @user" do put "update", id: user.id, user: Fabricate.to_params(:user) expect(assigns(:user)).to eq(user) end it "changes @user's attributes" do - login(member_admin.user) - put "update", id: user.id, user: Fabricate.to_params(:user, @@ -162,9 +188,9 @@ describe UsersController do context "with invalid params" do context "with a logged admin user" do - it "does not change @user's attributes" do - login(member_admin.user) + before { login(admin_user) } + it "does not change @user's attributes" do put :update, id: user.id, user: Fabricate.to_params(:user, @@ -176,6 +202,20 @@ describe UsersController do expect(user.phone).not_to eq("1234") expect(user.alt_phone).not_to eq("4321") end + + it "cannot create a user with invalid email" do + wrong_user[:email] = "sin mail" + subject { post "create", user: wrong_user } + wrong_user.valid? + wrong_user.errors[:email].count.should > 0 + end + + it "cannot create a user with dummy @example.com" do + user[:email] = "@example.com" + subject { post "create", user: user } + user.valid? + user.errors[:email].count.should > 0 + end end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb new file mode 100644 index 0000000..44032b4 --- /dev/null +++ b/spec/models/user_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe User do + pending "add some examples to (or delete) #{__FILE__}" +end