From b94d362125604249790d5ea344401be4d18cfea9 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 13:54:50 +0900 Subject: [PATCH 01/15] Wrap the unlogined spec with new description To distingish specs for logined user with the less indent. --- spec/features/users_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index 5eeb8a2..bfc52ab 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -1,9 +1,11 @@ require "spec_helper" describe "users" do - describe "edit" do + describe "unlogined" do let(:url) { user_path } it_should_behave_like "login required" end + describe "edit" do + end end From 2e2f4f66e825c602e435d5aab76a62c2414f5d29 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 15:50:17 +0900 Subject: [PATCH 02/15] Fix the bug that password is changed unexpectedly when confirm is wrong model spec is broken, so i fix it. --- app/models/user.rb | 4 +++- spec/features/users_spec.rb | 39 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index f432e62..ae410bd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -57,7 +57,9 @@ class User end def valid_password_confirmation - password == password_confirmation + unless password == password_confirmation + errors.add(:current_password, :wrong_password) + end end def stretching_cost diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index bfc52ab..75c336f 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -7,5 +7,44 @@ describe "users" do end describe "edit" do + let!(:user) { build(:user) } + + before do + login_with user + end + + after do + # reset password to the default + FileUtils.rm_rf(User::ENCRYPTED_PASSWORD_FILE) + end + + describe 'to change password' do + let(:password) { 'new_password' } + + before do + visit user_path + fill_in 'user[current_password]', with: user.password + + fill_in 'user[password]', with: password + fill_in 'user[password_confirmation]', with: password_confirmation + find('input[type="submit"]').click + end + + context 'when valid new password/confirmation is input' do + let(:password_confirmation) { password } + + it 'should update users password with new password' do + expect(page).to have_css('.alert-success') + end + end + + context 'when invalid new password/confirmation is input' do + let(:password_confirmation) { 'invalid_password' } + + it 'should not update users password with new password' do + expect(page).to have_css('.alert-danger') + end + end + end end end From 886c8be8e1236d76b9201badf693981744c6b497 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 15:51:13 +0900 Subject: [PATCH 03/15] Fix the broken model spec Before, validated user is invalid, but other validations cause (length, and wrong current_password) --- spec/models/user_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 07c3bb1..933632b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -6,8 +6,9 @@ describe User do describe "#valid?" do describe "password" do it "password != password_confirmation is invalid" do - user.password = "a" - user.password_confirmation = "b" + user.current_password = user.password + user.password = "aaaaaaaa" + user.password_confirmation = "bbbbbbbb" user.should_not be_valid end end From d1f7e7271a70c87d43305b6c72436e25ef0dd178 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 16:09:03 +0900 Subject: [PATCH 04/15] spec: Extract variables by `let` --- spec/models/user_spec.rb | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 933632b..ca0dcd5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -5,11 +5,20 @@ describe User do describe "#valid?" do describe "password" do - it "password != password_confirmation is invalid" do - user.current_password = user.password - user.password = "aaaaaaaa" - user.password_confirmation = "bbbbbbbb" - user.should_not be_valid + before do + user.current_password = current_password + user.password = password + user.password_confirmation = password_confirmation + end + + context 'when password != password_confirmation' do + let(:current_password) { user.password } + let(:password) { 'aaaaaaaa' } + let(:password_confirmation) { 'bbbbbbbb' } + + it 'should be false' do + user.should_not be_valid + end end end end From 7e8a9f154d18eef0dae8cf0c4879e26bf2ba8cb3 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 16:10:43 +0900 Subject: [PATCH 05/15] spec: use subject --- spec/models/user_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ca0dcd5..dd08a4e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4,6 +4,8 @@ describe User do let(:user) { build(:user) } describe "#valid?" do + subject { user.valid? } + describe "password" do before do user.current_password = current_password @@ -16,9 +18,7 @@ describe User do let(:password) { 'aaaaaaaa' } let(:password_confirmation) { 'bbbbbbbb' } - it 'should be false' do - user.should_not be_valid - end + it { should be_falsey } end end end From 8567315956dade7e6d4bc5d959242f4d52986882 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 16:16:58 +0900 Subject: [PATCH 06/15] spec: add specs for User#valid? --- spec/models/user_spec.rb | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index dd08a4e..04b14f6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -13,10 +13,35 @@ describe User do user.password_confirmation = password_confirmation end - context 'when password != password_confirmation' do + context 'when current_password is correct' do let(:current_password) { user.password } + + context 'when password/confirmation is 8 characters' do + let(:password) { 'aaaaaaaa' } + let(:password_confirmation) { password } + + it { should be_truthy } + end + + context 'when password is 7 characters' do + let(:password) { 'aaaaaaa' } + let(:password_confirmation) { password } + + it { should be_falsey } + end + + context 'when password != password_confirmation' do + let(:password) { 'aaaaaaaa' } + let(:password_confirmation) { 'bbbbbbbb' } + + it { should be_falsey } + end + end + + context 'when current_password is wrong' do + let(:current_password) { 'invalid_password' } let(:password) { 'aaaaaaaa' } - let(:password_confirmation) { 'bbbbbbbb' } + let(:password_confirmation) { password } it { should be_falsey } end From c0d90640a3dcc7f077f2cfdb26efe342dc88697b Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 16:40:43 +0900 Subject: [PATCH 07/15] Match method name with its behavior find_user -> set_user ^^^^ --- app/controllers/users_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index da8817c..5eae569 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,5 +1,5 @@ class UsersController < ApplicationController - before_action :find_user + before_action :set_user def show end @@ -14,7 +14,7 @@ class UsersController < ApplicationController private - def find_user + def set_user @user = User.new(name: session[:user_name]) end From 27f550e9eac62e5fa5598eb723aa7a269156d556 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 16:51:58 +0900 Subject: [PATCH 08/15] Use the correct error message to unmatched password with password_confirmation --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index ae410bd..de634fd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -58,7 +58,7 @@ class User def valid_password_confirmation unless password == password_confirmation - errors.add(:current_password, :wrong_password) + errors.add(:password, :confirmation, attribute: User.human_attribute_name(:password_confirmation)) end end From 761322280d15bdf1772d3fd5052058ddd84d2f89 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 18:25:38 +0900 Subject: [PATCH 09/15] Use `if` instead of `unless` for readability --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index de634fd..6860d3e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -57,7 +57,7 @@ class User end def valid_password_confirmation - unless password == password_confirmation + if password != password_confirmation errors.add(:password, :confirmation, attribute: User.human_attribute_name(:password_confirmation)) end end From d5e0dd30266d1a368f453c7b4a6868285c6ecfa9 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 18:41:01 +0900 Subject: [PATCH 10/15] Use more meaningful description --- spec/features/users_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index 75c336f..a4ee4e1 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe "users" do - describe "unlogined" do + describe "visit edit page before login" do let(:url) { user_path } it_should_behave_like "login required" end From d6d7aa0b9baf5dcc5b77c859840af1b2f0caf2da Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 18:41:53 +0900 Subject: [PATCH 11/15] Use buton value instead of type to nallow target --- spec/features/users_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index a4ee4e1..19448e0 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -27,7 +27,7 @@ describe "users" do fill_in 'user[password]', with: password fill_in 'user[password_confirmation]', with: password_confirmation - find('input[type="submit"]').click + click_button I18n.t("terms.update_password") end context 'when valid new password/confirmation is input' do From 2f8561a0718201661d46c2bc5d88f97f96568a01 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 18:45:39 +0900 Subject: [PATCH 12/15] spec: Clarify password lengths --- spec/models/user_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 04b14f6..b236ff2 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -17,22 +17,22 @@ describe User do let(:current_password) { user.password } context 'when password/confirmation is 8 characters' do - let(:password) { 'aaaaaaaa' } + let(:password) { 'a' * 8 } let(:password_confirmation) { password } it { should be_truthy } end context 'when password is 7 characters' do - let(:password) { 'aaaaaaa' } + let(:password) { 'a' * 7 } let(:password_confirmation) { password } it { should be_falsey } end context 'when password != password_confirmation' do - let(:password) { 'aaaaaaaa' } - let(:password_confirmation) { 'bbbbbbbb' } + let(:password) { 'a' * 8 } + let(:password_confirmation) { 'b' * 8 } it { should be_falsey } end @@ -40,7 +40,7 @@ describe User do context 'when current_password is wrong' do let(:current_password) { 'invalid_password' } - let(:password) { 'aaaaaaaa' } + let(:password) { 'a' * 8 } let(:password_confirmation) { password } it { should be_falsey } From c3d18ffe1dcf8896ac153dd69cf35300bd5ac438 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 18:55:22 +0900 Subject: [PATCH 13/15] spec: add expectations for password changed to feature specs --- spec/features/users_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index 19448e0..b746732 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -19,11 +19,12 @@ describe "users" do end describe 'to change password' do + let(:current_password) { user.password } let(:password) { 'new_password' } before do visit user_path - fill_in 'user[current_password]', with: user.password + fill_in 'user[current_password]', with: current_password fill_in 'user[password]', with: password fill_in 'user[password_confirmation]', with: password_confirmation @@ -35,6 +36,7 @@ describe "users" do it 'should update users password with new password' do expect(page).to have_css('.alert-success') + expect(user.stored_digest).to eq user.digest(password) end end @@ -43,6 +45,7 @@ describe "users" do it 'should not update users password with new password' do expect(page).to have_css('.alert-danger') + expect(user.stored_digest).to eq user.digest(current_password) end end end From a23133515998a2fd7eae1855bd0498d3a97f9acc Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 18:58:07 +0900 Subject: [PATCH 14/15] spec: Use more specialized method I want to remove a file only, not directory. --- spec/features/sessions_spec.rb | 2 +- spec/features/users_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/sessions_spec.rb b/spec/features/sessions_spec.rb index bc6e826..39978b0 100644 --- a/spec/features/sessions_spec.rb +++ b/spec/features/sessions_spec.rb @@ -43,7 +43,7 @@ describe "sessions" do after do # reset password to the default - FileUtils.rm_rf(User::ENCRYPTED_PASSWORD_FILE) + FileUtils.rm_f(User::ENCRYPTED_PASSWORD_FILE) end context "correct password" do diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb index b746732..177deb1 100644 --- a/spec/features/users_spec.rb +++ b/spec/features/users_spec.rb @@ -15,7 +15,7 @@ describe "users" do after do # reset password to the default - FileUtils.rm_rf(User::ENCRYPTED_PASSWORD_FILE) + FileUtils.rm_f(User::ENCRYPTED_PASSWORD_FILE) end describe 'to change password' do From 602e07696067930e487502e132d0a46ea97553f6 Mon Sep 17 00:00:00 2001 From: yoshihara Date: Wed, 8 Apr 2015 19:05:22 +0900 Subject: [PATCH 15/15] Add more expectation to User#valid? specs --- spec/models/user_spec.rb | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b236ff2..45f543a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -27,14 +27,20 @@ describe User do let(:password) { 'a' * 7 } let(:password_confirmation) { password } - it { should be_falsey } + it 'should return false' do + should be_falsey + user.errors.keys.should == [:password] + end end context 'when password != password_confirmation' do let(:password) { 'a' * 8 } let(:password_confirmation) { 'b' * 8 } - it { should be_falsey } + it 'should return false' do + should be_falsey + user.errors.keys.should == [:password] + end end end @@ -43,7 +49,10 @@ describe User do let(:password) { 'a' * 8 } let(:password_confirmation) { password } - it { should be_falsey } + it 'should return false' do + should be_falsey + user.errors.keys.should == [:current_password] + end end end end