Merge branch 'master' into fixes/api

spec/factories/user_team_project_relationships.rb

@@ -1,3 +1,15 @@
+# == Schema Information
+#
+# Table name: user_team_project_relationships
+#
+#  id              :integer          not null, primary key
+#  project_id      :integer
+#  user_team_id    :integer
+#  greatest_access :integer
+#  created_at      :datetime         not null
+#  updated_at      :datetime         not null
+#
+
 # Read about factories at https://github.com/thoughtbot/factory_girl
 
 FactoryGirl.define do

spec/factories/user_team_user_relationships.rb

@@ -1,3 +1,16 @@
+# == Schema Information
+#
+# Table name: user_team_user_relationships
+#
+#  id           :integer          not null, primary key
+#  user_id      :integer
+#  user_team_id :integer
+#  group_admin  :boolean
+#  permission   :integer
+#  created_at   :datetime         not null
+#  updated_at   :datetime         not null
+#
+
 # Read about factories at https://github.com/thoughtbot/factory_girl
 
 FactoryGirl.define do

spec/factories/user_teams.rb

@@ -1,3 +1,15 @@
+# == Schema Information
+#
+# Table name: user_teams
+#
+#  id         :integer          not null, primary key
+#  name       :string(255)
+#  path       :string(255)
+#  owner_id   :integer
+#  created_at :datetime         not null
+#  updated_at :datetime         not null
+#
+
 # Read about factories at https://github.com/thoughtbot/factory_girl
 
 FactoryGirl.define do

spec/lib/gitolite_config_spec.rb

@@ -1,16 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::GitoliteConfig do
-  let(:gitolite) { Gitlab::GitoliteConfig.new }
-
-  it { should respond_to :write_key }
-  it { should respond_to :rm_key }
-  it { should respond_to :update_project }
-  it { should respond_to :update_project! }
-  it { should respond_to :update_projects }
-  it { should respond_to :destroy_project }
-  it { should respond_to :destroy_project! }
-  it { should respond_to :apply }
-  it { should respond_to :admin_all_repo }
-  it { should respond_to :admin_all_repo! }
-end

spec/lib/gitolite_spec.rb

@@ -1,26 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Gitolite do
-  let(:project) { double('Project', id: 7, path: 'diaspora') }
-  let(:gitolite_config) { double('Gitlab::GitoliteConfig') }
-  let(:gitolite) { Gitlab::Gitolite.new }
-
-  before do
-    gitolite.stub(config: gitolite_config)
-    Project.stub(find: project)
-  end
-
-  it { should respond_to :set_key }
-  it { should respond_to :remove_key }
-
-  it { should respond_to :update_repository }
-  it { should respond_to :create_repository }
-  it { should respond_to :remove_repository }
-
-  it { gitolite.url_to_repo('diaspora').should == Gitlab.config.gitolite.ssh_path_prefix + "diaspora.git" }
-
-  it "should call config update" do
-    gitolite_config.should_receive(:update_project!)
-    gitolite.update_repository(project.id)
-  end
-end

spec/lib/shell_spec.rb

@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe Gitlab::Shell do
+  let(:project) { double('Project', id: 7, path: 'diaspora') }
+  let(:gitolite) { Gitlab::Shell.new }
+
+  before do
+    Project.stub(find: project)
+  end
+
+  it { should respond_to :add_key }
+  it { should respond_to :remove_key }
+  it { should respond_to :add_repository }
+  it { should respond_to :remove_repository }
+
+  it { gitolite.url_to_repo('diaspora').should == Gitlab.config.gitolite.ssh_path_prefix + "diaspora.git" }
+end

spec/mailers/notify_spec.rb

@@ -266,7 +266,7 @@ describe Notify do
 
         before(:each) { note.stub(:noteable).and_return(commit) }
 
-        subject { Notify.note_commit_email(recipient.email, note.id) }
+        subject { Notify.note_commit_email(recipient.id, note.id) }
 
         it_behaves_like 'a note email'
 

spec/models/key_spec.rb

@@ -46,9 +46,9 @@ describe Key do
         key.should_not be_valid
       end
 
-      it "does accept the same key for another project" do
+      it "does not accept the same key for another project" do
         key = build(:key, project_id: 0)
