Merge branch '301143-refactor-new-dropdown-to-view-model' into 'master'

Move new_dropdown logic from haml to view_model

See merge request gitlab-org/gitlab!62866

app/controllers/projects_controller.rb

@@ -39,6 +39,11 @@ class ProjectsController < Projects::ApplicationController
     push_frontend_feature_flag(:refactor_blob_viewer, @project, default_enabled: :yaml)
   end
 
+  before_action only: [:new] do
+    # Run experiment before render so it will be written to the `gon` for FE
+    helpers.new_repo_experiment_text
+  end
+
   layout :determine_layout
 
   feature_category :projects, [

app/helpers/invite_members_helper.rb

@@ -21,17 +21,6 @@ module InviteMembersHelper
     experiment_enabled?(:invite_members_empty_group_version_a) && Ability.allowed?(current_user, :admin_group_member, group)
   end
 
-  def dropdown_invite_members_link(form_model)
-    link_to invite_members_url(form_model),
-            data: {
-              'track-event': 'click_link',
-              'track-label': tracking_label,
-              'track-property': experiment_tracking_category_and_group(:invite_members_new_dropdown)
-            } do
-      invite_member_link_content
-    end
-  end
-
   def invite_accepted_notice(member)
     case member.source
     when Project
@@ -42,23 +31,4 @@ module InviteMembersHelper
         { member_human_access: member.human_access, name: member.source.name }
     end
   end
-
-  private
-
-  def invite_members_url(form_model)
-    case form_model
-    when Project
-      project_project_members_path(form_model)
-    when Group
-      group_group_members_path(form_model)
-    end
-  end
-
-  def invite_member_link_content
-    text = s_('InviteMember|Invite members')
-
-    return text unless experiment_enabled?(:invite_members_new_dropdown)
-
-    "#{text} #{emoji_icon('shaking_hands', 'aria-hidden': true, class: 'gl-font-base gl-vertical-align-baseline')}".html_safe
-  end
 end

app/helpers/nav/new_dropdown_helper.rb

+# frozen_string_literal: true
+
+module Nav
+  module NewDropdownHelper
+    def new_dropdown_view_model(group:, project:)
+      return unless current_user
+
+      menu_sections = []
+
+      if group&.persisted?
+        menu_sections.push(group_menu_section(group))
+      elsif project&.persisted?
+        menu_sections.push(project_menu_section(project))
+      end
+
+      menu_sections.push(general_menu_section)
+
+      {
+        title: _("New..."),
+        menu_sections: menu_sections.select { |x| x.fetch(:menu_items).any? }
+      }
+    end
+
+    def new_repo_experiment_text
+      experiment(:new_repo, user: current_user) do |e|
+        e.use { _('New project') }
+        e.try { _('New project/repository') }
+      end.run
+    end
+
+    private
+
+    def group_menu_section(group)
+      menu_items = []
+
+      if can?(current_user, :create_projects, group)
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'new_project',
+            title: new_repo_experiment_text,
+            href: new_project_path(namespace_id: group.id),
+            data: { track_experiment: 'new_repo', track_event: 'click_link_new_project_group', track_label: 'plus_menu_dropdown' }
+          )
+        )
+      end
+
+      if can?(current_user, :create_subgroup, group)
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'new_subgroup',
+            title: _('New subgroup'),
+            href: new_group_path(parent_id: group.id),
+            data: { track_event: 'click_link_new_subgroup', track_label: 'plus_menu_dropdown' }
+          )
+        )
+      end
+
+      menu_items.push(create_epic_menu_item(group))
+
+      if Gitlab::Experimentation.active?(:invite_members_new_dropdown) && can?(current_user, :admin_group_member, group)
+        menu_items.push(
+          invite_members_menu_item(
+            href: group_group_members_path(group)
+          )
+        )
+      end
+
+      {
+        title: _('This group'),
+        menu_items: menu_items.compact
+      }
+    end
+
+    def project_menu_section(project)
+      menu_items = []
+      merge_project = merge_request_source_project_for_project(project)
+
+      if show_new_issue_link?(project)
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'new_issue',
+            title: _('New issue'),
+            href: new_project_issue_path(project),
+            data: { track_event: 'click_link_new_issue', track_label: 'plus_menu_dropdown', qa_selector: 'new_issue_link' }
+          )
+        )
+      end
+
+      if merge_project
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'new_mr',
+            title: _('New merge request'),
+            href: project_new_merge_request_path(merge_project),
+            data: { track_event: 'click_link_new_mr', track_label: 'plus_menu_dropdown' }
+          )
+        )
+      end
+
+      if can?(current_user, :create_snippet, project)
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'new_snippet',
+            title: _('New snippet'),
+            href: new_project_snippet_path(project),
+            data: { track_event: 'click_link_new_snippet_project', track_label: 'plus_menu_dropdown' }
+          )
+        )
+      end
+
+      if Gitlab::Experimentation.active?(:invite_members_new_dropdown) && can_import_members?
+        menu_items.push(
+          invite_members_menu_item(
+            href: project_project_members_path(project)
+          )
+        )
+      end
+
+      {
+        title: _('This project'),
+        menu_items: menu_items
+      }
+    end
+
+    def general_menu_section
+      menu_items = []
+
+      if current_user.can_create_project?
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'general_new_project',
+            title: new_repo_experiment_text,
+            href: new_project_path,
+            data: { track_experiment: 'new_repo', track_event: 'click_link_new_project', track_label: 'plus_menu_dropdown', qa_selector: 'global_new_project_link' }
+          )
+        )
+      end
+
+      if current_user.can_create_group?
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'general_new_group',
+            title: _('New group'),
+            href: new_group_path,
+            data: { track_event: 'click_link_new_group', track_label: 'plus_menu_dropdown' }
+          )
+        )
+      end
+
+      if current_user.can?(:create_snippet)
+        menu_items.push(
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'general_new_snippet',
+            title: _('New snippet'),
+            href: new_snippet_path,
+            data: { track_event: 'click_link_new_snippet_parent', track_label: 'plus_menu_dropdown', qa_selector: 'global_new_snippet_link' }
+          )
+        )
+      end
+
+      {
+        title: _('GitLab'),
+        menu_items: menu_items
+      }
+    end
+
+    def invite_members_menu_item(href:)
+      ::Gitlab::Nav::TopNavMenuItem.build(
+        id: 'invite',
+        title: s_('InviteMember|Invite members'),
+        emoji: ('shaking_hands' if experiment_enabled?(:invite_members_new_dropdown)),
+        href: href,
+        data: {
+          track_event: 'click_link',
+          track_label: tracking_label,
+          track_property: experiment_tracking_category_and_group(:invite_members_new_dropdown)
+        }
+      )
+    end
+
+    # Overridden in EE
+    def create_epic_menu_item(group)
+      nil
+    end
+  end
+end
+
+Nav::NewDropdownHelper.prepend_mod

