Merge branch 'feature/public_groups' into 'master'

Public Groups

This is the initial work (meaning no tests) for making groups public if they have a public project (or internal for logged in users). This allows issues and merge requests to be viewed, but _not_ group membership. As part of this I have also added back the link in the public project title section (it was removed as it didn't make sense before).

This addesses the following suggestions/issues:

http://feedback.gitlab.com/forums/176466-general/suggestions/5314461-groups-containing-one-or-more-public-projects-shou
Issue #32
https://github.com/gitlabhq/gitlabhq/issues/5203
as well as a few closed issues.

This also changes the public user page to only show groups that are accessible to the user in some manner.

22 files changed, 154 insertions, 88 deletions

diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb
index b95233c0e91..8f204d09af1 100644
--- a/app/controllers/dashboard_controller.rb
+++ b/app/controllers/dashboard_controller.rb
@@ -77,5 +77,6 @@ class DashboardController < ApplicationController
   def default_filter
     params[:scope] = 'assigned-to-me' if params[:scope].blank?
     params[:state] = 'opened' if params[:state].blank?
+    params[:authorized_only] = true
   end
 end
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index b927dd2f20a..f6f7e3b3ecd 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -1,4 +1,5 @@
 class GroupsController < ApplicationController
+  skip_before_filter :authenticate_user!, only: [:show, :issues, :members, :merge_requests]
   respond_to :html
   before_filter :group, except: [:new, :create]
 
@@ -36,7 +37,7 @@ class GroupsController < ApplicationController
     @events = Event.in_projects(project_ids)
     @events = event_filter.apply_filter(@events)
     @events = @events.limit(20).offset(params[:offset] || 0)
-    @last_push = current_user.recent_push
+    @last_push = current_user.recent_push if current_user
 
     respond_to do |format|
       format.html
@@ -98,17 +99,21 @@ class GroupsController < ApplicationController
   end
 
   def projects
-    @projects ||= current_user.authorized_projects.where(namespace_id: group.id).sorted_by_activity
+    @projects ||= group.projects_accessible_to(current_user).sorted_by_activity
   end
 
   def project_ids
-    projects.map(&:id)
+    projects.pluck(:id)
   end
 
   # Dont allow unauthorized access to group
   def authorize_read_group!
     unless @group and (projects.present? or can?(current_user, :read_group, @group))
-      return render_404
+      if current_user.nil?
+        return authenticate_user!
+      else
+        return render_404
+      end
     end
   end
 
@@ -131,13 +136,21 @@ class GroupsController < ApplicationController
   def determine_layout
     if [:new, :create].include?(action_name.to_sym)
       'navless'
-    else
+    elsif current_user
       'group'
+    else
+      'public_group'
     end
   end
 
   def default_filter
-    params[:scope] = 'assigned-to-me' if params[:scope].blank?
+    if params[:scope].blank?
+      if current_user
+        params[:scope] = 'assigned-to-me'
+      else
+        params[:scope] = 'all'
+      end
+    end
     params[:state] = 'opened' if params[:state].blank?
     params[:group_id] = @group.id
   end
diff --git a/app/controllers/public/projects_controller.rb b/app/controllers/public/projects_controller.rb
index d7297161c22..d6238f79547 100644
--- a/app/controllers/public/projects_controller.rb
+++ b/app/controllers/public/projects_controller.rb
@@ -6,7 +6,7 @@ class Public::ProjectsController < ApplicationController
   layout 'public'
 
   def index
-    @projects = Project.public_or_internal_only(current_user)
+    @projects = Project.publicish(current_user)
     @projects = @projects.search(params[:search]) if params[:search].present?
     @projects = @projects.sort(@sort = params[:sort])
     @projects = @projects.includes(:namespace).page(params[:page]).per(20)
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index e86601a439e..9461174b950 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -1,15 +1,15 @@
 class UsersController < ApplicationController
-
   skip_before_filter :authenticate_user!, only: [:show]
   layout :determine_layout
 
   def show
     @user = User.find_by_username!(params[:username])
-    @projects = @user.authorized_projects.includes(:namespace).select {|project| can?(current_user, :read_project, project)}
+    @projects = @user.authorized_projects.accessible_to(current_user)
     if !current_user && @projects.empty?
       return authenticate_user!
     end
-    @events = @user.recent_events.where(project_id: @projects.map(&:id)).limit(20)
+    @groups = @user.groups.accessible_to(current_user)
+    @events = @user.recent_events.where(project_id: @projects.pluck(:id)).limit(20)
     @title = @user.name
   end
 
diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb
index 5865396b698..cfc9a572cac 100644
--- a/app/helpers/groups_helper.rb
+++ b/app/helpers/groups_helper.rb
@@ -6,6 +6,14 @@ module GroupsHelper
   def leave_group_message(group)
     "Are you sure you want to leave \"#{group}\" group?"
   end
+  
+  def should_user_see_group_roles?(user, group)
+    if user
+      user.is_admin? || group.members.exists?(user_id: user.id)
+    else
+      false
+    end
+  end
 
   def group_head_title
     title = @group.name
diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb
index 470a495f036..d7f3da7e537 100644
--- a/app/helpers/search_helper.rb
+++ b/app/helpers/search_helper.rb
@@ -4,8 +4,7 @@ module SearchHelper
 
     resources_results = [
       groups_autocomplete(term),
-      projects_autocomplete(term),
-      public_projects_autocomplete(term),
+      projects_autocomplete(term)
     ].flatten
 
     generic_results = project_autocomplete + default_autocomplete + help_autocomplete
@@ -82,17 +81,7 @@ module SearchHelper
 
   # Autocomplete results for the current user's projects
   def projects_autocomplete(term, limit = 5)
-    current_user.authorized_projects.search_by_title(term).non_archived.limit(limit).map do |p|
-      {
-        label: "project: #{search_result_sanitize(p.name_with_namespace)}",
-        url: project_path(p)
-      }
-    end
-  end
-
-  # Autocomplete results for the current user's projects
-  def public_projects_autocomplete(term, limit = 5)
-    Project.public_or_internal_only(current_user).search_by_title(term).non_archived.limit(limit).map do |p|
+    Project.accessible_to(current_user).search_by_title(term).non_archived.limit(limit).map do |p|
       {
         label: "project: #{search_result_sanitize(p.name_with_namespace)}",
         url: project_path(p)
diff --git a/app/models/ability.rb b/app/models/ability.rb
index ba0ce527f64..89f8f320da9 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -43,7 +43,19 @@ class Ability
           :download_code
         ]
       else
-        []
+        group = if subject.kind_of?(Group)
+                  subject
+                elsif subject.respond_to?(:group)
+                  subject.group
+                else
+                  nil
+                end
+        
+        if group && group.has_projects_accessible_to?(nil)
+          [:read_group]
+        else
+          []
+        end
       end
     end
 
@@ -172,7 +184,7 @@ class Ability
     def group_abilities user, group
       rules = []
 
-      if group.users.include?(user) || user.admin?
+      if user.admin? || group.users.include?(user) || group.has_projects_accessible_to?(user)
         rules << :read_group
       end
 
diff --git a/app/models/group.rb b/app/models/group.rb
index 8de0c78c158..0d4d5f4e836 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -25,6 +25,12 @@ class Group < Namespace
   validates :avatar, file_size: { maximum: 100.kilobytes.to_i }
 
   mount_uploader :avatar, AttachmentUploader
+  
+  def self.accessible_to(user)
+    accessible_ids = Project.accessible_to(user).pluck(:namespace_id)
+    accessible_ids += user.groups.pluck(:id) if user
+    where(id: accessible_ids)
+  end
 
   def human_name
     name
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index 0bc5e1862eb..468c93bd426 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -47,6 +47,14 @@ class Namespace < ActiveRecord::Base
   def self.global_id
     'GLN'
   end
+  
+  def projects_accessible_to(user)
+    projects.accessible_to(user)
+  end
+  
+  def has_projects_accessible_to?(user)
+    projects_accessible_to(user).present?
+  end
 
   def to_param
     path
diff --git a/app/models/project.rb b/app/models/project.rb
index 2c926ff8a9a..60f0e40d7c7 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -115,8 +115,6 @@ class Project < ActiveRecord::Base
   scope :sorted_by_activity, -> { reorder("projects.last_activity_at DESC") }
   scope :personal, ->(user) { where(namespace_id: user.namespace_id) }
   scope :joined, ->(user) { where("namespace_id != ?", user.namespace_id) }
-  scope :public_only, -> { where(visibility_level: PUBLIC) }
-  scope :public_or_internal_only, ->(user) { where("visibility_level IN (:levels)", levels: user ? [ INTERNAL, PUBLIC ] : [ PUBLIC ]) }
 
   scope :non_archived, -> { where(archived: false) }
 
@@ -126,6 +124,18 @@ class Project < ActiveRecord::Base
     def abandoned
       where('projects.last_activity_at < ?', 6.months.ago)
     end
+    
+    def publicish(user)
+      visibility_levels = [Project::PUBLIC]
+      visibility_levels += [Project::INTERNAL] if user
+      where(visibility_level: visibility_levels)
+    end
+    
+    def accessible_to(user)
+      accessible_ids = publicish(user).pluck(:id)
+      accessible_ids += user.authorized_projects.pluck(:id) if user
+      where(id: accessible_ids)
+    end
 
     def with_push
       includes(:events).where('events.action = ?', Event::PUSHED)
diff --git a/app/services/filtering_service.rb b/app/services/filtering_service.rb
index 52537f7ba4f..cbbb04ccba9 100644
--- a/app/services/filtering_service.rb
+++ b/app/services/filtering_service.rb
@@ -41,16 +41,16 @@ class FilteringService
   def init_collection
     table_name = klass.table_name
 
-    return klass.of_projects(Project.public_only) unless current_user
-
     if project
-      if current_user.can?(:read_project, project)
+      if project.public? || (current_user && current_user.can?(:read_project, project))
         project.send(table_name)
       else
         []
       end
-    else
+    elsif current_user && params[:authorized_only].presence
       klass.of_projects(current_user.authorized_projects)
+    else
+      klass.of_projects(Project.accessible_to(current_user))
     end
   end
 
diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index c1130401578..09c7cb25dd5 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -11,12 +11,8 @@ module Search
       query = Shellwords.shellescape(query) if query.present?
       return result unless query.present?
 
-      authorized_projects_ids = []
-      authorized_projects_ids += current_user.authorized_projects.pluck(:id) if current_user
-      authorized_projects_ids += Project.public_or_internal_only(current_user).pluck(:id)
-
       group = Group.find_by(id: params[:group_id]) if params[:group_id].present?
-      projects = Project.where(id: authorized_projects_ids)
+      projects = Project.accessible_to(current_user)
       projects = projects.where(namespace_id: group.id) if group
       projects = projects.search(query)
       project_ids = projects.pluck(:id)
diff --git a/app/views/groups/issues.html.haml b/app/views/groups/issues.html.haml
index e24df310910..4b11d91dc98 100644
--- a/app/views/groups/issues.html.haml
+++ b/app/views/groups/issues.html.haml
@@ -5,7 +5,9 @@
 %p.light
   Only issues from
   %strong #{@group.name}
-  group are listed here. To see all issues you should visit #{link_to 'dashboard', issues_dashboard_path} page.
+  group are listed here.
+  - if current_user
+    To see all issues you should visit #{link_to 'dashboard', issues_dashboard_path} page.
 %hr
 
 .row
diff --git a/app/views/groups/members.html.haml b/app/views/groups/members.html.haml
index 38069feb37b..0c972622f88 100644
--- a/app/views/groups/members.html.haml
+++ b/app/views/groups/members.html.haml
@@ -1,9 +1,11 @@
+- show_roles = should_user_see_group_roles?(current_user, @group)
 %h3.page-title
   Group members
-%p.light
-  Members of group have access to all group projects.
-  Read more about permissions
-  %strong= link_to "here", help_permissions_path, class: "vlink"
+- if show_roles
+  %p.light
+    Members of group have access to all group projects.
+    Read more about permissions
+    %strong= link_to "here", help_permissions_path, class: "vlink"
 
 %hr
 
@@ -13,7 +15,7 @@
       = search_field_tag :search, params[:search], { placeholder: 'Find member by name', class: 'form-control search-text-input input-mn-300' }
     = submit_tag 'Search', class: 'btn'
 
-  - if current_user.can? :manage_group, @group
+  - if current_user && current_user.can?(:manage_group, @group)
     .pull-right
       = link_to '#', class: 'btn btn-new js-toggle-visibility-link' do
         Add members
@@ -30,7 +32,7 @@
       (#{@members.total_count})
   %ul.well-list
     - @members.each do |member|
-      = render 'users_groups/users_group', member: member, show_controls: true
+      = render 'users_groups/users_group', member: member, show_roles: show_roles, show_controls: true
 = paginate @members, theme: 'gitlab'
 
 :coffeescript
diff --git a/app/views/groups/merge_requests.html.haml b/app/views/groups/merge_requests.html.haml
index eaf85bbdbc8..209130ec444 100644
--- a/app/views/groups/merge_requests.html.haml
+++ b/app/views/groups/merge_requests.html.haml
@@ -5,7 +5,9 @@
 %p.light
   Only merge requests from
   %strong #{@group.name}
-  group are listed here. To see all merge requests you should visit #{link_to 'dashboard', merge_requests_dashboard_path} page.
+  group are listed here.
+  - if current_user
+    To see all merge requests you should visit #{link_to 'dashboard', merge_requests_dashboard_path} page.
 %hr
 .row
   .col-md-3
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index a2d62d64f35..0343670c203 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -1,9 +1,10 @@
 .dashboard
   .activities.col-md-8.hidden-sm
-    = render "events/event_last_push", event: @last_push
-    = link_to dashboard_path, class: 'btn btn-tiny' do
-      &larr; To dashboard
-    &nbsp;
+    - if current_user
+      = render "events/event_last_push", event: @last_push
+      = link_to dashboard_path, class: 'btn btn-tiny' do
+        &larr; To dashboard
+      &nbsp;
     %span.cgray You will only see events from projects in this group
     %hr
     = render 'shared/event_filter'
@@ -21,11 +22,12 @@
         - if @group.description.present?
           %p= @group.description
     = render "projects", projects: @projects
-    .prepend-top-20
-      = link_to group_path(@group, { format: :atom, private_token: current_user.private_token }), title: "Feed" do
-        %strong
-          %i.icon-rss
-          News Feed
+    - if current_user
+      .prepend-top-20
+        = link_to group_path(@group, { format: :atom, private_token: current_user.private_token }), title: "Feed" do
+          %strong
+            %i.icon-rss
+            News Feed
 
     %hr
     = render 'shared/promo'
diff --git a/app/views/layouts/nav/_group.html.haml b/app/views/layouts/nav/_group.html.haml
index a8bb3b91559..36b102dc25a 100644
--- a/app/views/layouts/nav/_group.html.haml
+++ b/app/views/layouts/nav/_group.html.haml
@@ -5,11 +5,13 @@
   = nav_link(path: 'groups#issues') do
     = link_to issues_group_path(@group) do
       Issues
-      %span.count= current_user.assigned_issues.opened.of_group(@group).count
+      - if current_user
+        %span.count= current_user.assigned_issues.opened.of_group(@group).count
   = nav_link(path: 'groups#merge_requests') do
     = link_to merge_requests_group_path(@group) do
       Merge Requests
-      %span.count= current_user.cared_merge_requests.opened.of_group(@group).count
+      - if current_user
+        %span.count= current_user.cared_merge_requests.opened.of_group(@group).count
   = nav_link(path: 'groups#members') do
     = link_to "Members", members_group_path(@group)
 
diff --git a/app/views/layouts/public_group.html.haml b/app/views/layouts/public_group.html.haml
new file mode 100644
index 00000000000..a289b784725
--- /dev/null
+++ b/app/views/layouts/public_group.html.haml
@@ -0,0 +1,10 @@
+!!! 5
+%html{ lang: "en"}
+  = render "layouts/head", title: group_head_title
+  %body{class: "#{app_theme} application", :'data-page' => body_data_page}
+    = render "layouts/broadcast"
+    = render "layouts/public_head_panel", title: "group: #{@group.name}"
+    %nav.main-nav.navbar-collapse.collapse
+      .container= render 'layouts/nav/group'
+    .container
+      .content= yield
diff --git a/app/views/layouts/public_projects.html.haml b/app/views/layouts/public_projects.html.haml
index 5fcf9f99e75..cf4ca9c7a84 100644
--- a/app/views/layouts/public_projects.html.haml
+++ b/app/views/layouts/public_projects.html.haml
@@ -3,7 +3,7 @@
   = render "layouts/head", title: @project.name_with_namespace
   %body{class: "#{app_theme} application", :'data-page' => body_data_page}
     = render "layouts/broadcast"
-    = render "layouts/public_head_panel", title: @project.name_with_namespace
+    = render "layouts/public_head_panel", title: project_title(@project)
     %nav.main-nav
       .container= render 'layouts/nav/project'
     .container
diff --git a/app/views/shared/_filter.html.haml b/app/views/shared/_filter.html.haml
index 6063b4a0732..0becf531cc3 100644
--- a/app/views/shared/_filter.html.haml
+++ b/app/views/shared/_filter.html.haml
@@ -1,16 +1,17 @@
 .side-filters.hidden-xs.hidden-sm
   = form_tag filter_path(entity), method: 'get' do
-    %fieldset.scope-filter
-      %ul.nav.nav-pills.nav-stacked
-        %li{class: ("active" if params[:scope] == 'assigned-to-me')}
-          = link_to filter_path(entity, scope: 'assigned-to-me') do
-            Assigned to me
-        %li{class: ("active" if params[:scope] == 'authored')}
-          = link_to filter_path(entity, scope: 'authored') do
-            Created by me
-        %li{class: ("active" if params[:scope] == 'all')}
-          = link_to filter_path(entity, scope: 'all') do
-            Everyone's
+    - if current_user
+      %fieldset.scope-filter
+        %ul.nav.nav-pills.nav-stacked
+          %li{class: ("active" if params[:scope] == 'assigned-to-me')}
+            = link_to filter_path(entity, scope: 'assigned-to-me') do
+              Assigned to me
+          %li{class: ("active" if params[:scope] == 'authored')}
+            = link_to filter_path(entity, scope: 'authored') do
+              Created by me
+          %li{class: ("active" if params[:scope] == 'all')}
+            = link_to filter_path(entity, scope: 'all') do
+              Everyone's
 
     %fieldset.status-filter
       %legend State
diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml
index 98210af1e3d..edcaf3acf98 100644
--- a/app/views/users/show.html.haml
+++ b/app/views/users/show.html.haml
@@ -14,10 +14,10 @@
       %small member since #{@user.created_at.stamp("Nov 12, 2031")}
     .clearfix
     %h4 Groups:
-    = render 'groups', groups: @user.groups 
+    = render 'groups', groups: @groups
     %hr
     %h4 User Activity:
     = render @events
   .col-md-4
     = render 'profile', user: @user
-    = render 'projects', user: @user
+    = render 'projects'
diff --git a/app/views/users_groups/_users_group.html.haml b/app/views/users_groups/_users_group.html.haml
index b66b486fbc5..1784dab35b4 100644
--- a/app/views/users_groups/_users_group.html.haml
+++ b/app/views/users_groups/_users_group.html.haml
@@ -1,5 +1,6 @@
 - user = member.user
 - return unless user
+- show_roles = true if show_roles.nil?
 %li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)}
   = image_tag avatar_icon(user.email, 16), class: "avatar s16"
   %strong= user.name