-        key.should be_valid
+        key.should_not be_valid
       end
     end
 

spec/models/project_spec.rb

@@ -8,7 +8,6 @@
 #  description            :text
 #  created_at             :datetime         not null
 #  updated_at             :datetime         not null
-#  private_flag           :boolean          default(TRUE), not null
 #  creator_id             :integer
 #  default_branch         :string(255)
 #  issues_enabled         :boolean          default(TRUE), not null
@@ -16,6 +15,7 @@
 #  merge_requests_enabled :boolean          default(TRUE), not null
 #  wiki_enabled           :boolean          default(TRUE), not null
 #  namespace_id           :integer
+#  public                 :boolean          default(FALSE), not null
 #
 
 require 'spec_helper'
@@ -42,7 +42,6 @@ describe Project do
   describe "Mass assignment" do
     it { should_not allow_mass_assignment_of(:namespace_id) }
     it { should_not allow_mass_assignment_of(:creator_id) }
-    it { should_not allow_mass_assignment_of(:private_flag) }
   end
 
   describe "Validation" do
@@ -78,8 +77,6 @@ describe Project do
     it { should respond_to(:url_to_repo) }
     it { should respond_to(:repo_exists?) }
     it { should respond_to(:satellite) }
-    it { should respond_to(:update_repository) }
-    it { should respond_to(:destroy_repository) }
     it { should respond_to(:observe_push) }
     it { should respond_to(:update_merge_requests) }
     it { should respond_to(:execute_hooks) }

spec/models/protected_branch_spec.rb

@@ -24,19 +24,4 @@ describe ProtectedBranch do
     it { should validate_presence_of(:project) }
     it { should validate_presence_of(:name) }
   end
-
-  describe 'Callbacks' do
-    let(:branch) { build(:protected_branch) }
-
-    it 'call update_repository after save' do
-      branch.should_receive(:update_repository)
-      branch.save
-    end
-
-    it 'call update_repository after destroy' do
-      branch.save
-      branch.should_receive(:update_repository)
-      branch.destroy
-    end
-  end
 end

spec/models/user_spec.rb

@@ -31,6 +31,8 @@
 #  extern_uid             :string(255)
 #  provider               :string(255)
 #  username               :string(255)
+#  can_create_group       :boolean          default(TRUE), not null
+#  can_create_team        :boolean          default(TRUE), not null
 #
 
 require 'spec_helper'

spec/models/user_team_project_relationship_spec.rb

@@ -1,3 +1,15 @@
+# == Schema Information
+#
+# Table name: user_team_project_relationships
+#
+#  id              :integer          not null, primary key
+#  project_id      :integer
+#  user_team_id    :integer
+#  greatest_access :integer
+#  created_at      :datetime         not null
+#  updated_at      :datetime         not null
+#
+
 require 'spec_helper'
 
 describe UserTeamProjectRelationship do

spec/models/user_team_spec.rb

@@ -1,3 +1,15 @@
+# == Schema Information
+#
+# Table name: user_teams
+#
+#  id         :integer          not null, primary key
+#  name       :string(255)
+#  path       :string(255)
+#  owner_id   :integer
+#  created_at :datetime         not null
+#  updated_at :datetime         not null
+#
+
 require 'spec_helper'
 
 describe UserTeam do

spec/models/user_team_user_relationship_spec.rb

@@ -1,3 +1,16 @@
+# == Schema Information
+#
+# Table name: user_team_user_relationships
+#
+#  id           :integer          not null, primary key
+#  user_id      :integer
+#  user_team_id :integer
+#  group_admin  :boolean
+#  permission   :integer
+#  created_at   :datetime         not null
+#  updated_at   :datetime         not null
+#
+
 require 'spec_helper'
 
 describe UserTeamUserRelationship do

