Merge branch '3258-fix-geo-selective-replication-dropdown' into 'master'

Geo - Selective replication allows admins to select any groups

Closes #3258

See merge request gitlab-org/gitlab-ee!2779

app/assets/javascripts/dispatcher.js

@@ -55,7 +55,6 @@ import UserCallout from './user_callout';
 import ShortcutsWiki from './shortcuts_wiki';
 import Pipelines from './pipelines';
 import BlobViewer from './blob/viewer/index';
-import GeoNodeForm from './geo/geo_node_form';
 import GeoNodes from './geo_nodes';
 import AutoWidthDropdownSelect from './issuable/auto_width_dropdown_select';
 import UsersSelect from './users_select';
@@ -640,7 +639,9 @@ import initGroupAnalytics from './init_group_analytics';
               break;
             case 'geo_nodes':
               new GeoNodes($('.geo-nodes'));
-              new GeoNodeForm($('.js-geo-node-form'));
+              import(/* webpackChunkName: 'geo_node_form' */ './geo/geo_node_form')
+                .then(geoNodeForm => geoNodeForm.default($('.js-geo-node-form')))
+                .catch(() => {});
               break;
           }
           break;

app/assets/javascripts/geo/geo_node_form.js

-export default class GeoNodeForm {
-  constructor(container) {
-    this.$container = container;
-    this.$namespaces = this.$container.find('.js-hide-if-geo-primary');
-    this.$namespacesSelect = this.$namespaces.find('.select2');
-    this.$primaryCheckbox = this.$container.find("input[type='checkbox']");
-    this.$primaryCheckbox.on('change', () => this.onPrimaryCheckboxChange());
-  }
-
-  onPrimaryCheckboxChange() {
-    this.$namespacesSelect.select2('data', null);
-    this.$namespaces.toggleClass('hidden', this.$primaryCheckbox.is(':checked'));
-  }
+/* global Flash */
+import {
+  s__,
+} from '../locale';
+import '../flash';
+import Api from '../api';
+
+const onPrimaryCheckboxChange = function onPrimaryCheckboxChange(e, $namespaces) {
+  const $namespacesSelect = $('.select2', $namespaces);
+
+  $namespacesSelect.select2('data', null);
+  $namespaces.toggleClass('hidden', e.currentTarget.checked);
+};
+
+export default function geoNodeForm($container) {
+  const $namespaces = $('.js-hide-if-geo-primary', $container);
+  const $primaryCheckbox = $('input[type="checkbox"]', $container);
+  const $select2Dropdown = $('.js-geo-node-namespaces', $container);
+
+  $primaryCheckbox.on('change', e => onPrimaryCheckboxChange(e, $namespaces));
+
+  $select2Dropdown.select2({
+    placeholder: s__('Geo|Select groups to replicate.'),
+    multiple: true,
+    initSelection($el, callback) {
+      callback($el.data('selected'));
+    },
+    ajax: {
+      url: Api.buildUrl(Api.groupsPath),
+      dataType: 'JSON',
+      quietMillis: 250,
+      data(search) {
+        return {
+          search,
+        };
+      },
+      results(data) {
+        return {
+          results: data.map(group => ({
+            id: group.id,
+            text: group.full_name,
+          })),
+        };
+      },
+    },
+  });
 }

app/services/geo/node_create_service.rb

+module Geo
+  class NodeCreateService
+    attr_reader :params
+
+    def initialize(params)
+      @params = params.dup
+      @params[:namespace_ids] = @params[:namespace_ids].to_s.split(',')
+    end
+
+    def execute
+      GeoNode.create(params).persisted?
+    end
+  end
+end

app/services/geo/node_update_service.rb

@@ -6,6 +6,7 @@ module Geo
       @geo_node = geo_node
       @old_namespace_ids = geo_node.namespace_ids
       @params = params.slice(:url, :primary, :namespace_ids)
+      @params[:namespace_ids] = @params[:namespace_ids].to_s.split(',')
     end
 
     def execute
@@ -15,7 +16,7 @@ module Geo
         Geo::RepositoriesChangedEventStore.new(geo_node).create
       end
 
-      geo_node
+      true
     end
 
     private

app/views/admin/geo_nodes/_form.html.haml

@@ -24,8 +24,8 @@
           = link_to 'here', help_page_path('gitlab-geo/configuration.html', anchor: 'step-5-enabling-the-secondary-gitlab-node')
 
 .form-group.js-hide-if-geo-primary{ class: ('hidden' unless geo_node.secondary?) }
-  = form.label :namespace_ids, 'Namespaces to replicate', class: 'control-label'
+  = form.label :namespace_ids, s_('Geo|Groups to replicate'), class: 'control-label'
   .col-sm-10
-    = form.select :namespace_ids, namespaces_options(geo_node.namespace_ids), { include_hidden: true }, multiple: true, class: 'select2 select-wide', data: { field: 'namespace_ids' }
+    = hidden_field_tag "#{form.object_name}[namespace_ids]", geo_node.namespace_ids.join(","), class: 'js-geo-node-namespaces', data: { selected: node_namespaces_options(geo_node.namespaces).to_json }
     .help-block
-      Choose which namespaces you wish to replicate to this secondary node. Leave blank to replicate all.
+      #{ s_("Choose which groups you wish to replicate to this secondary node. Leave blank to replicate all.") }

changelogs/unreleased-ee/3258-fix-geo-selective-replication-dropdown.yml

+---
+title: Geo - Selective replication allows admins to select any groups
+merge_request: 2779
+author:
+type: fixed

doc/gitlab-geo/configuration.md

@@ -281,7 +281,7 @@ Point your users to the [after setup steps](after_setup.md).
 ## Selective replication
 
 With GitLab 9.5, GitLab Geo now supports the first iteration of selective
-replication, which allows admins to choose which namespaces should be
+replication, which allows admins to choose which groups should be
 replicated by secondary nodes.
 
 It is important to notice that selective replication:
@@ -291,7 +291,7 @@ It is important to notice that selective replication:
 relies on PostgreSQL replication, all project metadata gets replicated to
 secondary nodes, but repositories that have not been selected will be empty.
 1. Secondary nodes won't pull repositories that do not belong to the selected
-namespaces to be replicated.
+groups to be replicated.
 
 ## Adding another secondary Geo node
 

ee/app/controllers/admin/geo_nodes_controller.rb

@@ -14,9 +14,7 @@ class Admin::GeoNodesController < Admin::ApplicationController
   end
 
   def create
-    @node = GeoNode.new(geo_node_params)
-
-    if @node.save
+    if Geo::NodeCreateService.new(geo_node_params).execute
       redirect_to admin_geo_nodes_path, notice: 'Node was successfully created.'
     else
       @nodes = GeoNode.all
@@ -26,9 +24,9 @@ class Admin::GeoNodesController < Admin::ApplicationController
 
   def update
     if Geo::NodeUpdateService.new(@node, geo_node_params).execute
-      redirect_to admin_geo_nodes_path, notice: 'Geo Node was successfully updated.'
+      redirect_to admin_geo_nodes_path, notice: 'Node was successfully updated.'
     else
-      render 'edit'
+      render :edit
     end
   end
 
@@ -79,7 +77,7 @@ class Admin::GeoNodesController < Admin::ApplicationController
   private
 
   def geo_node_params
-    params.require(:geo_node).permit(:url, :primary, namespace_ids: [], geo_node_key_attributes: [:key])
+    params.require(:geo_node).permit(:url, :primary, :namespace_ids, geo_node_key_attributes: [:key])
   end
 
   def check_license

ee/app/helpers/ee/geo_helper.rb

 module EE
   module GeoHelper
+    def node_namespaces_options(namespaces)
+      namespaces.map { |g| { id: g.id, text: g.full_name } }
+    end
+
     def node_selected_namespaces_to_replicate(node)
       node.namespaces.map(&:human_name).sort.join(', ')
     end

locale/gitlab.pot

@@ -8,8 +8,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-10-02 15:48-0400\n"
-"PO-Revision-Date: 2017-10-02 15:48-0400\n"
+"POT-Creation-Date: 2017-10-04 17:48+0100\n"
+"PO-Revision-Date: 2017-10-04 17:48+0100\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
 "Language: \n"
@@ -382,6 +382,9 @@ msgstr ""
 msgid "Cherry-pick this merge request"
 msgstr ""
 
+msgid "Choose which groups you wish to replicate to this secondary node. Leave blank to replicate all."
+msgstr ""
+
 msgid "CiStatusLabel|canceled"
 msgstr ""
 
@@ -712,6 +715,12 @@ msgstr ""
 msgid "Geo Nodes"
 msgstr ""
 
+msgid "Geo|Groups to replicate"
+msgstr ""
+
+msgid "Geo|Select groups to replicate."
+msgstr ""
+
 msgid "Git storage health information has been reset"
 msgstr ""
 
@@ -1195,6 +1204,21 @@ msgstr ""
 msgid "ProjectNetworkGraph|Graph"
 msgstr ""
 
+msgid "ProjectSettings|Contact an admin to change this setting."
+msgstr ""
+
+msgid "ProjectSettings|Only signed commits can be pushed to this repository."
+msgstr ""
+
+msgid "ProjectSettings|This setting is applied on the server level and can be overridden by an admin."
+msgstr ""
+
+msgid "ProjectSettings|This setting is applied on the server level but has been overridden for this project."
+msgstr ""
+
+msgid "ProjectSettings|This setting will be applied to all projects unless overridden by an admin."
+msgstr ""
+
 msgid "ProjectsDropdown|Frequently visited"
 msgstr ""
 

spec/controllers/admin/geo_nodes_controller_spec.rb

@@ -105,8 +105,10 @@ describe Admin::GeoNodesController, :postgresql do
         allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
       end
 
-      it 'creates the node' do
-        expect { go }.to change { GeoNode.count }.by(1)
+      it 'delegates the create of the Geo node to Geo::NodeCreateService' do
+        expect_any_instance_of(Geo::NodeCreateService).to receive(:execute).once.and_call_original
+
+        go
       end
     end
   end

spec/services/geo/node_create_service_spec.rb

+require 'spec_helper'
+
+describe Geo::NodeCreateService do
+  describe '#execute' do
+    it 'creates a new node with valid params' do
+      params = { url: 'http://example.com', geo_node_key_attributes: attributes_for(:key) }
+      service = described_class.new(params)
+
+      expect { service.execute }.to change(GeoNode, :count).by(1)
+    end
+
+    it 'does not create a node with invalid params' do
+      service = described_class.new({ url: 'http://example.com' })
+
+      expect { service.execute }.not_to change(GeoNode, :count)
+    end
+
+    it 'returns true when creation succeeds' do
+      params = { url: 'http://example.com', geo_node_key_attributes: attributes_for(:key) }
+      service = described_class.new(params)
+
+      expect(service.execute).to eq true
+    end
+
+    it 'returns false when creation fails' do
+      params = { url: 'http://example.com' }
+      service = described_class.new(params)
+
+      expect(service.execute).to eq false
+    end
+
+    it 'parses the namespace_ids when node have namespace restrictions' do
+      groups = create_list(:group, 2)
+      params = { url: 'http://example.com', geo_node_key_attributes: attributes_for(:key), namespace_ids: groups.map(&:id).join(',') }
+      service = described_class.new(params)
+
+      service.execute
+
+      expect(GeoNode.last.namespace_ids).to match_array(groups.map(&:id))
+    end
+  end
+end

spec/services/geo/node_update_service_spec.rb

 require 'spec_helper'
 
 describe Geo::NodeUpdateService do
-  let(:group) { create(:group) }
+  let(:groups) { create_list(:group, 2) }
   let!(:primary) { create(:geo_node, :primary) }
   let(:geo_node) { create(:geo_node) }
-  let(:geo_node_with_restrictions) { create(:geo_node, namespace_ids: [group.id]) }
+  let(:geo_node_with_restrictions) { create(:geo_node, namespace_ids: [groups.first.id]) }
 
   describe '#execute' do
     it 'updates the node without changing the key' do
@@ -19,6 +19,12 @@ describe Geo::NodeUpdateService do
       expect(geo_node.geo_node_key.fingerprint).to eq(original_fingerprint)
     end
 
+    it 'returns true when update succeeds' do
+      service = described_class.new(geo_node, { url: 'http://example.com' })
+
+      expect(service.execute).to eq true
+    end
+
     it 'returns false when update fails' do
       allow(geo_node).to receive(:update).and_return(false)
 
@@ -28,19 +34,19 @@ describe Geo::NodeUpdateService do
     end
 
     it 'logs an event to the Geo event log when namespaces change' do
-      service = described_class.new(geo_node, namespace_ids: [group.id])
+      service = described_class.new(geo_node, namespace_ids: groups.map(&:id).join(','))
 
       expect { service.execute }.to change(Geo::RepositoriesChangedEvent, :count).by(1)
     end
 
     it 'does not log an event to the Geo event log when removing namespace restrictions' do
-      service = described_class.new(geo_node_with_restrictions, namespace_ids: [])
+      service = described_class.new(geo_node_with_restrictions, namespace_ids: '')
 
       expect { service.execute }.not_to change(Geo::RepositoriesChangedEvent, :count)
     end
 
     it 'does not log an event to the Geo event log when node is a primary node' do
-      service = described_class.new(primary, namespace_ids: [group.id])
+      service = described_class.new(primary, namespace_ids: groups.map(&:id).join(','))
 
       expect { service.execute }.not_to change(Geo::RepositoriesChangedEvent, :count)
     end