diff --git a/app/controllers/explore/groups_controller.rb b/app/controllers/explore/groups_controller.rb index a9bf4321f735a93efde68384d0e03e3e79feb410..9575a87ee4157bdc69bb7a5a7913e1399af91480 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 = Group.order_id_desc + @groups = GroupsFinder.new.execute(current_user) @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/groups_controller.rb b/app/controllers/groups_controller.rb index f05c29e9974f38119fa326cec4d5fe3706f180f2..13de19bc141ca386c3e005430ea3f3966342a40b 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -131,7 +131,7 @@ class GroupsController < Groups::ApplicationController end def group_params - params.require(:group).permit(:name, :description, :path, :avatar, :public) + params.require(:group).permit(:name, :description, :path, :avatar, :public, :visibility_level) end def load_events diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..a3a8cd541de6ee9b710010da30a5b3ae86b82b22 --- /dev/null +++ b/app/finders/groups_finder.rb @@ -0,0 +1,31 @@ +class GroupsFinder + def execute(current_user = nil) + + segments = all_groups(current_user) + + if segments.length > 1 + union = Gitlab::SQL::Union.new(segments.map { |s| s.select(:id) }) + Group.where("namespaces.id IN (#{union.to_sql})").order_id_desc + else + segments.first + end + end + + private + + def all_groups(current_user) + if current_user + [current_user.authorized_groups, public_and_internal_groups] + else + [Group.public_only] + end + end + + def public_groups + Group.unscoped.public_only + end + + def public_and_internal_groups + Group.unscoped.public_and_internal_only + end +end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 1d36969cd62c49a061c65341a0084b9f6f3a12c7..b1f0a765bb90655bc41fa416ee2bf48b34216432 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -19,6 +19,10 @@ module GroupsHelper end end + def can_change_group_visibility_level?(group) + can?(current_user, :change_visibility_level, group) + end + def group_icon(group) if group.is_a?(String) group = Group.find_by(path: group) diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index 71d33b445c22bea272c664e004ef0f01401ea4db..c47342534a8efeab8811e6d18e2b4b973c6822f7 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -19,6 +19,8 @@ module VisibilityLevelHelper case form_model when Project project_visibility_level_description(level) + when Group + group_visibility_level_description(level) when Snippet snippet_visibility_level_description(level, form_model) end @@ -35,6 +37,17 @@ module VisibilityLevelHelper end end + def group_visibility_level_description(level) + case level + when Gitlab::VisibilityLevel::PRIVATE + "The group can be accessed only by members." + when Gitlab::VisibilityLevel::INTERNAL + "The group can be accessed by any logged user." + when Gitlab::VisibilityLevel::PUBLIC + "The group can be accessed without any authentication." + end + end + def snippet_visibility_level_description(level, snippet = nil) case level when Gitlab::VisibilityLevel::PRIVATE diff --git a/app/models/ability.rb b/app/models/ability.rb index fe9e0aab71732052f451ece80c425aaa4ad16736..c84ded61606a1d39ba18910054868d55e3d24a10 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -275,11 +275,12 @@ class Ability rules << :read_group end - # Only group masters and group owners can create new projects in group + # Only group masters and group owners can create new projects and change permission level if group.has_master?(user) || group.has_owner?(user) || user.admin? rules += [ :create_projects, - :admin_milestones + :admin_milestones, + :change_visibility_level ] end diff --git a/app/models/group.rb b/app/models/group.rb index 76042b3e3fd341ef46c1e8bc3d2cf0acd13d0aa1..26914f55541e38d0ff03635483224f646c14936f 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -2,15 +2,16 @@ # # Table name: namespaces # -# id :integer not null, primary key -# name :string(255) not null -# path :string(255) not null -# owner_id :integer -# created_at :datetime -# updated_at :datetime -# type :string(255) -# description :string(255) default(""), not null -# avatar :string(255) +# id :integer not null, primary key +# name :string(255) not null +# path :string(255) not null +# owner_id :integer +# visibility_level :integer default(20), not null +# created_at :key => "value", datetime +# updated_at :datetime +# type :string(255) +# description :string(255) default(""), not null +# avatar :string(255) # require 'carrierwave/orm/activerecord' @@ -18,8 +19,10 @@ require 'file_size_validator' class Group < Namespace include Gitlab::ConfigHelper + include Gitlab::VisibilityLevel include Referable + has_many :group_members, dependent: :destroy, as: :source, class_name: 'GroupMember' alias_method :members, :group_members has_many :users, through: :group_members @@ -32,6 +35,10 @@ class Group < Namespace after_create :post_create_hook after_destroy :post_destroy_hook + scope :public_only, -> { where(visibility_level: Group::PUBLIC) } + scope :public_and_internal_only, -> { where(visibility_level: [Group::PUBLIC, Group::INTERNAL] ) } + + class << self def search(query) where("LOWER(namespaces.name) LIKE :query or LOWER(namespaces.path) LIKE :query", query: "%#{query.downcase}%") diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml index 3430f56a9c98890681fba9d0974beab6ebe7d471..ea223d2209f3e3a8ce1a0921359b0cd14f1ac0bf 100644 --- a/app/views/groups/edit.html.haml +++ b/app/views/groups/edit.html.haml @@ -23,6 +23,8 @@ %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" + = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: can_change_group_visibility_level?(@group), form_model: @group + .form-actions = f.submit 'Save group', class: "btn btn-save" diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml index 4bc31cabea61def06d9e0e50420ba3dce7c23548..1526ca42634415e19c00d5efd977e5a977190c4c 100644 --- a/app/views/groups/new.html.haml +++ b/app/views/groups/new.html.haml @@ -17,6 +17,8 @@ .col-sm-10 = render 'shared/choose_group_avatar_button', f: f + = render 'shared/visibility_level', f: f, visibility_level: @group.visibility_level, can_change_visibility_level: true, form_model: @group + .form-group .col-sm-offset-2.col-sm-10 = render 'shared/group_tips' diff --git a/app/views/shared/_group_tips.html.haml b/app/views/shared/_group_tips.html.haml index e5cf783beb7ae21505440dfc6296c159bd382a74..46e4340511a5b18ef002f195660b6f97d330e731 100644 --- a/app/views/shared/_group_tips.html.haml +++ b/app/views/shared/_group_tips.html.haml @@ -1,6 +1,5 @@ %ul %li A group is a collection of several projects - %li Groups are private by default %li Members of a group may only view projects they have permission to access %li Group project URLs are prefixed with the group namespace %li Existing projects may be moved into a group diff --git a/db/migrate/20160301124843_add_visibility_level_to_groups.rb b/db/migrate/20160301124843_add_visibility_level_to_groups.rb new file mode 100644 index 0000000000000000000000000000000000000000..b0afe795f42e72e84412f7ce2fa4f7d99e2b7b7d --- /dev/null +++ b/db/migrate/20160301124843_add_visibility_level_to_groups.rb @@ -0,0 +1,9 @@ +class AddVisibilityLevelToGroups < ActiveRecord::Migration + def change + #All groups will be private when created + add_column :namespaces, :visibility_level, :integer, null: false, default: 0 + + #Set all existing groups to public + Group.update_all(visibility_level: 20) + end +end diff --git a/db/schema.rb b/db/schema.rb index a74b86d8e2fd75f1b0652dd4eb7e854843668bad..d0e3bb769d2c99d71f05be7d2a0538eef5a9ea8b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,8 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160309140734) do - +ActiveRecord::Schema.define(version: 20160305220806) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -568,14 +567,15 @@ ActiveRecord::Schema.define(version: 20160309140734) do add_index "milestones", ["title"], name: "index_milestones_on_title", 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.integer "visibility_level", default: 0, null: false end add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ed24954af7add73c5f02bc586c5ce63f3280a1a9 --- /dev/null +++ b/spec/finders/groups_finder_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe GroupsFinder do + describe '#execute' do + let(:user) { create(:user) } + let!(:private_group) { create(:group, visibility_level: 0) } + let!(:internal_group) { create(:group, visibility_level: 10) } + let!(:public_group) { create(:group, visibility_level: 20) } + let(:finder) { described_class.new } + + describe 'execute' do + describe 'without a user' do + subject { finder.execute } + + it { is_expected.to eq([public_group]) } + end + + describe 'with a user' do + subject { finder.execute(user) } + + it { is_expected.to eq([public_group, internal_group]) } + end + end + end +end diff --git a/spec/helpers/groups_helper.rb b/spec/helpers/groups_helper.rb index 4ea90a80a926649ba2597bf08ddad0831a8c14e8..01ec9e5a07f2a0f40ec6ee391dc4110a32e3ea4d 100644 --- a/spec/helpers/groups_helper.rb +++ b/spec/helpers/groups_helper.rb @@ -18,4 +18,19 @@ describe GroupsHelper do expect(group_icon(group.path)).to match('group_avatar.png') end end + + describe 'permissions' do + let(:group) { create(:group) } + let!(:user) { create(:user) } + + before do + allow(self).to receive(:current_user).and_return(user) + allow(self).to receive(:can?) { true } + end + + it 'checks user ability to change permissions' do + expect(self).to receive(:can?).with(user, :change_visibility_level, group) + can_change_group_visibility_level?(group) + end + end end diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index cd7596a763d9930c37afc812700b391fdd554017..ef60ef75fbedc375ff39ff3a84b94889600c97c5 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -8,6 +8,7 @@ describe VisibilityLevelHelper do end let(:project) { build(:project) } + let(:group) { build(:group) } let(:personal_snippet) { build(:personal_snippet) } let(:project_snippet) { build(:project_snippet) } @@ -19,6 +20,13 @@ describe VisibilityLevelHelper do end end + context 'used with a Group' do + it 'delegates groups to #group_visibility_level_description' do + expect(visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, group)) + .to match /group/i + end + end + context 'called with a Snippet' do it 'delegates snippets to #snippet_visibility_level_description' do expect(visibility_level_description(Gitlab::VisibilityLevel::INTERNAL, project_snippet)) diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 3c995053eecfd350c20b6b6d985b387923928b07..25aa77dc4e812bd13ec5ea0ba4acca82a8180d91 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -56,6 +56,24 @@ describe Group, models: true do end end + describe 'scopes' do + let!(:private_group) { create(:group, visibility_level: 0) } + let!(:internal_group) { create(:group, visibility_level: 10) } + let!(:public_group) { create(:group, visibility_level: 20) } + + describe 'public_only' do + subject { described_class.public_only } + + it{ is_expected.to eq([public_group]) } + end + + describe 'public_and_internal_only' do + subject { described_class.public_and_internal_only } + + it{ is_expected.to eq([public_group, internal_group]) } + end + end + describe '#to_reference' do it 'returns a String reference to the object' do expect(group.to_reference).to eq "@#{group.name}"