spec/observers/key_observer_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe KeyObserver do
   before do
     @key = double('Key',
-      identifier: 'admin_654654',
+      shell_id: 'key-32',
       key: '== a vaild ssh key',
       projects: [],
       is_deploy_key: false
@@ -14,14 +14,14 @@ describe KeyObserver do
 
   context :after_save do
     it do
-      GitoliteWorker.should_receive(:perform_async).with(:set_key, @key.identifier, @key.key, @key.projects.map(&:id))
+      GitoliteWorker.should_receive(:perform_async).with(:add_key, @key.shell_id, @key.key)
       @observer.after_save(@key)
     end
   end
 
   context :after_destroy do
     it do
-      GitoliteWorker.should_receive(:perform_async).with(:remove_key, @key.identifier, @key.projects.map(&:id))
+      GitoliteWorker.should_receive(:perform_async).with(:remove_key, @key.shell_id, @key.key)
       @observer.after_destroy(@key)
     end
   end

spec/requests/api/groups_spec.rb

@@ -0,0 +1,93 @@
+require 'spec_helper'
+
+describe Gitlab::API do
+  include ApiHelpers
+
+  let(:user1)  { create(:user) }
+  let(:user2)  { create(:user) }
+  let(:admin) { create(:admin) }
+  let!(:group1)  { create(:group, owner: user1) }
+  let!(:group2)  { create(:group, owner: user2) }
+
+  describe "GET /groups" do
+    context "when unauthenticated" do
+      it "should return authentication error" do
+        get api("/groups")
+        response.status.should == 401
+      end
+    end
+
+    context "when authenticated as user" do
+      it "normal user: should return an array of groups of user1" do
+        get api("/groups", user1)
+        response.status.should == 200
+        json_response.should be_an Array
+        json_response.length.should == 1
+        json_response.first['name'].should == group1.name
+      end
+    end
+    
+    context "when authenticated as  admin" do
+      it "admin: should return an array of all groups" do
+        get api("/groups", admin)
+        response.status.should == 200
+        json_response.should be_an Array
+        json_response.length.should == 2
+      end
+    end
+  end
+  
+  describe "GET /groups/:id" do
+    context "when authenticated as user" do
+      it "should return one of user1's groups" do
+        get api("/groups/#{group1.id}", user1)
+        response.status.should == 200
+        json_response['name'] == group1.name
+      end
+      
+      it "should not return a non existing group" do
+        get api("/groups/1328", user1)
+        response.status.should == 404
+      end
+      
+      it "should not return a group not attached to user1" do
+        get api("/groups/#{group2.id}", user1)
+        response.status.should == 404
+      end
+    end
+    
+    context "when authenticated as admin" do
+      it "should return any existing group" do
+        get api("/groups/#{group2.id}", admin)
+        response.status.should == 200
+        json_response['name'] == group2.name
+      end
+      
+      it "should not return a non existing group" do
+        get api("/groups/1328", admin)
+        response.status.should == 404
+      end
+    end
+  end
+  
+  describe "POST /groups" do
+    context "when authenticated as user" do
+      it "should not create group" do
+        post api("/groups", user1), attributes_for(:group)
+        response.status.should == 403
+      end
+    end
+    
+    context "when authenticated as admin" do
+      it "should create group" do
+        post api("/groups", admin), attributes_for(:group)
+        response.status.should == 201
+      end
+
+      it "should not create group, duplicate" do
+        post api("/groups", admin), {:name => "Duplicate Test", :path => group2.path}
+        response.status.should == 404
+      end
+    end
+  end
+end

spec/requests/api/notes_spec.rb

@@ -6,8 +6,10 @@ describe Gitlab::API do
   let(:user) { create(:user) }
   let!(:project) { create(:project, namespace: user.namespace ) }
   let!(:issue) { create(:issue, project: project, author: user) }
+  let!(:merge_request) { create(:merge_request, project: project, author: user) }
   let!(:snippet) { create(:snippet, project: project, author: user) }
   let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) }
+  let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) }
   let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) }
   let!(:wall_note) { create(:note, project: project, author: user) }
   before { project.team << [user, :reporter] }
@@ -84,6 +86,15 @@ describe Gitlab::API do
         response.status.should == 404
       end
     end
+
+    context "when noteable is a Merge Request" do
+      it "should return an array of merge_requests notes" do
+        get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/notes", user)
+        response.status.should == 200
+        json_response.should be_an Array
+        json_response.first['body'].should == merge_request_note.note
+      end
+    end
   end
 
   describe "GET /projects/:id/noteable/:noteable_id/notes/:note_id" do

