Merge branch '267570-prevent-design-comments-without-signing-in' into 'master'

Prevent design discussions without user login

See merge request gitlab-org/gitlab!77563

app/assets/javascripts/design_management/components/design_notes/design_discussion.vue

@@ -7,6 +7,7 @@ import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vu
 import { updateGlobalTodoCount } from '~/vue_shared/components/sidebar/todo_toggle/utils';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import DesignNotePin from '~/vue_shared/components/design_management/design_note_pin.vue';
+import { isLoggedIn } from '~/lib/utils/common_utils';
 import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants';
 import createNoteMutation from '../../graphql/mutations/create_note.mutation.graphql';
 import toggleResolveDiscussionMutation from '../../graphql/mutations/toggle_resolve_discussion.mutation.graphql';
@@ -17,6 +18,7 @@ import { hasErrors } from '../../utils/cache_update';
 import { extractDesign } from '../../utils/design_management_utils';
 import { ADD_DISCUSSION_COMMENT_ERROR } from '../../utils/error_messages';
 import DesignNote from './design_note.vue';
+import DesignNoteSignedOut from './design_note_signed_out.vue';
 import DesignReplyForm from './design_reply_form.vue';
 import ToggleRepliesWidget from './toggle_replies_widget.vue';
 
@@ -24,6 +26,7 @@ export default {
   components: {
     ApolloMutation,
     DesignNote,
+    DesignNoteSignedOut,
     ReplyPlaceholder,
     DesignReplyForm,
     GlIcon,
@@ -55,6 +58,14 @@ export default {
       required: false,
       default: '',
     },
+    registerPath: {
+      type: String,
+      required: true,
+    },
+    signInPath: {
+      type: String,
+      required: true,
+    },
     resolvedDiscussionsExpanded: {
       type: Boolean,
       required: true,
@@ -93,6 +104,7 @@ export default {
       isResolving: false,
       shouldChangeResolvedStatus: false,
       areRepliesCollapsed: this.discussion.resolved,
+      isLoggedIn: isLoggedIn(),
     };
   },
   computed: {
@@ -226,7 +238,7 @@ export default {
         :class="{ 'gl-bg-blue-50': isDiscussionActive }"
         @error="$emit('update-note-error', $event)"
       >
-        <template v-if="discussion.resolvable" #resolve-discussion>
+        <template v-if="isLoggedIn && discussion.resolvable" #resolve-discussion>
           <button
             v-gl-tooltip
             :class="{ 'is-active': discussion.resolved }"
@@ -269,7 +281,15 @@ export default {
         :class="{ 'gl-bg-blue-50': isDiscussionActive }"
         @error="$emit('update-note-error', $event)"
       />
-      <li v-show="isReplyPlaceholderVisible" class="reply-wrapper discussion-reply-holder">
+      <li
+        v-show="isReplyPlaceholderVisible"
+        class="reply-wrapper discussion-reply-holder"
+        :class="{ 'gl-bg-gray-10': !isLoggedIn }"
+      >
+        <template v-if="!isLoggedIn">
+          <design-note-signed-out :register-path="registerPath" :sign-in-path="signInPath" />
+        </template>
+        <template v-else>
           <reply-placeholder
             v-if="!isFormVisible"
             class="qa-discussion-reply"
@@ -301,6 +321,7 @@ export default {
               </template>
             </design-reply-form>
           </apollo-mutation>
+        </template>
       </li>
     </ul>
   </div>

app/assets/javascripts/design_management/components/design_notes/design_note_signed_out.vue

+<script>
+import { GlSprintf, GlLink } from '@gitlab/ui';
+import { __ } from '~/locale';
+
+export default {
+  components: {
+    GlSprintf,
+    GlLink,
+  },
+  props: {
+    registerPath: {
+      type: String,
+      required: true,
+    },
+    signInPath: {
+      type: String,
+      required: true,
+    },
+    isAddDiscussion: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
+  },
+  computed: {
+    signedOutText() {
+      return this.isAddDiscussion
+        ? __(
+            'Please %{registerLinkStart}register%{registerLinkEnd} or %{signInLinkStart}sign in%{signInLinkEnd} to start a new discussion.',
+          )
+        : __(
+            'Please %{registerLinkStart}register%{registerLinkEnd} or %{signInLinkStart}sign in%{signInLinkEnd} to reply.',
+          );
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="disabled-comment text-center">
+    <gl-sprintf :message="signedOutText">
+      <template #registerLink="{ content }">
+        <gl-link :href="registerPath">{{ content }}</gl-link>
+      </template>
+      <template #signInLink="{ content }">
+        <gl-link :href="signInPath">{{ content }}</gl-link>
+      </template>
+    </gl-sprintf>
+  </div>
+</template>

app/assets/javascripts/design_management/components/design_presentation.vue

 <script>
 import { throttle } from 'lodash';
+import { isLoggedIn } from '~/lib/utils/common_utils';
 import DesignOverlay from './design_overlay.vue';
 import DesignImage from './image.vue';
 
@@ -54,6 +55,7 @@ export default {
       initialLoad: true,
       lastDragPosition: null,
       isDraggingDesign: false,
+      isLoggedIn: isLoggedIn(),
     };
   },
   computed: {
@@ -311,7 +313,7 @@ export default {
         :position="overlayPosition"
         :notes="discussionStartingNotes"
         :current-comment-form="currentCommentForm"
-        :disable-commenting="isDraggingDesign"
+        :disable-commenting="!isLoggedIn || isDraggingDesign"
         :resolved-discussions-expanded="resolvedDiscussionsExpanded"
         @openCommentForm="openCommentForm"
         @closeCommentForm="closeCommentForm"

app/assets/javascripts/design_management/components/design_sidebar.vue

 <script>
 import { GlCollapse, GlButton, GlPopover } from '@gitlab/ui';
 import Cookies from 'js-cookie';
-import { parseBoolean } from '~/lib/utils/common_utils';
+import { parseBoolean, isLoggedIn } from '~/lib/utils/common_utils';
 import { s__ } from '~/locale';
 import Participants from '~/sidebar/components/participants/participants.vue';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
@@ -9,11 +9,13 @@ import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../constants';
 import updateActiveDiscussionMutation from '../graphql/mutations/update_active_discussion.mutation.graphql';
 import { extractDiscussions, extractParticipants } from '../utils/design_management_utils';
 import DesignDiscussion from './design_notes/design_discussion.vue';
+import DesignNoteSignedOut from './design_notes/design_note_signed_out.vue';
 import DesignTodoButton from './design_todo_button.vue';
 
 export default {
   components: {
     DesignDiscussion,
+    DesignNoteSignedOut,
     Participants,
     GlCollapse,
     GlButton,
@@ -28,6 +30,12 @@ export default {
     issueIid: {
       default: '',
     },
+    registerPath: {
+      default: '',
+    },
+    signInPath: {
+      default: '',
+    },
   },
   props: {
     design: {
@@ -47,6 +55,7 @@ export default {
     return {
       isResolvedCommentsPopoverHidden: parseBoolean(Cookies.get(this.$options.cookieKey)),
       discussionWithOpenForm: '',
+      isLoggedIn: isLoggedIn(),
     };
   },
   computed: {
@@ -134,12 +143,19 @@ export default {
       class="gl-mb-4"
     />
     <h2
-      v-if="unresolvedDiscussions.length === 0"
+      v-if="isLoggedIn && unresolvedDiscussions.length === 0"
       class="new-discussion-disclaimer gl-font-base gl-m-0 gl-mb-4"
       data-testid="new-discussion-disclaimer"
     >
       {{ s__("DesignManagement|Click the image where you'd like to start a new discussion") }}
     </h2>
+    <design-note-signed-out
+      v-if="!isLoggedIn"
+      class="gl-mb-4"
+      :register-path="registerPath"
+      :sign-in-path="signInPath"
+      :is-add-discussion="true"
+    />
     <design-discussion
       v-for="discussion in unresolvedDiscussions"
       :key="discussion.id"
@@ -147,6 +163,8 @@ export default {
       :design-id="$route.params.id"
       :noteable-id="design.id"
       :markdown-preview-path="markdownPreviewPath"
+      :register-path="registerPath"
+      :sign-in-path="signInPath"
       :resolved-discussions-expanded="resolvedDiscussionsExpanded"
       :discussion-with-open-form="discussionWithOpenForm"
       data-testid="unresolved-discussion"
@@ -197,6 +215,8 @@ export default {
           :design-id="$route.params.id"
           :noteable-id="design.id"
           :markdown-preview-path="markdownPreviewPath"
+          :register-path="registerPath"
+          :sign-in-path="signInPath"
           :resolved-discussions-expanded="resolvedDiscussionsExpanded"
           :discussion-with-open-form="discussionWithOpenForm"
           data-testid="resolved-discussion"

app/assets/javascripts/design_management/index.js

@@ -8,7 +8,7 @@ import createRouter from './router';
 
 export default () => {
   const el = document.querySelector('.js-design-management');
-  const { issueIid, projectPath, issuePath } = el.dataset;
+  const { issueIid, projectPath, issuePath, registerPath, signInPath } = el.dataset;
   const router = createRouter(issuePath);
 
   apolloProvider.clients.defaultClient.cache.writeQuery({
@@ -29,6 +29,8 @@ export default () => {
     provide: {
       projectPath,
       issueIid,
+      registerPath,
+      signInPath,
     },
     mounted() {
       performanceMarkAndMeasure({

app/views/projects/issues/_design_management.html.haml

@@ -8,7 +8,11 @@
 - if @project.design_management_enabled?
   - add_page_startup_graphql_call('design_management/get_design_list', { fullPath: @project.full_path, iid: @issue.iid.to_s, atVersion: nil })
   - add_page_startup_graphql_call('design_management/design_permissions', { fullPath: @project.full_path, iid: @issue.iid.to_s })
-  .js-design-management{ data: { project_path: @project.full_path, issue_iid: @issue.iid, issue_path: project_issue_path(@project, @issue) } }
+  .js-design-management{ data: { project_path: @project.full_path,
+    issue_iid: @issue.iid,
+    issue_path: project_issue_path(@project, @issue),
+    register_path: new_user_registration_path(redirect_to_referer: 'yes'),
+    sign_in_path: new_session_path(:user, redirect_to_referer: 'yes') } }
 - else
   .gl-border-solid.gl-border-1.gl-border-gray-100.gl-rounded-base.gl-mt-5.gl-p-3.gl-text-center
     = enable_lfs_message

locale/gitlab.pot

@@ -26470,6 +26470,12 @@ msgstr ""
 msgid "Please %{link_to_register} or %{link_to_sign_in} to comment"
 msgstr ""
 
+msgid "Please %{registerLinkStart}register%{registerLinkEnd} or %{signInLinkStart}sign in%{signInLinkEnd} to reply."
+msgstr ""
+
+msgid "Please %{registerLinkStart}register%{registerLinkEnd} or %{signInLinkStart}sign in%{signInLinkEnd} to start a new discussion."
+msgstr ""
+
 msgid "Please %{startTagRegister}register%{endRegisterTag} or %{startTagSignIn}sign in%{endSignInTag} to reply"
 msgstr ""
 

spec/frontend/design_management/components/design_notes/__snapshots__/design_note_signed_out_spec.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`DesignNoteSignedOut renders message containing register and sign-in links while user wants to reply to a discussion 1`] = `
+<div
+  class="disabled-comment text-center"
+>
+  Please 
+  <gl-link-stub
+    href="/users/sign_up?redirect_to_referer=yes"
+  >
+    register
+  </gl-link-stub>
+   or 
+  <gl-link-stub
+    href="/users/sign_in?redirect_to_referer=yes"
+  >
+    sign in
+  </gl-link-stub>
+   to reply.
+</div>
+`;
+
+exports[`DesignNoteSignedOut renders message containing register and sign-in links while user wants to start a new discussion 1`] = `
+<div
+  class="disabled-comment text-center"
+>
+  Please 
+  <gl-link-stub
+    href="/users/sign_up?redirect_to_referer=yes"
+  >
+    register
+  </gl-link-stub>
+   or 
+  <gl-link-stub
+    href="/users/sign_in?redirect_to_referer=yes"
+  >
+    sign in
+  </gl-link-stub>
+   to start a new discussion.
+</div>
+`;

spec/frontend/design_management/components/design_notes/design_discussion_spec.js

 import { GlLoadingIcon } from '@gitlab/ui';
 import { mount } from '@vue/test-utils';
+import { ApolloMutation } from 'vue-apollo';
 import DesignDiscussion from '~/design_management/components/design_notes/design_discussion.vue';
 import DesignNote from '~/design_management/components/design_notes/design_note.vue';
+import DesignNoteSignedOut from '~/design_management/components/design_notes/design_note_signed_out.vue';
 import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue';
 import ToggleRepliesWidget from '~/design_management/components/design_notes/toggle_replies_widget.vue';
 import createNoteMutation from '~/design_management/graphql/mutations/create_note.mutation.graphql';
@@ -20,6 +22,7 @@ const defaultMockDiscussion = {
 const DEFAULT_TODO_COUNT = 2;
 
 describe('Design discussions component', () => {
+  const originalGon = window.gon;
   let wrapper;
 
   const findDesignNotes = () => wrapper.findAll(DesignNote);
@@ -31,6 +34,7 @@ describe('Design discussions component', () => {
   const findResolvedMessage = () => wrapper.find('[data-testid="resolved-message"]');
   const findResolveLoadingIcon = () => wrapper.find(GlLoadingIcon);
   const findResolveCheckbox = () => wrapper.find('[data-testid="resolve-checkbox"]');
+  const findApolloMutation = () => wrapper.findComponent(ApolloMutation);
 
   const mutationVariables = {
     mutation: createNoteMutation,
@@ -42,6 +46,8 @@ describe('Design discussions component', () => {
       },
     },
   };
+  const registerPath = '/users/sign_up?redirect_to_referer=yes';
+  const signInPath = '/users/sign_in?redirect_to_referer=yes';
   const mutate = jest.fn().mockResolvedValue({ data: { createNote: { errors: [] } } });
   const readQuery = jest.fn().mockReturnValue({
     project: {
@@ -62,6 +68,8 @@ describe('Design discussions component', () => {
         designId: 'design-id',
         discussionIndex: 1,
         discussionWithOpenForm: '',
+        registerPath,
+        signInPath,
         ...props,
       },
       data() {
@@ -88,8 +96,13 @@ describe('Design discussions component', () => {
     });
   }
 
+  beforeEach(() => {
+    window.gon = { current_user_id: 1 };
+  });
+
   afterEach(() => {
     wrapper.destroy();
+    window.gon = originalGon;
   });
 
   describe('when discussion is not resolvable', () => {
@@ -349,4 +362,41 @@ describe('Design discussions component', () => {
 
     expect(wrapper.emitted('open-form')).toBeTruthy();
   });
+
+  describe('when user is not logged in', () => {
+    const findDesignNoteSignedOut = () => wrapper.findComponent(DesignNoteSignedOut);
+
+    beforeEach(() => {
+      window.gon = { current_user_id: null };
+      createComponent(
+        {
+          discussion: {
+            ...defaultMockDiscussion,
+          },
+          discussionWithOpenForm: defaultMockDiscussion.id,
+        },
+        { discussionComment: 'test', isFormRendered: true },
+      );
+    });
+
+    it('does not render resolve discussion button', () => {
+      expect(findResolveButton().exists()).toBe(false);
+    });
+
+    it('does not render replace-placeholder component', () => {
+      expect(findReplyPlaceholder().exists()).toBe(false);
+    });
+
+    it('does not render apollo-mutation component', () => {
+      expect(findApolloMutation().exists()).toBe(false);
+    });
+
+    it('renders design-note-signed-out component', () => {
+      expect(findDesignNoteSignedOut().exists()).toBe(true);
+      expect(findDesignNoteSignedOut().props()).toMatchObject({
+        registerPath,
+        signInPath,
+      });
+    });
+  });
 });

spec/frontend/design_management/components/design_notes/design_note_signed_out_spec.js

+import { GlSprintf } from '@gitlab/ui';
+import { shallowMount } from '@vue/test-utils';
+import DesignNoteSignedOut from '~/design_management/components/design_notes/design_note_signed_out.vue';
+
+function createComponent(isAddDiscussion = false) {
+  return shallowMount(DesignNoteSignedOut, {
+    propsData: {
+      registerPath: '/users/sign_up?redirect_to_referer=yes',
+      signInPath: '/users/sign_in?redirect_to_referer=yes',
+      isAddDiscussion,
+    },
+    stubs: {
+      GlSprintf,
+    },
+  });
+}
+
+describe('DesignNoteSignedOut', () => {
+  let wrapper;
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  it('renders message containing register and sign-in links while user wants to reply to a discussion', () => {
+    wrapper = createComponent();
+
+    expect(wrapper.element).toMatchSnapshot();
+  });
+
+  it('renders message containing register and sign-in links while user wants to start a new discussion', () => {
+    wrapper = createComponent(true);
+
+    expect(wrapper.element).toMatchSnapshot();
+  });
+});

spec/frontend/design_management/components/design_presentation_spec.js

@@ -15,6 +15,7 @@ const mockOverlayData = {
 };
 
 describe('Design management design presentation component', () => {
+  const originalGon = window.gon;
   let wrapper;
 
   function createComponent(
@@ -115,8 +116,13 @@ describe('Design management design presentation component', () => {
       });
   }
 
+  beforeEach(() => {
+    window.gon = { current_user_id: 1 };
+  });
+
   afterEach(() => {
     wrapper.destroy();
+    window.gon = originalGon;
   });
 
   it('renders image and overlay when image provided', () => {
@@ -552,4 +558,23 @@ describe('Design management design presentation component', () => {
       });
     });
   });
+
+  describe('when user is not logged in', () => {
+    beforeEach(() => {
+      window.gon = { current_user_id: null };
+      createComponent(
+        {
+          image: 'test.jpg',
+          imageName: 'test',
+        },
+        mockOverlayData,
+      );
+    });
+
+    it('disables commenting from design overlay', () => {
+      expect(wrapper.findComponent(DesignOverlay).props()).toMatchObject({
+        disableCommenting: true,
+      });
+    });
+  });
 });

spec/frontend/design_management/components/design_sidebar_spec.js

@@ -2,6 +2,7 @@ import { GlCollapse, GlPopover } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import Cookies from 'js-cookie';
 import DesignDiscussion from '~/design_management/components/design_notes/design_discussion.vue';
+import DesignNoteSignedOut from '~/design_management/components/design_notes/design_note_signed_out.vue';
 import DesignSidebar from '~/design_management/components/design_sidebar.vue';
 import DesignTodoButton from '~/design_management/components/design_todo_button.vue';
 import updateActiveDiscussionMutation from '~/design_management/graphql/mutations/update_active_discussion.mutation.graphql';
@@ -30,6 +31,7 @@ const cookieKey = 'hide_design_resolved_comments_popover';
 const mutate = jest.fn().mockResolvedValue();
 
 describe('Design management design sidebar component', () => {
+  const originalGon = window.gon;
   let wrapper;
 
   const findDiscussions = () => wrapper.findAll(DesignDiscussion);
@@ -58,11 +60,20 @@ describe('Design management design sidebar component', () => {
         },
       },
       stubs: { GlPopover },
+      provide: {
+        registerPath: '/users/sign_up?redirect_to_referer=yes',
+        signInPath: '/users/sign_in?redirect_to_referer=yes',
+      },
     });
   }
 
+  beforeEach(() => {
+    window.gon = { current_user_id: 1 };
+  });
+
   afterEach(() => {
     wrapper.destroy();
+    window.gon = originalGon;
   });
 
   it('renders participants', () => {
@@ -248,4 +259,44 @@ describe('Design management design sidebar component', () => {
       expect(Cookies.set).toHaveBeenCalledWith(cookieKey, 'true', { expires: 365 * 10 });
     });
   });
+
+  describe('when user is not logged in', () => {
+    const findDesignNoteSignedOut = () => wrapper.findComponent(DesignNoteSignedOut);
+
+    beforeEach(() => {
+      window.gon = { current_user_id: null };
+    });
+
+    describe('design has no discussions', () => {
+      beforeEach(() => {
+        createComponent({
+          design: {
+            ...design,
+            discussions: {
+              nodes: [],
+            },
+          },
+        });
+      });
+
+      it('does not render a message about possibility to create a new discussion', () => {
+        expect(findNewDiscussionDisclaimer().exists()).toBe(false);
+      });
+
+      it('renders design-note-signed-out component', () => {
+        expect(findDesignNoteSignedOut().exists()).toBe(true);
+      });
+    });
+
+    describe('design has discussions', () => {
+      beforeEach(() => {
+        Cookies.set(cookieKey, true);
+        createComponent();
+      });
+
+      it('renders design-note-signed-out component', () => {
+        expect(findDesignNoteSignedOut().exists()).toBe(true);
+      });
+    });
+  });
 });

spec/frontend/design_management/pages/design/__snapshots__/index_spec.js.snap

@@ -70,6 +70,13 @@ exports[`Design management design index page renders design index 1`] = `
      
     <!---->
      
+    <design-note-signed-out-stub
+      class="gl-mb-4"
+      isadddiscussion="true"
+      registerpath=""
+      signinpath=""
+    />
+     
     <design-discussion-stub
       data-testid="unresolved-discussion"
       designid="gid::/gitlab/Design/1"
@@ -77,6 +84,8 @@ exports[`Design management design index page renders design index 1`] = `
       discussionwithopenform=""
       markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
       noteableid="gid::/gitlab/Design/1"
+      registerpath=""
+      signinpath=""
     />
      
     <gl-button-stub
@@ -126,6 +135,8 @@ exports[`Design management design index page renders design index 1`] = `
         discussionwithopenform=""
         markdownpreviewpath="/project-path/preview_markdown?target_type=Issue"
         noteableid="gid::/gitlab/Design/1"
+        registerpath=""
+        signinpath=""
       />
     </gl-collapse-stub>
      
@@ -231,14 +242,14 @@ exports[`Design management design index page with error GlAlert is rendered in c
       participants="[object Object]"
     />
      
-    <h2
-      class="new-discussion-disclaimer gl-font-base gl-m-0 gl-mb-4"
-      data-testid="new-discussion-disclaimer"
-    >
-      
-    Click the image where you'd like to start a new discussion
+    <!---->
      
-    </h2>
+    <design-note-signed-out-stub
+      class="gl-mb-4"
+      isadddiscussion="true"
+      registerpath=""
+      signinpath=""
+    />
       
     <!---->
      

spec/frontend/design_management/pages/index_spec.js

@@ -91,6 +91,8 @@ const designToMove = {
 };
 
 describe('Design management index page', () => {
+  const registerPath = '/users/sign_up?redirect_to_referer=yes';
+  const signInPath = '/users/sign_in?redirect_to_referer=yes';
   let mutate;
   let wrapper;
   let fakeApollo;
@@ -164,6 +166,8 @@ describe('Design management index page', () => {
       provide: {
         projectPath: 'project-path',
         issueIid: '1',
+        registerPath,
+        signInPath,
       },
     });
   }
@@ -186,6 +190,10 @@ describe('Design management index page', () => {
       apolloProvider: fakeApollo,
       router,
       stubs: { VueDraggable },
+      provide: {
+        registerPath,
+        signInPath,
+      },
     });
   }