Refactoring. Clean up advanced settings. Migrate from gotlab_url to path

20 files changed, 65 insertions, 90 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5f157e0..d7a601b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,8 @@ v7.11.0
   - Running and Pending tabs on admin builds page
   - Fix [ci skip] tag, so you can skip CI triggering now
   - Add HipChat notifications
+  - Clean up project advanced settings.
+  - Add a GitLab project path parameter to the project API
 
 v7.10.1
   - Fix failing migration when update to 7.10 from 7.8 and older versions
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index dfef9bf..33cec6a 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -118,10 +118,9 @@ class ProjectsController < ApplicationController
   end
 
   def project_params
-    params.require(:project).permit(:name, :path, :timeout, :token, :timeout_in_minutes,
-    :default_ref, :gitlab_url, :always_build, :polling_interval,
-    :public, :ssh_url_to_repo, :gitlab_id, :allow_git_fetch, :skip_refs,
-    :email_recipients, :email_add_pusher, :email_only_broken_builds, :coverage_regex, :shared_runners_enabled,
-    { jobs_attributes: [:id, :name, :build_branches, :build_tags, :tag_list, :commands, :refs, :_destroy, :job_type] })
+    params.require(:project).permit(:path, :timeout, :timeout_in_minutes, :default_ref, :always_build,
+      :polling_interval, :public, :ssh_url_to_repo, :allow_git_fetch, :skip_refs, :email_recipients,
+      :email_add_pusher, :email_only_broken_builds, :coverage_regex, :shared_runners_enabled, :token,
+      { jobs_attributes: [:id, :name, :build_branches, :build_tags, :tag_list, :commands, :refs, :_destroy, :job_type] })
   end
 end
diff --git a/app/helpers/builds_helper.rb b/app/helpers/builds_helper.rb
index 9f4eb11..6e63dc0 100644
--- a/app/helpers/builds_helper.rb
+++ b/app/helpers/builds_helper.rb
@@ -1,24 +1,14 @@
 module BuildsHelper
   def build_ref_link build
-    if build.commit.gitlab?
-      gitlab_ref_link build.project, build.ref
-    else
-      build.ref
-    end
+    gitlab_ref_link build.project, build.ref
   end
 
   def build_compare_link build
-    if build.commit.gitlab?
-      gitlab_compare_link build.project, build.commit.short_before_sha, build.short_sha
-    end
+    gitlab_compare_link build.project, build.commit.short_before_sha, build.short_sha
   end
 
   def build_commit_link build
-    if build.commit.gitlab?
-      gitlab_commit_link build.project, build.short_sha
-    else
-      build.short_sha
-    end
+    gitlab_commit_link build.project, build.short_sha
   end
 
   def build_url(build)
diff --git a/app/models/commit.rb b/app/models/commit.rb
index bc33350..6c876bd 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -51,11 +51,7 @@ class Commit < ActiveRecord::Base
   end
 
   def compare?
-    gitlab? && !new_branch?
-  end
-
-  def gitlab?
-    project.gitlab?
+    !new_branch?
   end
 
   def git_author_name
diff --git a/app/models/project.rb b/app/models/project.rb
index c48b079..f648833 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -9,7 +9,7 @@
 #  updated_at               :datetime
 #  token                    :string(255)
 #  default_ref              :string(255)
-#  gitlab_url               :string(255)
+#  path                     :string(255)
 #  always_build             :boolean          default(FALSE), not null
 #  polling_interval         :integer
 #  public                   :boolean          default(FALSE), not null
@@ -47,7 +47,7 @@ class Project < ActiveRecord::Base
   # Validations
   #
   validates_presence_of :name, :timeout, :token, :default_ref,
-    :gitlab_url, :ssh_url_to_repo, :gitlab_id
+    :path, :ssh_url_to_repo, :gitlab_id
 
   validates_uniqueness_of :name
 
