Commit 63ca9209c01e18938dcb1c09f5545065f794c2a7
1 parent
103b7ebe
Exists in
master
and in
29 other branches
change_password: accept email or login
Also including support for other fields later. (ActionItem2857)
Showing
2 changed files
with
64 additions
and
55 deletions
Show diff stats
app/models/change_password.rb
1 | 1 | class ChangePassword < Task |
2 | 2 | |
3 | - attr_accessor :login, :email, :password, :password_confirmation, :environment_id | |
3 | + settings_items :field, :value | |
4 | + attr_accessor :password, :password_confirmation, :environment_id | |
4 | 5 | |
5 | 6 | def self.human_attribute_name(attrib) |
6 | 7 | case attrib.to_sym |
7 | - when :login: | |
8 | - _('Username') | |
9 | - when :email | |
10 | - _('e-mail') | |
8 | + when :field | |
9 | + _('Field') | |
10 | + when :value | |
11 | + _('Value') | |
11 | 12 | when :password |
12 | 13 | _('Password') |
13 | 14 | when :password_confirmation |
... | ... | @@ -20,25 +21,21 @@ class ChangePassword < Task |
20 | 21 | ################################################### |
21 | 22 | # validations for creating a ChangePassword task |
22 | 23 | |
23 | - validates_presence_of :login, :email, :environment_id, :on => :create, :message => _('must be filled in') | |
24 | + validates_presence_of :field, :value, :environment_id, :on => :create, :message => _('must be filled in') | |
25 | + validates_inclusion_of :field, :in => %w[login email] | |
24 | 26 | |
25 | - validates_format_of :email, :on => :create, :with => Noosfero::Constants::EMAIL_FORMAT, :if => (lambda { |obj| !obj.email.blank? }) | |
26 | - | |
27 | - validates_each :login, :on => :create do |data,attr,value| | |
28 | - unless data.login.blank? || data.email.blank? | |
29 | - user = User.find_by_login_and_environment_id(data.login, data.environment_id) | |
27 | + validates_each :value, :on => :create do |data,attr,value| | |
28 | + unless data.field.blank? || data.value.blank? | |
29 | + user = data.user_find | |
30 | 30 | if user.nil? |
31 | - data.errors.add(:login, _('is invalid or user does not exists.')) | |
32 | - else | |
33 | - if user.email != data.email | |
34 | - data.errors.add(:email, _('does not match the username you filled in')) | |
35 | - end | |
31 | + data.errors.add(:value, _('is invalid for the selected field.')) | |
36 | 32 | end |
37 | 33 | end |
38 | 34 | end |
39 | 35 | |
40 | 36 | before_validation_on_create do |change_password| |
41 | - change_password.requestor = Person.find_by_identifier_and_environment_id(change_password.login, change_password.environment_id) | |
37 | + user = change_password.user_find | |
38 | + change_password.requestor = user.person if user | |
42 | 39 | end |
43 | 40 | |
44 | 41 | ################################################### |
... | ... | @@ -49,6 +46,16 @@ class ChangePassword < Task |
49 | 46 | validates_presence_of :password_confirmation, :on => :update, :if => lambda { |change| change.status != Task::Status::CANCELLED } |
50 | 47 | validates_confirmation_of :password, :if => lambda { |change| change.status != Task::Status::CANCELLED } |
51 | 48 | |
49 | + def user_find | |
50 | + begin | |
51 | + method = "find_by_#{field}_and_environment_id" | |
52 | + user = User.send(method, value, environment_id) | |
53 | + rescue | |
54 | + nil | |
55 | + end | |
56 | + user | |
57 | + end | |
58 | + | |
52 | 59 | def title |
53 | 60 | _("Change password") |
54 | 61 | end | ... | ... |
test/unit/change_password_test.rb
... | ... | @@ -8,63 +8,65 @@ class ChangePasswordTest < ActiveSupport::TestCase |
8 | 8 | data = ChangePassword.new |
9 | 9 | assert !data.valid? |
10 | 10 | end |
11 | - | |
12 | - should 'refuse invalid username' do | |
13 | - User.destroy_all | |
14 | 11 | |
12 | + should 'validate field is login or email' do | |
15 | 13 | data = ChangePassword.new |
16 | - data.login = 'unexisting' | |
17 | - data.email = 'example@example.com' | |
18 | - data.environment_id = Environment.default.id | |
14 | + data.field = 'anything' | |
19 | 15 | data.valid? |
20 | - assert data.errors.invalid?(:login) | |
21 | - end | |
16 | + assert data.errors.invalid?(:field) | |
22 | 17 | |
23 | - should 'require a valid username' do | |
24 | - User.destroy_all | |
25 | - create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com') | |
18 | + data.field = 'login' | |
19 | + data.valid? | |
20 | + assert !data.errors.invalid?(:field) | |
26 | 21 | |
27 | - data = ChangePassword.new | |
28 | - data.login = 'testuser' | |
22 | + data.field = 'email' | |
29 | 23 | data.valid? |
30 | - assert !data.errors.invalid?(:login) | |
24 | + assert !data.errors.invalid?(:field) | |
31 | 25 | end |
32 | 26 | |
33 | - should 'refuse incorrect e-mail address' do | |
27 | + should 'refuse invalid field' do | |
34 | 28 | User.destroy_all |
35 | - create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com') | |
36 | 29 | |
37 | 30 | data = ChangePassword.new |
38 | - data.login = 'testuser' | |
39 | - data.email = 'wrong@example.com' | |
40 | 31 | data.environment_id = Environment.default.id |
41 | 32 | |
33 | + data.field = 'login' | |
34 | + data.value = 'unexisting' | |
35 | + data.valid? | |
36 | + assert data.errors.invalid?(:value) | |
37 | + | |
38 | + data.field = 'email' | |
39 | + data.value = 'example@example.com' | |
42 | 40 | data.valid? |
43 | - assert !data.errors.invalid?(:login) | |
44 | - assert data.errors.invalid?(:email) | |
41 | + assert data.errors.invalid?(:value) | |
45 | 42 | end |
46 | 43 | |
47 | - should 'require the correct e-mail address' do | |
44 | + should 'require only a valid field-value' do | |
48 | 45 | User.destroy_all |
49 | - create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com') | |
46 | + create_user('testuser', :email => 'test@example.com') | |
50 | 47 | |
51 | 48 | data = ChangePassword.new |
52 | - data.login = 'testuser' | |
53 | - data.email = 'test@example.com' | |
54 | 49 | data.environment_id = Environment.default.id |
50 | + assert !data.valid? | |
51 | + assert data.errors.invalid?(:value) | |
55 | 52 | |
53 | + data.field = 'login' | |
54 | + data.value = 'testuser' | |
56 | 55 | data.valid? |
57 | - assert !data.errors.invalid?(:login) | |
58 | - assert !data.errors.invalid?(:email) | |
56 | + assert data.valid? | |
57 | + | |
58 | + data.field = 'email' | |
59 | + data.value = 'test@example.com' | |
60 | + assert data.valid? | |
59 | 61 | end |
60 | 62 | |
61 | 63 | should 'require correct passsword confirmation' do |
62 | 64 | create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com') |
63 | 65 | |
64 | 66 | change = ChangePassword.new |
65 | - change.login = 'testuser' | |
66 | - change.email = 'test@example.com' | |
67 | 67 | change.environment_id = Environment.default.id |
68 | + change.field = 'login' | |
69 | + change.value = 'testuser' | |
68 | 70 | change.save! |
69 | 71 | |
70 | 72 | change.status = Task::Status::FINISHED |
... | ... | @@ -83,9 +85,9 @@ class ChangePasswordTest < ActiveSupport::TestCase |
83 | 85 | person = create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com').person |
84 | 86 | |
85 | 87 | change = ChangePassword.new |
86 | - change.login = 'testuser' | |
87 | - change.email = 'test@example.com' | |
88 | 88 | change.environment_id = Environment.default.id |
89 | + change.field = 'login' | |
90 | + change.value = 'testuser' | |
89 | 91 | change.save! |
90 | 92 | |
91 | 93 | change.expects(:requestor).returns(person).at_least_once |
... | ... | @@ -102,9 +104,9 @@ class ChangePasswordTest < ActiveSupport::TestCase |
102 | 104 | person = create_user('testuser', :password => 'test', :password_confirmation => 'test', :email => 'test@example.com').person |
103 | 105 | |
104 | 106 | change = ChangePassword.new |
105 | - change.login = 'testuser' | |
106 | - change.email = 'test@example.com' | |
107 | 107 | change.environment_id = Environment.default.id |
108 | + change.field = 'login' | |
109 | + change.value = 'testuser' | |
108 | 110 | change.save! |
109 | 111 | |
110 | 112 | assert_nothing_raised do |
... | ... | @@ -125,25 +127,25 @@ class ChangePasswordTest < ActiveSupport::TestCase |
125 | 127 | p1 = create_user('sample-user', :password => 'test', :password_confirmation => 'test', :email => 'sample-user@test.com', :environment => e1).person |
126 | 128 | p2 = create_user('sample-user', :password => 'test', :password_confirmation => 'test', :email => 'sample-user@test.com', :environment => e2).person |
127 | 129 | |
128 | - c1 = ChangePassword.create!(:login => 'sample-user', :email => 'sample-user@test.com', :environment_id => e1.id) | |
129 | - c2 = ChangePassword.create!(:login => 'sample-user', :email => 'sample-user@test.com', :environment_id => e2.id) | |
130 | + c1 = ChangePassword.create!(:field => 'login', :value => 'sample-user', :environment_id => e1.id) | |
131 | + c2 = ChangePassword.create!(:field => 'login', :value => 'sample-user', :environment_id => e2.id) | |
130 | 132 | |
131 | 133 | assert_equal c1.requestor, p1 |
132 | 134 | assert_equal c2.requestor, p2 |
133 | 135 | end |
134 | 136 | |
135 | 137 | should 'have target notification description' do |
136 | - person = fast_create(Person, :identifier => 'testuser') | |
138 | + person = create_user('testuser').person | |
137 | 139 | |
138 | - change = ChangePassword.create(:login => 'testuser', :email => 'test@example.com', :environment_id => Environment.default.id) | |
140 | + change = ChangePassword.create(:field => 'login', :value => 'testuser', :environment_id => Environment.default.id) | |
139 | 141 | |
140 | 142 | assert_match(/#{change.requestor.name} wants to change its password/, change.target_notification_description) |
141 | 143 | end |
142 | 144 | |
143 | 145 | should 'deliver task created message' do |
144 | - person = fast_create(Person, :identifier => 'testuser') | |
146 | + person = create_user('testuser').person | |
145 | 147 | |
146 | - task = ChangePassword.create(:login => 'testuser', :email => 'test@example.com', :environment_id => Environment.default.id) | |
148 | + task = ChangePassword.create(:field => 'login', :value => 'testuser', :environment_id => Environment.default.id) | |
147 | 149 | |
148 | 150 | email = TaskMailer.deliver_task_created(task) |
149 | 151 | assert_match(/#{task.requestor.name} wants to change its password/, email.subject) | ... | ... |