Commit f8d862a7 authored by Nicolò Maria Mezzopera's avatar Nicolò Maria Mezzopera

Merge branch 'ntepluhina-move-design-management' into 'master'

Move design management app to the issue tab

Closes #223193

See merge request gitlab-org/gitlab!35042
parents 658b5eae e97fb6f7
......@@ -13,13 +13,14 @@ export default {
type: Array,
required: true,
},
},
inject: {
projectPath: {
type: String,
required: true,
default: '',
},
iid: {
type: String,
required: true,
from: 'issueIid',
defaut: '',
},
},
computed: {
......
......@@ -60,7 +60,7 @@ export default {
},
mounted() {
if (this.isNoteLinked) {
this.$refs.anchor.$el.scrollIntoView({ behavior: 'smooth', inline: 'start' });
this.$el.scrollIntoView({ behavior: 'smooth', inline: 'start' });
}
},
methods: {
......@@ -80,7 +80,7 @@ export default {
</script>
<template>
<timeline-entry-item :id="`note_${noteAnchorId}`" ref="anchor" class="design-note note-form">
<timeline-entry-item :id="`note_${noteAnchorId}`" class="design-note note-form">
<user-avatar-link
:link-href="author.webUrl"
:img-src="author.avatarUrl"
......
......@@ -6,7 +6,6 @@ import timeagoMixin from '~/vue_shared/mixins/timeago';
import Pagination from './pagination.vue';
import DeleteButton from '../delete_button.vue';
import permissionsQuery from '../../graphql/queries/design_permissions.query.graphql';
import appDataQuery from '../../graphql/queries/app_data.query.graphql';
import { DESIGNS_ROUTE_NAME } from '../../router/constants';
export default {
......@@ -55,19 +54,17 @@ export default {
permissions: {
createDesign: false,
},
projectPath: '',
issueIid: null,
};
},
apollo: {
appData: {
query: appDataQuery,
manual: true,
result({ data: { projectPath, issueIid } }) {
this.projectPath = projectPath;
this.issueIid = issueIid;
inject: {
projectPath: {
default: '',
},
issueIid: {
default: '',
},
},
apollo: {
permissions: {
query: permissionsQuery,
variables() {
......@@ -102,6 +99,7 @@ export default {
query: $route.query,
}"
:aria-label="s__('DesignManagement|Go back to designs')"
data-testid="close-design"
class="mr-3 text-plain d-flex justify-content-center align-items-center"
>
<icon :size="18" name="close" />
......
query projectFullPath {
projectPath @client
issueIid @client
}
import $ from 'jquery';
import Vue from 'vue';
import createRouter from './router';
import App from './components/app.vue';
import apolloProvider from './graphql';
import getDesignListQuery from './graphql/queries/get_design_list.query.graphql';
import { DESIGNS_ROUTE_NAME, ROOT_ROUTE_NAME } from './router/constants';
export default () => {
const el = document.querySelector('.js-design-management-new');
const badge = document.querySelector('.js-designs-count');
const { issueIid, projectPath, issuePath } = el.dataset;
const router = createRouter(issuePath);
$('.js-issue-tabs').on('shown.bs.tab', ({ target: { id } }) => {
if (id === 'designs' && router.currentRoute.name === ROOT_ROUTE_NAME) {
router.push({ name: DESIGNS_ROUTE_NAME });
} else if (id === 'discussion') {
router.push({ name: ROOT_ROUTE_NAME });
}
});
apolloProvider.clients.defaultClient.cache.writeData({
data: {
projectPath,
issueIid,
activeDiscussion: {
__typename: 'ActiveDiscussion',
id: null,
......@@ -32,25 +18,14 @@ export default () => {
},
});
apolloProvider.clients.defaultClient
.watchQuery({
query: getDesignListQuery,
variables: {
fullPath: projectPath,
iid: issueIid,
atVersion: null,
},
})
.subscribe(({ data }) => {
if (badge) {
badge.textContent = data.project.issue.designCollection.designs.edges.length;
}
});
return new Vue({
el,
router,
apolloProvider,
provide: {
projectPath,
issueIid,
},
render(createElement) {
return createElement(App);
},
......
import getDesignListQuery from '../graphql/queries/get_design_list.query.graphql';
import appDataQuery from '../graphql/queries/app_data.query.graphql';
import { findVersionId } from '../utils/design_management_utils';
export default {
apollo: {
appData: {
query: appDataQuery,
manual: true,
result({ data: { projectPath, issueIid } }) {
this.projectPath = projectPath;
this.issueIid = issueIid;
},
},
allVersions: {
query: getDesignListQuery,
variables() {
......@@ -24,6 +15,14 @@ export default {
update: data => data.project.issue.designCollection.versions.edges,
},
},
inject: {
projectPath: {
default: '',
},
issueIid: {
default: '',
},
},
computed: {
hasValidVersion() {
return (
......@@ -55,8 +54,6 @@ export default {
data() {
return {
allVersions: [],
projectPath: '',
issueIid: null,
};
},
};
......@@ -12,7 +12,6 @@ import DesignPresentation from '../../components/design_presentation.vue';
import DesignReplyForm from '../../components/design_notes/design_reply_form.vue';
import DesignSidebar from '../../components/design_sidebar.vue';
import getDesignQuery from '../../graphql/queries/get_design.query.graphql';
import appDataQuery from '../../graphql/queries/app_data.query.graphql';
import createImageDiffNoteMutation from '../../graphql/mutations/create_image_diff_note.mutation.graphql';
import updateImageDiffNoteMutation from '../../graphql/mutations/update_image_diff_note.mutation.graphql';
import updateActiveDiscussionMutation from '../../graphql/mutations/update_active_discussion.mutation.graphql';
......@@ -62,22 +61,12 @@ export default {
design: {},
comment: '',
annotationCoordinates: null,
projectPath: '',
errorMessage: '',
issueIid: '',
scale: 1,
resolvedDiscussionsExpanded: false,
};
},
apollo: {
appData: {
query: appDataQuery,
manual: true,
result({ data: { projectPath, issueIid } }) {
this.projectPath = projectPath;
this.issueIid = issueIid;
},
},
design: {
query: getDesignQuery,
// We want to see cached design version if we have one, and fetch newer version on the background to update discussions
......
......@@ -259,7 +259,7 @@ export default {
</script>
<template>
<div>
<div data-testid="designs-root">
<header v-if="showToolbar" class="row-content-block border-top-0 p-2 d-flex">
<div class="d-flex justify-content-between align-items-center w-100">
<design-version-dropdown />
......@@ -274,8 +274,6 @@ export default {
<design-destroyer
#default="{ mutate, loading }"
:filenames="selectedDesigns"
:project-path="projectPath"
:iid="issueIid"
@done="onDesignDelete"
@error="onDesignDeleteError"
>
......
export const ROOT_ROUTE_NAME = 'root';
export const DESIGNS_ROUTE_NAME = 'designs';
export const DESIGN_ROUTE_NAME = 'design';
import $ from 'jquery';
import Vue from 'vue';
import VueRouter from 'vue-router';
import routes from './routes';
......@@ -16,9 +15,7 @@ export default function createRouter(base) {
});
const pageEl = getPageLayoutElement();
router.beforeEach(({ meta: { el }, name }, _, next) => {
$(`#${el}`).tab('show');
router.beforeEach(({ name }, _, next) => {
// apply a fullscreen layout style in Design View (a.k.a design detail)
if (pageEl) {
if (name === DESIGN_ROUTE_NAME) {
......
import Home from '../pages/index.vue';
import DesignDetail from '../pages/design/index.vue';
import { ROOT_ROUTE_NAME, DESIGNS_ROUTE_NAME, DESIGN_ROUTE_NAME } from './constants';
import { DESIGNS_ROUTE_NAME, DESIGN_ROUTE_NAME } from './constants';
export default [
{
name: ROOT_ROUTE_NAME,
path: '/',
component: Home,
meta: {
el: 'discussion',
},
},
{
name: DESIGNS_ROUTE_NAME,
path: '/designs',
path: '/',
component: Home,
meta: {
el: 'designs',
},
children: [
{
name: DESIGN_ROUTE_NAME,
path: ':id',
path: '/designs/:id',
component: DesignDetail,
meta: {
el: 'designs',
},
beforeEnter(
{
params: { id },
......@@ -39,6 +25,4 @@ export default [
},
props: ({ params: { id } }) => ({ id }),
},
],
},
];
......@@ -77,6 +77,9 @@
- if @issue.sentry_issue.present?
#js-sentry-error-stack-trace{ data: error_details_data(@project, @issue.sentry_issue.sentry_issue_identifier) }
- if Feature.enabled?(:design_management_moved, @project)
= render 'projects/issues/design_management'
= render_if_exists 'projects/issues/related_issues'
#js-related-merge-requests{ data: { endpoint: expose_path(api_v4_projects_issues_related_merge_requests_path(id: @project.id, issue_iid: @issue.iid)), project_namespace: @project.namespace.path, project_path: @project.path } }
......@@ -94,6 +97,9 @@
#js-vue-discussion-filter{ data: { default_filter: current_user&.notes_filter_for(@issue), notes_filters: UserPreference.notes_filters.to_json } }
= render 'new_branch' if show_new_branch_button?
- if Feature.enabled?(:design_management_moved, @project)
= render 'projects/issues/discussion'
- else
= render 'projects/issues/tabs'
= render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @issue.assignees
......@@ -10,6 +10,7 @@ RSpec.describe 'Issue page tabs', :js do
describe 'discussions tab counter' do
before do
allow(Ability).to receive(:allowed?) { true }
stub_feature_flags(design_management_moved: false)
end
subject do
......
......@@ -27,10 +27,6 @@ RSpec.describe 'viewing issues with design references' do
MD
end
before do
stub_feature_flags(design_management_moved: false)
end
def visit_page_with_design_references
public_issue = create(:issue, project: public_project, description: description)
visit project_issue_path(public_issue.project, public_issue)
......
......@@ -8,18 +8,40 @@ RSpec.describe 'User paginates issue designs', :js do
let(:project) { create(:project_empty_repo, :public) }
let(:issue) { create(:issue, project: project) }
context 'design_management_moved flag disabled' do
before do
enable_design_management
stub_feature_flags(design_management_moved: false)
enable_design_management
create_list(:design, 2, :with_file, issue: issue)
visit project_issue_path(project, issue)
click_link 'Designs'
wait_for_requests
find('.js-design-list-item', match: :first).click
end
it 'paginates to next design' do
expect(find('.js-previous-design')[:disabled]).to eq('true')
page.within(find('.js-design-header')) do
expect(page).to have_content('1 of 2')
end
find('.js-next-design').click
expect(find('.js-previous-design')[:disabled]).not_to eq('true')
page.within(find('.js-design-header')) do
expect(page).to have_content('2 of 2')
end
end
end
context 'design_management_moved flag enabled' do
before do
enable_design_management
create_list(:design, 2, :with_file, issue: issue)
visit project_issue_path(project, issue)
find('.js-design-list-item', match: :first).click
end
......@@ -38,4 +60,5 @@ RSpec.describe 'User paginates issue designs', :js do
expect(page).to have_content('2 of 2')
end
end
end
end
......@@ -8,6 +8,7 @@ RSpec.describe 'User design permissions', :js do
let(:project) { create(:project_empty_repo, :public) }
let(:issue) { create(:issue, project: project) }
context 'design_management_moved flag disabled' do
before do
enable_design_management
stub_feature_flags(design_management_moved: false)
......@@ -22,4 +23,17 @@ RSpec.describe 'User design permissions', :js do
it 'user does not have permissions to upload design' do
expect(page).not_to have_field('design_file')
end
end
context 'design_management_moved flag enabled' do
before do
enable_design_management
visit project_issue_path(project, issue)
end
it 'user does not have permissions to upload design' do
expect(page).not_to have_field('design_file')
end
end
end
......@@ -13,6 +13,7 @@ RSpec.describe 'User uploads new design', :js do
sign_in(user)
end
context 'design_management_moved flag disabled' do
context "when the feature is available" do
before do
enable_design_management
......@@ -42,6 +43,7 @@ RSpec.describe 'User uploads new design', :js do
context 'when the feature is not available' do
before do
stub_feature_flags(design_management_moved: false)
visit project_issue_path(project, issue)
click_link 'Designs'
......@@ -53,6 +55,41 @@ RSpec.describe 'User uploads new design', :js do
expect(page).to have_content("To enable design management, you'll need to meet the requirements.")
end
end
end
context 'design_management_moved flag enabled' do
context "when the feature is available" do
before do
enable_design_management
visit project_issue_path(project, issue)
end
it 'uploads designs' do
attach_file(:design_file, logo_fixture, make_visible: true)
expect(page).to have_selector('.js-design-list-item', count: 1)
within first('[data-testid="designs-root"] .js-design-list-item') do
expect(page).to have_content('dk.png')
end
attach_file(:design_file, gif_fixture, make_visible: true)
expect(page).to have_selector('.js-design-list-item', count: 2)
end
end
context 'when the feature is not available' do
before do
visit project_issue_path(project, issue)
end
it 'shows the message about requirements' do
expect(page).to have_content("To enable design management, you'll need to meet the requirements.")
end
end
end
def logo_fixture
Rails.root.join('spec', 'fixtures', 'dk.png')
......
......@@ -13,7 +13,6 @@ RSpec.describe 'Users views raw design image files' do
before do
enable_design_management
stub_feature_flags(design_management_moved: false)
end
it 'serves the latest design version when no ref is given' do
......
......@@ -9,6 +9,7 @@ RSpec.describe 'User views issue designs', :js do
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:design) { create(:design, :with_file, issue: issue) }
context 'design_management_moved flag disabled' do
before do
enable_design_management
stub_feature_flags(design_management_moved: false)
......@@ -27,4 +28,23 @@ RSpec.describe 'User views issue designs', :js do
expect(page).to have_selector('.js-design-image')
end
end
context 'design_management_moved flag enabled' do
before do
enable_design_management
visit project_issue_path(project, issue)
end
it 'opens design detail' do
click_link design.filename
page.within(find('.js-design-header')) do
expect(page).to have_content(design.filename)
end
expect(page).to have_selector('.js-design-image')
end
end
end
......@@ -9,6 +9,7 @@ RSpec.describe 'User views issue designs', :js do
let_it_be(:issue) { create(:issue, project: project) }
let_it_be(:design) { create(:design, :with_file, issue: issue) }
context 'design_management_moved flag disabled' do
before do
enable_design_management
stub_feature_flags(design_management_moved: false)
......@@ -45,4 +46,41 @@ RSpec.describe 'User views issue designs', :js do
expect(page).to have_selector('.js-design-detail')
end
end
end
context 'design_management_moved flag enabled' do
before do
enable_design_management
end
context 'navigates from the issue view' do
before do
visit project_issue_path(project, issue)
end
it 'fetches list of designs' do
expect(page).to have_selector('.js-design-list-item', count: 1)
end
end
context 'navigates directly to the design collection view' do
before do
visit designs_project_issue_path(project, issue)
end
it 'expands the sidebar' do
expect(page).to have_selector('.layout-page.right-sidebar-expanded')
end
end
context 'navigates directly to the individual design view' do
before do
visit designs_project_issue_path(project, issue, vueroute: design.filename)
end
it 'sees the design' do
expect(page).to have_selector('.js-design-detail')
end
end
end
end
......@@ -12,7 +12,6 @@ RSpec.describe 'User views an SVG design that contains XSS', :js do
before do
enable_design_management
stub_feature_flags(design_management_moved: false)
visit designs_project_issue_path(
project,
......@@ -30,6 +29,7 @@ RSpec.describe 'User views an SVG design that contains XSS', :js do
end
it 'displays the SVG' do
find("[data-testid='close-design']").click
expect(page).to have_selector("img.design-img[alt='xss.svg']", count: 1, visible: false)
end
......
......@@ -61,6 +61,10 @@ describe('Design discussions component', () => {
...data,
};
},
provide: {
projectPath: 'project-path',
issueIid: '1',
},
mocks: {
$apollo,
$route: {
......
......@@ -7,6 +7,7 @@ exports[`Design management toolbar component renders design and updated data 1`]
<a
aria-label="Go back to designs"
class="mr-3 text-plain d-flex justify-content-center align-items-center"
data-testid="close-design"
>
<icon-stub
name="close"
......
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`Design management index page designs does not render toolbar when there is no permission 1`] = `
<div>
<div
data-testid="designs-root"
>
<!---->
<div
......@@ -73,7 +75,9 @@ exports[`Design management index page designs does not render toolbar when there
`;
exports[`Design management index page designs renders designs list and header with upload button 1`] = `
<div>
<div
data-testid="designs-root"
>
<header
class="row-content-block border-top-0 p-2 d-flex"
>
......@@ -188,7 +192,9 @@ exports[`Design management index page designs renders designs list and header wi
`;
exports[`Design management index page designs renders error 1`] = `
<div>
<div
data-testid="designs-root"
>
<!---->
<div
......@@ -216,7 +222,9 @@ exports[`Design management index page designs renders error 1`] = `
`;
exports[`Design management index page designs renders loading icon 1`] = `
<div>
<div
data-testid="designs-root"
>
<!---->
<div
......@@ -236,7 +244,9 @@ exports[`Design management index page designs renders loading icon 1`] = `
`;
exports[`Design management index page when has no designs renders empty text 1`] = `
<div>
<div
data-testid="designs-root"
>
<!---->
<div
......
......@@ -10,7 +10,7 @@ exports[`Design management design index page renders design index 1`] = `
<design-destroyer-stub
filenames="test.jpg"
iid="1"
projectpath=""
project-path="project-path"
/>
<!---->
......@@ -60,7 +60,7 @@ exports[`Design management design index page renders design index 1`] = `
designid="test"
discussion="[object Object]"
discussionwithopenform=""
markdownpreviewpath="//preview_markdown?target_type=Issue"
markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
noteableid="design-id"
/>
......@@ -108,7 +108,7 @@ exports[`Design management design index page renders design index 1`] = `
designid="test"
discussion="[object Object]"
discussionwithopenform=""
markdownpreviewpath="//preview_markdown?target_type=Issue"
markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
noteableid="design-id"
/>
</gl-collapse-stub>
......@@ -140,7 +140,7 @@ exports[`Design management design index page with error GlAlert is rendered in c
<design-destroyer-stub
filenames="test.jpg"
iid="1"
projectpath=""
project-path="project-path"
/>
<div
......
......@@ -95,9 +95,12 @@ describe('Design management design index page', () => {
DesignSidebar,
DesignReplyForm,
},
provide: {
issueIid: '1',
projectPath: 'project-path',
},
data() {
return {
issueIid: '1',
activeDiscussion: {
id: null,
source: null,
......@@ -149,7 +152,7 @@ describe('Design management design index page', () => {
expect(findSidebar().props()).toEqual({
design,
markdownPreviewPath: '//preview_markdown?target_type=Issue',
markdownPreviewPath: '/project-path/preview_markdown?target_type=Issue',
resolvedDiscussionsExpanded: false,
});
});
......
......@@ -92,19 +92,23 @@ describe('Design management index page', () => {
};
wrapper = shallowMount(Index, {
data() {
return {
designs,
allVersions,
permissions: {
createDesign,
},
};
},
mocks: { $apollo },
localVue,
router,
stubs: { DesignDestroyer, ApolloMutation, ...stubs },
attachToDocument: true,
});
wrapper.setData({
designs,
allVersions,
provide: {
projectPath: 'project-path',
issueIid: '1',
permissions: {
createDesign,
},
});
}
......@@ -117,10 +121,8 @@ describe('Design management index page', () => {
it('renders loading icon', () => {
createComponent({ loading: true });
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.element).toMatchSnapshot();
});
});
it('renders error', () => {
createComponent();
......@@ -135,27 +137,21 @@ describe('Design management index page', () => {
it('renders a toolbar with buttons when there are designs', () => {
createComponent({ designs: mockDesigns, allVersions: [mockVersion] });
return wrapper.vm.$nextTick().then(() => {
expect(findToolbar().exists()).toBe(true);
});
});
it('renders designs list and header with upload button', () => {
createComponent({ designs: mockDesigns, allVersions: [mockVersion] });
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.element).toMatchSnapshot();
});
});
it('does not render toolbar when there is no permission', () => {
createComponent({ designs: mockDesigns, allVersions: [mockVersion], createDesign: false });
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.element).toMatchSnapshot();
});
});
});
describe('when has no designs', () => {
beforeEach(() => {
......@@ -185,7 +181,7 @@ describe('Design management index page', () => {
mutation: uploadDesignQuery,
variables: {
files: [{ name: 'test' }],
projectPath: '',
projectPath: 'project-path',
iid: '1',
},
optimisticResponse: {
......@@ -442,9 +438,9 @@ describe('Design management index page', () => {
});
});
it('on latest version when has no designs does not render toolbar buttons', () => {
it('on latest version when has no designs toolbar buttons are invisible', () => {
createComponent({ designs: [], allVersions: [mockVersion] });
expect(findToolbar().exists()).toBe(false);
expect(findToolbar().classes()).toContain('d-none');
});
describe('on non-latest version', () => {
......@@ -535,7 +531,7 @@ describe('Design management index page', () => {
it('ensures fullscreen layout is not applied', () => {
createComponent(true);
wrapper.vm.$router.push('/designs');
wrapper.vm.$router.push('/');
expect(mockPageEl.classList.remove).toHaveBeenCalledTimes(1);
expect(mockPageEl.classList.remove).toHaveBeenCalledWith(...DESIGN_DETAIL_LAYOUT_CLASSLIST);
});
......
......@@ -5,11 +5,7 @@ import App from '~/design_management_new/components/app.vue';
import Designs from '~/design_management_new/pages/index.vue';
import DesignDetail from '~/design_management_new/pages/design/index.vue';
import createRouter from '~/design_management_new/router';
import {
ROOT_ROUTE_NAME,
DESIGNS_ROUTE_NAME,
DESIGN_ROUTE_NAME,
} from '~/design_management_new/router/constants';
import { DESIGNS_ROUTE_NAME, DESIGN_ROUTE_NAME } from '~/design_management_new/router/constants';
import '~/commons/bootstrap';
function factory(routeArg) {
......@@ -49,7 +45,7 @@ describe('Design management router', () => {
window.location.hash = '';
});
describe.each([['/'], [{ name: ROOT_ROUTE_NAME }]])('root route', routeArg => {
describe.each([['/'], [{ name: DESIGNS_ROUTE_NAME }]])('root route', routeArg => {
it('pushes home component', () => {
const wrapper = factory(routeArg);
......@@ -57,14 +53,6 @@ describe('Design management router', () => {
});
});
describe.each([['/designs'], [{ name: DESIGNS_ROUTE_NAME }]])('designs route', routeArg => {
it('pushes designs root component', () => {
const wrapper = factory(routeArg);
expect(wrapper.find(Designs).exists()).toBe(true);
});
});
describe.each([['/designs/1'], [{ name: DESIGN_ROUTE_NAME, params: { id: '1' } }]])(
'designs detail route',
routeArg => {
......
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