refactor dropdown

clear hidden inputs after posting update

app/assets/javascripts/api.js

@@ -23,7 +23,7 @@ var Api = {
       return callback(group);
     });
   },
-  users: function(search, options, callback) {
+  users: function(search, options, callback = $.noop) {
     var url = Api.buildUrl("/autocomplete/users.json");
     return $.ajax({
       url,
@@ -35,7 +35,7 @@ var Api = {
     }).done(callback);
   },
   // Return groups list. Filtered by query
-  groups: function(query, options, callback) {
+  groups: function(query, options, callback = $.noop) {
     var url = Api.buildUrl(Api.groupsPath);
     return $.ajax({
       url: url,

app/assets/javascripts/approvers_select.js

+/* global Api */
+
+export default class ApproversSelect {
+  constructor() {
+    const approverSelect = document.querySelector('.js-select-user-and-group');
+    const name = approverSelect.dataset.name;
+    this.fieldNames = [`${name}[approver_ids]`, `${name}[approver_group_ids]`];
+
+    this.bindEvents();
+    this.addEvents();
+
+    $('.js-select-user-and-group').select2({
+      placeholder: 'Search for users or groups',
+      multiple: true,
+      minimumInputLength: 0,
+      query: (query) => {
+        const fetchGroups = this.fetchGroups(query.term);
+        const fetchUsers = this.fetchUsers(query.term);
+        return $.when(fetchGroups, fetchUsers).then((groups, users) => {
+          const data = {
+            results: groups[0].concat(users[0]),
+          };
+          return query.callback(data);
+        });
+      },
+      formatResult: ApproversSelect.formatResult,
+      formatSelection: ApproversSelect.formatSelection,
+      dropdownCssClass: 'ajax-groups-dropdown',
+    })
+    .on('change', this.handleSelectChange);
+  }
+
+  bindEvents() {
+    this.addApprover = this.addApprover.bind(this);
+    this.handleSelectChange = this.handleSelectChange.bind(this);
+    this.fetchGroups = this.fetchGroups.bind(this);
+    this.fetchUsers = this.fetchUsers.bind(this);
+  }
+
+  addEvents() {
+    $(document).on('click', '.js-approvers', this.addApprover);
+    $(document).on('click', '.js-approver-remove', ApproversSelect.removeApprover);
+  }
+
+  static getOptions(fieldName, selector, key) {
+    const input = $(`[name="${fieldName}"]`);
+    const existingApprovers = [].map.call(
+      document.querySelectorAll(selector),
+      item => parseInt(item.getAttribute('data-id'), 10),
+    );
+    const selectedApprovers = input.val()
+      .split(',')
+      .filter(val => val !== '');
+    const options = {
+      [key]: [...existingApprovers, ...selectedApprovers],
+    };
+    return options;
+  }
+
+  fetchGroups(term) {
+    const options = ApproversSelect.getOptions(this.fieldNames[1], '.js-approver-group', 'skip_groups');
+    return Api.groups(term, options);
+  }
+
+  fetchUsers(term) {
+    const options = ApproversSelect.getOptions(this.fieldNames[0], '.js-approver', 'skip_users');
+    return Api.users(term, options);
+  }
+
+  handleSelectChange(evt) {
+    const { added, removed } = evt;
+    const userInput = $(`[name="${this.fieldNames[0]}"]`);
+    const groupInput = $(`[name="${this.fieldNames[1]}"]`);
+
+    if (added) {
+      if (added.full_name) {
+        groupInput.val(`${groupInput.val()},${added.id}`.replace(/^,/, ''));
+      } else {
+        userInput.val(`${userInput.val()},${added.id}`.replace(/^,/, ''));
+      }
+    }
+
+    if (removed) {
+      if (removed.full_name) {
+        groupInput.val(groupInput.val().replace(new RegExp(`,?${removed.id}`), ''));
+      } else {
+        userInput.val(userInput.val().replace(new RegExp(`,?${removed.id}`), ''));
+      }
+    }
+  }
+
+  static formatSelection(group) {
+    return group.full_name || group.name;
+  }
+
+  static formatResult({
+    avatar_url: avatarUrl,
+    full_name: fullName,
+    full_path: fullPath,
+    name,
+    username,
+  }) {
+    if (username) {
+      const avatar = avatarUrl || gon.default_avatar_url;
+      return `
+        <div class="user-result">
+          <div class="user-image">
+            <img class="avatar s24" src="${avatar}">
+          </div>
+          <div class="user-name">${name}</div>
+          <div class="user-username">@${username}</div>
+        </div>
+      `;
+    }
+
+    return `
+      <div class="group-result">
+        <div class="group-name">${fullName}</div>
+        <div class="group-path">${fullPath}</div>
+      </div>
+    `;
+  }
+
+  addApprover() {
+    this.fieldNames.forEach(ApproversSelect.saveApprovers);
+  }
+
+  static saveApprovers(fieldName) {
+    const $input = $(`[name="${fieldName}"]`);
+    const newValue = $input.val();
+
+    if (!newValue) {
+      return;
+    }
+
+    const $form = $('.js-approvers').closest('form');
+    $('.load-wrapper').removeClass('hidden');
+    $.ajax({
+      url: $form.attr('action'),
+      type: 'POST',
+      data: {
+        _method: 'PATCH',
+        [fieldName]: newValue,
+      },
+      success: ApproversSelect.updateApproverList,
+      complete() {
+        $input.val('val', '');
+        $('.js-select-user-and-group').select2('val', '');
+        $('.load-wrapper').addClass('hidden');
+      },
+      error() {
+        // TODO: scroll into view or toast
+        window.Flash('Failed to add Approver', 'alert');
+      },
+    });
+  }
+
+  static removeApprover(evt) {
+    evt.preventDefault();
+    const target = evt.currentTarget;
+    $('.load-wrapper').removeClass('hidden');
+    $.ajax({
+      url: target.getAttribute('href'),
+      type: 'POST',
+      data: {
+        _method: 'DELETE',
+      },
+      success: ApproversSelect.updateApproverList,
+      complete: () => $('.load-wrapper').addClass('hidden'),
+      error() {
+        window.Flash('Failed to remove Approver', 'alert');
+      },
+    });
+  }
+
+  static updateApproverList(html) {
+    $('.approver-list').html($(html).find('.approver-list').html());
+  }
+}

app/assets/javascripts/project_new.js

 /* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, wrap-iife, no-unused-vars, one-var, no-underscore-dangle, prefer-template, no-else-return, prefer-arrow-callback, max-len */
 /* global Api */
 
-(function() {
-  var bind = function(fn, me) { return function() { return fn.apply(me, arguments); }; };
+import ApproversSelect from './approvers_select';
 
+(function() {
   this.ProjectNew = (function() {
     function ProjectNew() {
-      this.toggleSettings = bind(this.toggleSettings, this);
-      this.addApprover = this.addApprover.bind(this);
-      this.removeApprover = this.removeApprover.bind(this);
-
       this.$selects = $('.project-feature select');
       this.$repoSelects = this.$selects.filter('.js-repo-select');
       this.$enableApprovers = $('.js-require-approvals-toggle');
@@ -27,173 +23,17 @@
       this.addEvents();
       this.toggleRepoVisibility();
 
-      this.initApproverSelect();
+      this.approversSelect = new ApproversSelect();
     }
 
-    ProjectNew.prototype.addEvents = function() {
-      this.$selects.on('change', this.toggleSettings);
-      $('.js-approvers').on('click', this.addApprover);
-      $(document).on('click', '.js-approver-remove', this.removeApprover);
-
-      $('#require_approvals').on('change', function() {
-        const enabled = $(this).prop('checked');
-        const val = enabled ? 1 : 0;
-        $('#project_approvals_before_merge').val(val);
-        $('#project_approvals_before_merge').prop('min', val);
-        $('.nested-settings').toggleClass('hidden', !enabled);
-      });
+    ProjectNew.prototype.bindEvents = function() {
+      this.toggleSettings = this.toggleSettings.bind(this);
+      this.toggleApproverSettingsVisibility = this.toggleApproverSettingsVisibility.bind(this);
     };
 
-    ProjectNew.prototype.initApproverSelect = function() {
-      $('.js-select-user-and-group').select2({
-        placeholder: 'Search for users or groups',
-        multiple: true,
-        minimumInputLength: 0,
-        query(query) {
-          const existingGroupApprovers = [].map.call(
-            document.querySelectorAll('.js-approver-group'),
-            item => parseInt(item.getAttribute('data-id'), 10),
-          );
-          const selectedGroupApprovers = $('[name="project[approver_group_ids]"]').val()
-            .split(',')
-            .filter(val => val !== '');
-          const groupOptions = {
-            skip_groups: [...existingGroupApprovers, ...selectedGroupApprovers],
-          };
-          const groupsApi = Api.groups(query.term, groupOptions, function(groups) {
-            return groups;
-          });
-
-          const existingApprovers = [].map.call(
-            document.querySelectorAll('.js-approver'),
-            item => parseInt(item.getAttribute('data-id'), 10),
-          );
-          const selectedApprovers = $('[name="project[approver_ids]"]').val()
-            .split(',')
-            .filter(id => id !== '');
-          const userOptions = {
-            skip_users: [...existingApprovers, ...selectedApprovers],
-          };
-          const usersApi = Api.users(query.term, userOptions, function(groups) {
-            return groups;
-          });
-
-          return $.when(groupsApi, usersApi).then((groups, users) => {
-            const data = {
-              results: groups[0].concat(users[0]),
-            };
-            return query.callback(data);
-          });
-        },
-        formatResult: this.formatResult,
-        formatSelection: this.formatSelection,
-        dropdownCssClass: 'ajax-groups-dropdown',
-      })
-      .on('change', (evt) => {
-        const { added, removed } = evt;
-        const groupInput = $('[name="project[approver_group_ids]"]');
-        const userInput = $('[name="project[approver_ids]"]');
-
-        if (added) {
-          if (added.full_name) {
-            groupInput.val(`${groupInput.val()},${added.id}`.replace(/^,/, ''));
-          } else {
-            userInput.val(`${userInput.val()},${added.id}`.replace(/^,/, ''));
-          }
-        }
-
-        if (removed) {
-          if (removed.full_name) {
-            groupInput.val(groupInput.val().replace(new RegExp(`,?${removed.id}`), ''));
-          } else {
-            userInput.val(userInput.val().replace(new RegExp(`,?${removed.id}`), ''));
-          }
-        }
-      });
-    };
-
-    ProjectNew.prototype.formatResult = function(group) {
-      if (group.username) {
-        return "<div class='group-result'> <div class='group-name'>" + group.name + "</div> <div class='group-path'></div> </div>";
-      }
-
-      let avatar;
-      if (group.avatar_url) {
-        avatar = group.avatar_url;
-      } else {
-        avatar = gon.default_avatar_url;
-      }
-      return `
-        <div class='group-result'>
-          <div class='group-name'>
-            ${group.full_name}
-          </div>
-          <div class='group-path'>
-            ${group.full_path}
-          </div>
-        </div>
-      `;
-    };
-
-    ProjectNew.prototype.formatSelection = function(group) {
-      return group.full_name || group.name;
-    };
-
-    ProjectNew.prototype.addApprover = function(evt) {
-      const fieldNames = ['project[approver_ids]', 'project[approver_group_ids]'];
-      fieldNames.forEach((fieldName) => {
-        const $select = $(`[name="${fieldName}"]`);
-        const newValue = $select.val();
-
-        if (!newValue) {
-          return;
-        }
-
-        const $form = $('.js-approvers').closest('form');
-        $('.load-wrapper').removeClass('hidden');
-        $.ajax({
-          url: $form.attr('action'),
-          type: 'POST',
-          data: {
-            _method: 'PATCH',
-            [fieldName]: newValue,
-          },
-          success: this.updateApproverList,
-          complete() {
-            $select.select2('val', '');
-            $('.js-select-user-and-group').select2('val', '');
-            $('.load-wrapper').addClass('hidden');
-          },
-          error(err) {
-            // TODO: scroll into view or toast
-            window.Flash('Failed to add Approver', 'alert');
-          },
-        });
-      });
-    };
-
-    ProjectNew.prototype.removeApprover = function(evt) {
-      evt.preventDefault();
-      const target = evt.currentTarget;
-      $('.load-wrapper').removeClass('hidden');
-      $.ajax({
-        url: target.getAttribute('href'),
-        type: 'POST',
-        data: {
-          _method: 'DELETE',
-        },
-        success: this.updateApproverList,
-        complete: () => $('.load-wrapper').addClass('hidden'),
-        error(err) {
-          window.Flash('Failed to remove Approver', 'alert');
-        },
-      });
-    };
-
-    ProjectNew.prototype.updateApproverList = function(html) {
-      const fakeEl = document.createElement('template');
-      fakeEl.innerHTML = html;
-      document.querySelector('.well-list.approver-list').innerHTML = fakeEl.content.querySelector('.well-list.approver-list').innerHTML;
+    ProjectNew.prototype.addEvents = function() {
+      this.$selects.on('change', this.toggleSettings);
+      $('#require_approvals').on('change', this.toggleApproverSettingsVisibility);
     };
 
     ProjectNew.prototype.initVisibilitySelect = function() {
@@ -204,8 +44,12 @@
     };
 
     ProjectNew.prototype.toggleApproverSettingsVisibility = function(evt) {
-      const enabled = evt.value;
-      $('.nested-settings').toggleClass('hidden', enabled);
+      this.$requiredApprovals = $('#project_approvals_before_merge');
+      const enabled = $(evt.target).prop('checked');
+      const val = enabled ? 1 : 0;
+      this.$requiredApprovals.val(val);
+      this.$requiredApprovals.prop('min', val);
+      $('.nested-settings').toggleClass('hidden', !enabled);
     };
 
     ProjectNew.prototype.toggleSettings = function() {

app/views/projects/ee/_merge_request_settings.html.haml

@@ -58,7 +58,7 @@
     = hidden_field_tag "project[approver_ids]"
     = hidden_field_tag "project[approver_group_ids]"
     .input-group.input-btn-group
-      = hidden_field_tag :approver_user_and_group_ids, '', { class: 'js-select-user-and-group input-large', tabindex: 1 }
+      = hidden_field_tag :approver_user_and_group_ids, '', { class: 'js-select-user-and-group input-large', tabindex: 1, data: { name: 'project' }}
       %button.btn.btn-success.js-approvers{ type: 'button', title: 'Add approvers(s)' }
         Add
     .help-block
@@ -67,6 +67,7 @@
     .panel.panel-default.prepend-top-10
       .panel-heading
         Approvers
+        %span.badge
         -# TODO: badge with project.approver_group_ids.count + project.approver_ids.count
       %ul.well-list.approver-list
         .load-wrapper.hidden

spec/javascripts/project_new_spec.js → spec/javascripts/approvers_select_spec.js

-/* global ProjectNew */
+import ApproversSelect from '~/approvers_select';
 
-require('~/project_new');
-
-describe('Project settings', function () {
+describe('ApproversSelect', function () {
   const projectSettingsTemplate = 'projects/edit.html.raw';
   preloadFixtures(projectSettingsTemplate);
 
   beforeEach(() => {
     loadFixtures(projectSettingsTemplate);
     this.$requireApprovalsToggle = $('.js-require-approvals-toggle');
-    this.project = new ProjectNew();
+    this.project = new ApproversSelect();
   });
 
   it('shows approver settings if enabled', () => {
@@ -37,4 +35,10 @@ describe('Project settings', function () {
     this.$requireApprovalsToggle.click();
     expect($('[name="project[approvals_before_merge]"]').val()).toBe('1');
   });
+
+  it('sets minimum for approvers field if enabled', () => {
+    expect($('[name="project[approvals_before_merge]"]').attr('min')).toBe('0');
+    this.$requireApprovalsToggle.click();
+    expect($('[name="project[approvals_before_merge]"]').attr('min')).toBe('1');
+  });
 });