Commit 01e1b6bd authored by Valery Sizov's avatar Valery Sizov

Merge branch 'namespace-clean_up' into 'master'

Remove public field from namespace

https://gitlab.com/gitlab-org/gitlab-ce/issues/3580

See merge request !2222
parents 57074d60 8b184491
class Explore::GroupsController < Explore::ApplicationController class Explore::GroupsController < Explore::ApplicationController
def index def index
@groups = GroupsFinder.new.execute(current_user) @groups = Group.order_id_desc
@groups = @groups.search(params[:search]) if params[:search].present? @groups = @groups.search(params[:search]) if params[:search].present?
@groups = @groups.sort(@sort = params[:sort]) @groups = @groups.sort(@sort = params[:sort])
@groups = @groups.page(params[:page]).per(PER_PAGE) @groups = @groups.page(params[:page]).per(PER_PAGE)
......
...@@ -7,7 +7,7 @@ class UsersController < ApplicationController ...@@ -7,7 +7,7 @@ class UsersController < ApplicationController
@projects = PersonalProjectsFinder.new(@user).execute(current_user) @projects = PersonalProjectsFinder.new(@user).execute(current_user)
@groups = JoinedGroupsFinder.new(@user).execute(current_user) @groups = @user.groups.order_id_desc
respond_to do |format| respond_to do |format|
format.html format.html
......
class GroupsFinder
# Finds the groups available to the given user.
#
# current_user - The user to find the groups for.
#
# Returns an ActiveRecord::Relation.
def execute(current_user = nil)
if current_user
relation = groups_visible_to_user(current_user)
else
relation = public_groups
end
relation.order_id_desc
end
private
# This method returns the groups "current_user" can see.
def groups_visible_to_user(current_user)
base = groups_for_projects(public_and_internal_projects)
union = Gitlab::SQL::Union.
new([base.select(:id), current_user.authorized_groups.select(:id)])
Group.where("namespaces.id IN (#{union.to_sql})")
end
def public_groups
groups_for_projects(public_projects)
end
def groups_for_projects(projects)
Group.public_and_given_groups(projects.select(:namespace_id))
end
def public_projects
Project.unscoped.public_only
end
def public_and_internal_projects
Project.unscoped.public_and_internal_only
end
end
# Class for finding the groups a user is a member of.
class JoinedGroupsFinder
def initialize(user = nil)
@user = user
end
# Finds the groups of the source user, optionally limited to those visible to
# the current user.
#
# current_user - If given the groups of "@user" will only include the groups
# "current_user" can also see.
#
# Returns an ActiveRecord::Relation.
def execute(current_user = nil)
if current_user
relation = groups_visible_to_user(current_user)
else
relation = public_groups
end
relation.order_id_desc
end
private
# Returns the groups the user in "current_user" can see.
#
# This list includes all public/internal projects as well as the projects of
# "@user" that "current_user" also has access to.
def groups_visible_to_user(current_user)
base = @user.authorized_groups.visible_to_user(current_user)
extra = public_and_internal_groups
union = Gitlab::SQL::Union.new([base.select(:id), extra.select(:id)])
Group.where("namespaces.id IN (#{union.to_sql})")
end
def public_groups
groups_for_projects(@user.authorized_projects.public_only)
end
def public_and_internal_groups
groups_for_projects(@user.authorized_projects.public_and_internal_only)
end
def groups_for_projects(projects)
@user.groups.public_and_given_groups(projects.select(:namespace_id))
end
end
...@@ -70,7 +70,7 @@ module SearchHelper ...@@ -70,7 +70,7 @@ module SearchHelper
# Autocomplete results for the current user's groups # Autocomplete results for the current user's groups
def groups_autocomplete(term, limit = 5) def groups_autocomplete(term, limit = 5)
GroupsFinder.new.execute(current_user).search(term).limit(limit).map do |group| Group.search(term).limit(limit).map do |group|
{ {
label: "group: #{search_result_sanitize(group.name)}", label: "group: #{search_result_sanitize(group.name)}",
url: group_path(group) url: group_path(group)
......
...@@ -69,7 +69,7 @@ class Ability ...@@ -69,7 +69,7 @@ class Ability
subject.group subject.group
end end
if group && group.public_profile? if group && group.projects.public_only.any?
[:read_group] [:read_group]
else else
[] []
......
...@@ -50,10 +50,6 @@ class Group < Namespace ...@@ -50,10 +50,6 @@ class Group < Namespace
User.reference_pattern User.reference_pattern
end end
def public_and_given_groups(ids)
where('public IS TRUE OR namespaces.id IN (?)', ids)
end
def visible_to_user(user) def visible_to_user(user)
where(id: user.authorized_groups.select(:id).reorder(nil)) where(id: user.authorized_groups.select(:id).reorder(nil))
end end
...@@ -125,10 +121,6 @@ class Group < Namespace ...@@ -125,10 +121,6 @@ class Group < Namespace
end end
end end
def public_profile?
self.public || projects.public_only.any?
end
def post_create_hook def post_create_hook
Gitlab::AppLogger.info("Group \"#{name}\" was created") Gitlab::AppLogger.info("Group \"#{name}\" was created")
......
...@@ -24,15 +24,6 @@ ...@@ -24,15 +24,6 @@
%hr %hr
= link_to 'Remove avatar', group_avatar_path(@group.to_param), data: { confirm: "Group avatar will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-sm remove-avatar" = link_to 'Remove avatar', group_avatar_path(@group.to_param), data: { confirm: "Group avatar will be removed. Are you sure?"}, method: :delete, class: "btn btn-remove btn-sm remove-avatar"
.form-group
%hr
= f.label :public, class: 'control-label' do
Public
.col-sm-10
.checkbox
= f.check_box :public
%span.descr Make this group public (even if there are no public projects inside this group)
.form-actions .form-actions
= f.submit 'Save group', class: "btn btn-save" = f.submit 'Save group', class: "btn btn-save"
......
...@@ -47,5 +47,5 @@ ...@@ -47,5 +47,5 @@
= render "projects", projects: @projects = render "projects", projects: @projects
- else - else
%p %p.center-top-menu.no-top
This group does not have public projects No projects to show
# Migration type: online
class RemovePublicFromNamespace < ActiveRecord::Migration
def change
remove_column :namespaces, :public, :boolean
end
end
...@@ -547,22 +547,20 @@ ActiveRecord::Schema.define(version: 20151229112614) do ...@@ -547,22 +547,20 @@ ActiveRecord::Schema.define(version: 20151229112614) do
add_index "milestones", ["project_id"], name: "index_milestones_on_project_id", using: :btree add_index "milestones", ["project_id"], name: "index_milestones_on_project_id", using: :btree
create_table "namespaces", force: :cascade do |t| create_table "namespaces", force: :cascade do |t|
t.string "name", null: false t.string "name", null: false
t.string "path", null: false t.string "path", null: false
t.integer "owner_id" t.integer "owner_id"
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_at" t.datetime "updated_at"
t.string "type" t.string "type"
t.string "description", default: "", null: false t.string "description", default: "", null: false
t.string "avatar" t.string "avatar"
t.boolean "public", default: false
end end
add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree
add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree
add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree
add_index "namespaces", ["path"], name: "index_namespaces_on_path", unique: true, using: :btree add_index "namespaces", ["path"], name: "index_namespaces_on_path", unique: true, using: :btree
add_index "namespaces", ["public"], name: "index_namespaces_on_public", using: :btree
add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree
create_table "notes", force: :cascade do |t| create_table "notes", force: :cascade do |t|
......
...@@ -105,15 +105,6 @@ Feature: Explore Groups ...@@ -105,15 +105,6 @@ Feature: Explore Groups
When I visit the public groups area When I visit the public groups area
Then I should see group "TestGroup" Then I should see group "TestGroup"
Scenario: I should not see group with internal project in public groups area
Given group "TestGroup" has internal project "Internal"
When I visit the public groups area
Then I should not see group "TestGroup"
Scenario: I should not see group with private project in public groups area
When I visit the public groups area
Then I should not see group "TestGroup"
Scenario: I should see group with public project in public groups area as user Scenario: I should see group with public project in public groups area as user
Given group "TestGroup" has public project "Community" Given group "TestGroup" has public project "Community"
When I sign in as a user When I sign in as a user
...@@ -125,9 +116,3 @@ Feature: Explore Groups ...@@ -125,9 +116,3 @@ Feature: Explore Groups
When I sign in as a user When I sign in as a user
And I visit the public groups area And I visit the public groups area
Then I should see group "TestGroup" Then I should see group "TestGroup"
Scenario: I should not see group with private project in public groups area as user
When I sign in as a user
And I visit the public groups area
Then I should not see group "TestGroup"
require 'spec_helper'
describe GroupsFinder do
describe '#execute' do
let(:user) { create(:user) }
let(:group1) { create(:group) }
let(:group2) { create(:group) }
let(:group3) { create(:group) }
let(:group4) { create(:group, public: true) }
let!(:public_project) { create(:project, :public, group: group1) }
let!(:internal_project) { create(:project, :internal, group: group2) }
let!(:private_project) { create(:project, :private, group: group3) }
let(:finder) { described_class.new }
describe 'with a user' do
subject { finder.execute(user) }
describe 'when the user is not a member of any groups' do
it { is_expected.to eq([group4, group2, group1]) }
end
describe 'when the user is a member of a group' do
before do
group3.add_user(user, Gitlab::Access::DEVELOPER)
end
it { is_expected.to eq([group4, group3, group2, group1]) }
end
describe 'when the user is a member of a private project' do
before do
private_project.team.add_user(user, Gitlab::Access::DEVELOPER)
end
it { is_expected.to eq([group4, group3, group2, group1]) }
end
end
describe 'without a user' do
subject { finder.execute }
it { is_expected.to eq([group4, group1]) }
end
end
end
require 'spec_helper'
describe JoinedGroupsFinder do
describe '#execute' do
let(:source_user) { create(:user) }
let(:current_user) { create(:user) }
let(:group1) { create(:group) }
let(:group2) { create(:group) }
let(:group3) { create(:group) }
let(:group4) { create(:group, public: true) }
let!(:public_project) { create(:project, :public, group: group1) }
let!(:internal_project) { create(:project, :internal, group: group2) }
let!(:private_project) { create(:project, :private, group: group3) }
let(:finder) { described_class.new(source_user) }
before do
[group1, group2, group3, group4].each do |group|
group.add_user(source_user, Gitlab::Access::MASTER)
end
end
describe 'with a current user' do
describe 'when the current user has access to the projects of the source user' do
before do
private_project.team.add_user(current_user, Gitlab::Access::DEVELOPER)
end
subject { finder.execute(current_user) }
it { is_expected.to eq([group4, group3, group2, group1]) }
end
describe 'when the current user does not have access to the projects of the source user' do
subject { finder.execute(current_user) }
it { is_expected.to eq([group4, group2, group1]) }
end
end
describe 'without a current user' do
subject { finder.execute }
it { is_expected.to eq([group4, group1]) }
end
end
end
...@@ -43,7 +43,7 @@ describe SearchHelper do ...@@ -43,7 +43,7 @@ describe SearchHelper do
end end
it "includes the public group" do it "includes the public group" do
group = create(:group, public: true) group = create(:group)
expect(search_autocomplete_opts(group.name).size).to eq(1) expect(search_autocomplete_opts(group.name).size).to eq(1)
end end
......
...@@ -38,14 +38,6 @@ describe Group, models: true do ...@@ -38,14 +38,6 @@ describe Group, models: true do
it { is_expected.not_to validate_presence_of :owner } it { is_expected.not_to validate_presence_of :owner }
end end
describe '.public_and_given_groups' do
let!(:public_group) { create(:group, public: true) }
subject { described_class.public_and_given_groups([group.id]) }
it { is_expected.to eq([public_group, group]) }
end
describe '.visible_to_user' do describe '.visible_to_user' do
let!(:group) { create(:group) } let!(:group) { create(:group) }
let!(:user) { create(:user) } let!(:user) { create(:user) }
...@@ -112,23 +104,4 @@ describe Group, models: true do ...@@ -112,23 +104,4 @@ describe Group, models: true do
expect(group.avatar_type).to eq(["only images allowed"]) expect(group.avatar_type).to eq(["only images allowed"])
end end
end end
describe "public_profile?" do
it "returns true for public group" do
group = create(:group, public: true)
expect(group.public_profile?).to be_truthy
end
it "returns true for non-public group with public project" do
group = create(:group)
create(:project, :public, group: group)
expect(group.public_profile?).to be_truthy
end
it "returns false for non-public group with no public projects" do
group = create(:group)
create(:project, group: group)
expect(group.public_profile?).to be_falsy
end
end
end end
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