From 2e7696630b0219b2d4c2ed6a2e7ed0a39cabb1dc Mon Sep 17 00:00:00 2001
From: Scott Albertson <scott@thoughtbot.com>
Date: Fri, 1 Nov 2013 11:12:25 -0700
Subject: [PATCH 1/4] Make #update specs consistent

* Use expect syntax
* Avoid lets
* Stub Guardian method used in the controller
---
 spec/controllers/users_controller_spec.rb | 52 ++++++++++++-----------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 609242792c5..9ed7902d578 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -824,40 +824,43 @@ describe UsersController do
 
   end
 
-  describe '.update' do
-
-    context 'not logged in' do
-      it 'raises an error when not logged in' do
+  describe '#update' do
+    context 'with guest' do
+      it 'raises an error' do
         expect do
-          xhr :put, :update, username: 'somename'
+          xhr :put, :update, username: 'guest'
         end.to raise_error(Discourse::NotLoggedIn)
       end
     end
 
-    context 'logged in' do
-      let!(:user) { log_in }
+    context 'with authenticated user' do
+      context 'with permission to update' do
+        it 'allows the update' do
+          user = Fabricate(:user, name: 'Billy Bob')
+          log_in_user(user)
+          guardian = Guardian.new(user)
+          guardian.stubs(:ensure_can_edit!)
+          Guardian.stubs(new: guardian).with(user)
 
-      context 'without a token' do
-        it 'should ensure you can update the user' do
-          Guardian.any_instance.expects(:can_edit?).with(user).returns(false)
-          put :update, username: user.username
-          response.should be_forbidden
+          put :update, username: user.username, name: 'Jim Tom'
+
+          expect(response).to be_success
+          expect(user.reload.name).to eq 'Jim Tom'
         end
+      end
 
-        context 'as a user who can edit the user' do
+      context 'without permission to update' do
+        it 'does not allow the update' do
+          user = Fabricate(:user, name: 'Billy Bob')
+          log_in_user(user)
+          guardian = Guardian.new(user)
+          guardian.stubs(:ensure_can_edit!).raises(Discourse::InvalidAccess.new)
+          Guardian.stubs(new: guardian).with(user)
 
-          before do
-            put :update, username: user.username, bio_raw: 'brand new bio'
-            user.reload
-          end
+          put :update, username: user.username, name: 'Jim Tom'
 
-          it 'updates the user' do
-            user.bio_raw.should == 'brand new bio'
-          end
-
-          it 'returns json success' do
-            response.should be_success
-          end
+          expect(response).to be_forbidden
+          expect(user.reload.name).not_to eq 'Jim Tom'
         end
       end
     end
@@ -1102,5 +1105,4 @@ describe UsersController do
     end
 
   end
-
 end

From 58f96bdfb5a94b9df2b5a915f044d06a08bbc465 Mon Sep 17 00:00:00 2001
From: Scott Albertson <scott@thoughtbot.com>
Date: Fri, 1 Nov 2013 11:22:52 -0700
Subject: [PATCH 2/4] Remove duplication in test setup

---
 spec/controllers/users_controller_spec.rb | 31 +++++++++++++++--------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 9ed7902d578..c378af6d956 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -836,11 +836,10 @@ describe UsersController do
     context 'with authenticated user' do
       context 'with permission to update' do
         it 'allows the update' do
-          user = Fabricate(:user, name: 'Billy Bob')
-          log_in_user(user)
-          guardian = Guardian.new(user)
-          guardian.stubs(:ensure_can_edit!)
-          Guardian.stubs(new: guardian).with(user)
+          user = create_authenticated_user('Billy Bob')
+          stub_guardian(user) do |guardian|
+            guardian.stubs(:ensure_can_edit!).with(user)
+          end
 
           put :update, username: user.username, name: 'Jim Tom'
 
@@ -851,11 +850,11 @@ describe UsersController do
 
       context 'without permission to update' do
         it 'does not allow the update' do
-          user = Fabricate(:user, name: 'Billy Bob')
-          log_in_user(user)
-          guardian = Guardian.new(user)
-          guardian.stubs(:ensure_can_edit!).raises(Discourse::InvalidAccess.new)
-          Guardian.stubs(new: guardian).with(user)
+          user = create_authenticated_user('Billy Bob')
+          stub_guardian(user) do |guardian|
+            guardian.stubs(:ensure_can_edit!).
+              with(user).raises(Discourse::InvalidAccess.new)
+          end
 
           put :update, username: user.username, name: 'Jim Tom'
 
