From 0568b90c97dcbad3ab100e060fef91e0786aafe8 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 10 Jun 2016 16:53:20 -0300
Subject: [PATCH] Remove deprecated issues_tracker and issues_tracker_id from
 project

---
 app/models/project.rb                         |  5 -----
 .../project_services/issue_tracker_service.rb | 18 +++------------
 ...ed_issues_tracker_columns_from_projects.rb |  6 +++++
 db/schema.rb                                  |  2 --
 spec/factories/projects.rb                    |  6 -----
 spec/helpers/issues_helper_spec.rb            | 16 +++-----------
 spec/models/project_spec.rb                   | 22 -------------------
 7 files changed, 12 insertions(+), 63 deletions(-)
 create mode 100644 db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb

diff --git a/app/models/project.rb b/app/models/project.rb
index e2f7ffe493c..dfa99fe0df2 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -146,7 +146,6 @@ class Project < ActiveRecord::Base
               message: Gitlab::Regex.project_path_regex_message }
   validates :issues_enabled, :merge_requests_enabled,
             :wiki_enabled, inclusion: { in: [true, false] }
-  validates :issues_tracker_id, length: { maximum: 255 }, allow_blank: true
   validates :namespace, presence: true
   validates_uniqueness_of :name, scope: :namespace_id
   validates_uniqueness_of :path, scope: :namespace_id
@@ -589,10 +588,6 @@ class Project < ActiveRecord::Base
     update_column(:has_external_issue_tracker, services.external_issue_trackers.any?)
   end
 
-  def can_have_issues_tracker_id?
-    self.issues_enabled && !self.default_issues_tracker?
-  end
-
   def build_missing_services
     services_templates = Service.where(template: true)
 
diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb
index 6ae9b16d3ce..87ecb3b8b86 100644
--- a/app/models/project_services/issue_tracker_service.rb
+++ b/app/models/project_services/issue_tracker_service.rb
@@ -38,9 +38,9 @@ class IssueTrackerService < Service
       if enabled_in_gitlab_config
         self.properties = {
           title: issues_tracker['title'],
-          project_url: add_issues_tracker_id(issues_tracker['project_url']),
-          issues_url: add_issues_tracker_id(issues_tracker['issues_url']),
-          new_issue_url: add_issues_tracker_id(issues_tracker['new_issue_url'])
+          project_url: issues_tracker['project_url'],
+          issues_url: issues_tracker['issues_url'],
+          new_issue_url: issues_tracker['new_issue_url']
         }
       else
         self.properties = {}
@@ -83,16 +83,4 @@ class IssueTrackerService < Service
   def issues_tracker
     Gitlab.config.issues_tracker[to_param]
   end
-
-  def add_issues_tracker_id(url)
-    if self.project
-      id = self.project.issues_tracker_id
-
-      if id
-        url = url.gsub(":issues_tracker_id", id)
-      end
-    end
-
-    url
-  end
 end
diff --git a/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb b/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb
new file mode 100644
index 00000000000..477b2106dea
--- /dev/null
+++ b/db/migrate/20160610194713_remove_deprecated_issues_tracker_columns_from_projects.rb
@@ -0,0 +1,6 @@
+class RemoveDeprecatedIssuesTrackerColumnsFromProjects < ActiveRecord::Migration
+  def change
+    remove_column :projects, :issues_tracker, :string, default: 'gitlab', null: false
+    remove_column :projects, :issues_tracker_id, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 3c947d62e82..3dccbbd50ba 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -751,8 +751,6 @@ ActiveRecord::Schema.define(version: 20160610301627) do
     t.boolean  "merge_requests_enabled",             default: true,     null: false
     t.boolean  "wiki_enabled",                       default: true,     null: false
     t.integer  "namespace_id"
-    t.string   "issues_tracker",                     default: "gitlab", null: false
-    t.string   "issues_tracker_id"
     t.boolean  "snippets_enabled",                   default: true,     null: false
     t.datetime "last_activity_at"
     t.string   "import_url"
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index da8d97c9f82..5c8ddbebf0d 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -67,9 +67,6 @@ FactoryGirl.define do
           'new_issue_url' => 'http://redmine/projects/project_name_in_redmine/issues/new'
         }
       )