app/views/layouts/_head.html.haml

@@ -54,8 +54,6 @@
 
   = stylesheet_link_tag 'performance_bar' if performance_bar_enabled?
 
-  -# Rendering this above Gon, to use in JS later
-  = render 'layouts/header/new_repo_experiment'
   = Gon::Base.render_data(nonce: content_security_policy_nonce)
 
   = javascript_include_tag locale_path unless I18n.locale == :en

app/views/layouts/header/_group_invite_members_new_dropdown_item.html.haml

-- return unless Gitlab::Experimentation.active?(:invite_members_new_dropdown) && can?(current_user, :admin_group_member, @group)
-
-%li= dropdown_invite_members_link(@group)

app/views/layouts/header/_new_dropdown.html.haml

-- new_repo_experiment_text = content_for(:new_repo_experiment)
+- view_model = new_dropdown_view_model(project: @project, group: @group)
+- menu_sections = view_model.fetch(:menu_sections)
+- title = view_model.fetch(:title)
+- show_headers = menu_sections.length > 1
+
+- return if menu_sections.empty?
+
 %li.header-new.dropdown{ data: { track_label: "new_dropdown", track_event: "click_dropdown", track_experiment: "new_repo" } }
-  = link_to new_project_path, class: "header-new-dropdown-toggle has-tooltip qa-new-menu-toggle", id: "js-onboarding-new-project-link", title: _("New..."), ref: 'tooltip', aria: { label: _("New...") }, data: { toggle: 'dropdown', placement: 'bottom', container: 'body', display: 'static' } do
+  = link_to new_project_path, class: "header-new-dropdown-toggle has-tooltip", id: "js-onboarding-new-project-link", title: title, ref: 'tooltip', aria: { label: title }, data: { toggle: 'dropdown', placement: 'bottom', container: 'body', display: 'static', qa_selector: 'new_menu_toggle' } do
     = sprite_icon('plus-square')
     = sprite_icon('chevron-down', css_class: 'caret-down')
   .dropdown-menu.dropdown-menu-right.dropdown-extended-height
     %ul
-      - if @group&.persisted?
-        - create_group_project = can?(current_user, :create_projects, @group)
-        - create_group_subgroup = can?(current_user, :create_subgroup, @group)
-
-        - if create_group_project || create_group_subgroup
-          %li.dropdown-bold-header
-            = _('This group')
-          - if create_group_project
-            %li= link_to new_repo_experiment_text, new_project_path(namespace_id: @group.id), data: { track_experiment: 'new_repo', track_event: 'click_link_new_project_group', track_label: 'plus_menu_dropdown' }
-          - if create_group_subgroup
-            %li= link_to _('New subgroup'), new_group_path(parent_id: @group.id), data: { track_event: 'click_link_new_subgroup', track_label: 'plus_menu_dropdown' }
-          = render_if_exists 'layouts/header/create_epic_new_dropdown_item'
-          = render 'layouts/header/group_invite_members_new_dropdown_item'
+      - menu_sections.each_with_index do |section, index|
+        - if index > 0
           %li.divider
-          %li.dropdown-bold-header GitLab
-
-      - if @project&.persisted?
-        - create_project_issue = show_new_issue_link?(@project)
-        - merge_project = merge_request_source_project_for_project(@project)
-        - create_project_snippet = can?(current_user, :create_snippet, @project)
-
-        - if create_project_issue || merge_project || create_project_snippet
+        - if show_headers
           %li.dropdown-bold-header
-            = _('This project')
-          - if create_project_issue
-            %li= link_to _('New issue'), new_project_issue_path(@project), data: { track_event: 'click_link_new_issue', track_label: 'plus_menu_dropdown' }
-          - if merge_project
-            %li= link_to _('New merge request'), project_new_merge_request_path(merge_project), data: { track_event: 'click_link_new_mr', track_label: 'plus_menu_dropdown' }
-
-          - if create_project_snippet
-            %li= link_to _('New snippet'), new_project_snippet_path(@project), data: { track_event: 'click_link_new_snippet_project', track_label: 'plus_menu_dropdown' }
-          = render 'layouts/header/project_invite_members_new_dropdown_item'
-          %li.divider
-          %li.dropdown-bold-header GitLab
-      - if current_user.can_create_project?
-        %li= link_to new_repo_experiment_text, new_project_path, class: 'qa-global-new-project-link', data: { track_experiment: 'new_repo', track_event: 'click_link_new_project', track_label: 'plus_menu_dropdown' }
-      - if current_user.can_create_group?
-        %li= link_to _('New group'), new_group_path, data: { track_event: 'click_link_new_group', track_label: 'plus_menu_dropdown' }
-      - if current_user.can?(:create_snippet)
-        %li= link_to _('New snippet'), new_snippet_path, data: { track_event: 'click_link_new_snippet_parent', track_label: 'plus_menu_dropdown' }, class: 'qa-global-new-snippet-link'
+            = section.fetch(:title)
+        - section.fetch(:menu_items).each do |menu_item|
+          %li<
+            = link_to menu_item.fetch(:href), class: menu_item.fetch(:css_class), data: menu_item.fetch(:data) do
+              = menu_item.fetch(:title)
+              - if menu_item.fetch(:emoji)
+                -# We need to insert a space between the title and emoji
+                = " #{emoji_icon(menu_item.fetch(:emoji), 'aria-hidden': true, class: "gl-font-base gl-vertical-align-baseline")}".html_safe

app/views/layouts/header/_new_repo_experiment.html.haml

-- content_for :new_repo_experiment do
-  - experiment(:new_repo, user: current_user) do |e|
-    - e.use do
-      = _('New project')
-    - e.try do
-      = _('New project/repository')

app/views/layouts/header/_project_invite_members_new_dropdown_item.html.haml

-- return unless Gitlab::Experimentation.active?(:invite_members_new_dropdown) && can_import_members?
-
-%li= dropdown_invite_members_link(@project)

ee/app/helpers/ee/nav/new_dropdown_helper.rb

+# frozen_string_literal: true
+
+module EE
+  module Nav
+    module NewDropdownHelper
+      extend ::Gitlab::Utils::Override
+
+      private
+
+      override :create_epic_menu_item
+      def create_epic_menu_item(group)
+        if can?(current_user, :create_epic, group)
+          ::Gitlab::Nav::TopNavMenuItem.build(
+            id: 'create_epic',
+            title: _('New epic'),
+            href: new_group_epic_path(group),
+            data: { track_event: 'click_link_new_epic', track_label: 'plus_menu_dropdown' }
+          )
+        end
+      end
+    end
+  end
+end

