summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorValery Sizov <vsv2711@gmail.com>2015-12-28 13:32:18 +0200
committerValery Sizov <vsv2711@gmail.com>2016-01-04 16:00:29 +0200
commit8b1844912561a7e6dd0cc361ea1514f2a340e275 (patch)
tree9a6c705fb9ac1c1e6c28f5ed712025c0b020fd1c
parent57074d606b20516921ff2297e2f94262b58e3d3e (diff)
downloadgitlab-ce-8b1844912561a7e6dd0cc361ea1514f2a340e275.tar.gz
remove public field from namespace and refactoringnamespace-clean_up
-rw-r--r--app/controllers/explore/groups_controller.rb2
-rw-r--r--app/controllers/users_controller.rb2
-rw-r--r--app/finders/groups_finder.rb44
-rw-r--r--app/finders/joined_groups_finder.rb49
-rw-r--r--app/helpers/search_helper.rb2
-rw-r--r--app/models/ability.rb2
-rw-r--r--app/models/group.rb8
-rw-r--r--app/views/groups/edit.html.haml9
-rw-r--r--app/views/groups/show.html.haml4
-rw-r--r--db/migrate/20151228111122_remove_public_from_namespace.rb6
-rw-r--r--db/schema.rb8
-rw-r--r--features/explore/groups.feature15
-rw-r--r--spec/finders/groups_finder_spec.rb48
-rw-r--r--spec/finders/joined_groups_finder_spec.rb49
-rw-r--r--spec/helpers/search_helper_spec.rb2
-rw-r--r--spec/models/group_spec.rb27
16 files changed, 16 insertions, 261 deletions
diff --git a/app/controllers/explore/groups_controller.rb b/app/controllers/explore/groups_controller.rb
index 9575a87ee41..a9bf4321f73 100644
--- a/app/controllers/explore/groups_controller.rb
+++ b/app/controllers/explore/groups_controller.rb
@@ -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)
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 30cb869eb2a..280228dbcc0 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -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
diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
deleted file mode 100644
index 91cb0f228f0..00000000000
--- a/app/finders/groups_finder.rb
+++ /dev/null
@@ -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
diff --git a/app/finders/joined_groups_finder.rb b/app/finders/joined_groups_finder.rb
deleted file mode 100644
index e7523136fea..00000000000
--- a/app/finders/joined_groups_finder.rb
+++ /dev/null
@@ -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
diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb
index a6ee6880247..d4f78258626 100644
--- a/app/helpers/search_helper.rb
+++ b/app/helpers/search_helper.rb
@@ -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)
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 1b3ee757040..5a1a67db8e1 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -69,7 +69,7 @@ class Ability
subject.group
end
- if group && group.public_profile?
+ if group && group.projects.public_only.any?
[:read_group]
else
[]
diff --git a/app/models/group.rb b/app/models/group.rb
index 1b5b875a19e..b8f2ab6ae5d 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -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")
diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml
index 1dea77c2e96..7e3e2e28bc9 100644
--- a/app/views/groups/edit.html.haml
+++ b/app/views/groups/edit.html.haml
@@ -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"
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index c2c7c581b3e..a607d860d7d 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -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
diff --git a/db/migrate/20151228111122_remove_public_from_namespace.rb b/db/migrate/20151228111122_remove_public_from_namespace.rb
new file mode 100644
index 00000000000..f4c848bbf47
--- /dev/null
+++ b/db/migrate/20151228111122_remove_public_from_namespace.rb
@@ -0,0 +1,6 @@
+# Migration type: online
+class RemovePublicFromNamespace < ActiveRecord::Migration
+ def change
+ remove_column :namespaces, :public, :boolean
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index df7f72d5ad4..7a6d34b8153 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -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|
diff --git a/features/explore/groups.feature b/features/explore/groups.feature
index a42e59c98f2..5fc9b135601 100644
--- a/features/explore/groups.feature
+++ b/features/explore/groups.feature
@@ -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"
-
diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb
deleted file mode 100644
index 4f6a000822e..00000000000
--- a/spec/finders/groups_finder_spec.rb
+++ /dev/null
@@ -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
diff --git a/spec/finders/joined_groups_finder_spec.rb b/spec/finders/joined_groups_finder_spec.rb
deleted file mode 100644
index 2d9068cc720..00000000000
--- a/spec/finders/joined_groups_finder_spec.rb
+++ /dev/null
@@ -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
diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb
index ebe9c29d91c..f0d553f5f1d 100644
--- a/spec/helpers/search_helper_spec.rb
+++ b/spec/helpers/search_helper_spec.rb
@@ -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
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 646f767e6fe..ba5acceadff 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -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