spec/requests/api/projects_spec.rb

@@ -113,6 +113,29 @@ describe Gitlab::API do
 
       json_response['name'].should == 'new_design'
       json_response['commit']['id'].should == '621491c677087aa243f165eab467bfdfbee00be1'
+      json_response['protected'].should == false
+    end
+  end
+
+  describe "PUT /projects/:id/repository/branches/:branch/protect" do
+    it "should protect a single branch" do
+      put api("/projects/#{project.id}/repository/branches/new_design/protect", user)
+      response.status.should == 200
+
+      json_response['name'].should == 'new_design'
+      json_response['commit']['id'].should == '621491c677087aa243f165eab467bfdfbee00be1'
+      json_response['protected'].should == true
+    end
+  end
+
+  describe "PUT /projects/:id/repository/branches/:branch/unprotect" do
+    it "should unprotect a single branch" do
+      put api("/projects/#{project.id}/repository/branches/new_design/unprotect", user)
+      response.status.should == 200
+
+      json_response['name'].should == 'new_design'
+      json_response['commit']['id'].should == '621491c677087aa243f165eab467bfdfbee00be1'
+      json_response['protected'].should == false
     end
   end
 

spec/requests/api/users_spec.rb

@@ -83,6 +83,54 @@ describe Gitlab::API do
     end
   end
 
+  describe "PUT /users/:id" do
+    before { admin }
+
+    it "should update user" do
+      put api("/users/#{user.id}", admin), {bio: 'new test bio'}
+      response.status.should == 200
+      json_response['bio'].should == 'new test bio'
+      user.reload.bio.should == 'new test bio'
+    end
+
+    it "should not allow invalid update" do
+      put api("/users/#{user.id}", admin), {email: 'invalid email'}
+      response.status.should == 404
+      user.reload.email.should_not == 'invalid email'
+    end
+
+    it "shouldn't available for non admin users" do
+      put api("/users/#{user.id}", user), attributes_for(:user)
+      response.status.should == 403
+    end
+
+    it "should return 404 for non-existing user" do
+      put api("/users/999999", admin), {bio: 'update should fail'}
+      response.status.should == 404
+    end
+  end
+
+  describe "DELETE /users/:id" do
+    before { admin }
+
+    it "should delete user" do
+      delete api("/users/#{user.id}", admin)
+      response.status.should == 200
+      expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound
+      json_response['email'].should == user.email
+    end
+
+    it "shouldn't available for non admin users" do
+      delete api("/users/#{user.id}", user)
+      response.status.should == 403
+    end
+
+    it "should return 404 for non-existing user" do
+      delete api("/users/999999", admin)
+      response.status.should == 404
+    end
+  end
+
   describe "GET /user" do
     it "should return current user" do
       get api("/user", user)

spec/requests/notes_on_merge_requests_spec.rb

@@ -22,9 +22,9 @@ describe "On a merge request", js: true do
     it { within(".js-main-target-form") { should_not have_link("Cancel") } }
 
     # notifiactions
-    it { within(".js-main-target-form") { should have_checked_field("Project team") } }
-    it { within(".js-main-target-form") { should_not have_checked_field("Commit author") } }
-    it { within(".js-main-target-form") { should_not have_unchecked_field("Commit author") } }
+    it { within(".js-main-target-form") { should have_checked_field("Notify team via email") } }
+    it { within(".js-main-target-form") { should_not have_checked_field("Notify commit author") } }
+    it { within(".js-main-target-form") { should_not have_unchecked_field("Notify commit author") } }
 
     describe "without text" do
       it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: false) } }
@@ -125,7 +125,7 @@ describe "On a merge request diff", js: true, focus: true do
       it { should have_css(".js-close-discussion-note-form", text: "Cancel") }
 
       # notification options
-      it { should have_checked_field("Project team") }
+      it { should have_checked_field("Notify team via email") }
 
       it "shouldn't add a second form for same row" do
         find("#4735dfc552ad7bf15ca468adc3cad9d05b624490_185_185.line_holder .js-add-diff-note-button").trigger("click")

spec/requests/notes_on_wall_spec.rb

