Merge branch 'dashboard-groups' into 'master'

Move profile groups page to dashboard

For #2069

See merge request !1654

CHANGELOG

@@ -31,6 +31,7 @@ v 7.9.0 (unreleased)
   - Condense commits already in target branch when updating merge request source branch.
   - Send notifications and leave system comments when bulk updating issues.
   - Automatically link commit ranges to compare page: sha1...sha4 or sha1..sha4 (includes sha1 in comparison)
+  - Move groups page from profile to dashboard
 
 v 7.8.2
   - Fix service migration issue when upgrading from versions prior to 7.3

app/controllers/profiles/groups_controller.rb → app/controllers/dashboard/groups_controller.rb

-class Profiles::GroupsController < ApplicationController
-  layout "profile"
-
+class Dashboard::GroupsController < ApplicationController
   def index
     @user_groups = current_user.group_members.page(params[:page]).per(20)
   end
@@ -9,7 +7,7 @@ class Profiles::GroupsController < ApplicationController
     @users_group = group.group_members.where(user_id: current_user.id).first
     if can?(current_user, :destroy, @users_group)
       @users_group.destroy
-      redirect_to(profile_groups_path, info: "You left #{group.name} group.")
+      redirect_to(dashboard_groups_path, info: "You left #{group.name} group.")
     else
       return render_403
     end

app/views/profiles/groups/index.html.haml → app/views/dashboard/groups/index.html.haml

@@ -23,7 +23,7 @@
               Settings
 
           - if can?(current_user, :destroy, user_group)
-            = link_to leave_profile_group_path(group), data: { confirm: leave_group_message(group.name) }, method: :delete, class: "btn-small btn btn-grouped", title: 'Remove user from group' do
+            = link_to leave_dashboard_group_path(group), data: { confirm: leave_group_message(group.name) }, method: :delete, class: "btn-small btn btn-grouped", title: 'Remove user from group' do
               %i.fa.fa-sign-out
               Leave
 

app/views/groups/group_members/_group_member.html.haml

@@ -19,7 +19,7 @@
             %i.fa.fa-pencil-square-o
         - if can?(current_user, :destroy, member)
           - if current_user == member.user
-            = link_to leave_profile_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
+            = link_to leave_dashboard_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
               %i.fa.fa-minus.fa-inverse
           - else
             = link_to group_group_member_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do

app/views/layouts/nav/_dashboard.html.haml

@@ -9,6 +9,11 @@
       %i.fa.fa-cube
       %span
         Projects
+  = nav_link(controller: :groups) do
+    = link_to dashboard_groups_path, title: 'Groups' do
+      %i.fa.fa-group
+      %span
+        Groups
   = nav_link(controller: :milestones) do
     = link_to dashboard_milestones_path, title: 'Milestones' do
       %i.fa.fa-clock-o

app/views/layouts/nav/_profile.html.haml

@@ -43,11 +43,6 @@
       %i.fa.fa-image
       %span
         Design
-  = nav_link(controller: :groups) do
-    = link_to profile_groups_path, title: 'Groups' do
-      %i.fa.fa-group
-      %span
-        Groups
   = nav_link(path: 'profiles#history') do
     = link_to history_profile_path, title: 'History' do
       %i.fa.fa-history

config/routes.rb

@@ -193,11 +193,6 @@ Gitlab::Application.routes.draw do
       end
       resources :keys
       resources :emails, only: [:index, :create, :destroy]
-      resources :groups, only: [:index] do
-        member do
-          delete :leave
-        end
-      end
       resource :avatar, only: [:destroy]
     end
   end
@@ -220,6 +215,12 @@ Gitlab::Application.routes.draw do
 
     scope module: :dashboard do
       resources :milestones, only: [:index, :show]
+
+      resources :groups, only: [:index] do
+        member do
+          delete :leave
+        end
+      end
     end
   end
 

features/profile/group.feature → features/dashboard/group.feature

-@profile
-Feature: Profile Group
+@dashboard
+Feature: Dashboard Group
   Background:
     Given I sign in as "John Doe"
     And "John Doe" is owner of group "Owned"
@@ -10,18 +10,18 @@ Feature: Profile Group
   @javascript
   Scenario: Owner should be able to leave from group if he is not the last owner
     Given "Mary Jane" is owner of group "Owned"
-    When I visit profile groups page
+    When I visit dashboard groups page
     Then I should see group "Owned" in group list
     Then I should see group "Guest" in group list
     When I click on the "Leave" button for group "Owned"
-    And I visit profile groups page
+    And I visit dashboard groups page
     Then I should not see group "Owned" in group list
     Then I should see group "Guest" in group list
 
   @javascript
   Scenario: Owner should not be able to leave from group if he is the last owner
     Given "Mary Jane" is guest of group "Owned"
-    When I visit profile groups page
+    When I visit dashboard groups page
     Then I should see group "Owned" in group list
     Then I should see group "Guest" in group list
     Then I should not see the "Leave" button for group "Owned"
