diff options
| -rw-r--r-- | app/controllers/explore/groups_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/users_controller.rb | 2 | ||||
| -rw-r--r-- | app/finders/groups_finder.rb | 44 | ||||
| -rw-r--r-- | app/finders/joined_groups_finder.rb | 49 | ||||
| -rw-r--r-- | app/helpers/search_helper.rb | 2 | ||||
| -rw-r--r-- | app/models/ability.rb | 2 | ||||
| -rw-r--r-- | app/models/group.rb | 8 | ||||
| -rw-r--r-- | app/views/groups/edit.html.haml | 9 | ||||
| -rw-r--r-- | app/views/groups/show.html.haml | 4 | ||||
| -rw-r--r-- | db/migrate/20151228111122_remove_public_from_namespace.rb | 6 | ||||
| -rw-r--r-- | db/schema.rb | 8 | ||||
| -rw-r--r-- | features/explore/groups.feature | 15 | ||||
| -rw-r--r-- | spec/finders/groups_finder_spec.rb | 48 | ||||
| -rw-r--r-- | spec/finders/joined_groups_finder_spec.rb | 49 | ||||
| -rw-r--r-- | spec/helpers/search_helper_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/group_spec.rb | 27 |
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 |