diff options
| -rw-r--r-- | app/helpers/users_helper.rb | 22 | ||||
| -rw-r--r-- | app/policies/user_policy.rb | 6 | ||||
| -rw-r--r-- | app/views/layouts/header/_current_user_dropdown.html.haml | 22 | ||||
| -rw-r--r-- | app/views/layouts/header/_default.html.haml | 17 | ||||
| -rw-r--r-- | qa/qa/page/menu/main.rb | 7 | ||||
| -rw-r--r-- | spec/helpers/users_helper_spec.rb | 25 | ||||
| -rw-r--r-- | spec/policies/user_policy_spec.rb | 18 |
7 files changed, 92 insertions, 25 deletions
diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 01af68088df..517268175e6 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -23,9 +23,31 @@ module UsersHelper profile_tabs.include?(tab) end + def current_user_menu_items + @current_user_menu_items ||= get_current_user_menu_items + end + + def current_user_menu?(item) + current_user_menu_items.include?(item) + end + private def get_profile_tabs [:activity, :groups, :contributed, :projects, :snippets] end + + def get_current_user_menu_items + items = [:help, :sign_out] + + if can?(current_user, :read_user, current_user) + items << :profile + end + + if can?(current_user, :update_user, current_user) + items << :settings + end + + items + end end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 0905ddd9b38..ee219f0a0d0 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -8,6 +8,8 @@ class UserPolicy < BasePolicy rule { ~restricted_public_level }.enable :read_user rule { ~anonymous }.enable :read_user - rule { user_is_self | admin }.enable :destroy_user - rule { subject_ghost }.prevent :destroy_user + rule { ~subject_ghost & (user_is_self | admin) }.policy do + enable :destroy_user + enable :update_user + end end diff --git a/app/views/layouts/header/_current_user_dropdown.html.haml b/app/views/layouts/header/_current_user_dropdown.html.haml new file mode 100644 index 00000000000..24b6c490a5a --- /dev/null +++ b/app/views/layouts/header/_current_user_dropdown.html.haml @@ -0,0 +1,22 @@ +- return unless current_user + +%ul + %li.current-user + .user-name.bold + = current_user.name + = current_user.to_reference + %li.divider + - if current_user_menu?(:profile) + %li + = link_to s_("CurrentUser|Profile"), current_user, class: 'profile-link', data: { user: current_user.username } + - if current_user_menu?(:settings) + %li + = link_to s_("CurrentUser|Settings"), profile_path + - if current_user_menu?(:help) + %li + = link_to _("Help"), help_path + - if current_user_menu?(:help) || current_user_menu?(:settings) || current_user_menu?(:profile) + %li.divider + - if current_user_menu?(:sign_out) + %li + = link_to _("Sign out"), destroy_user_session_path, class: "sign-out-link" diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index e6238c0dddb..dc121812406 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -53,22 +53,7 @@ = image_tag avatar_icon_for_user(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" = sprite_icon('angle-down', css_class: 'caret-down') .dropdown-menu-nav.dropdown-menu-align-right - %ul - %li.current-user - .user-name.bold - = current_user.name - @#{current_user.username} - %li.divider - %li - = link_to "Profile", current_user, class: 'profile-link', data: { user: current_user.username } - %li - = link_to "Settings", profile_path - - if current_user - %li - = link_to "Help", help_path - %li.divider - %li - = link_to "Sign out", destroy_user_session_path, class: "sign-out-link" + = render 'layouts/header/current_user_dropdown' - if header_link?(:admin_impersonation) %li.impersonation = link_to admin_impersonation_path, class: 'impersonation-btn', method: :delete, title: "Stop impersonation", aria: { label: 'Stop impersonation' }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do diff --git a/qa/qa/page/menu/main.rb b/qa/qa/page/menu/main.rb index df93a5fa2d2..d3562effaab 100644 --- a/qa/qa/page/menu/main.rb +++ b/qa/qa/page/menu/main.rb @@ -2,12 +2,15 @@ module QA module Page module Menu class Main < Page::Base + view 'app/views/layouts/header/_current_user_dropdown.html.haml' do + element :user_sign_out_link, 'link_to _("Sign out")' + element :settings_link, 'link_to s_("CurrentUser|Settings")' + end + view 'app/views/layouts/header/_default.html.haml' do element :navbar element :user_avatar element :user_menu, '.dropdown-menu-nav' - element :user_sign_out_link, 'link_to "Sign out"' - element :settings_link, 'link_to "Settings"' end view 'app/views/layouts/nav/_dashboard.html.haml' do diff --git a/spec/helpers/users_helper_spec.rb b/spec/helpers/users_helper_spec.rb index 6332217b920..9f67277801f 100644 --- a/spec/helpers/users_helper_spec.rb +++ b/spec/helpers/users_helper_spec.rb @@ -27,4 +27,29 @@ describe UsersHelper do expect(tabs).to include(:activity, :groups, :contributed, :projects, :snippets) end end + + describe '#current_user_menu_items' do + subject(:items) { helper.current_user_menu_items } + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?).and_return(false) + end + + it 'includes all default items' do + expect(items).to include(:help, :sign_out) + end + + it 'includes the profile tab if the user can read themself' do + expect(helper).to receive(:can?).with(user, :read_user, user) { true } + + expect(items).to include(:profile) + end + + it 'includes the settings tab if the user can update themself' do + expect(helper).to receive(:can?).with(user, :read_user, user) { true } + + expect(items).to include(:profile) + end + end end diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 6593a6ca3b9..a7a77abc3ee 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -10,28 +10,36 @@ describe UserPolicy do it { is_expected.to be_allowed(:read_user) } end - describe "destroying a user" do + shared_examples 'changing a user' do |ability| context "when a regular user tries to destroy another regular user" do - it { is_expected.not_to be_allowed(:destroy_user) } + it { is_expected.not_to be_allowed(ability) } end context "when a regular user tries to destroy themselves" do let(:current_user) { user } - it { is_expected.to be_allowed(:destroy_user) } + it { is_expected.to be_allowed(ability) } end context "when an admin user tries to destroy a regular user" do let(:current_user) { create(:user, :admin) } - it { is_expected.to be_allowed(:destroy_user) } + it { is_expected.to be_allowed(ability) } end context "when an admin user tries to destroy a ghost user" do let(:current_user) { create(:user, :admin) } let(:user) { create(:user, :ghost) } - it { is_expected.not_to be_allowed(:destroy_user) } + it { is_expected.not_to be_allowed(ability) } end end + + describe "destroying a user" do + it_behaves_like 'changing a user', :destroy_user + end + + describe "updating a user" do + it_behaves_like 'changing a user', :update_user + end end |