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

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