Improve invalidation of stored service password if the endpoint URL is changed

Password can now be specified at the same time as the new URL, and the service
template admin pages now work.

3 files changed, 215 insertions, 42 deletions

diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index f8a3493f52d..c34b2487ecf 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -30,27 +30,65 @@ describe BambooService, models: true do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
 
-    before do
-      @bamboo_service = BambooService.create(
-        project: create(:project),
-        properties: {
-          bamboo_url: 'http://gitlab.com',
-          username: 'mic',
-          password: "password"
-        }
-      )
-    end
+    context "when a password was previously set" do
+      before do
+        @bamboo_service = BambooService.create(
+          project: create(:project),
+          properties: {
+            bamboo_url: 'http://gitlab.com',
+            username: 'mic',
+            password: "password"
+          }
+        )
+      end
+  
+      it "reset password if url changed" do
+        @bamboo_service.bamboo_url = 'http://gitlab1.com'
+        @bamboo_service.save
+        expect(@bamboo_service.password).to be_nil
+      end
+  
+      it "does not reset password if username changed" do
+        @bamboo_service.username = "some_name"
+        @bamboo_service.save
+        expect(@bamboo_service.password).to eq("password")
+      end
+
+      it "does not reset password if new url is set together with password, even if it's the same password" do
+        @bamboo_service.bamboo_url = 'http://gitlab_edited.com'
+        @bamboo_service.password = 'password'
+        @bamboo_service.save
+        expect(@bamboo_service.password).to eq("password")
+        expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
+      end
 
-    it "reset password if url changed" do
-      @bamboo_service.bamboo_url = 'http://gitlab1.com'
-      @bamboo_service.save
-      expect(@bamboo_service.password).to be_nil
+      it "should reset password if url changed, even if setter called multiple times" do
+        @bamboo_service.bamboo_url = 'http://gitlab1.com'
+        @bamboo_service.bamboo_url = 'http://gitlab1.com'
+        @bamboo_service.save
+        expect(@bamboo_service.password).to be_nil
+      end
     end
+    
+    context "when no password was previously set" do
+      before do
+        @bamboo_service = BambooService.create(
+          project: create(:project),
+          properties: {
+            bamboo_url: 'http://gitlab.com',
+            username: 'mic'
+          }
+        )
+      end
+
+      it "saves password if new url is set together with password" do
+        @bamboo_service.bamboo_url = 'http://gitlab_edited.com'
+        @bamboo_service.password = 'password'
+        @bamboo_service.save
+        expect(@bamboo_service.password).to eq("password")
+        expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
+      end
 
-    it "does not reset password if username changed" do
-      @bamboo_service.username = "some_name"
-      @bamboo_service.save
-      expect(@bamboo_service.password).to eq("password")
     end
   end
 end
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index 3dbd2346bcc..f26b47a856c 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -30,27 +30,64 @@ describe TeamcityService, models: true do
     let(:user)    { create(:user) }
     let(:project) { create(:project) }
 
-    before do
-      @teamcity_service = TeamcityService.create(
-        project: create(:project),
-        properties: {
-          teamcity_url: 'http://gitlab.com',
-          username: 'mic',
-          password: "password"
-        }
-      )
-    end
+    context "when a password was previously set" do
+      before do
+        @teamcity_service = TeamcityService.create(
+          project: create(:project),
+          properties: {
+            teamcity_url: 'http://gitlab.com',
+            username: 'mic',
+            password: "password"
+          }
+        )
+      end
+  
+      it "reset password if url changed" do
+        @teamcity_service.teamcity_url = 'http://gitlab1.com'
+        @teamcity_service.save
+        expect(@teamcity_service.password).to be_nil
+      end
+  
+      it "does not reset password if username changed" do
+        @teamcity_service.username = "some_name"
+        @teamcity_service.save
+        expect(@teamcity_service.password).to eq("password")
+      end
+
+      it "does not reset password if new url is set together with password, even if it's the same password" do
+        @teamcity_service.teamcity_url = 'http://gitlab_edited.com'
+        @teamcity_service.password = 'password'
+        @teamcity_service.save
+        expect(@teamcity_service.password).to eq("password")
+        expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com")
+      end
 
-    it "reset password if url changed" do
-      @teamcity_service.teamcity_url = 'http://gitlab1.com'
-      @teamcity_service.save
-      expect(@teamcity_service.password).to be_nil
+      it "should reset password if url changed, even if setter called multiple times" do
+        @teamcity_service.teamcity_url = 'http://gitlab1.com'
+        @teamcity_service.teamcity_url = 'http://gitlab1.com'
+        @teamcity_service.save
+        expect(@teamcity_service.password).to be_nil
+      end
     end