@@ -79,7 +79,7 @@ ls -la
       params = {
         name:                    project.name_with_namespace,
         gitlab_id:               project.id,
-        gitlab_url:              project.web_url,
+        path:                    project.path_with_namespace,
         default_ref:             project.default_branch || 'master',
         ssh_url_to_repo:         project.ssh_url_to_repo,
         email_add_pusher:        GitlabCi.config.gitlab_ci.add_pusher,
@@ -130,10 +130,6 @@ ls -la
     self.token = SecureRandom.hex(15) if self.token.blank?
   end
 
-  def gitlab?
-    gitlab_url.present?
-  end
-
   def tracked_refs
     @tracked_refs ||= default_ref.split(",").map{|ref| ref.strip}
   end
@@ -236,6 +232,10 @@ ls -la
     end
   end
 
+  def gitlab_url
+    File.join(GitlabCi.config.gitlab_server.url, path)
+  end
+
   def setup_finished?
     commits.any?
   end
diff --git a/app/models/project_services/slack_message.rb b/app/models/project_services/slack_message.rb
index 8d8bfcf..c95499e 100644
--- a/app/models/project_services/slack_message.rb
+++ b/app/models/project_services/slack_message.rb
@@ -69,19 +69,11 @@ class SlackMessage
   end
 
   def commit_sha_link
-    if commit.project.gitlab?
-      "#{project.gitlab_url}/commit/#{commit.sha}"
-    else
-      commit.ref
-    end
+    "#{project.gitlab_url}/commit/#{commit.sha}"
   end
 
   def commit_ref_link
-    if commit.project.gitlab?
-      "#{project.gitlab_url}/commits/#{commit.ref}"
-    else
-      commit.ref
-    end
+    "#{project.gitlab_url}/commits/#{commit.ref}"
   end
 
   def attachment_color
diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml
index 231bded..6ee2788 100644
--- a/app/views/layouts/project.html.haml
+++ b/app/views/layouts/project.html.haml
@@ -12,9 +12,8 @@
             %i.icon-globe
             Public
 
-        - if @project.gitlab_url.present?
-          .pull-right
-            = link_to 'View on GitLab', @project.gitlab_url, no_turbolink.merge( class: 'btn btn-small' )
+        .pull-right
+          = link_to 'View on GitLab', @project.gitlab_url, no_turbolink.merge( class: 'btn btn-small' )
     %hr
     .container
       - if current_user && current_user.can_manage_project?(@project.gitlab_id)
diff --git a/app/views/projects/_form.html.haml b/app/views/projects/_form.html.haml
index 3702f7e..af78db9 100644
--- a/app/views/projects/_form.html.haml
+++ b/app/views/projects/_form.html.haml
@@ -83,26 +83,10 @@
   %fieldset
     %legend Advanced settings
     .form-group
-      = f.label :name, class: 'control-label'
-      .col-sm-10
-        = f.text_field :name, class: 'form-control', placeholder: 'my-project'
-    .form-group
-      = f.label :gitlab_url, "GitLab url to project", class: 'control-label'
-      .col-sm-10
-        = f.text_field :gitlab_url, class: 'form-control', placeholder: 'http://gitlab.domain.com/project-slug'
-    .form-group
-      = f.label :gitlab_id, "GitLab project id", class: 'control-label'
-      .col-sm-10
-        = f.text_field :gitlab_id, class: 'form-control', placeholder: '17'
-    .form-group
       = f.label :token, "CI token", class: 'control-label'
       .col-sm-10
         = f.text_field :token, class: 'form-control', placeholder: 'xEeFCaDAB89'
     .form-group
-      = f.label :url, "CI project URL", class: 'control-label'
-      .col-sm-10
-        = text_field_tag :url, project_url(@project), class: 'form-control', readonly: true
-    .form-group
       = f.label :skip_refs, "Skip refs", class: 'control-label'
       .col-sm-10
         = f.text_field :skip_refs, class: 'form-control', placeholder: 'branch1, branch2, feature/*'
diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml
deleted file mode 100644
index c287d33..0000000
--- a/app/views/projects/new.html.haml
+++ /dev/null
@@ -1,6 +0,0 @@
-%h3 New project
-
-= link_to projects_path do
-  &larr; Back to projects
-%hr
-= render 'form'
diff --git a/db/migrate/20150504010150_migrate_url_to_path.rb b/db/migrate/20150504010150_migrate_url_to_path.rb
new file mode 100644
index 0000000..31d8c54
--- /dev/null
+++ b/db/migrate/20150504010150_migrate_url_to_path.rb
@@ -0,0 +1,11 @@
+class MigrateUrlToPath < ActiveRecord::Migration
+  def up
+    select_all("SELECT id, gitlab_url FROM projects").each do |project|
+      path = project['gitlab_url'].sub(/.*\/(.*\/.*)$/, '\1')
+      execute("UPDATE projects SET gitlab_url = '#{path}' WHERE id = '#{project['id']}'")
+    end
+  end
+
+  def down
+  end
+end
diff --git a/db/migrate/20150504010250_rename_gitlab_url_to_path.rb b/db/migrate/20150504010250_rename_gitlab_url_to_path.rb
new file mode 100644
index 0000000..6040c32
--- /dev/null
+++ b/db/migrate/20150504010250_rename_gitlab_url_to_path.rb
@@ -0,0 +1,5 @@
+class RenameGitlabUrlToPath < ActiveRecord::Migration
+  def change
+    rename_column :projects, :gitlab_url, :path
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f16ae4c..596e11f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150417000045) do
+ActiveRecord::Schema.define(version: 20150504010250) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -87,7 +87,7 @@ ActiveRecord::Schema.define(version: 20150417000045) do
     t.datetime "updated_at"
     t.string   "token"
     t.string   "default_ref"