@@ -21,9 +21,9 @@ describe "On the project wall", js: true do
     it { within(".js-main-target-form") { should_not have_link("Cancel") } }
 
     # notifiactions
-    it { within(".js-main-target-form") { should have_checked_field("Project team") } }
-    it { within(".js-main-target-form") { should_not have_checked_field("Commit author") } }
-    it { within(".js-main-target-form") { should_not have_unchecked_field("Commit author") } }
+    it { within(".js-main-target-form") { should have_checked_field("Notify team via email") } }
+    it { within(".js-main-target-form") { should_not have_checked_field("Notify commit author") } }
+    it { within(".js-main-target-form") { should_not have_unchecked_field("Notify commit author") } }
 
     describe "without text" do
       it { within(".js-main-target-form") { should have_css(".js-note-preview-button", visible: false) } }

spec/routing/project_routing_spec.rb

@@ -76,7 +76,7 @@ describe ProjectsController, "routing" do
   end
 
   it "to #graph" do
-    get("/gitlabhq/graph").should route_to('projects#graph', id: 'gitlabhq')
+    get("/gitlabhq/graph/master").should route_to('graph#show', project_id: 'gitlabhq', id: 'master')
   end
 
   it "to #files" do

spec/spec_helper.rb

@@ -24,7 +24,6 @@ RSpec.configure do |config|
   config.mock_with :rspec
 
   config.include LoginHelpers, type: :request
-  config.include GitoliteStub
   config.include FactoryGirl::Syntax::Methods
   config.include Devise::TestHelpers, type: :controller
 
@@ -34,8 +33,6 @@ RSpec.configure do |config|
   config.use_transactional_fixtures = false
 
   config.before do
-    stub_gitolite!
-
     # Use tmp dir for FS manipulations
     temp_repos_path = Rails.root.join('tmp', 'test-git-base-path')
     Gitlab.config.gitolite.stub(repos_path: temp_repos_path)

spec/support/gitolite_stub.rb

@@ -1,21 +0,0 @@
-module GitoliteStub
-  def stub_gitolite!
-    stub_gitlab_gitolite
-    stub_gitolite_admin
-  end
-
-  def stub_gitolite_admin
-    gitolite_admin = double('Gitolite::GitoliteAdmin')
-    gitolite_admin.as_null_object
-
-    Gitolite::GitoliteAdmin.stub(new: gitolite_admin)
-  end
-
-  def stub_gitlab_gitolite
-    gitolite_config = double('Gitlab::GitoliteConfig')
-    gitolite_config.stub(apply: ->() { yield(self) })
-    gitolite_config.as_null_object
-
-    Gitlab::GitoliteConfig.stub(new: gitolite_config)
-  end
-end

spec/support/stubbed_repository.rb

@@ -1,5 +1,6 @@
 require "repository"
 require "project"
+require "shell"
 
 # Stubs out all Git repository access done by models so that specs can run
 # against fake repositories without Grit complaining that they don't exist.
@@ -36,3 +37,23 @@ class GitLabTestRepo < Repository
     @repo ||= Grit::Repo.new(Rails.root.join('tmp', 'repositories', 'gitlabhq'))
   end
 end
+
+module Gitlab
+  class Shell
+    def add_repository name
+      true
+    end
+
+    def remove_repository name
+      true
+    end
+
+    def add_key id, key
+      true
+    end
+
+    def remove_key id, key
+      true
+    end
+  end
+end

spec/workers/post_receive_spec.rb

@@ -11,7 +11,7 @@ describe PostReceive do
   context "web hook" do
     let(:project) { create(:project) }
     let(:key) { create(:key, user: project.owner) }
-    let(:key_id) { key.identifier }
+    let(:key_id) { key.shell_id }
 
     it "fetches the correct project" do
       Project.should_receive(:find_with_namespace).with(project.path_with_namespace).and_return(project)
@@ -19,7 +19,7 @@ describe PostReceive do
     end
 
     it "does not run if the author is not in the project" do
-      Key.stub(find_by_identifier: nil)
+      Key.stub(find_by_id: nil)
 
       project.should_not_receive(:observe_push)
       project.should_not_receive(:execute_hooks)