+    
+    context "when no password was previously set" do
+      before do
+        @teamcity_service = TeamcityService.create(
+          project: create(:project),
+          properties: {
+            teamcity_url: 'http://gitlab.com',
+            username: 'mic'
+          }
+        )
+      end
 
-    it "does not reset password if username changed" do
-      @teamcity_service.username = "some_name"
-      @teamcity_service.save
-      expect(@teamcity_service.password).to eq("password")
+      it "saves password if new url is set together with password" do
+        @teamcity_service.teamcity_url = 'http://gitlab_edited.com'
+        @teamcity_service.password = 'password'
+        @teamcity_service.save
+        expect(@teamcity_service.password).to eq("password")
+        expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com")
+      end
     end
   end
 end
diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb
index da87ea5b84f..692e5fda3ba 100644
--- a/spec/models/service_spec.rb
+++ b/spec/models/service_spec.rb
@@ -104,7 +104,7 @@ describe Service do
     end
   end
 
-  describe "#prop_updated?" do
+  describe "{property}_changed?" do
     let(:service) do
       BambooService.create(
         project: create(:project),
@@ -116,14 +116,112 @@ describe Service do
       )
     end
 
-    it "returns false" do
+    it "returns false when the property has not been assigned a new value" do
       service.username = "key_changed"
-      expect(service.prop_updated?(:bamboo_url)).to be_falsy
+      expect(service.bamboo_url_changed?).to be_falsy
     end
 
-    it "returns true" do
-      service.bamboo_url = "http://other.com"
-      expect(service.prop_updated?(:bamboo_url)).to be_truthy
+    it "returns true when the property has been assigned a different value" do
+      service.bamboo_url = "http://example.com"
+      expect(service.bamboo_url_changed?).to be_truthy
+    end
+
+    it "returns true when the property has been assigned a different value twice" do
+      service.bamboo_url = "http://example.com"
+      service.bamboo_url = "http://example.com"
+      expect(service.bamboo_url_changed?).to be_truthy
+    end
+
+    it "returns false when the property has been re-assigned the same value" do
+      service.bamboo_url = 'http://gitlab.com'
+      expect(service.bamboo_url_changed?).to be_falsy
+    end
+
+    it "returns false when the property has been assigned a new value then saved" do
+      service.bamboo_url = 'http://example.com'
+      service.save
+      expect(service.bamboo_url_changed?).to be_falsy
+    end
+  end
+
+  describe "{property}_touched?" do
+    let(:service) do
+      BambooService.create(
+        project: create(:project),
+        properties: {
+          bamboo_url: 'http://gitlab.com',
+          username: 'mic',
+          password: "password"
+        }
+      )
+    end
+
+    it "returns false when the property has not been assigned a new value" do
+      service.username = "key_changed"
+      expect(service.bamboo_url_touched?).to be_falsy
+    end
+
+    it "returns true when the property has been assigned a different value" do
+      service.bamboo_url = "http://example.com"
+      expect(service.bamboo_url_touched?).to be_truthy
+    end
+
+    it "returns true when the property has been assigned a different value twice" do
+      service.bamboo_url = "http://example.com"
+      service.bamboo_url = "http://example.com"
+      expect(service.bamboo_url_touched?).to be_truthy
+    end
+
+    it "returns true when the property has been re-assigned the same value" do
+      service.bamboo_url = 'http://gitlab.com'
+      expect(service.bamboo_url_touched?).to be_truthy
+    end
+
+    it "returns false when the property has been assigned a new value then saved" do
+      service.bamboo_url = 'http://example.com'
+      service.save
+      expect(service.bamboo_url_changed?).to be_falsy
+    end
+  end
+
+  describe "{property}_was" do
+    let(:service) do
+      BambooService.create(
+        project: create(:project),
+        properties: {
+          bamboo_url: 'http://gitlab.com',
+          username: 'mic',
+          password: "password"
+        }
+      )
+    end
+
+
+    it "returns nil when the property has not been assigned a new value" do
+      service.username = "key_changed"
+      expect(service.bamboo_url_was).to be_nil
+    end
+
+    it "returns the previous value when the property has been assigned a different value" do
+      service.bamboo_url = "http://example.com"
+      expect(service.bamboo_url_was).to eq('http://gitlab.com')
+    end
+
+    it "returns initial value when the property has been re-assigned the same value" do
+      service.bamboo_url = 'http://gitlab.com'
+      expect(service.bamboo_url_was).to eq('http://gitlab.com')
+    end
+
+    it "returns initial value when the property has been assigned multiple values" do
+      service.bamboo_url = "http://example.com"
+      service.bamboo_url = "http://example2.com"
+      expect(service.bamboo_url_was).to eq('http://gitlab.com')
+    end
+
+    it "returns nil when the property has been assigned a new value then saved" do
+      service.bamboo_url = 'http://example.com'
+      service.save
+      expect(service.bamboo_url_was).to be_nil
     end
   end
 end