ee/app/views/layouts/header/_create_epic_new_dropdown_item.haml

-- return unless can?(current_user, :create_epic, @group)
-
-%li= link_to _('New epic'), new_group_epic_path(@group), data: { track_event: 'click_link_new_epic', track_label: 'plus_menu_dropdown' }

ee/spec/helpers/nav/new_dropdown_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Nav::NewDropdownHelper do
+  describe '#new_dropdown_view_model' do
+    let_it_be(:user) { build_stubbed(:user) }
+    let_it_be(:group) { build_stubbed(:group) }
+
+    let(:subject) { helper.new_dropdown_view_model(group: group, project: nil) }
+
+    before do
+      allow(helper).to receive(:current_user) { user }
+      allow(helper).to receive(:can?) { false }
+      allow(helper).to receive(:can?).with(user, :create_epic, group) { true }
+    end
+
+    context 'with group and can create_epic' do
+      it 'shows create epic menu item' do
+        expect(subject[:menu_sections][0]).to eq({
+          title: 'This group',
+          menu_items: [
+            ::Gitlab::Nav::TopNavMenuItem.build(
+              id: 'create_epic',
+              title: 'New epic',
+              href: "/groups/#{group.path}/-/epics/new",
+              data: { track_event: 'click_link_new_epic', track_label: 'plus_menu_dropdown' }
+            )
+          ]
+        })
+      end
+    end
+  end
+end

lib/gitlab/nav/top_nav_menu_item.rb

@@ -8,7 +8,7 @@ module Gitlab
       # this is already :/. We could also take a hash and manually check every
       # entry, but it's much more maintainable to do rely on native Ruby.
       # rubocop: disable Metrics/ParameterLists
-      def self.build(id:, title:, active: false, icon: '', href: '', view: '', css_class: '', data: {})
+      def self.build(id:, title:, active: false, icon: '', href: '', view: '', css_class: nil, data: {}, emoji: nil)
         {
           id: id,
           title: title,
@@ -17,7 +17,8 @@ module Gitlab
           href: href,
           view: view.to_s,
           css_class: css_class,
-          data: data
+          data: data,
+          emoji: emoji
         }
       end
       # rubocop: enable Metrics/ParameterLists

qa/qa/page/dashboard/snippet/index.rb

@@ -7,6 +7,9 @@ module QA
         class Index < Page::Base
           view 'app/views/layouts/header/_new_dropdown.html.haml' do
             element :new_menu_toggle
+          end
+
+          view 'app/helpers/nav/new_dropdown_helper.rb' do
             element :global_new_snippet_link
           end
 

qa/qa/page/project/show.rb

@@ -24,7 +24,10 @@ module QA
 
         view 'app/views/layouts/header/_new_dropdown.html.haml' do
           element :new_menu_toggle
-          element :new_issue_link, "link_to _('New issue'), new_project_issue_path(@project)" # rubocop:disable QA/ElementWithPattern
+        end
+
+        view 'app/helpers/nav/new_dropdown_helper.rb' do
+          element :new_issue_link
         end
 
         view 'app/views/projects/_last_push.html.haml' do
@@ -115,7 +118,7 @@ module QA
 
         def go_to_new_issue
           click_element :new_menu_toggle
-          click_link 'New issue'
+          click_element(:new_issue_link)
         end
 
         def has_file?(name)

spec/features/projects/show/user_sees_collaboration_links_spec.rb

@@ -12,6 +12,10 @@ RSpec.describe 'Projects > Show > Collaboration links', :js do
     sign_in(user)
   end
 
+  def find_new_menu_toggle
+    find('#js-onboarding-new-project-link')
+  end
+
   context 'with developer user' do
     before do
       project.add_developer(user)
@@ -22,7 +26,7 @@ RSpec.describe 'Projects > Show > Collaboration links', :js do
 
       # The navigation bar
       page.within('.header-new') do
-        find('.qa-new-menu-toggle').click
+        find_new_menu_toggle.click
 
         aggregate_failures 'dropdown links in the navigation bar' do
           expect(page).to have_link('New issue')
@@ -30,7 +34,7 @@ RSpec.describe 'Projects > Show > Collaboration links', :js do
           expect(page).to have_link('New snippet', href: new_project_snippet_path(project))
         end
 
-        find('.qa-new-menu-toggle').click
+        find_new_menu_toggle.click
       end
 
       # The dropdown above the tree
@@ -56,7 +60,7 @@ RSpec.describe 'Projects > Show > Collaboration links', :js do
       visit project_path(project)
 
       page.within('.header-new') do