-    t.string   "gitlab_url"
+    t.string   "path"
     t.boolean  "always_build",             default: false, null: false
     t.integer  "polling_interval"
     t.boolean  "public",                   default: false, null: false
diff --git a/doc/api/projects.md b/doc/api/projects.md
index 766ee12..bdc9862 100644
--- a/doc/api/projects.md
+++ b/doc/api/projects.md
@@ -26,6 +26,7 @@ Returns:
     "token" : "iPWx6WM4lhHNedGfBpPJNP",
     "default_ref" : "master",
     "gitlab_url" : "http://demo.gitlabhq.com/gitlab/gitlab-shell",
+    "path" : "gitlab/gitlab-shell",
     "always_build" : false,
     "polling_interval" : null,
     "public" : false,
@@ -39,6 +40,7 @@ Returns:
     "token" : "iPWx6WM4lhHNedGfBpPJNP",
     "default_ref" : "master",
     "gitlab_url" : "http://demo.gitlabhq.com/gitlab/gitlab-shell",
+    "path" : "gitlab/gitlab-shell",
     "always_build" : false,
     "polling_interval" : null,
     "public" : false,
@@ -67,6 +69,7 @@ Returns:
     "token" : "iPWx6WM4lhHNedGfBpPJNP",
     "default_ref" : "master",
     "gitlab_url" : "http://demo.gitlabhq.com/gitlab/gitlab-shell",
+    "path" : "gitlab/gitlab-shell",
     "always_build" : false,
     "polling_interval" : null,
     "public" : false,
@@ -97,7 +100,7 @@ Parameters:
 
   * `name` (required) - The name of the project
   * `gitlab_id` (required) - The ID of the project on the Gitlab instance
-  * `gitlab_url` (required) - The web url of the project on the Gitlab instance
+  * `path` (required) - The gitlab project path
   * `ssh_url_to_repo` (required) - The gitlab SSH url to the repo
   * `default_ref` (optional) - The branch to run on (default to `master`)
 
@@ -112,7 +115,7 @@ Parameters:
 
   * `name` - The name of the project
   * `gitlab_id` - The ID of the project on the Gitlab instance
-  * `gitlab_url` - The web url of the project on the Gitlab instance
+  * `path` - The gitlab project path
   * `ssh_url_to_repo` - The gitlab SSH url to the repo
   * `default_ref` - The branch to run on (default to `master`)
 
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 3d98866..7417ef5 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -17,7 +17,7 @@ module API
     end
 
     class Project < Grape::Entity
-      expose :id, :name, :timeout, :token, :default_ref, :gitlab_url,
+      expose :id, :name, :timeout, :token, :default_ref, :gitlab_url, :path,
         :always_build, :polling_interval, :public, :ssh_url_to_repo, :gitlab_id
     end
 
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index b7927c9..ea2ceeb 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -185,7 +185,7 @@ module API
       # Parameters:
       #   name (required)            - The name of the project
       #   gitlab_id (required)       - The gitlab id of the project
-      #   gitlab_url (required)      - The gitlab web url to the project
+      #   path (required)            - The gitlab project path, ex. randx/six
       #   ssh_url_to_repo (required) - The gitlab ssh url to the repo
       #   default_ref                - The branch to run against (defaults to `master`)
       # Example Request:
@@ -196,7 +196,8 @@ module API
         filtered_params = {
           name:            params[:name],
           gitlab_id:       params[:gitlab_id],
-          gitlab_url:      params[:gitlab_url],
+          # we accept gitlab_url for backward compatibility for a while (added to 7.11)
+          path:            params[:post] || params[:gitlab_url].sub(/.*\/(.*\/.*)$/, '\1'),
           default_ref:     params[:default_ref] || 'master',
           ssh_url_to_repo: params[:ssh_url_to_repo]
         }
@@ -219,7 +220,7 @@ module API
       #   id (required)   - The ID of a project
       #   name            - The name of the project
       #   gitlab_id       - The gitlab id of the project
