diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2017-09-29 15:55:14 +0000 |
---|---|---|
committer | Jose Ivan Vargas <jvargas@gitlab.com> | 2017-10-04 11:56:16 -0500 |
commit | 6c4bf7fbbfeee4d5c2f49deebfec36d45f80c9b4 (patch) | |
tree | 049b3c444ed037d3ad444d4b5c6cb2fb294c465c | |
parent | 512460981d1dd5a29183ad7ee730bfe2cf01792f (diff) | |
download | gitlab-ce-6c4bf7fbbfeee4d5c2f49deebfec36d45f80c9b4.tar.gz |
Merge branch 'fork-btn-enabled-user-groups' into 'master'
Fix fork button being disabled for users who can fork to group
Closes #38462
See merge request gitlab-org/gitlab-ce!14551
-rw-r--r-- | app/assets/javascripts/dispatcher.js | 5 | ||||
-rw-r--r-- | app/assets/javascripts/main.js | 1 | ||||
-rw-r--r-- | app/assets/javascripts/project_fork.js | 19 | ||||
-rw-r--r-- | app/assets/stylesheets/pages/projects.scss | 11 | ||||
-rw-r--r-- | app/policies/global_policy.rb | 6 | ||||
-rw-r--r-- | app/policies/namespace_policy.rb | 4 | ||||
-rw-r--r-- | app/views/projects/buttons/_fork.html.haml | 9 | ||||
-rw-r--r-- | app/views/projects/forks/new.html.haml | 10 | ||||
-rw-r--r-- | changelogs/unreleased/fork-btn-enabled-user-groups.yml | 5 | ||||
-rw-r--r-- | spec/features/projects/fork_spec.rb | 57 | ||||
-rw-r--r-- | spec/policies/global_policy_spec.rb | 23 | ||||
-rw-r--r-- | spec/policies/namespace_policy_spec.rb | 20 |
12 files changed, 146 insertions, 24 deletions
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index a0ed5c23ffe..be21116f6fb 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -14,7 +14,6 @@ /* global NotificationsDropdown */ /* global GroupAvatar */ /* global LineHighlighter */ -/* global ProjectFork */ /* global BuildArtifacts */ /* global GroupsSelect */ /* global Search */ @@ -468,7 +467,9 @@ import initChangesDropdown from './init_changes_dropdown'; shortcut_handler = true; break; case 'projects:forks:new': - new ProjectFork(); + import(/* webpackChunkName: 'project_fork' */ './project_fork') + .then(fork => fork.default()) + .catch(() => {}); break; case 'projects:artifacts:browse': new ShortcutsNavigation(); diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 534eee9101a..0af3eba2a87 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -124,7 +124,6 @@ import './preview_markdown'; import './project'; import './project_avatar'; import './project_find_file'; -import './project_fork'; import './project_import'; import './project_label_subscription'; import './project_new'; diff --git a/app/assets/javascripts/project_fork.js b/app/assets/javascripts/project_fork.js index 47197db39d3..68cf47fd54e 100644 --- a/app/assets/javascripts/project_fork.js +++ b/app/assets/javascripts/project_fork.js @@ -1,13 +1,8 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, prefer-arrow-callback, max-len */ -(function() { - this.ProjectFork = (function() { - function ProjectFork() { - $('.fork-thumbnail a').on('click', function() { - $('.fork-namespaces').hide(); - return $('.save-project-loader').show(); - }); - } +export default () => { + $('.fork-thumbnail a').on('click', function forkThumbnailClicked() { + if ($(this).hasClass('disabled')) return false; - return ProjectFork; - })(); -}).call(window); + $('.fork-namespaces').hide(); + return $('.save-project-loader').show(); + }); +}; diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index d01326637ea..c53ad008567 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -417,7 +417,7 @@ a.deploy-project-label { text-align: center; width: 169px; - &:hover, + &:hover:not(.disabled), &.forked { background-color: $row-hover; border-color: $row-hover-border; @@ -444,6 +444,15 @@ a.deploy-project-label { padding-top: $gl-padding; color: $gl-text-color; + &.disabled { + opacity: .3; + cursor: not-allowed; + + &:hover { + text-decoration: none; + } + } + .caption { min-height: 30px; padding: $gl-padding 0; diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 1be7bbe9953..0bf1e12b6db 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -11,6 +11,8 @@ class GlobalPolicy < BasePolicy with_options scope: :user, score: 0 condition(:access_locked) { @user.access_locked? } + condition(:can_create_fork, scope: :user) { @user.manageable_namespaces.any? { |namespace| @user.can?(:create_projects, namespace) } } + rule { anonymous }.policy do prevent :log_in prevent :access_api @@ -40,6 +42,10 @@ class GlobalPolicy < BasePolicy enable :create_group end + rule { can_create_fork }.policy do + enable :create_fork + end + rule { access_locked }.policy do prevent :log_in end diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb index 85b67f0a237..92213f0155e 100644 --- a/app/policies/namespace_policy.rb +++ b/app/policies/namespace_policy.rb @@ -1,10 +1,14 @@ class NamespacePolicy < BasePolicy rule { anonymous }.prevent_all + condition(:personal_project, scope: :subject) { @subject.kind == 'user' } + condition(:can_create_personal_project, scope: :user) { @user.can_create_project? } condition(:owner) { @subject.owner == @user } rule { owner | admin }.policy do enable :create_projects enable :admin_namespace end + + rule { personal_project & ~can_create_personal_project }.prevent :create_projects end diff --git a/app/views/projects/buttons/_fork.html.haml b/app/views/projects/buttons/_fork.html.haml index f45cc7f0f45..f880556a9f7 100644 --- a/app/views/projects/buttons/_fork.html.haml +++ b/app/views/projects/buttons/_fork.html.haml @@ -4,12 +4,11 @@ = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: _('Go to your fork'), class: 'btn has-tooltip' do = custom_icon('icon_fork') %span= s_('GoToYourFork|Fork') - - elsif !current_user.can_create_project? - = link_to new_project_fork_path(@project), title: _('You have reached your project limit'), class: 'btn has-tooltip disabled' do - = custom_icon('icon_fork') - %span= s_('CreateNewFork|Fork') - else - = link_to new_project_fork_path(@project), class: 'btn' do + - can_create_fork = current_user.can?(:create_fork) + = link_to new_project_fork_path(@project), + class: "btn btn-default #{'has-tooltip disabled' unless can_create_fork}", + title: (_('You have reached your project limit') unless can_create_fork) do = custom_icon('icon_fork') %span= s_('CreateNewFork|Fork') .count-with-arrow diff --git a/app/views/projects/forks/new.html.haml b/app/views/projects/forks/new.html.haml index 0f36e1a7353..906774a21e3 100644 --- a/app/views/projects/forks/new.html.haml +++ b/app/views/projects/forks/new.html.haml @@ -13,7 +13,7 @@ - if @namespaces.present? %label.label-light %span - Click to fork the project to a user or group + Click to fork the project - @namespaces.in_groups_of(6, false) do |group| .row - group.each do |namespace| @@ -29,8 +29,12 @@ .caption = namespace.human_name - else - .fork-thumbnail - = link_to project_forks_path(@project, namespace_key: namespace.id), method: "POST" do + - can_create_project = current_user.can?(:create_projects, namespace) + .fork-thumbnail{ class: ("disabled" unless can_create_project) } + = link_to project_forks_path(@project, namespace_key: namespace.id), + method: "POST", + class: ("disabled has-tooltip" unless can_create_project), + title: (_('You have reached your project limit') unless can_create_project) do - if /no_((\w*)_)*avatar/.match(avatar) .no-avatar = icon 'question' diff --git a/changelogs/unreleased/fork-btn-enabled-user-groups.yml b/changelogs/unreleased/fork-btn-enabled-user-groups.yml new file mode 100644 index 00000000000..3bd7581a961 --- /dev/null +++ b/changelogs/unreleased/fork-btn-enabled-user-groups.yml @@ -0,0 +1,5 @@ +--- +title: Fixed fork button being disabled for users who can fork to a group +merge_request: +author: +type: fixed diff --git a/spec/features/projects/fork_spec.rb b/spec/features/projects/fork_spec.rb new file mode 100644 index 00000000000..e10d29e5eea --- /dev/null +++ b/spec/features/projects/fork_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe 'Project fork' do + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + + before do + sign_in user + end + + it 'allows user to fork project' do + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + end + + it 'disables fork button when user has exceeded project limit' do + user.projects_limit = 0 + user.save! + + visit project_path(project) + + expect(page).to have_css('a.disabled', text: 'Fork') + end + + context 'master in group' do + before do + group = create(:group) + group.add_master(user) + end + + it 'allows user to fork project to group or to user namespace' do + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + + click_link 'Fork' + + expect(page).to have_css('.fork-thumbnail', count: 2) + expect(page).not_to have_css('.fork-thumbnail.disabled') + end + + it 'allows user to fork project to group and not user when exceeded project limit' do + user.projects_limit = 0 + user.save! + + visit project_path(project) + + expect(page).not_to have_css('a.disabled', text: 'Fork') + + click_link 'Fork' + + expect(page).to have_css('.fork-thumbnail', count: 2) + expect(page).to have_css('.fork-thumbnail.disabled') + end + end +end diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index a6bf70c1e09..0ca1d03c2d3 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -51,4 +51,27 @@ describe GlobalPolicy do end end end + + describe "create fork" do + context "when user has not exceeded project limit" do + it { is_expected.to be_allowed(:create_fork) } + end + + context "when user has exceeded project limit" do + let(:current_user) { create(:user, projects_limit: 0) } + + it { is_expected.not_to be_allowed(:create_fork) } + end + + context "when user is a master in a group" do + let(:group) { create(:group) } + let(:current_user) { create(:user, projects_limit: 0) } + + before do + group.add_master(current_user) + end + + it { is_expected.to be_allowed(:create_fork) } + end + end end diff --git a/spec/policies/namespace_policy_spec.rb b/spec/policies/namespace_policy_spec.rb new file mode 100644 index 00000000000..e52ff02e5f0 --- /dev/null +++ b/spec/policies/namespace_policy_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe NamespacePolicy do + let(:current_user) { create(:user) } + let(:namespace) { current_user.namespace } + + subject { described_class.new(current_user, namespace) } + + context "create projects" do + context "user namespace" do + it { is_expected.to be_allowed(:create_projects) } + end + + context "user who has exceeded project limit" do + let(:current_user) { create(:user, projects_limit: 0) } + + it { is_expected.not_to be_allowed(:create_projects) } + end + end +end |