@@ -29,20 +29,20 @@ Feature: Profile Group
   @javascript
   Scenario: Guest should be able to leave from group
     Given "Mary Jane" is guest of group "Guest"
-    When I visit profile groups page
+    When I visit dashboard groups page
     Then I should see group "Owned" in group list
     Then I should see group "Guest" in group list
     When I click on the "Leave" button for group "Guest"
-    When I visit profile groups page
+    When I visit dashboard groups page
     Then I should see group "Owned" in group list
     Then I should not see group "Guest" in group list
 
   @javascript
   Scenario: Guest should be able to leave from group even if he is the only user in the group
-    When I visit profile groups page
+    When I visit dashboard groups page
     Then I should see group "Owned" in group list
     Then I should see group "Guest" in group list
     When I click on the "Leave" button for group "Guest"
-    When I visit profile groups page
+    When I visit dashboard groups page
     Then I should see group "Owned" in group list
     Then I should not see group "Guest" in group list

features/steps/profile/group.rb → features/steps/dashboard/group.rb

-class Spinach::Features::ProfileGroup < Spinach::FeatureSteps
+class Spinach::Features::DashboardGroup < Spinach::FeatureSteps
   include SharedAuthentication
   include SharedGroup
   include SharedPaths

features/steps/shared/paths.rb

@@ -87,6 +87,14 @@ module SharedPaths
     visit help_path
   end
 
+  step 'I visit dashboard groups page' do
+    visit dashboard_groups_path
+  end
+
+  step 'I should be redirected to the dashboard groups page' do
+    current_path.should == dashboard_groups_path
+  end
+
   # ----------------------------------------
   # Profile
   # ----------------------------------------
@@ -119,14 +127,6 @@ module SharedPaths
     visit history_profile_path
   end
 
-  step 'I visit profile groups page' do
-    visit profile_groups_path
-  end
-
-  step 'I should be redirected to the profile groups page' do
-    current_path.should == profile_groups_path
-  end
-
   # ----------------------------------------
   # Admin
   # ----------------------------------------

spec/features/security/dashboard_access_spec.rb

@@ -52,4 +52,12 @@ describe "Dashboard access", feature: true  do
     it { expect(new_group_path).to be_allowed_for :user }
     it { expect(new_group_path).to be_denied_for :visitor }
   end
+
+  describe "GET /profile/groups" do
+    subject { dashboard_groups_path }
+
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
+  end
 end

spec/features/security/profile_access_spec.rb

 require 'spec_helper'
 
-describe "Users Security", feature: true  do
-  describe "Project" do
-    before do
-      @u1 = create(:user)
-    end
-
-    describe "GET /login" do
-      it { expect(new_user_session_path).not_to be_404_for :visitor }
-    end
-
-    describe "GET /profile/keys" do
-      subject { profile_keys_path }
+describe "Profile access", feature: true  do
+  before do
+    @u1 = create(:user)
+  end
 
-      it { is_expected.to be_allowed_for @u1 }
-      it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
-    end
+  describe "GET /login" do
+    it { expect(new_user_session_path).not_to be_404_for :visitor }
+  end
 
-    describe "GET /profile" do
-      subject { profile_path }
+  describe "GET /profile/keys" do
+    subject { profile_keys_path }
 
-      it { is_expected.to be_allowed_for @u1 }
-      it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
-    end
+    it { is_expected.to be_allowed_for @u1 }
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
+  end
 
-    describe "GET /profile/account" do
-      subject { profile_account_path }
+  describe "GET /profile" do
+    subject { profile_path }
 
-      it { is_expected.to be_allowed_for @u1 }
-      it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
-    end
+    it { is_expected.to be_allowed_for @u1 }
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
+  end
 
-    describe "GET /profile/design" do
-      subject { design_profile_path }
+  describe "GET /profile/account" do
+    subject { profile_account_path }
 
-      it { is_expected.to be_allowed_for @u1 }
-      it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
-    end
+    it { is_expected.to be_allowed_for @u1 }
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
+  end
 
-    describe "GET /profile/history" do
-      subject { history_profile_path }
+  describe "GET /profile/design" do
+    subject { design_profile_path }
 
-      it { is_expected.to be_allowed_for @u1 }
-      it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
-    end
+    it { is_expected.to be_allowed_for @u1 }
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
+  end
 
-    describe "GET /profile/notifications" do
-      subject { profile_notifications_path }
+  describe "GET /profile/history" do
+    subject { history_profile_path }
 
-      it { is_expected.to be_allowed_for @u1 }
-      it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
-    end
+    it { is_expected.to be_allowed_for @u1 }
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
+  end
 
-    describe "GET /profile/groups" do
-      subject { profile_groups_path }
+  describe "GET /profile/notifications" do
+    subject { profile_notifications_path }
 
-      it { is_expected.to be_allowed_for @u1 }
-      it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
-    end
+    it { is_expected.to be_allowed_for @u1 }
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
   end
 end