Merge branch 'master' into issue-board-sidebar

3 files changed, 151 insertions, 225 deletions

diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb
index a0870891cf4..ad15b3f8f40 100644
--- a/spec/controllers/groups/group_members_controller_spec.rb
+++ b/spec/controllers/groups/group_members_controller_spec.rb
@@ -2,16 +2,10 @@ require 'spec_helper'
 
 describe Groups::GroupMembersController do
   let(:user)  { create(:user) }
+  let(:group) { create(:group, :public) }
 
-  describe '#index' do
-    let(:group) { create(:group) }
-
-    before do
-      group.add_owner(user)
-      stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
-    end
-
-    it 'renders index with group members' do
+  describe 'GET index' do
+    it 'renders index with 200 status code' do
       get :index, group_id: group
 
       expect(response).to have_http_status(200)
@@ -19,74 +13,56 @@ describe Groups::GroupMembersController do
     end
   end
 
-  describe '#destroy' do
-    let(:group) { create(:group, :public) }
+  describe 'DELETE destroy' do
+    let(:member) { create(:group_member, :developer, group: group) }
+
+    before { sign_in(user) }
 
     context 'when member is not found' do
       it 'returns 403' do
-        delete :destroy, group_id: group,
-                         id: 42
+        delete :destroy, group_id: group, id: 42
 
         expect(response).to have_http_status(403)
       end
     end
 
     context 'when member is found' do
-      let(:user) { create(:user) }
-      let(:group_user) { create(:user) }
-      let(:member) do
-        group.add_developer(group_user)
-        group.members.find_by(user_id: group_user)
-      end
-
       context 'when user does not have enough rights' do
-        before do
-          group.add_developer(user)
-          sign_in(user)
-        end
+        before { group.add_developer(user) }
 
         it 'returns 403' do
-          delete :destroy, group_id: group,
-                           id: member
+          delete :destroy, group_id: group, id: member
 
           expect(response).to have_http_status(403)
-          expect(group.users).to include group_user
+          expect(group.members).to include member
         end
       end
 
       context 'when user has enough rights' do
-        before do
-          group.add_owner(user)
-          sign_in(user)
-        end
+        before { group.add_owner(user) }
 
         it '[HTML] removes user from members' do
-          delete :destroy, group_id: group,
-                           id: member
+          delete :destroy, group_id: group, id: member
 
           expect(response).to set_flash.to 'User was successfully removed from group.'
           expect(response).to redirect_to(group_group_members_path(group))
-          expect(group.users).not_to include group_user
+          expect(group.members).not_to include member
         end
 
         it '[JS] removes user from members' do
-          xhr :delete, :destroy, group_id: group,
-                                 id: member
+          xhr :delete, :destroy, group_id: group, id: member
 
           expect(response).to be_success
-          expect(group.users).not_to include group_user
+          expect(group.members).not_to include member
         end
       end
     end
   end
 
-  describe '#leave' do
-    let(:group) { create(:group, :public) }
-    let(:user) { create(:user) }
+  describe 'DELETE leave' do
+    before { sign_in(user) }
 
     context 'when member is not found' do
-      before { sign_in(user) }
-
       it 'returns 404' do
         delete :leave, group_id: group
 
@@ -96,10 +72,7 @@ describe Groups::GroupMembersController do
 
     context 'when member is found' do
       context 'and is not an owner' do
-        before do
-          group.add_developer(user)
-          sign_in(user)
-        end
+        before { group.add_developer(user) }
 
         it 'removes user from members' do
           delete :leave, group_id: group
@@ -111,10 +84,7 @@ describe Groups::GroupMembersController do
       end
 
       context 'and is an owner' do
-        before do
-          group.add_owner(user)
-          sign_in(user)
-        end
+        before { group.add_owner(user) }
 
         it 'cannot removes himself from the group' do
           delete :leave, group_id: group
@@ -124,10 +94,7 @@ describe Groups::GroupMembersController do
       end
 
       context 'and is a requester' do
-        before do
-          group.request_access(user)
-          sign_in(user)
-        end
+        before { group.request_access(user) }
 
         it 'removes user from members' do
           delete :leave, group_id: group
@@ -141,13 +108,8 @@ describe Groups::GroupMembersController do
     end
   end
 
-  describe '#request_access' do
-    let(:group) { create(:group, :public) }
-    let(:user) { create(:user) }
-
-    before do
-      sign_in(user)
-    end
+  describe 'POST request_access' do
+    before { sign_in(user) }
 
     it 'creates a new GroupMember that is not a team member' do
       post :request_access, group_id: group
@@ -159,53 +121,39 @@ describe Groups::GroupMembersController do
     end
   end
 
