Commit f5e30567 authored by Fatih Acet's avatar Fatih Acet

Merge branch 'fix_group_links_404' into 'master'

fix group links 404

### What does this MR do?
This MR should fix the 404 page, when creating a new group links without selecting any group from the group dropdown list.

### What are the relevant issue numbers?
Closes #20766

See merge request !6155
parents 966bbb9f 35ced4da
...@@ -23,12 +23,13 @@ ...@@ -23,12 +23,13 @@
}, },
// Return groups list. Filtered by query // Return groups list. Filtered by query
// Only active groups retrieved // Only active groups retrieved
groups: function(query, skip_ldap, callback) { groups: function(query, skip_ldap, skip_groups, callback) {
var url = Api.buildUrl(Api.groupsPath); var url = Api.buildUrl(Api.groupsPath);
return $.ajax({ return $.ajax({
url: url, url: url,
data: { data: {
search: query, search: query,
skip_groups: skip_groups,
per_page: 20 per_page: 20
}, },
dataType: "json" dataType: "json"
......
...@@ -5,14 +5,15 @@ ...@@ -5,14 +5,15 @@
function GroupsSelect() { function GroupsSelect() {
$('.ajax-groups-select').each((function(_this) { $('.ajax-groups-select').each((function(_this) {
return function(i, select) { return function(i, select) {
var skip_ldap; var skip_ldap, skip_groups;
skip_ldap = $(select).hasClass('skip_ldap'); skip_ldap = $(select).hasClass('skip_ldap');
skip_groups = $(select).data('skip-groups') || [];
return $(select).select2({ return $(select).select2({
placeholder: "Search for a group", placeholder: "Search for a group",
multiple: $(select).hasClass('multiselect'), multiple: $(select).hasClass('multiselect'),
minimumInputLength: 0, minimumInputLength: 0,
query: function(query) { 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; var data;
data = { data = {
results: groups results: groups
......
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
data = groups.concat(projects); data = groups.concat(projects);
return finalCallback(data); return finalCallback(data);
}; };
return Api.groups(term, false, groupsCallback); return Api.groups(term, false, false, groupsCallback);
}; };
} else { } else {
projectsCallback = finalCallback; projectsCallback = finalCallback;
...@@ -72,7 +72,7 @@ ...@@ -72,7 +72,7 @@
data = groups.concat(projects); data = groups.concat(projects);
return finalCallback(data); return finalCallback(data);
}; };
return Api.groups(query.term, false, groupsCallback); return Api.groups(query.term, false, false, groupsCallback);
}; };
} else { } else {
projectsCallback = finalCallback; projectsCallback = finalCallback;
......
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
filterable: true, filterable: true,
fieldName: 'group_id', fieldName: 'group_id',
data: function(term, callback) { data: function(term, callback) {
return Api.groups(term, null, function(data) { return Api.groups(term, false, false, function(data) {
data.unshift({ data.unshift({
name: 'Any' name: 'Any'
}); });
......
...@@ -4,10 +4,15 @@ class Projects::GroupLinksController < Projects::ApplicationController ...@@ -4,10 +4,15 @@ class Projects::GroupLinksController < Projects::ApplicationController
def index def index
@group_links = project.project_group_links.all @group_links = project.project_group_links.all
@skip_groups = @group_links.pluck(:group_id)
@skip_groups << project.group.try(:id)
end end
def create def create
group = Group.find(params[:link_group_id]) 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) return render_404 unless can?(current_user, :read_group, group)
project.project_group_links.create( project.project_group_links.create(
...@@ -15,6 +20,9 @@ class Projects::GroupLinksController < Projects::ApplicationController ...@@ -15,6 +20,9 @@ class Projects::GroupLinksController < Projects::ApplicationController
group_access: params[:link_group_access], group_access: params[:link_group_access],
expires_at: params[:expires_at] expires_at: params[:expires_at]
) )
else
flash[:alert] = 'Please select a group.'
end
redirect_to namespace_project_group_links_path(project.namespace, project) redirect_to namespace_project_group_links_path(project.namespace, project)
end end
......
...@@ -49,12 +49,10 @@ module SelectsHelper ...@@ -49,12 +49,10 @@ module SelectsHelper
end end
def select2_tag(id, opts = {}) def select2_tag(id, opts = {})
css_class = '' opts[:class] << ' multiselect' if opts[:multiple]
css_class << 'multiselect ' if opts[:multiple]
css_class << (opts[:class] || '')
value = opts[:selected] || '' value = opts[:selected] || ''
hidden_field_tag(id, value, class: css_class) hidden_field_tag(id, value, opts)
end end
private private
......
...@@ -8,10 +8,10 @@ ...@@ -8,10 +8,10 @@
.col-lg-9 .col-lg-9
%h5.prepend-top-0 %h5.prepend-top-0
Set a group to share 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 .form-group
= label_tag :link_group_id, "Group", class: "label-light" = 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 .form-group
= label_tag :link_group_access, "Max access level", class: "label-light" = label_tag :link_group_access, "Max access level", class: "label-light"
.select-wrapper .select-wrapper
......
...@@ -6,6 +6,8 @@ module API ...@@ -6,6 +6,8 @@ module API
resource :groups do resource :groups do
# Get a groups list # Get a groups list
# #
# Parameters:
# skip_groups (optional) - Array of group ids to exclude from list
# Example Request: # Example Request:
# GET /groups # GET /groups
get do get do
...@@ -16,6 +18,7 @@ module API ...@@ -16,6 +18,7 @@ module API
end end
@groups = @groups.search(params[:search]) if params[:search].present? @groups = @groups.search(params[:search]) if params[:search].present?
@groups = @groups.where.not(id: params[:skip_groups]) if params[:skip_groups].present?
@groups = paginate @groups @groups = paginate @groups
present @groups, with: Entities::Group present @groups, with: Entities::Group
end end
......
require 'spec_helper' require 'spec_helper'
describe Projects::GroupLinksController do describe Projects::GroupLinksController do
let(:project) { create(:project, :private) }
let(:group) { create(:group, :private) } let(:group) { create(:group, :private) }
let(:group2) { create(:group, :private) }
let(:project) { create(:project, :private, group: group2) }
let(:user) { create(:user) } let(:user) { create(:user) }
before do before do
...@@ -46,5 +47,39 @@ describe Projects::GroupLinksController do ...@@ -46,5 +47,39 @@ describe Projects::GroupLinksController do
expect(group.shared_projects).not_to include project expect(group.shared_projects).not_to include project
end end
end end
context 'when project group id equal link group id' do
before do
post(:create, namespace_id: project.namespace.to_param,
project_id: project.to_param,
link_group_id: group2.id,
link_group_access: ProjectGroupLink.default_access)
end
it 'does not share project with selected group' do
expect(group2.shared_projects).not_to include project
end
it 'redirects to project group links page' do
expect(response).to redirect_to(
namespace_project_group_links_path(project.namespace, project)
)
end
end
context 'when link group id is not present' do
before do
post(:create, namespace_id: project.namespace.to_param,
project_id: project.to_param,
link_group_access: ProjectGroupLink.default_access)
end
it 'redirects to project group links page' do
expect(response).to redirect_to(
namespace_project_group_links_path(project.namespace, project)
)
expect(flash[:alert]).to eq('Please select a group.')
end
end
end end
end end
...@@ -45,6 +45,16 @@ describe API::API, api: true do ...@@ -45,6 +45,16 @@ describe API::API, api: true do
expect(json_response.length).to eq(2) expect(json_response.length).to eq(2)
end end
end end
context "when using skip_groups in request" do
it "returns all groups excluding skipped groups" do
get api("/groups", admin), skip_groups: [group2.id]
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
end
end
end end
describe "GET /groups/:id" do describe "GET /groups/:id" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment