diff --git a/CHANGELOG b/CHANGELOG index b8f95994213..d88e45c0fc3 100644 --- a/CHANGELOG +++ b/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 diff --git a/app/controllers/profiles/groups_controller.rb b/app/controllers/dashboard/groups_controller.rb similarity index 71% rename from app/controllers/profiles/groups_controller.rb rename to app/controllers/dashboard/groups_controller.rb index ce9dd50df67..61d691e6368 100644 --- a/app/controllers/profiles/groups_controller.rb +++ b/app/controllers/dashboard/groups_controller.rb @@ -1,6 +1,4 @@ -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 diff --git a/app/views/profiles/groups/index.html.haml b/app/views/dashboard/groups/index.html.haml similarity index 83% rename from app/views/profiles/groups/index.html.haml rename to app/views/dashboard/groups/index.html.haml index daf76636ff2..fd7bbb5500c 100644 --- a/app/views/profiles/groups/index.html.haml +++ b/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 diff --git a/app/views/groups/group_members/_group_member.html.haml b/app/views/groups/group_members/_group_member.html.haml index 21029c3a07d..30c3c2b00df 100644 --- a/app/views/groups/group_members/_group_member.html.haml +++ b/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 diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index 304744ba251..a22ddaf1cfd 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/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 diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 0914d2a167a..d88e862829d 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index 6dd9ded0193..1b855cd7a32 100644 --- a/config/routes.rb +++ b/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 diff --git a/features/profile/group.feature b/features/dashboard/group.feature similarity index 83% rename from features/profile/group.feature rename to features/dashboard/group.feature index e2fbfde77be..92c1379ba77 100644 --- a/features/profile/group.feature +++ b/features/dashboard/group.feature @@ -1,5 +1,5 @@ -@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 diff --git a/features/steps/profile/group.rb b/features/steps/dashboard/group.rb similarity index 95% rename from features/steps/profile/group.rb rename to features/steps/dashboard/group.rb index 0a10e04e219..09d7717b67b 100644 --- a/features/steps/profile/group.rb +++ b/features/steps/dashboard/group.rb @@ -1,4 +1,4 @@ -class Spinach::Features::ProfileGroup < Spinach::FeatureSteps +class Spinach::Features::DashboardGroup < Spinach::FeatureSteps include SharedAuthentication include SharedGroup include SharedPaths diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 835b644e6c7..db6417bf951 100644 --- a/features/steps/shared/paths.rb +++ b/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 # ---------------------------------------- diff --git a/spec/features/security/dashboard_access_spec.rb b/spec/features/security/dashboard_access_spec.rb index d1f00a3dd82..3d2d8a3502c 100644 --- a/spec/features/security/dashboard_access_spec.rb +++ b/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 diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb index 5f254c42e58..2512a9c0e3d 100644 --- a/spec/features/security/profile_access_spec.rb +++ b/spec/features/security/profile_access_spec.rb @@ -1,76 +1,65 @@ require 'spec_helper' -describe "Users Security", feature: true do - describe "Project" do - before do - @u1 = create(:user) - end +describe "Profile access", feature: true 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 /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 "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" do - subject { profile_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/account" do - subject { profile_account_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/design" do - subject { design_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/history" do - subject { history_profile_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/notifications" do - subject { profile_notifications_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 - - describe "GET /profile/groups" do - subject { profile_groups_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