-  describe '#approve_access_request' do
-    let(:group) { create(:group, :public) }
+  describe 'POST approve_access_request' do
+    let(:member) { create(:group_member, :access_request, group: group) }
+
+    before { sign_in(user) }
 
     context 'when member is not found' do
       it 'returns 403' do
-        post :approve_access_request, group_id: group,
-                                      id: 42
+        post :approve_access_request, group_id: group, id: 42
 
         expect(response).to have_http_status(403)
       end
     end
 
     context 'when member is found' do
-      let(:user) { create(:user) }
-      let(:group_requester) { create(:user) }
-      let(:member) do
-        group.request_access(group_requester)
-        group.requesters.find_by(user_id: group_requester)
-      end
-
       context 'when user does not have enough rights' do
-        before do
-          group.add_developer(user)
-          sign_in(user)
-        end
+        before { group.add_developer(user) }
 
         it 'returns 403' do
-          post :approve_access_request, group_id: group,
-                                        id: member
+          post :approve_access_request, group_id: group, id: member
 
           expect(response).to have_http_status(403)
-          expect(group.users).not_to include group_requester
+          expect(group.members).not_to include member
         end
       end
 
       context 'when user has enough rights' do
-        before do
-          group.add_owner(user)
-          sign_in(user)
-        end
+        before { group.add_owner(user) }
 
         it 'adds user to members' do
-          post :approve_access_request, group_id: group,
-                                        id: member
+          post :approve_access_request, group_id: group, id: member
 
           expect(response).to redirect_to(group_group_members_path(group))
-          expect(group.users).to include group_requester
+          expect(group.members).to include member
         end
       end
     end
diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb
index 3492b6ffbbb..622ab154493 100644
--- a/spec/controllers/projects/labels_controller_spec.rb
+++ b/spec/controllers/projects/labels_controller_spec.rb
@@ -1,52 +1,73 @@
 require 'spec_helper'
 
 describe Projects::LabelsController do
-  let(:project) { create(:project) }
+  let(:group)   { create(:group) }
+  let(:project) { create(:project, namespace: group) }
   let(:user)    { create(:user) }
 
   before do
     project.team << [user, :master]
+
     sign_in(user)
   end
 
   describe 'GET #index' do
-    def create_label(attributes)
-      create(:label, attributes.merge(project: project))
-    end
+    let!(:label_1) { create(:label, project: project, priority: 1, title: 'Label 1') }
+    let!(:label_2) { create(:label, project: project, priority: 3, title: 'Label 2') }
+    let!(:label_3) { create(:label, project: project, priority: 1, title: 'Label 3') }
+    let!(:label_4) { create(:label, project: project, title: 'Label 4') }
+    let!(:label_5) { create(:label, project: project, title: 'Label 5') }
 
-    before do
-      15.times { |i| create_label(priority: (i % 3) + 1, title: "label #{15 - i}") }
-      5.times { |i| create_label(title: "label #{100 - i}") }
+    let!(:group_label_1) { create(:group_label, group: group, title: 'Group Label 1') }
+    let!(:group_label_2) { create(:group_label, group: group, title: 'Group Label 2') }
+    let!(:group_label_3) { create(:group_label, group: group, title: 'Group Label 3') }
+    let!(:group_label_4) { create(:group_label, group: group, title: 'Group Label 4') }
 
-      get :index, namespace_id: project.namespace.to_param, project_id: project.to_param
+    before do
+      create(:label_priority, project: project, label: group_label_1, priority: 3)
+      create(:label_priority, project: project, label: group_label_2, priority: 1)
     end
 
     context '@prioritized_labels' do
-      let(:prioritized_labels) { assigns(:prioritized_labels) }
+      before do
+        list_labels
+      end
+
+      it 'does not include labels without priority' do
+        list_labels
 
-      it 'contains only prioritized labels' do
-        expect(prioritized_labels).to all(have_attributes(priority: a_value > 0))
+        expect(assigns(:prioritized_labels)).not_to include(group_label_3, group_label_4, label_4, label_5)
       end
 
       it 'is sorted by priority, then label title' do
-        priorities_and_titles = prioritized_labels.pluck(:priority, :title)
-
-        expect(priorities_and_titles.sort).to eq(priorities_and_titles)
+        expect(assigns(:prioritized_labels)).to eq [group_label_2, label_1, label_3, group_label_1, label_2]
       end
     end
 
     context '@labels' do
-      let(:labels) { assigns(:labels) }
+      it 'is sorted by label title' do
+        list_labels
 
-      it 'contains only unprioritized labels' do
-        expect(labels).to all(have_attributes(priority: nil))
+        expect(assigns(:labels)).to eq [group_label_3, group_label_4, label_4, label_5]
       end
 
-      it 'is sorted by label title' do
-        titles = labels.pluck(:title)
+      it 'does not include labels with priority' do
+        list_labels
+
+        expect(assigns(:labels)).not_to include(group_label_2, label_1, label_3, group_label_1, label_2)
+      end
+
+      it 'does not include group labels when project does not belong to a group' do
+        project.update(namespace: create(:namespace))
 
-        expect(titles.sort).to eq(titles)
+        list_labels
+
+        expect(assigns(:labels)).not_to include(group_label_3, group_label_4)
       end
     end
+
+    def list_labels
+      get :index, namespace_id: project.namespace.to_param, project_id: project.to_param
+    end
   end
 end
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index 074f85157de..8519ebc1d5f 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -1,69 +1,22 @@
 require('spec_helper')
 
 describe Projects::ProjectMembersController do
-  describe '#apply_import' do
-    let(:project) { create(:project) }
-    let(:another_project) { create(:project, :private) }
-    let(:user) { create(:user) }
-    let(:member) { create(:user) }
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :public) }
 
-    before do
-      project.team << [user, :master]
-      another_project.team << [member, :guest]
-      sign_in(user)
-    end
+  describe 'GET index' do
+    it 'renders index with 200 status code' do
+      get :index, namespace_id: project.namespace, project_id: project
 
-    shared_context 'import applied' do
-      before do
-        post(:apply_import, namespace_id: project.namespace,
-                            project_id: project,
-                            source_project_id: another_project.id)
-      end
-    end
-
-    context 'when user can access source project members' do
-      before { another_project.team << [user, :guest] }
-      include_context 'import applied'
-
-      it 'imports source project members' do
-        expect(project.team_members).to include member
-        expect(response).to set_flash.to 'Successfully imported'
-        expect(response).to redirect_to(
-          namespace_project_project_members_path(project.namespace, project)
-        )
-      end
-    end
-
-    context 'when user is not member of a source project' do
-      include_context 'import applied'
-
-      it 'does not import team members' do
-        expect(project.team_members).not_to include member
-      end
-
-      it 'responds with not found' do
-        expect(response.status).to eq 404
-      end
+      expect(response).to have_http_status(200)
+      expect(response).to render_template(:index)
     end
   end
 
-  describe '#index' do
-    context 'when user is member' do
-      before do
-        project = create(:project, :private)
-        member = create(:user)
-        project.team << [member, :guest]
-        sign_in(member)
-
-        get :index, namespace_id: project.namespace, project_id: project
-      end
-
-      it { expect(response).to have_http_status(200) }
-    end
-  end
+  describe 'DELETE destroy' do
+    let(:member) { create(:project_member, :developer, project: project) }
 
-  describe '#destroy' do
-    let(:project) { create(:project, :public) }
+    before { sign_in(user) }
 
     context 'when member is not found' do
       it 'returns 404' do
@@ -76,18 +29,8 @@ describe Projects::ProjectMembersController do
     end
 
     context 'when member is found' do
-      let(:user) { create(:user) }
-      let(:team_user) { create(:user) }
-      let(:member) do
-        project.team << [team_user, :developer]
-        project.members.find_by(user_id: team_user.id)
-      end
-
       context 'when user does not have enough rights' do
-        before do
-          project.team << [user, :developer]
-          sign_in(user)
-        end
+        before { project.team << [user, :developer] }
 
         it 'returns 404' do
           delete :destroy, namespace_id: project.namespace,
@@ -95,15 +38,12 @@ describe Projects::ProjectMembersController do
                            id: member
 
           expect(response).to have_http_status(404)
-          expect(project.users).to include team_user
+          expect(project.members).to include member
         end
       end
 
       context 'when user has enough rights' do
-        before do
-          project.team << [user, :master]
-          sign_in(user)
-        end
+        before { project.team << [user, :master] }
 
         it '[HTML] removes user from members' do
           delete :destroy, namespace_id: project.namespace,
@@ -113,7 +53,7 @@ describe Projects::ProjectMembersController do
           expect(response).to redirect_to(
             namespace_project_project_members_path(project.namespace, project)
           )
-          expect(project.users).not_to include team_user
+          expect(project.members).not_to include member
         end
 
         it '[JS] removes user from members' do
@@ -122,19 +62,16 @@ describe Projects::ProjectMembersController do
                                  id: member
 
           expect(response).to be_success
-          expect(project.users).not_to include team_user
+          expect(project.members).not_to include member
         end
       end
     end
   end
 
-  describe '#leave' do
-    let(:project) { create(:project, :public) }
-    let(:user) { create(:user) }
+  describe 'DELETE leave' do
+    before { sign_in(user) }
 
     context 'when member is not found' do