-
-      project.issues_tracker = 'redmine'
-      project.issues_tracker_id = 'project_name_in_redmine'
     end
   end
 
@@ -84,9 +81,6 @@ FactoryGirl.define do
           'new_issue_url' => 'http://jira.example/secure/CreateIssue.jspa'
         }
       )
-
-      project.issues_tracker = 'jira'
-      project.issues_tracker_id = 'project_name_in_jira'
     end
   end
 end
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index eae61a54dfc..831ae7fb69c 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -7,10 +7,7 @@ describe IssuesHelper do
 
   describe "url_for_project_issues" do
     let(:project_url) { ext_project.external_issue_tracker.project_url }
-    let(:ext_expected) do
-      project_url.gsub(':project_id', ext_project.id.to_s)
-                 .gsub(':issues_tracker_id', ext_project.issues_tracker_id.to_s)
-    end
+    let(:ext_expected) { project_url.gsub(':project_id', ext_project.id.to_s) }
     let(:int_expected) { polymorphic_path([@project.namespace, project]) }
 
     it "should return internal path if used internal tracker" do
@@ -56,11 +53,7 @@ describe IssuesHelper do
 
   describe "url_for_issue" do
     let(:issues_url) { ext_project.external_issue_tracker.issues_url}
-    let(:ext_expected) do
-      issues_url.gsub(':id', issue.iid.to_s)
-        .gsub(':project_id', ext_project.id.to_s)
-        .gsub(':issues_tracker_id', ext_project.issues_tracker_id.to_s)
-    end
+    let(:ext_expected) { issues_url.gsub(':id', issue.iid.to_s).gsub(':project_id', ext_project.id.to_s) }
     let(:int_expected) { polymorphic_path([@project.namespace, project, issue]) }
 
     it "should return internal path if used internal tracker" do
@@ -106,10 +99,7 @@ describe IssuesHelper do
 
   describe 'url_for_new_issue' do
     let(:issues_url) { ext_project.external_issue_tracker.new_issue_url }
-    let(:ext_expected) do
-      issues_url.gsub(':project_id', ext_project.id.to_s)
-        .gsub(':issues_tracker_id', ext_project.issues_tracker_id.to_s)
-    end
+    let(:ext_expected) { issues_url.gsub(':project_id', ext_project.id.to_s) }
     let(:int_expected) { new_namespace_project_issue_path(project.namespace, project) }
 
     it "should return internal path if used internal tracker" do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f3590f72cfe..de8815f5a38 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -53,7 +53,6 @@ describe Project, models: true do
     it { is_expected.to validate_length_of(:path).is_within(0..255) }
     it { is_expected.to validate_length_of(:description).is_within(0..2000) }
     it { is_expected.to validate_presence_of(:creator) }
-    it { is_expected.to validate_length_of(:issues_tracker_id).is_within(0..255) }
     it { is_expected.to validate_presence_of(:namespace) }
 
     it 'should not allow new projects beyond user limits' do
@@ -321,27 +320,6 @@ describe Project, models: true do
     end
   end
 
-  describe :can_have_issues_tracker_id? do
-    let(:project) { create(:project) }
-    let(:ext_project) { create(:redmine_project) }
-
-    it 'should be true for projects with external issues tracker if issues enabled' do
-      expect(ext_project.can_have_issues_tracker_id?).to be_truthy
-    end
-
-    it 'should be false for projects with internal issue tracker if issues enabled' do
-      expect(project.can_have_issues_tracker_id?).to be_falsey
-    end
-
-    it 'should be always false if issues disabled' do
-      project.issues_enabled = false
-      ext_project.issues_enabled = false
-
-      expect(project.can_have_issues_tracker_id?).to be_falsey
-      expect(ext_project.can_have_issues_tracker_id?).to be_falsey
-    end
-  end
-
   describe :open_branches do
     let(:project) { create(:project) }
 
-- 
GitLab