diff --git a/app/models/change_password.rb b/app/models/change_password.rb index 9cf32c6..89656c0 100644 --- a/app/models/change_password.rb +++ b/app/models/change_password.rb @@ -1,14 +1,12 @@ class ChangePassword < Task - settings_items :field, :value + settings_items :value attr_accessor :password, :password_confirmation, :environment_id include Noosfero::Plugin::HotSpot def self.human_attribute_name(attrib) case attrib.to_sym - when :field - _('Field') when :value _('Value') when :password @@ -20,49 +18,54 @@ class ChangePassword < Task end end - def plugins_fields - plugins.dispatch(:change_password_fields).inject({}) { |result, fields| result.merge!(fields)} - end - def environment (requestor.environment if requestor) || Environment.find_by_id(environment_id) end + def plugins_options + plugins.dispatch(:change_password_fields) + end + + def user_fields + %w[login email] + plugins_options.select {|options| options[:model].to_sym == :user }.map { |options| options[:field].to_s } + end + + def person_fields + %w[] + plugins_options.select {|options| options[:model].to_sym == :person }.map { |options| options[:field].to_s } + end + def fields - %w[login email] + plugins_fields.map { |field, name| field.to_s } + user_fields + person_fields end - def fields_choice - [ - [_('Username'), 'login'], - [_('Email'), 'email'], - ] + plugins_fields.map { |field, name| [name, field] } + def fields_label + labels = [ + _('Username'), + _('Email'), + ] + plugins_options.map { |options| options[:name] } + + last = labels.pop + label = labels.join(', ') + "#{label} #{_('or')} #{last}" end ################################################### # validations for creating a ChangePassword task - validates_presence_of :field, :value, :environment_id, :on => :create, :message => _('must be filled in') - # TODO Only on rails3 - # validates_inclusion_of :field, :in => lambda { |data| data.fields } - validates_each :field do |data, attr, value| - unless data.fields.include?(value) - data.errors.add(attr, _('is not in the list of valid fields.')) - end - end + validates_presence_of :value, :environment_id, :on => :create, :message => _('must be filled in') validates_each :value, :on => :create do |data,attr,value| - unless data.field.blank? || data.value.blank? - user = data.user_find - if user.nil? - data.errors.add(:value, _('"%s" is not a valid %s.') % [value.to_s, human_attribute_name(data.field)]) + unless data.value.blank? + users = data.find_users + if users.blank? + data.errors.add(:value, _('"%s" is not valid.') % value.to_s) end end end before_validation do |change_password| - user = change_password.user_find - change_password.requestor = user.person if user + users = change_password.find_users + change_password.requestor ||= users.first.person if users.present? end ################################################### @@ -73,15 +76,18 @@ class ChangePassword < Task validates_presence_of :password_confirmation, :on => :update, :if => lambda { |change| change.status != Task::Status::CANCELLED } validates_confirmation_of :password, :if => lambda { |change| change.status != Task::Status::CANCELLED } - def user_find - begin - method = "find_by_#{field}_and_environment_id" - user = nil - user = User.send(method, value, environment_id) if User.respond_to?(method) - user = Person.send(method, value, environment_id).user if user.nil? && Person.respond_to?(method) - rescue - end - user + def build_query(fields) + fields.map {|field| "#{field} = '#{value}'"}.join(' OR ') + end + + def find_users + results = [] + person_query = build_query(person_fields) + user_query = build_query(user_fields) + + results += Person.where(person_query).where(:environment_id => environment.id).map(&:user) + results += User.where(user_query).where(:environment_id => environment.id) + results end def title diff --git a/app/views/account/forgot_password.rhtml b/app/views/account/forgot_password.rhtml index ba48087..da72d24 100644 --- a/app/views/account/forgot_password.rhtml +++ b/app/views/account/forgot_password.rhtml @@ -4,8 +4,7 @@ <% labelled_form_for :change_password, @change_password, :url => { :action => 'forgot_password' } do |f| %> - <%= labelled_form_field(_('What do you remember?'), f.select(:field, @change_password.fields_choice)) %> - <%= text_field_tag('change_password[value]') %> + <%= labelled_form_field @change_password.fields_label, text_field_tag('change_password[value]') %>
<% button_bar do %> diff --git a/lib/noosfero/plugin.rb b/lib/noosfero/plugin.rb index 0070688..7be3581 100644 --- a/lib/noosfero/plugin.rb +++ b/lib/noosfero/plugin.rb @@ -461,7 +461,7 @@ class Noosfero::Plugin end # -> Adds aditional fields for change_password - # returns = {:field1 => 'field1 name', :field2 => 'field2 name' ... } + # returns = [{:field => 'field1', :name => 'field 1 name', :model => 'person'}, {...}] def change_password_fields nil end diff --git a/plugins/stoa/lib/stoa_plugin.rb b/plugins/stoa/lib/stoa_plugin.rb index 05e908d..96d1457 100644 --- a/plugins/stoa/lib/stoa_plugin.rb +++ b/plugins/stoa/lib/stoa_plugin.rb @@ -121,7 +121,7 @@ class StoaPlugin < Noosfero::Plugin end def change_password_fields - {:usp_id => _('USP Number')} + {:field => :usp_id, :name => _('USP Number'), :model => 'person'} end end diff --git a/plugins/stoa/test/functional/account_controller_test.rb b/plugins/stoa/test/functional/account_controller_test.rb index 86af2c2..32f8b5c 100644 --- a/plugins/stoa/test/functional/account_controller_test.rb +++ b/plugins/stoa/test/functional/account_controller_test.rb @@ -67,7 +67,7 @@ class AccountControllerTest < ActionController::TestCase end should 'be able to recover password with usp_id' do - post :forgot_password, :change_password => { :field => 'usp_id', :value => '87654321' } + post :forgot_password, :change_password => { :value => '87654321' } assert_template 'password_recovery_sent' end end diff --git a/test/functional/account_controller_test.rb b/test/functional/account_controller_test.rb index 330cd65..226e115 100644 --- a/test/functional/account_controller_test.rb +++ b/test/functional/account_controller_test.rb @@ -217,19 +217,19 @@ class AccountControllerTest < ActionController::TestCase should 'respond to forgotten password change request with login' do change = ChangePassword.new - ChangePassword.expects(:new).with('field' => 'login', 'value' => 'test').returns(change) + ChangePassword.expects(:new).with('value' => 'test').returns(change) change.expects(:save!).returns(true) - post :forgot_password, :change_password => { :field => 'login', :value => 'test' } + post :forgot_password, :change_password => { :value => 'test' } assert_template 'password_recovery_sent' end should 'respond to forgotten password change request with email' do change = ChangePassword.new - ChangePassword.expects(:new).with('field' => 'email', 'value' => 'test@localhost.localdomain').returns(change) + ChangePassword.expects(:new).with('value' => 'test@localhost.localdomain').returns(change) change.expects(:save!).returns(true) - post :forgot_password, :change_password => { :field => 'email', :value => 'test@localhost.localdomain' } + post :forgot_password, :change_password => { :value => 'test@localhost.localdomain' } assert_template 'password_recovery_sent' end @@ -276,7 +276,7 @@ class AccountControllerTest < ActionController::TestCase should 'require password confirmation correctly to enter new pasword' do user = create_user('testuser', :email => 'testuser@example.com', :password => 'test', :password_confirmation => 'test') - change = ChangePassword.create!(:field => 'login', :value => 'testuser', :environment_id => Environment.default.id) + change = ChangePassword.create!(:value => 'testuser', :environment_id => Environment.default.id) post :new_password, :code => change.code, :change_password => { :password => 'onepass', :password_confirmation => 'another_pass' } assert_response :success @@ -862,7 +862,7 @@ class AccountControllerTest < ActionController::TestCase assert_response :redirect #Redirect on post action - post :forgot_password, :change_password => { :field => 'login', :value => 'test' } + post :forgot_password, :change_password => { :value => 'test' } assert_response :redirect end diff --git a/test/unit/change_password_test.rb b/test/unit/change_password_test.rb index 420be42..0d1c72a 100644 --- a/test/unit/change_password_test.rb +++ b/test/unit/change_password_test.rb @@ -9,39 +9,7 @@ class ChangePasswordTest < ActiveSupport::TestCase assert !data.valid? end - should 'validate field is login or email' do - data = ChangePassword.new(:environment_id => Environment.default) - data.field = 'anything' - data.valid? - assert data.errors.invalid?(:field) - - data.field = 'login' - data.valid? - assert !data.errors.invalid?(:field) - - data.field = 'email' - data.valid? - assert !data.errors.invalid?(:field) - end - - should 'refuse invalid field' do - User.destroy_all - - data = ChangePassword.new - data.environment_id = Environment.default.id - - data.field = 'login' - data.value = 'unexisting' - data.valid? - assert data.errors.invalid?(:value) - - data.field = 'email' - data.value = 'example@example.com' - data.valid? - assert data.errors.invalid?(:value) - end - - should 'require only a valid field-value' do + should 'require only a valid value' do User.destroy_all create_user('testuser', :email => 'test@example.com') @@ -50,12 +18,10 @@ class ChangePasswordTest < ActiveSupport::TestCase assert !data.valid? assert data.errors.invalid?(:value) - data.field = 'login' data.value = 'testuser' data.valid? assert data.valid? - data.field = 'email' data.value = 'test@example.com' assert data.valid? end @@ -65,7 +31,6 @@ class ChangePasswordTest < ActiveSupport::TestCase change = ChangePassword.new change.environment_id = Environment.default.id - change.field = 'login' change.value = 'testuser' change.save! @@ -86,7 +51,6 @@ class ChangePasswordTest < ActiveSupport::TestCase change = ChangePassword.new change.environment_id = Environment.default.id - change.field = 'login' change.value = 'testuser' change.save! @@ -105,7 +69,6 @@ class ChangePasswordTest < ActiveSupport::TestCase change = ChangePassword.new change.environment_id = Environment.default.id - change.field = 'login' change.value = 'testuser' change.save! @@ -127,8 +90,8 @@ class ChangePasswordTest < ActiveSupport::TestCase p1 = create_user('sample-user', :password => 'test', :password_confirmation => 'test', :email => 'sample-user@test.com', :environment => e1).person p2 = create_user('sample-user', :password => 'test', :password_confirmation => 'test', :email => 'sample-user@test.com', :environment => e2).person - c1 = ChangePassword.create!(:field => 'login', :value => 'sample-user', :environment_id => e1.id) - c2 = ChangePassword.create!(:field => 'login', :value => 'sample-user', :environment_id => e2.id) + c1 = ChangePassword.create!(:value => 'sample-user', :environment_id => e1.id) + c2 = ChangePassword.create!(:value => 'sample-user', :environment_id => e2.id) assert_equal c1.requestor, p1 assert_equal c2.requestor, p2 @@ -137,7 +100,7 @@ class ChangePasswordTest < ActiveSupport::TestCase should 'have target notification description' do person = create_user('testuser').person - change = ChangePassword.create(:field => 'login', :value => 'testuser', :environment_id => Environment.default.id) + change = ChangePassword.create(:value => 'testuser', :environment_id => Environment.default.id) assert_match(/#{change.requestor.name} wants to change its password/, change.target_notification_description) end @@ -145,7 +108,7 @@ class ChangePasswordTest < ActiveSupport::TestCase should 'deliver task created message' do person = create_user('testuser').person - task = ChangePassword.create(:field => 'login', :value => 'testuser', :environment_id => Environment.default.id) + task = ChangePassword.create(:value => 'testuser', :environment_id => Environment.default.id) email = TaskMailer.deliver_task_created(task) assert_match(/#{task.requestor.name} wants to change its password/, email.subject) @@ -154,12 +117,13 @@ class ChangePasswordTest < ActiveSupport::TestCase should 'allow extra fields provided by plugins' do class Plugin1 < Noosfero::Plugin def change_password_fields - {:f1 => 'F1'} + {:field => 'f1', :name => 'F1', :model => 'person'} end end class Plugin2 < Noosfero::Plugin def change_password_fields - {:f2 => 'F2', :f3 => 'F3'} + [{:field => 'f2', :name => 'F2', :model => 'person'}, + {:field => 'f3', :name => 'F3', :model => 'person'}] end end -- libgit2 0.21.2