remove public field from namespace and refactoring

This commit is contained in:
Valery Sizov 2015-12-28 13:32:18 +02:00
parent 57074d606b
commit 8b18449125
16 changed files with 16 additions and 261 deletions

View file

@ -1,6 +1,6 @@
class Explore::GroupsController < Explore::ApplicationController
def index
@groups = GroupsFinder.new.execute(current_user)
@groups = Group.order_id_desc
@groups = @groups.search(params[:search]) if params[:search].present?
@groups = @groups.sort(@sort = params[:sort])
@groups = @groups.page(params[:page]).per(PER_PAGE)

View file

@ -7,7 +7,7 @@ class UsersController < ApplicationController
@projects = PersonalProjectsFinder.new(@user).execute(current_user)
@groups = JoinedGroupsFinder.new(@user).execute(current_user)
@groups = @user.groups.order_id_desc
respond_to do |format|
format.html

View file

@ -1,44 +0,0 @@
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

View file

@ -1,49 +0,0 @@
# 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

View file

@ -70,7 +70,7 @@ module SearchHelper
# Autocomplete results for the current user's groups
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)}",
url: group_path(group)

View file

@ -69,7 +69,7 @@ class Ability
subject.group
end
if group && group.public_profile?
if group && group.projects.public_only.any?
[:read_group]
else
[]

View file

@ -50,10 +50,6 @@ class Group < Namespace
User.reference_pattern
end
def public_and_given_groups(ids)
where('public IS TRUE OR namespaces.id IN (?)', ids)
end
def visible_to_user(user)
where(id: user.authorized_groups.select(:id).reorder(nil))
end
@ -125,10 +121,6 @@ class Group < Namespace
end
end
def public_profile?
self.public || projects.public_only.any?
end
def post_create_hook
Gitlab::AppLogger.info("Group \"#{name}\" was created")

View file

@ -24,15 +24,6 @@
%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"
.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
= f.submit 'Save group', class: "btn btn-save"

View file

@ -47,5 +47,5 @@
= render "projects", projects: @projects
- else
%p
This group does not have public projects
%p.center-top-menu.no-top
No projects to show

View file

@ -0,0 +1,6 @@
# Migration type: online
class RemovePublicFromNamespace < ActiveRecord::Migration
def change
remove_column :namespaces, :public, :boolean
end
end

View file

@ -547,22 +547,20 @@ ActiveRecord::Schema.define(version: 20151229112614) do
add_index "milestones", ["project_id"], name: "index_milestones_on_project_id", using: :btree
create_table "namespaces", force: :cascade do |t|
t.string "name", null: false
t.string "path", null: false
t.string "name", null: false
t.string "path", null: false
t.integer "owner_id"
t.datetime "created_at"
t.datetime "updated_at"
t.string "type"
t.string "description", default: "", null: false
t.string "description", default: "", null: false
t.string "avatar"
t.boolean "public", default: false
end
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", ["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", ["public"], name: "index_namespaces_on_public", using: :btree
add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree
create_table "notes", force: :cascade do |t|

View file

@ -105,15 +105,6 @@ Feature: Explore Groups
When I visit the public groups area
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
Given group "TestGroup" has public project "Community"
When I sign in as a user
@ -125,9 +116,3 @@ Feature: Explore Groups
When I sign in as a user
And I visit the public groups area
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"

View file

@ -1,48 +0,0 @@
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

View file

@ -1,49 +0,0 @@
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

View file

@ -43,7 +43,7 @@ describe SearchHelper do
end
it "includes the public group" do
group = create(:group, public: true)
group = create(:group)
expect(search_autocomplete_opts(group.name).size).to eq(1)
end

View file

@ -38,14 +38,6 @@ describe Group, models: true do
it { is_expected.not_to validate_presence_of :owner }
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
let!(:group) { create(:group) }
let!(:user) { create(:user) }
@ -112,23 +104,4 @@ describe Group, models: true do
expect(group.avatar_type).to eq(["only images allowed"])
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