fix group links 404

7 files changed, 28 insertions, 20 deletions

diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js
index 1cd2302111e..4f04392513f 100644
--- a/app/assets/javascripts/api.js
+++ b/app/assets/javascripts/api.js
@@ -23,12 +23,13 @@
     },
     // Return groups list. Filtered by query
     // Only active groups retrieved
-    groups: function(query, skip_ldap, callback) {
+    groups: function(query, skip_ldap, skip_groups, callback) {
       var url = Api.buildUrl(Api.groupsPath);
       return $.ajax({
         url: url,
         data: {
           search: query,
+          skip_groups: skip_groups,
           per_page: 20
         },
         dataType: "json"
diff --git a/app/assets/javascripts/groups_select.js b/app/assets/javascripts/groups_select.js
index 7c2eebcdd44..5f06186504b 100644
--- a/app/assets/javascripts/groups_select.js
+++ b/app/assets/javascripts/groups_select.js
@@ -5,14 +5,15 @@
     function GroupsSelect() {
       $('.ajax-groups-select').each((function(_this) {
         return function(i, select) {
-          var skip_ldap;
+          var skip_ldap, skip_groups;
           skip_ldap = $(select).hasClass('skip_ldap');
+          skip_groups = $(select).data('skip-groups') || [];
           return $(select).select2({
             placeholder: "Search for a group",
             multiple: $(select).hasClass('multiselect'),
             minimumInputLength: 0,
             query: function(query) {
-              return Api.groups(query.term, skip_ldap, function(groups) {
+              return Api.groups(query.term, skip_ldap, skip_groups, function(groups) {
                 var data;
                 data = {
                   results: groups
diff --git a/app/assets/javascripts/project_select.js b/app/assets/javascripts/project_select.js
index 20b147500cf..4239ed2f889 100644
--- a/app/assets/javascripts/project_select.js
+++ b/app/assets/javascripts/project_select.js
@@ -23,7 +23,7 @@
                   data = groups.concat(projects);
                   return finalCallback(data);
                 };
-                return Api.groups(term, false, groupsCallback);
+                return Api.groups(term, false, false, groupsCallback);
               };
             } else {
               projectsCallback = finalCallback;
@@ -72,7 +72,7 @@
                     data = groups.concat(projects);
                     return finalCallback(data);
                   };
-                  return Api.groups(query.term, false, groupsCallback);
+                  return Api.groups(query.term, false, false, groupsCallback);
                 };
               } else {
                 projectsCallback = finalCallback;
diff --git a/app/assets/javascripts/search.js b/app/assets/javascripts/search.js
index d34346f862b..8074a94f33e 100644
--- a/app/assets/javascripts/search.js
+++ b/app/assets/javascripts/search.js
@@ -10,7 +10,7 @@
         filterable: true,
         fieldName: 'group_id',
         data: function(term, callback) {
-          return Api.groups(term, null, function(data) {
+          return Api.groups(term, false, false, function(data) {
             data.unshift({
               name: 'Any'
             });
diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb
index d0c4550733c..7a7475a7345 100644
--- a/app/controllers/projects/group_links_controller.rb
+++ b/app/controllers/projects/group_links_controller.rb
@@ -4,17 +4,25 @@ class Projects::GroupLinksController < Projects::ApplicationController
 
   def index
     @group_links = project.project_group_links.all
+
+    @skip_groups = @group_links.pluck(:group_id)
+    @skip_groups << project.group.try(:id)
   end
 
   def create
-    group = Group.find(params[:link_group_id])
-    return render_404 unless can?(current_user, :read_group, group)
-
-    project.project_group_links.create(
-      group: group,
-      group_access: params[:link_group_access],
-      expires_at: params[:expires_at]
-    )
+    group = Group.find(params[:link_group_id]) if params[:link_group_id].present?
+
+    if group
+      return render_404 unless can?(current_user, :read_group, group)
+
+      project.project_group_links.create(
+        group: group,
+        group_access: params[:link_group_access],
+        expires_at: params[:expires_at]
+      )
+    else
+      flash[:alert] = 'Please select a group.'
+    end
 
     redirect_to namespace_project_group_links_path(project.namespace, project)
   end
diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb
index 5f27e33c6ad..8706876ae4a 100644
--- a/app/helpers/selects_helper.rb
+++ b/app/helpers/selects_helper.rb
@@ -49,12 +49,10 @@ module SelectsHelper
   end
 
   def select2_tag(id, opts = {})
-    css_class = ''
-    css_class << 'multiselect ' if opts[:multiple]
-    css_class << (opts[:class] || '')
+    opts[:class] << ' multiselect' if opts[:multiple]
     value = opts[:selected] || ''
 
-    hidden_field_tag(id, value, class: css_class)
+    hidden_field_tag(id, value, opts)
   end
 
   private
diff --git a/app/views/projects/group_links/index.html.haml b/app/views/projects/group_links/index.html.haml
index ca700cb3a3b..4c5dd9b88bf 100644
--- a/app/views/projects/group_links/index.html.haml
+++ b/app/views/projects/group_links/index.html.haml
@@ -8,10 +8,10 @@
   .col-lg-9
     %h5.prepend-top-0
       Set a group to share
-    = form_tag namespace_project_group_links_path(@project.namespace, @project), method: :post do
+    = form_tag namespace_project_group_links_path(@project.namespace, @project), class: 'js-requires-input', method: :post do
       .form-group
         = label_tag :link_group_id, "Group", class: "label-light"
-        = groups_select_tag(:link_group_id, skip_group: @project.group.try(:path))
+        = groups_select_tag(:link_group_id, data: { skip_groups: @skip_groups }, required: true)
       .form-group
         = label_tag :link_group_access, "Max access level", class: "label-light"
         .select-wrapper