-      before { sign_in(user) }
-
       it 'returns 404' do
         delete :leave, namespace_id: project.namespace,
                        project_id: project
@@ -145,10 +82,7 @@ describe Projects::ProjectMembersController do
 
     context 'when member is found' do
       context 'and is not an owner' do
-        before do
-          project.team << [user, :developer]
-          sign_in(user)
-        end
+        before { project.team << [user, :developer] }
 
         it 'removes user from members' do
           delete :leave, namespace_id: project.namespace,
@@ -161,11 +95,9 @@ describe Projects::ProjectMembersController do
       end
 
       context 'and is an owner' do
-        before do
-          project.update(namespace_id: user.namespace_id)
-          project.team << [user, :master, user]
-          sign_in(user)
-        end
+        let(:project) { create(:project, namespace: user.namespace) }
+
+        before { project.team << [user, :master] }
 
         it 'cannot remove himself from the project' do
           delete :leave, namespace_id: project.namespace,
@@ -176,10 +108,7 @@ describe Projects::ProjectMembersController do
       end
 
       context 'and is a requester' do
-        before do
-          project.request_access(user)
-          sign_in(user)
-        end
+        before { project.request_access(user) }
 
         it 'removes user from members' do
           delete :leave, namespace_id: project.namespace,
@@ -194,13 +123,8 @@ describe Projects::ProjectMembersController do
     end
   end
 
-  describe '#request_access' do
-    let(:project) { create(:project, :public) }
-    let(:user) { create(:user) }
-
-    before do
-      sign_in(user)
-    end
+  describe 'POST request_access' do
+    before { sign_in(user) }
 
     it 'creates a new ProjectMember that is not a team member' do
       post :request_access, namespace_id: project.namespace,
@@ -215,8 +139,10 @@ describe Projects::ProjectMembersController do
     end
   end
 
-  describe '#approve' do
-    let(:project) { create(:project, :public) }
+  describe 'POST approve' do
+    let(:member) { create(:project_member, :access_request, project: project) }
+
+    before { sign_in(user) }
 
     context 'when member is not found' do
       it 'returns 404' do
@@ -229,18 +155,8 @@ describe Projects::ProjectMembersController do
     end
 
     context 'when member is found' do
-      let(:user) { create(:user) }
-      let(:team_requester) { create(:user) }
-      let(:member) do
-        project.request_access(team_requester)
-        project.requesters.find_by(user_id: team_requester.id)
-      end
-
       context 'when user does not have enough rights' do
-        before do
-          project.team << [user, :developer]
-          sign_in(user)
-        end
+        before { project.team << [user, :developer] }
 
         it 'returns 404' do
           post :approve_access_request, namespace_id: project.namespace,
@@ -248,15 +164,12 @@ describe Projects::ProjectMembersController do
                                         id: member
 
           expect(response).to have_http_status(404)
-          expect(project.users).not_to include team_requester
+          expect(project.members).not_to include member
         end
       end
 
       context 'when user has enough rights' do
-        before do
-          project.team << [user, :master]
-          sign_in(user)
-        end
+        before { project.team << [user, :master] }
 
         it 'adds user to members' do
           post :approve_access_request, namespace_id: project.namespace,
@@ -266,9 +179,53 @@ describe Projects::ProjectMembersController do
           expect(response).to redirect_to(
             namespace_project_project_members_path(project.namespace, project)
           )
-          expect(project.users).to include team_requester
+          expect(project.members).to include member
         end
       end
     end
   end
+
+  describe 'POST apply_import' do
+    let(:another_project) { create(:project, :private) }
+    let(:member) { create(:user) }
+
+    before do
+      project.team << [user, :master]
+      another_project.team << [member, :guest]
+      sign_in(user)
+    end
+
+    shared_context 'import applied' do
+      before do
+        post(:apply_import, namespace_id: project.namespace,
+                            project_id: project,
+                            source_project_id: another_project.id)
+      end
+    end
+
+    context 'when user can access source project members' do
+      before { another_project.team << [user, :guest] }
+      include_context 'import applied'
+
+      it 'imports source project members' do
+        expect(project.team_members).to include member
+        expect(response).to set_flash.to 'Successfully imported'
+        expect(response).to redirect_to(
+          namespace_project_project_members_path(project.namespace, project)
+        )
+      end
+    end
+
+    context 'when user is not member of a source project' do
+      include_context 'import applied'
+
+      it 'does not import team members' do
+        expect(project.team_members).not_to include member
+      end
+
+      it 'responds with not found' do
+        expect(response.status).to eq 404
+      end
+    end
+  end
 end