From 6051c28fc03b4d9928ee2f2855f210845f9c0579 Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Thu, 5 Nov 2015 12:38:00 +0200
Subject: [PATCH]  Allow groups to appear in the search results if the group
 owner allows it

---
 CHANGELOG                                     |  1 +
 app/controllers/groups_controller.rb          |  6 +-
 app/finders/groups_finder.rb                  | 57 ++++++++++---------
 app/helpers/search_helper.rb                  |  2 +-
 app/models/group.rb                           |  2 +-
 app/views/groups/edit.html.haml               |  9 +++
 .../20151103001141_add_public_to_group.rb     |  5 ++
 db/schema.rb                                  |  9 +--
 spec/finders/group_finder_spec.rb             | 15 +++++
 spec/helpers/search_helper_spec.rb            |  5 ++
 spec/models/group_spec.rb                     | 19 +++++++
 11 files changed, 93 insertions(+), 37 deletions(-)
 create mode 100644 db/migrate/20151103001141_add_public_to_group.rb
 create mode 100644 spec/finders/group_finder_spec.rb

diff --git a/CHANGELOG b/CHANGELOG
index 0ec6030b130..3a75f50e9a2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -22,6 +22,7 @@ v 8.2.0 (unreleased)
   - Include commit logs in project search
   - Add "added", "modified" and "removed" properties to commit object in webhook
   - Rename "Back to" links to "Go to" because its not always a case it point to place user come from
+  - Allow groups to appear in the search results if the group owner allows it
 
 v 8.1.3
   - Spread out runner contacted_at updates
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 40fb15a5b36..fb4eb094f27 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -4,12 +4,12 @@ class GroupsController < Groups::ApplicationController
   before_action :group, except: [:new, :create]
 
   # Authorize
-  before_action :authorize_read_group!, except: [:show, :new, :create]
+  before_action :authorize_read_group!, except: [:show, :new, :create, :autocomplete]
   before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects]
   before_action :authorize_create_group!, only: [:new, :create]
 
   # Load group projects
-  before_action :load_projects, except: [:new, :create, :projects, :edit, :update]
+  before_action :load_projects, except: [:new, :create, :projects, :edit, :update, :autocomplete]
   before_action :event_filter, only: :show
 
   layout :determine_layout
@@ -133,7 +133,7 @@ class GroupsController < Groups::ApplicationController
   end
 
   def group_params
-    params.require(:group).permit(:name, :description, :path, :avatar)
+    params.require(:group).permit(:name, :description, :path, :avatar, :public)
   end
 
   def load_events
diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index d3597ef0901..b5f3176461c 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -6,33 +6,34 @@ class GroupsFinder
   private
 
   def all_groups(current_user)
-    if current_user
-      if current_user.authorized_groups.any?
-        # User has access to groups
-        #
-        # Return only:
-        #   groups with public projects
-        #   groups with internal projects
-        #   groups with joined projects
-        #
-        group_ids = Project.public_and_internal_only.pluck(:namespace_id) +
-          current_user.authorized_groups.pluck(:id)
-        Group.where(id: group_ids)
-      else
-        # User has no group membership
-        #
-        # Return only:
-        #   groups with public projects
-        #   groups with internal projects
-        #
-        Group.where(id: Project.public_and_internal_only.pluck(:namespace_id))
-      end
-    else
-      # Not authenticated
-      #
-      # Return only:
-      #   groups with public projects
-      Group.where(id: Project.public_only.pluck(:namespace_id))
-    end
+    group_ids = if current_user
+                  if current_user.authorized_groups.any?
+                    # User has access to groups
+                    #
+                    # Return only:
+                    #   groups with public projects
+                    #   groups with internal projects
+                    #   groups with joined projects
+                    #
+                    Project.public_and_internal_only.pluck(:namespace_id) +
+                      current_user.authorized_groups.pluck(:id)
+                  else
+                    # User has no group membership
+                    #
+                    # Return only:
+                    #   groups with public projects
+                    #   groups with internal projects
+                    #
+                    Project.public_and_internal_only.pluck(:namespace_id)
+                  end
+                else
+                  # Not authenticated
+                  #
+                  # Return only:
+                  #   groups with public projects
+                  Project.public_only.pluck(:namespace_id)
+                end
+
+    Group.where("public IS TRUE OR id IN(?)", group_ids)
   end
 end
diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb
index c31a556ff7b..a6ee6880247 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)
-    current_user.authorized_groups.search(term).limit(limit).map do |group|
+    GroupsFinder.new.execute(current_user).search(term).limit(limit).map do |group|
       {
         label: "group: #{search_result_sanitize(group.name)}",
         url: group_path(group)
diff --git a/app/models/group.rb b/app/models/group.rb
index 465c22d23ac..34904af3b5b 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -120,7 +120,7 @@ class Group < Namespace
   end
 
   def public_profile?
-    projects.public_only.any?
+    self.public || projects.public_only.any?
   end
 
   def post_create_hook
diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml
index ae8fc9f85f0..57308a661c0 100644
--- a/app/views/groups/edit.html.haml
+++ b/app/views/groups/edit.html.haml
@@ -25,6 +25,15 @@
             %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 is no any public project inside this group)
+
       .form-actions
         = f.submit 'Save group', class: "btn btn-save"
 
diff --git a/db/migrate/20151103001141_add_public_to_group.rb b/db/migrate/20151103001141_add_public_to_group.rb
new file mode 100644
index 00000000000..635346300c2
--- /dev/null
+++ b/db/migrate/20151103001141_add_public_to_group.rb
@@ -0,0 +1,5 @@
+class AddPublicToGroup < ActiveRecord::Migration
+  def change
+    add_column :namespaces, :public, :boolean, default: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 73fc83c3d6b..17d445a8baa 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20151026182941) do
+ActiveRecord::Schema.define(version: 20151103001141) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -501,14 +501,15 @@ ActiveRecord::Schema.define(version: 20151026182941) do
   add_index "milestones", ["project_id"], name: "index_milestones_on_project_id", using: :btree
 
   create_table "namespaces", force: true 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
diff --git a/spec/finders/group_finder_spec.rb b/spec/finders/group_finder_spec.rb
new file mode 100644
index 00000000000..78dc027837c
--- /dev/null
+++ b/spec/finders/group_finder_spec.rb
@@ -0,0 +1,15 @@
+require 'spec_helper'
+
+describe GroupsFinder do
+  let(:user) { create :user }
+  let!(:group) { create :group }
+  let!(:public_group) { create :group, public: true }
+  
+  describe :execute do
+    it 'finds public group' do
+      groups = GroupsFinder.new.execute(user)
+      expect(groups.size).to eq(1)
+      expect(groups.first).to eq(public_group)
+    end
+  end
+end
diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb
index b327f4f911a..ebe9c29d91c 100644
--- a/spec/helpers/search_helper_spec.rb
+++ b/spec/helpers/search_helper_spec.rb
@@ -42,6 +42,11 @@ describe SearchHelper do
         expect(search_autocomplete_opts(project.name).size).to eq(1)
       end
 
+      it "includes the public group" do
+        group = create(:group, public: true)
+        expect(search_autocomplete_opts(group.name).size).to eq(1)
+      end
+
       context "with a current project" do
         before { @project = create(:project) }
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 80638fc8db2..0f23e81ace9 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -84,4 +84,23 @@ describe Group 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
-- 
GitLab