@@ -1105,4 +1104,16 @@ describe UsersController do
     end
 
   end
+
+  private
+
+  def create_authenticated_user(name)
+    log_in_user(Fabricate(:user, name: name))
+  end
+
+  def stub_guardian(user)
+    guardian = Guardian.new(user)
+    yield(guardian)
+    Guardian.stubs(new: guardian).with(user)
+  end
 end

From 3cc17ad4cdc4a08853b9dda6b90c1e4d0aab3afd Mon Sep 17 00:00:00 2001
From: Scott Albertson <scott@thoughtbot.com>
Date: Fri, 1 Nov 2013 11:42:16 -0700
Subject: [PATCH 3/4] Add test coverage for #update

---
 spec/controllers/users_controller_spec.rb | 57 ++++++++++++++---------
 1 file changed, 36 insertions(+), 21 deletions(-)

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index c378af6d956..f8539afd7c1 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -836,25 +836,52 @@ describe UsersController do
     context 'with authenticated user' do
       context 'with permission to update' do
         it 'allows the update' do
-          user = create_authenticated_user('Billy Bob')
-          stub_guardian(user) do |guardian|
-            guardian.stubs(:ensure_can_edit!).with(user)
-          end
+          user = Fabricate(:user, name: 'Billy Bob')
+          log_in_user(user)
 
           put :update, username: user.username, name: 'Jim Tom'
 
           expect(response).to be_success
           expect(user.reload.name).to eq 'Jim Tom'
         end
+
+        it 'returns user JSON' do
+          user = log_in
+
+          put :update, username: user.username
+
+          json = JSON.parse(response.body)
+          expect(json['user']['id']).to eq user.id
+        end
+
+        context 'when website includes http' do
+          it 'does not add http before updating' do
+            user = log_in
+
+            put :update, username: user.username, website: 'http://example.com'
+
+            expect(user.reload.website).to eq 'http://example.com'
+          end
+        end
+
+        context 'when website does not include http' do
+          it 'adds http before updating' do
+            user = log_in
+
+            put :update, username: user.username, website: 'example.com'
+
+            expect(user.reload.website).to eq 'http://example.com'
+          end
+        end
       end
 
       context 'without permission to update' do
         it 'does not allow the update' do
-          user = create_authenticated_user('Billy Bob')
-          stub_guardian(user) do |guardian|
-            guardian.stubs(:ensure_can_edit!).
-              with(user).raises(Discourse::InvalidAccess.new)
-          end
+          user = Fabricate(:user, name: 'Billy Bob')
+          log_in_user(user)
+          guardian = Guardian.new(user)
+          guardian.stubs(:ensure_can_edit!).with(user).raises(Discourse::InvalidAccess.new)
+          Guardian.stubs(new: guardian).with(user)
 
           put :update, username: user.username, name: 'Jim Tom'
 
@@ -1104,16 +1131,4 @@ describe UsersController do
     end
 
   end
-
-  private
-
-  def create_authenticated_user(name)
-    log_in_user(Fabricate(:user, name: name))
-  end
-
-  def stub_guardian(user)
-    guardian = Guardian.new(user)
-    yield(guardian)
-    Guardian.stubs(new: guardian).with(user)
-  end
 end

From c0cffca1e65d7db4d5a6fe0edbb30438dcc03727 Mon Sep 17 00:00:00 2001
From: Scott Albertson <scott@thoughtbot.com>
Date: Fri, 1 Nov 2013 13:27:48 -0700
Subject: [PATCH 4/4] Test title updating

---
 spec/controllers/users_controller_spec.rb | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index f8539afd7c1..d5d2a24f0b6 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -875,7 +875,7 @@ describe UsersController do
         end
       end
 
-      context 'without permission to update' do
+      context 'without permission to update any attributes' do
         it 'does not allow the update' do
           user = Fabricate(:user, name: 'Billy Bob')
           log_in_user(user)
@@ -889,6 +889,20 @@ describe UsersController do
           expect(user.reload.name).not_to eq 'Jim Tom'
         end
       end
+
+      context 'without permission to update title' do
+        it 'does not allow the user to update their title' do
+          user = Fabricate(:user, title: 'Emperor')
+          log_in_user(user)
+          guardian = Guardian.new(user)
+          guardian.stubs(can_grant_title?: false).with(user)
+          Guardian.stubs(new: guardian).with(user)
+
+          put :update, username: user.username, title: 'Minion'
+
+          expect(user.reload.title).not_to eq 'Minion'
+        end
+      end
     end
   end