-        find('.qa-new-menu-toggle').click
+        find_new_menu_toggle.click
 
         aggregate_failures 'dropdown links' do
           expect(page).not_to have_link('New issue')
@@ -64,7 +68,7 @@ RSpec.describe 'Projects > Show > Collaboration links', :js do
           expect(page).not_to have_link('New snippet', href: new_project_snippet_path(project))
         end
 
-        find('.qa-new-menu-toggle').click
+        find_new_menu_toggle.click
       end
 
       expect(page).not_to have_selector('.qa-add-to-tree')

spec/helpers/invite_members_helper_spec.rb

@@ -114,69 +114,4 @@ RSpec.describe InviteMembersHelper do
       end
     end
   end
-
-  describe '#dropdown_invite_members_link' do
-    shared_examples_for 'dropdown invite members link' do
-      let(:link_regex) do
-        /data-track-event="click_link".*data-track-property="_track_property_".*Invite members/
-      end
-
-      before do
-        allow(helper).to receive(:experiment_tracking_category_and_group) { '_track_property_' }
-        allow(helper).to receive(:current_user) { owner }
-      end
-
-      it 'records the experiment' do
-        allow(helper).to receive(:experiment_enabled?)
-
-        helper.dropdown_invite_members_link(form_model)
-
-        expect(helper).to have_received(:experiment_tracking_category_and_group).with(:invite_members_new_dropdown)
-      end
-
-      context 'with experiment enabled' do
-        before do
-          allow(helper).to receive(:experiment_enabled?).with(:invite_members_new_dropdown) { true }
-        end
-
-        it 'returns link' do
-          link = helper.dropdown_invite_members_link(form_model)
-
-          expect(link).to match(link_regex)
-          expect(link).to include(link_href)
-          expect(link).to include('gl-emoji')
-        end
-      end
-
-      context 'with no experiment enabled' do
-        before do
-          allow(helper).to receive(:experiment_enabled?).with(:invite_members_new_dropdown) { false }
-        end
-
-        it 'returns link' do
-          link = helper.dropdown_invite_members_link(form_model)
-
-          expect(link).to match(link_regex)
-          expect(link).to include(link_href)
-          expect(link).not_to include('gl-emoji')
-        end
-      end
-    end
-
-    context 'with a project' do
-      let_it_be(:form_model) { project }
-
-      let(:link_href) { "href=\"#{project_project_members_path(form_model)}\"" }
-
-      it_behaves_like 'dropdown invite members link'
-    end
-
-    context 'with a group' do
-      let_it_be(:form_model) { create(:group) }
-
-      let(:link_href) { "href=\"#{group_group_members_path(form_model)}\"" }
-
-      it_behaves_like 'dropdown invite members link'
-    end
-  end
 end

spec/lib/gitlab/nav/top_nav_menu_item_spec.rb

@@ -13,7 +13,8 @@ RSpec.describe ::Gitlab::Nav::TopNavMenuItem do
         href: 'href',
         view: 'view',
         css_class: 'css_class',
-        data: {}
+        data: {},
+        emoji: 'smile'
       }
 
       expect(described_class.build(**item)).to eq(item)

spec/views/layouts/header/_new_dropdown.haml_spec.rb

@@ -52,7 +52,6 @@ RSpec.describe 'layouts/header/_new_dropdown' do
       end
 
       it 'has a "New project" link' do
-        render('layouts/header/new_repo_experiment')
         render
 
         expect(rendered).to have_link('New project', href: new_project_path(namespace_id: group.id))
@@ -164,7 +163,6 @@ RSpec.describe 'layouts/header/_new_dropdown' do
     end
 
     it 'has a "New project" link' do
-      render('layouts/header/new_repo_experiment')
       render
 
       expect(rendered).to have_link('New project', href: new_project_path)
@@ -182,13 +180,13 @@ RSpec.describe 'layouts/header/_new_dropdown' do
       expect(rendered).to have_link('New snippet', href: new_snippet_path)
     end
 
-    context 'when the user is not allowed to create snippets' do
+    context 'when the user is not allowed to do anything' do
       let(:user) { create(:user, :external) }
 
-      it 'has no "New snippet" link' do
-        render
-
-        expect(rendered).not_to have_link('New snippet', href: new_snippet_path)
+      it 'is nil' do
+        # We have to us `view.render` because `render` causes issues
+        # https://github.com/rails/rails/issues/41320
+        expect(view.render("layouts/header/new_dropdown")).to be_nil
       end
     end
   end