Merge branch 'protected-branches-dropdown-headers' into 'master'

Protected branches dropdown headers

Closes #1294

See merge request !1114

app/assets/javascripts/protected_branches/protected_branch_access_dropdown.js

@@ -25,7 +25,7 @@
       this.isAllowedToPushDropdown = false;
       this.groups = [];
       this.accessLevel = accessLevel;
-      this.accessLevelsData = accessLevelsData;
+      this.accessLevelsData = accessLevelsData.roles;
       this.$dropdown = $dropdown;
       this.$wrap = this.$dropdown.closest(`.${this.accessLevel}-container`);
       this.usersPath = '/autocomplete/users.json';
@@ -275,8 +275,7 @@
       let consolidatedData = [];
       const map = [];
       let roles = [];
-      const selectedUsers = [];
-      const unselectedUsers = [];
+      const users = [];
       let groups = [];
       const selectedItems = this.getSelectedItems();
 
@@ -319,12 +318,12 @@
         if (current.type !== LEVEL_TYPES.USER) { continue; }
 
         // Collect selected users
-        selectedUsers.push({
+        users.push({
           id: current.user_id,
           name: current.name,
           username: current.username,
           avatar_url: current.avatar_url,
-          type: LEVEL_TYPES.USER
+          type: LEVEL_TYPES.USER,
         });
 
         // Save identifiers for easy-checking more later
@@ -339,32 +338,24 @@
         // Add is it has not been added
         if (map.indexOf(LEVEL_TYPES.USER + u.id) === -1) {
           u.type = LEVEL_TYPES.USER;
-          unselectedUsers.push(u);
+          users.push(u);
         }
       }
 
-      if (groups.length) {
-        consolidatedData = consolidatedData.concat(groups);
+      if (roles.length) {
+        consolidatedData = consolidatedData.concat([{ header: 'Roles', }], roles);
       }
 
-      if (roles.length) {
       if (groups.length) {
+        if (roles.length) {
           consolidatedData = consolidatedData.concat(['divider']);
         }
 
-        consolidatedData = consolidatedData.concat(roles);
-      }
-
-      if (selectedUsers.length) {
-        consolidatedData = consolidatedData.concat(['divider'], selectedUsers);
-      }
-
-      if (unselectedUsers.length) {
-        if (!selectedUsers.length) {
-          consolidatedData = consolidatedData.concat(['divider']);
+        consolidatedData = consolidatedData.concat([{ header: 'Groups', }], groups);
       }
 
-        consolidatedData = consolidatedData.concat(unselectedUsers);
+      if (users.length) {
+        consolidatedData = consolidatedData.concat(['divider'], [{ header: 'Users', }], users);
       }
 
       return consolidatedData;
@@ -434,9 +425,8 @@
 
     groupRowHtml(group, isActive) {
       const avatarHtml = group.avatar_url ? `<img src='${group.avatar_url}' class='avatar avatar-inline' width='30'>` : '';
-      const nameHtml = `<strong class='dropdown-menu-group-full-name'>${group.name}</strong>`;
       const groupnameHtml = `<span class='dropdown-menu-group-groupname'>${group.name}</span>`;
-      return `<li><a href='#' class='${isActive ? 'is-active' : ''}'>${avatarHtml} ${nameHtml} ${groupnameHtml}</a></li>`;
+      return `<li><a href='#' class='${isActive ? 'is-active' : ''}'>${avatarHtml} ${groupnameHtml}</a></li>`;
     }
 
     roleRowHtml(role, isActive) {

app/controllers/projects/protected_branches_controller.rb

@@ -69,8 +69,14 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
 
   def access_levels_options
     {
-      push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
-      merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } }
+      push_access_levels: {
+        roles: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
+      },
+      merge_access_levels: {
+        roles: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } },
+      },
+      selected_merge_access_levels: @protected_branch.merge_access_levels.map { |access_level| access_level.user_id || access_level.access_level },
+      selected_push_access_levels: @protected_branch.push_access_levels.map { |access_level| access_level.user_id || access_level.access_level }
     }
   end
 

changelogs/unreleased-ee/protected-branches-dropdown-headers.yml

+---
+title: Added headers to protected branches access dropdowns
+merge_request:
+author:

spec/features/protected_branches/access_control_ee_spec.rb

@@ -92,6 +92,13 @@ RSpec.shared_examples "protected branches > access control > EE" do
       within(".protected-branches-list") do
         find(".js-allowed-to-#{git_operation}").click
         find(".dropdown-input-field").set(users.last.name) # Find a user that is not loaded
+
+        expect(page).to have_selector('.dropdown-header', count: 3)
+
+        %w{Roles Groups Users}.each_with_index do |header, index|
+          expect(all('.dropdown-header')[index]).to have_content(header)
+        end
+
         wait_for_ajax
         click_on users.last.name
         find(".js-allowed-to-#{git_operation}").click # close