Commit 85541e2d authored by Illya Klymov's avatar Illya Klymov Committed by Markus Koller

Improve fork page design

Introduce lazy loading of all groups except own namespace
parent d0d05df1
...@@ -35,7 +35,7 @@ export default { ...@@ -35,7 +35,7 @@ export default {
}, },
}, },
data() { data() {
return { namespaces: null }; return { namespaces: null, isForking: false };
}, },
computed: { computed: {
...@@ -67,6 +67,13 @@ export default { ...@@ -67,6 +67,13 @@ export default {
}, },
}, },
methods: {
fork() {
this.isForking = true;
this.$refs.form.submit();
},
},
i18n: { i18n: {
hasReachedProjectLimitMessage: __('You have reached your project limit'), hasReachedProjectLimitMessage: __('You have reached your project limit'),
insufficientPermissionsMessage: __( insufficientPermissionsMessage: __(
...@@ -124,14 +131,17 @@ export default { ...@@ -124,14 +131,17 @@ export default {
> >
<template v-else> <template v-else>
<div ref="selectButtonWrapper"> <div ref="selectButtonWrapper">
<form method="POST" :action="group.fork_path"> <form ref="form" method="POST" :action="group.fork_path">
<input type="hidden" name="authenticity_token" :value="$options.csrf.token" /> <input type="hidden" name="authenticity_token" :value="$options.csrf.token" />
<gl-button <gl-button
type="submit" type="submit"
class="gl-h-7 gl-text-decoration-none!" class="gl-h-7"
:data-qa-name="group.full_name" :data-qa-name="group.full_name"
category="secondary"
variant="success" variant="success"
:disabled="isSelectButtonDisabled" :disabled="isSelectButtonDisabled"
:loading="isForking"
@click="fork"
>{{ __('Select') }}</gl-button >{{ __('Select') }}</gl-button
> >
</form> </form>
......
import ProjectFork from '~/project_fork'; import Vue from 'vue';
import { parseBoolean } from '~/lib/utils/common_utils';
import ForkGroupsList from './components/fork_groups_list.vue';
document.addEventListener('DOMContentLoaded', () => new ProjectFork()); document.addEventListener('DOMContentLoaded', () => {
const mountElement = document.getElementById('fork-groups-mount-element');
const { endpoint, canCreateProject } = mountElement.dataset;
const hasReachedProjectLimit = !parseBoolean(canCreateProject);
return new Vue({
el: mountElement,
render(h) {
return h(ForkGroupsList, {
props: {
endpoint,
hasReachedProjectLimit,
},
});
},
});
});
import $ from 'jquery';
export default () => {
$('.js-fork-thumbnail').on('click', function forkThumbnailClicked() {
if ($(this).hasClass('disabled')) return false;
return $('.js-fork-content').toggleClass('hidden');
});
};
...@@ -472,17 +472,9 @@ ...@@ -472,17 +472,9 @@
margin-right: auto; margin-right: auto;
} }
a { a.disabled {
display: block; opacity: 0.3;
width: 100%; cursor: not-allowed;
height: 100%;
padding-top: $gl-padding;
text-decoration: none;
&.disabled {
opacity: 0.3;
cursor: not-allowed;
}
} }
} }
...@@ -1538,3 +1530,10 @@ pre.light-well { ...@@ -1538,3 +1530,10 @@ pre.light-well {
} }
} }
} }
@include media-breakpoint-down(xs) {
.fork-filtered-search {
width: 100%;
margin: $gl-spacing-scale-2 0;
}
}
...@@ -36,7 +36,19 @@ class Projects::ForksController < Projects::ApplicationController ...@@ -36,7 +36,19 @@ class Projects::ForksController < Projects::ApplicationController
end end
def new def new
@namespaces = fork_service.valid_fork_targets - [project.namespace] respond_to do |format|
format.html do
@own_namespace = current_user.namespace if fork_service.valid_fork_targets.include?(current_user.namespace)
@project = project
end
format.json do
namespaces = fork_service.valid_fork_targets - [current_user.namespace, project.namespace]
render json: {
namespaces: ForkNamespaceSerializer.new.represent(namespaces, project: project, current_user: current_user)
}
end
end
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
......
...@@ -14,10 +14,10 @@ module Projects ...@@ -14,10 +14,10 @@ module Projects
@valid_fork_targets ||= ForkTargetsFinder.new(@project, current_user).execute @valid_fork_targets ||= ForkTargetsFinder.new(@project, current_user).execute
end end
def valid_fork_target? def valid_fork_target?(namespace = target_namespace)
return true if current_user.admin? return true if current_user.admin?
valid_fork_targets.include?(target_namespace) valid_fork_targets.include?(namespace)
end end
private private
......
- avatar = namespace_icon(namespace, 100) - avatar = namespace_icon(namespace, 100)
- can_create_project = current_user.can?(:create_projects, namespace) - can_create_project = current_user.can?(:create_projects, namespace)
- if forked_project = namespace.find_fork_of(@project) .bordered-box.fork-thumbnail.text-center.gl-m-3{ class: ("disabled" unless can_create_project) }
.bordered-box.fork-thumbnail.text-center.gl-ml-3.gl-mr-3.gl-mt-3.gl-mb-3.forked - if /no_((\w*)_)*avatar/.match(avatar)
= link_to project_path(forked_project) do = group_icon(namespace, class: "avatar rect-avatar s100 identicon mx-auto")
- if /no_((\w*)_)*avatar/.match(avatar) - else
= group_icon(namespace, class: "avatar rect-avatar s100 identicon mx-auto") .avatar-container.s100.mx-auto.gl-mt-5
- else = image_tag(avatar, class: "avatar s100")
.avatar-container.s100.mx-auto %h5.gl-mt-3
= image_tag(avatar, class: "avatar s100") = namespace.human_name
%h5.gl-mt-3 - if forked_project = namespace.find_fork_of(@project)
= namespace.human_name = link_to _("Go to project"), project_path(forked_project), class: "btn"
- else - else
.bordered-box.fork-thumbnail.text-center.gl-ml-3.gl-mr-3.gl-mt-3.gl-mb-3{ class: ("disabled" unless can_create_project) } %div{ class: ('has-tooltip' unless can_create_project),
= link_to project_forks_path(@project, namespace_key: namespace.id), title: (_('You have reached your project limit') unless can_create_project) }
method: "POST", = link_to _("Select"), project_forks_path(@project, namespace_key: namespace.id),
class: ("disabled has-tooltip" unless can_create_project), data: { qa_selector: 'fork_namespace_button', qa_name: namespace.human_name },
title: (_('You have reached your project limit') unless can_create_project) do method: "POST",
- if /no_((\w*)_)*avatar/.match(avatar) class: ["btn btn-success", ("disabled" unless can_create_project)]
= group_icon(namespace, class: "avatar rect-avatar s100 identicon mx-auto")
- else
.avatar-container.s100.mx-auto
= image_tag(avatar, class: "avatar s100")
%h5.gl-mt-3{ data: { qa_selector: 'fork_namespace_content', qa_name: namespace.human_name } }
= namespace.human_name
...@@ -7,15 +7,10 @@ ...@@ -7,15 +7,10 @@
%p %p
= _("A fork is a copy of a project.<br />Forking a repository allows you to make changes without affecting the original project.").html_safe = _("A fork is a copy of a project.<br />Forking a repository allows you to make changes without affecting the original project.").html_safe
.col-lg-9 .col-lg-9
- if @namespaces.present? - if @own_namespace.present?
.fork-thumbnail-container.js-fork-content .fork-thumbnail-container.js-fork-content
%h5.gl-mt-0.gl-mb-0.gl-ml-3.gl-mr-3 %h5.gl-mt-0.gl-mb-0.gl-ml-3.gl-mr-3
= _("Select a namespace to fork the project") = _("Select a namespace to fork the project")
- @namespaces.each do |namespace| = render 'fork_button', namespace: @own_namespace
= render 'fork_button', namespace: namespace #fork-groups-mount-element{ data: { endpoint: new_project_fork_path(@project, format: :json), can_create_project: current_user.can_create_project?.to_s } }
- else
%strong
= _("No available namespaces to fork the project.")
%p.gl-mt-3
= _("You must have permission to create a project in a namespace before forking.")
---
title: Improved fork page design
merge_request: 35592
author:
type: changed
...@@ -26,7 +26,7 @@ Forking a project is, in most cases, a two-step process. ...@@ -26,7 +26,7 @@ Forking a project is, in most cases, a two-step process.
NOTE: **Note:** NOTE: **Note:**
The project path must be unique within the namespace. The project path must be unique within the namespace.
![Choose namespace](img/forking_workflow_choose_namespace.png) ![Choose namespace](img/forking_workflow_choose_namespace_v13_2.png)
The fork is created. The permissions you have in the namespace are the permissions you will have in the fork. The fork is created. The permissions you have in the namespace are the permissions you will have in the fork.
......
...@@ -15713,9 +15713,6 @@ msgstr "" ...@@ -15713,9 +15713,6 @@ msgstr ""
msgid "No available groups to fork the project." msgid "No available groups to fork the project."
msgstr "" msgstr ""
msgid "No available namespaces to fork the project."
msgstr ""
msgid "No branches found" msgid "No branches found"
msgstr "" msgstr ""
......
...@@ -6,11 +6,11 @@ module QA ...@@ -6,11 +6,11 @@ module QA
module Fork module Fork
class New < Page::Base class New < Page::Base
view 'app/views/projects/forks/_fork_button.html.haml' do view 'app/views/projects/forks/_fork_button.html.haml' do
element :fork_namespace_content element :fork_namespace_button
end end
def choose_namespace(namespace = Runtime::Namespace.path) def choose_namespace(namespace = Runtime::Namespace.path)
click_element(:fork_namespace_content, name: namespace) click_element(:fork_namespace_button, name: namespace)
end end
end end
end end
......
...@@ -162,9 +162,25 @@ RSpec.describe Projects::ForksController do ...@@ -162,9 +162,25 @@ RSpec.describe Projects::ForksController do
end end
context 'when user is signed in' do context 'when user is signed in' do
it 'responds with status 200' do before do
sign_in(user) sign_in(user)
end
context 'when JSON requested' do
it 'responds with available groups' do
get :new,
format: :json,
params: {
namespace_id: project.namespace,
project_id: project
}
expect(json_response['namespaces'].length).to eq(1)
expect(json_response['namespaces'].first['id']).to eq(group.id)
end
end
it 'responds with status 200' do
subject subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
......
...@@ -15,7 +15,7 @@ RSpec.describe 'Project fork' do ...@@ -15,7 +15,7 @@ RSpec.describe 'Project fork' do
it 'allows user to fork project' do it 'allows user to fork project' do
visit project_path(project) visit project_path(project)
expect(page).not_to have_css('a.disabled', text: 'Fork') expect(page).not_to have_css('a.disabled', text: 'Select')
end end
it 'disables fork button when user has exceeded project limit' do it 'disables fork button when user has exceeded project limit' do
...@@ -40,7 +40,7 @@ RSpec.describe 'Project fork' do ...@@ -40,7 +40,7 @@ RSpec.describe 'Project fork' do
visit project_path(project) visit project_path(project)
expect(page).to have_css('a', text: 'Fork') expect(page).to have_css('a', text: 'Fork')
expect(page).not_to have_css('a.disabled', text: 'Fork') expect(page).not_to have_css('a.disabled', text: 'Select')
end end
it 'renders new project fork page' do it 'renders new project fork page' do
...@@ -116,7 +116,7 @@ RSpec.describe 'Project fork' do ...@@ -116,7 +116,7 @@ RSpec.describe 'Project fork' do
click_link 'Fork' click_link 'Fork'
page.within '.fork-thumbnail-container' do page.within '.fork-thumbnail-container' do
click_link user.name click_link 'Select'
end end
expect(page).to have_content 'Forked from' expect(page).to have_content 'Forked from'
...@@ -156,7 +156,7 @@ RSpec.describe 'Project fork' do ...@@ -156,7 +156,7 @@ RSpec.describe 'Project fork' do
click_link 'Fork' click_link 'Fork'
page.within '.fork-thumbnail-container' do page.within '.fork-thumbnail-container' do
click_link user.name click_link 'Select'
end end
visit project_forks_path(project) visit project_forks_path(project)
...@@ -193,7 +193,7 @@ RSpec.describe 'Project fork' do ...@@ -193,7 +193,7 @@ RSpec.describe 'Project fork' do
click_link 'Fork' click_link 'Fork'
page.within '.fork-thumbnail-container' do page.within '.fork-thumbnail-container' do
click_link user.name click_link 'Select'
end end
visit project_forks_path(project) visit project_forks_path(project)
...@@ -218,7 +218,7 @@ RSpec.describe 'Project fork' do ...@@ -218,7 +218,7 @@ RSpec.describe 'Project fork' do
click_link 'Fork' click_link 'Fork'
page.within '.fork-thumbnail-container' do page.within '.fork-thumbnail-container' do
click_link user.name click_link 'Select'
end end
expect(page).to have_content "Name has already been taken" expect(page).to have_content "Name has already been taken"
...@@ -232,39 +232,43 @@ RSpec.describe 'Project fork' do ...@@ -232,39 +232,43 @@ RSpec.describe 'Project fork' do
group.add_maintainer(user) group.add_maintainer(user)
end end
it 'allows user to fork project to group or to user namespace' do it 'allows user to fork project to group or to user namespace', :js do
visit project_path(project) visit project_path(project)
wait_for_requests
expect(page).not_to have_css('a.disabled', text: 'Fork') expect(page).not_to have_css('a.disabled', text: 'Fork')
click_link 'Fork' click_link 'Fork'
expect(page).to have_css('.fork-thumbnail', count: 2) expect(page).to have_css('.fork-thumbnail')
expect(page).to have_css('.group-row')
expect(page).not_to have_css('.fork-thumbnail.disabled') expect(page).not_to have_css('.fork-thumbnail.disabled')
end end
it 'allows user to fork project to group and not user when exceeded project limit' do it 'allows user to fork project to group and not user when exceeded project limit', :js do
user.projects_limit = 0 user.projects_limit = 0
user.save! user.save!
visit project_path(project) visit project_path(project)
wait_for_requests
expect(page).not_to have_css('a.disabled', text: 'Fork') expect(page).not_to have_css('a.disabled', text: 'Fork')
click_link 'Fork' click_link 'Fork'
expect(page).to have_css('.fork-thumbnail', count: 2)
expect(page).to have_css('.fork-thumbnail.disabled') expect(page).to have_css('.fork-thumbnail.disabled')
expect(page).to have_css('.group-row')
end end
it 'links to the fork if the project was already forked within that namespace', :sidekiq_might_not_need_inline do it 'links to the fork if the project was already forked within that namespace', :sidekiq_might_not_need_inline, :js do
forked_project = fork_project(project, user, namespace: group, repository: true) forked_project = fork_project(project, user, namespace: group, repository: true)
visit new_project_fork_path(project) visit new_project_fork_path(project)
wait_for_requests
expect(page).to have_css('div.forked', text: group.full_name) expect(page).to have_css('.group-row a.btn', text: 'Go to fork')
click_link group.full_name click_link 'Go to fork'
expect(current_path).to eq(project_path(forked_project)) expect(current_path).to eq(project_path(forked_project))
end end
......
...@@ -439,37 +439,71 @@ RSpec.describe Projects::ForkService do ...@@ -439,37 +439,71 @@ RSpec.describe Projects::ForkService do
end end
describe '#valid_fork_target?' do describe '#valid_fork_target?' do
subject { described_class.new(project, user, params).valid_fork_target? }
let(:project) { Project.new } let(:project) { Project.new }
let(:params) { {} } let(:params) { {} }
context 'when current user is an admin' do context 'when target is not passed' do
let(:user) { build(:user, :admin) } subject { described_class.new(project, user, params).valid_fork_target? }
it { is_expected.to be_truthy } context 'when current user is an admin' do
end let(:user) { build(:user, :admin) }
context 'when current_user is not an admin' do it { is_expected.to be_truthy }
let(:user) { create(:user) } end
let(:finder_mock) { instance_double('ForkTargetsFinder', execute: [user.namespace]) } context 'when current_user is not an admin' do
let(:project) { create(:project) } let(:user) { create(:user) }
before do let(:finder_mock) { instance_double('ForkTargetsFinder', execute: [user.namespace]) }
allow(ForkTargetsFinder).to receive(:new).with(project, user).and_return(finder_mock) let(:project) { create(:project) }
before do
allow(ForkTargetsFinder).to receive(:new).with(project, user).and_return(finder_mock)
end
context 'when target namespace is in valid fork targets' do
let(:params) { { namespace: user.namespace } }
it { is_expected.to be_truthy }
end
context 'when target namespace is not in valid fork targets' do
let(:params) { { namespace: create(:group) } }
it { is_expected.to be_falsey }
end
end end
end
context 'when target is passed' do
let(:target) { create(:group) }
context 'when target namespace is in valid fork targets' do subject { described_class.new(project, user, params).valid_fork_target?(target) }
let(:params) { { namespace: user.namespace } }
context 'when current user is an admin' do
let(:user) { build(:user, :admin) }
it { is_expected.to be_truthy } it { is_expected.to be_truthy }
end end
context 'when target namespace is not in valid fork targets' do context 'when current user is not an admin' do
let(:params) { { namespace: create(:group) } } let(:user) { create(:user) }
before do
allow(ForkTargetsFinder).to receive(:new).with(project, user).and_return(finder_mock)
end
context 'when target namespace is in valid fork targets' do
let(:finder_mock) { instance_double('ForkTargetsFinder', execute: [target]) }
it { is_expected.to be_truthy }
end
context 'when target namespace is not in valid fork targets' do
let(:finder_mock) { instance_double('ForkTargetsFinder', execute: [create(:group)]) }
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment