diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 31214818496..bbaa4e4d91e 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 */ @@ -476,7 +475,9 @@ import { ajaxGet, convertPermissionToBoolean } from './lib/utils/common_utils'; 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 3b8e2c5b2f3..24abc5c5c9e 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 6400b72742c..1f7b6703909 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -516,7 +516,7 @@ a.deploy-project-label { text-align: center; width: 169px; - &:hover, + &:hover:not(.disabled), &.forked { background-color: $row-hover; border-color: $row-hover-border; @@ -543,6 +543,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 8f7c01bb71f..64e550d19d0 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 983f0e52d31..5b8cf2e6ab5 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -52,6 +52,29 @@ describe GlobalPolicy do 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 + describe 'custom attributes' do context 'regular user' do it { is_expected.not_to be_allowed(:read_custom_attribute) } 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