Make all group public

CHANGELOG

@@ -29,7 +29,7 @@ v 8.0.0 (unreleased)
   - Fix 500 error when submit project snippet without body
   - Improve search page usability
   - Bring more UI consistency in way how projects, snippets and groups lists are rendered
-  - Make all profiles public
+  - Make all profiles and group public
   - Fixed login failure when extern_uid changes (Joel Koglin)
   - Don't notify users without access to the project when they are (accidentally) mentioned in a note.
   - Retrieving oauth token with LDAP credentials

app/models/ability.rb

@@ -54,7 +54,7 @@ class Ability
                   nil
                 end
 
-        if group && group.public_profile?
+        if group
           [:read_group]
         else
           []

app/models/group.rb

@@ -119,10 +119,6 @@ class Group < Namespace
     end
   end
 
-  def public_profile?
-    projects.public_only.any?
-  end
-
   def post_create_hook
     Gitlab::AppLogger.info("Group \"#{name}\" was created")
 

app/views/groups/show.html.haml

@@ -22,16 +22,16 @@
         - if current_user
           = render "events/event_last_push", event: @last_push
 
-          - if current_user
-            %ul.nav.nav-pills.event_filter.pull-right
-              %li
-                = link_to group_path(@group, { format: :atom, private_token: current_user.private_token }), title: "Feed", class: 'rss-btn' do
-                  %i.fa.fa-rss
+          %ul.nav.nav-pills.event_filter.pull-right
+            %li
+              = link_to group_path(@group, { format: :atom, private_token: current_user.private_token }), title: "Feed", class: 'rss-btn' do
+                %i.fa.fa-rss
 
           = render 'shared/event_filter'
           %hr
 
       .content_list
       = spinner
-    %aside.side.col-md-5
-      = render "projects", projects: @projects
+    - if @projects.any?
+      %aside.side.col-md-5
+        = render "projects", projects: @projects

features/explore/groups.feature

@@ -3,20 +3,6 @@ Feature: Explore Groups
   Background:
     Given group "TestGroup" has private project "Enterprise"
 
-  Scenario: I should not see group with private projects as visitor
-    When I visit group "TestGroup" page
-    Then I should be redirected to sign in page
-
-  Scenario: I should not see group with private projects group as user
-    When I sign in as a user
-    And I visit group "TestGroup" page
-    Then page status code should be 404
-
-  Scenario: I should not see group with private and internal projects as visitor
-    Given group "TestGroup" has internal project "Internal"
-    When I visit group "TestGroup" page
-    Then I should be redirected to sign in page
-
   Scenario: I should see group with private and internal projects as user
     Given group "TestGroup" has internal project "Internal"
     When I sign in as a user

features/groups.feature

@@ -159,3 +159,14 @@ Feature: Groups
     When I visit group "Owned" projects page
     Then I should see group "Owned" projects list
     And I should see "archived" label
+
+  # Public group
+  @javascript
+  Scenario: Signed out user should see group
+    Given "Mary Jane" is owner of group "Owned"
+    And I am a signed out user
+    And Group "Owned" has a public project "Public-project"
+    When I visit group "Owned" page
+    Then I should see group "Owned"
+    Then I should see project "Public-project"
+   

features/steps/groups.rb

@@ -17,6 +17,26 @@ class Spinach::Features::Groups < Spinach::FeatureSteps
     find(:css, 'button.btn-new').click
   end
 
+  step 'I should see group "Owned"' do
+    expect(page).to have_content '@owned'
+  end
+
+  step 'I am a signed out user' do
+    logout
+  end
+
+  step 'Group "Owned" has a public project "Public-project"' do
+    group = Group.find_by(name: "Owned")
+
+    @project = create :empty_project, :public,
+                 group: group,
+                 name: "Public-project"
+  end
+
+  step 'I should see project "Public-project"' do
+    expect(page).to have_content 'Public-project'
+  end
+
   step 'I select "Mike" as "Reporter"' do
     user = User.find_by(name: "Mike")
 

spec/controllers/namespaces_controller_spec.rb

@@ -46,13 +46,11 @@ describe NamespacesController do
 
       context "when the project doesn't have public projects" do
         context "when not signed in" do
-          it "redirects to the sign in page" do
+          it "does not redirect to the sign in page" do
             get :show, id: group.path
-
-            expect(response).to redirect_to(new_user_session_path)
+            expect(response).not_to redirect_to(new_user_session_path)
           end
         end
-
         context "when signed in" do
           before do
             sign_in(user)

spec/controllers/uploads_controller_spec.rb

@@ -156,14 +156,6 @@ describe UploadsController do
       end
 
       context "when the project doesn't have public projects" do
-        context "when not signed in" do
-          it "redirects to the sign in page" do
-            get :show, model: "group", mounted_as: "avatar", id: group.id, filename: "image.png"
-
-            expect(response).to redirect_to(new_user_session_path)
-          end
-        end
-
         context "when signed in" do
           before do
             sign_in(user)

spec/features/security/group_access_spec.rb

@@ -68,7 +68,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
 
     context 'with no projects' do
@@ -77,8 +77,8 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:reporter) }
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
-      it { is_expected.to be_denied_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
     end
   end
 
@@ -118,7 +118,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
 
     context 'with no projects' do
@@ -128,7 +128,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_denied_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
   end
 
@@ -168,7 +168,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
 
     context 'with no projects' do
@@ -178,7 +178,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_denied_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
   end
 
@@ -218,7 +218,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_allowed_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
 
     context 'with no projects' do
@@ -228,7 +228,7 @@ describe 'Group access', feature: true do
       it { is_expected.to be_allowed_for group_member(:guest) }
       it { is_expected.to be_allowed_for :admin }
       it { is_expected.to be_denied_for :user }
-      it { is_expected.to be_denied_for :visitor }
+      it { is_expected.to be_allowed_for :visitor }
     end
   end