@@ -7,22 +8,23 @@
   - if user == current_user
     %span.label.label-success It's you
 
-  %span.pull-right
-    %strong= member.human_access
-    - if show_controls
-      - if can?(current_user, :modify, member)
-        = link_to '#', class: "btn-tiny btn js-toggle-button", title: 'Edit access level' do
-          %i.icon-edit
-      - if can?(current_user, :destroy, member)
-        - if current_user == member.user
-          = link_to leave_profile_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
-            %i.icon-minus.icon-white
-        - else
-          = link_to group_users_group_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
-            %i.icon-minus.icon-white
-
-  .edit-member.hide.js-toggle-content
-    = form_for [@group, member], remote: true do |f|
-      .alert.prepend-top-20
-        = f.select :group_access, options_for_select(UsersGroup.group_access_roles, member.group_access)
-        = f.submit 'Save', class: 'btn btn-save btn-small'
+  - if show_roles
+    %span.pull-right
+      %strong= member.human_access
+      - if show_controls
+        - if can?(current_user, :modify, member)
+          = link_to '#', class: "btn-tiny btn js-toggle-button", title: 'Edit access level' do
+            %i.icon-edit
+        - if can?(current_user, :destroy, member)
+          - if current_user == member.user
+            = link_to leave_profile_group_path(@group), data: { confirm: leave_group_message(@group.name)}, method: :delete, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
+              %i.icon-minus.icon-white
+          - else
+            = link_to group_users_group_path(@group, member), data: { confirm: remove_user_from_group_message(@group, user) }, method: :delete, remote: true, class: "btn-tiny btn btn-remove", title: 'Remove user from group' do
+              %i.icon-minus.icon-white
+  
+    .edit-member.hide.js-toggle-content
+      = form_for [@group, member], remote: true do |f|
+        .alert.prepend-top-20
+          = f.select :group_access, options_for_select(UsersGroup.group_access_roles, member.group_access)
+          = f.submit 'Save', class: 'btn btn-save btn-small'