-      #   gitlab_url      - The gitlab web url to the project
+      #   path            - The gitlab project path, ex. randx/six
       #   ssh_url_to_repo - The gitlab ssh url to the repo
       #   default_ref     - The branch to run against (defaults to `master`)
       # Example Request:
@@ -229,7 +230,12 @@ module API
 
         unauthorized! unless current_user.can_manage_project?(project.gitlab_id)
 
-        attrs = attributes_for_keys [:name, :gitlab_id, :gitlab_url, :default_ref, :ssh_url_to_repo]
+        attrs = attributes_for_keys [:name, :gitlab_id, :path, :gitlab_url, :default_ref, :ssh_url_to_repo]
+
+        # we accept gitlab_url for backward compatibility for a while (added to 7.11)
+        if attrs[:gitlab_url] && !attrs[:path]
+          attrs[:path] = attrs[:gitlab_url].sub(/.*\/(.*\/.*)$/, '\1')
+        end
 
         if project.update_attributes(attrs)
           present project, with: Entities::Project
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index e28d53e..95f43f8 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -9,7 +9,7 @@
 #  updated_at               :datetime
 #  token                    :string(255)
 #  default_ref              :string(255)
-#  gitlab_url               :string(255)
+#  path               :string(255)
 #  always_build             :boolean          default(FALSE), not null
 #  polling_interval         :integer
 #  public                   :boolean          default(FALSE), not null
@@ -34,8 +34,8 @@ FactoryGirl.define do
 
     default_ref 'master'
     
-    sequence :gitlab_url do |n|
-      "http://demo.gitlabhq.com/gitlab/gitlab-shell#{n}"
+    sequence :path do |n|
+      "gitlab/gitlab-shell#{n}"
     end
     
     sequence :ssh_url_to_repo do |n|
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index 1804db5..f01dc11 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -34,12 +34,12 @@ describe "Projects" do
     it { page.should have_content 'Build Schedule' }
 
     it "updates configuration" do
-      fill_in 'Name', with: 'Documentcloud / Underscore1'
+      fill_in 'Skip refs', with: 'deploy'
       click_button 'Save changes'
 
-      page.should have_content 'successfully updated'
+      page.should have_content 'was successfully updated'
 
-      find_field('Name').value.should eq 'Documentcloud / Underscore1'
+      find_field('Skip refs').value.should eq 'deploy'
     end
   end
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 28cd0ef..75d5610 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -111,7 +111,7 @@ describe Commit do
   describe :compare? do
     subject { commit_with_project.compare? }
 
-    context 'if project.gitlab_url and commit.before_sha are not nil' do
+    context 'if commit.before_sha are not nil' do
       it { should be_true }
     end
   end
@@ -130,12 +130,6 @@ describe Commit do
     it { commit.sha.should start_with(subject) }
   end
 
-  describe :gitlab? do
-    subject { commit_with_project.gitlab? }
-
-    it { should eq(project.gitlab?) }
-  end
-
   describe "create_deploy_builds" do
     it "creates deploy build" do
       FactoryGirl.create :job, job_type: :deploy, project: project
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 482a401..0b334d3 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -9,7 +9,7 @@
 #  updated_at               :datetime
 #  token                    :string(255)
 #  default_ref              :string(255)
-#  gitlab_url               :string(255)
+#  path                    :string(255)
 #  always_build             :boolean          default(FALSE), not null
 #  polling_interval         :integer
 #  public                   :boolean          default(FALSE), not null
@@ -132,7 +132,7 @@ describe Project do
     it { parsed_project.should be_kind_of(Project) }
     it { parsed_project.name.should eq("GitLab / api.gitlab.org") }
     it { parsed_project.gitlab_id.should eq(189) }
-    it { parsed_project.gitlab_url.should eq("http://localhost:3000/gitlab/api-gitlab-org") }
+    it { parsed_project.gitlab_url.should eq("http://demo.gitlab.com/gitlab/api-gitlab-org") }
 
     it "parses plain hash" do
       data = YAML.load(project_dump)
diff --git a/spec/requests/api/forks_spec.rb b/spec/requests/api/forks_spec.rb
index 8de1006..af52342 100644
--- a/spec/requests/api/forks_spec.rb
+++ b/spec/requests/api/forks_spec.rb
@@ -27,7 +27,7 @@ describe API::API do
         data: {
           id:                  2,
           name_with_namespace: "Gitlab.org / Underscore",
-          web_url:             "http://example.com/gitlab-org/underscore",
+          path_with_namespace: "gitlab-org/underscore",
           default_branch:      "master",
           ssh_url_to_repo:     "git@example.com:gitlab-org/underscore"
         }