FEATURE: Add new group visibility option for "logged on users" (#7814)

FEATURE: Add new group visibility option for “logged on users” (#7814)

Groups can now be marked as visible to “logged on users”. All automatic groups (except everyone) are now visible to “logged on users”, previously they were marked as public but suppressed in the group page for non-staff.

diff --git a/app/assets/javascripts/discourse/components/groups-form-interaction-fields.js.es6 b/app/assets/javascripts/discourse/components/groups-form-interaction-fields.js.es6
index 6ccc23d..3b792fd 100644
--- a/app/assets/javascripts/discourse/components/groups-form-interaction-fields.js.es6
+++ b/app/assets/javascripts/discourse/components/groups-form-interaction-fields.js.es6
@@ -13,19 +13,25 @@ export default Ember.Component.extend({
       },
       {
         name: I18n.t(
-          "admin.groups.manage.interaction.visibility_levels.members"
+          "admin.groups.manage.interaction.visibility_levels.logged_on_users"
         ),
         value: 1
       },
       {
-        name: I18n.t("admin.groups.manage.interaction.visibility_levels.staff"),
+        name: I18n.t(
+          "admin.groups.manage.interaction.visibility_levels.members"
+        ),
         value: 2
       },
       {
+        name: I18n.t("admin.groups.manage.interaction.visibility_levels.staff"),
+        value: 3
+      },
+      {
         name: I18n.t(
           "admin.groups.manage.interaction.visibility_levels.owners"
         ),
-        value: 3
+        value: 4
       }
     ];
 
diff --git a/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs b/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs
index 5ed87a5..b2dc61b 100644
--- a/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs
+++ b/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs
@@ -9,6 +9,10 @@
         content=visibilityLevelOptions
         castInteger=true
         class="groups-form-visibility-level"}}
+
+    <div class="control-instructions">
+      {{i18n 'admin.groups.manage.interaction.visibility_levels.description'}}
+    </div>
   </div>
 {{/if}}
 
diff --git a/app/models/group.rb b/app/models/group.rb
index 4d50dbe..19cad44 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -89,9 +89,10 @@ class Group < ActiveRecord::Base
   def self.visibility_levels
     @visibility_levels = Enum.new(
       public: 0,
-      members: 1,
-      staff: 2,
-      owners: 3
+      logged_on_users: 1,
+      members: 2,
+      staff: 3,
+      owners: 4
     )
   end
 
@@ -113,6 +114,11 @@ class Group < ActiveRecord::Base
           UNION ALL
 
           SELECT g.id FROM groups g
+          WHERE g.visibility_level = :logged_on_users AND :user_id IS NOT NULL
+
+          UNION ALL
+
+          SELECT g.id FROM groups g
           JOIN group_users gu ON gu.group_id = g.id AND
                                  gu.user_id = :user_id
           WHERE g.visibility_level = :members
@@ -324,6 +330,8 @@ class Group < ActiveRecord::Base
       group.update!(messageable_level: ALIAS_LEVELS[:everyone])
     end
 
+    group.update!(visibility_level: Group.visibility_levels[:logged_on_users]) if group.visibility_level == Group.visibility_levels[:public]
+
     # Remove people from groups they don't belong in.
     remove_subquery =
       case name
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 2157f17..b7d2452 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3185,9 +3185,11 @@ en:
             visibility_levels:
               title: "Who can see this group?"
               public: "Everyone"
-              members: "Group owners, members and admins"
+              logged_on_users: "Logged on users"
+              members: "Group owners and members"
               staff: "Group owners and staff"
-              owners: "Group owners and admins"
+              owners: "Group owners"
+              description: "Admins can see all groups."
           membership:
             automatic: Automatic
             trust_level: Trust Level
diff --git a/db/fixtures/002_groups.rb b/db/fixtures/002_groups.rb
index f3ed480..d9cc999 100644
--- a/db/fixtures/002_groups.rb
+++ b/db/fixtures/002_groups.rb
@@ -5,4 +5,4 @@ if g = Group.find_by(name: 'trust_level_5', id: 15)
   g.destroy!
 end
 
-Group.where(name: 'everyone').update_all(visibility_level: Group.visibility_levels[:owners])
+Group.where(name: 'everyone').update_all(visibility_level: Group.visibility_levels[:staff])
diff --git a/db/migrate/20190705173948_renumber_group_visibility_levels.rb b/db/migrate/20190705173948_renumber_group_visibility_levels.rb
new file mode 100644
index 0000000..fce98d6
--- /dev/null
+++ b/db/migrate/20190705173948_renumber_group_visibility_levels.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class RenumberGroupVisibilityLevels < ActiveRecord::Migration[5.2]
+  def up
+    execute "UPDATE groups SET visibility_level = 4 WHERE visibility_level = 3"
+    execute "UPDATE groups SET visibility_level = 3 WHERE visibility_level = 2"
+    execute "UPDATE groups SET visibility_level = 2 WHERE visibility_level = 1"
+  end
+
+  def down
+    execute "UPDATE groups SET visibility_level = 0 WHERE visibility_level = 1"
+    execute "UPDATE groups SET visibility_level = 1 WHERE visibility_level = 2"
+    execute "UPDATE groups SET visibility_level = 2 WHERE visibility_level = 3"
+    execute "UPDATE groups SET visibility_level = 3 WHERE visibility_level = 4"
+  end
+end
diff --git a/lib/guardian.rb b/lib/guardian.rb
index 27bdff3..e2d4c24 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -194,6 +194,7 @@ class Guardian
     return true if group.visibility_level == Group.visibility_levels[:public]
     return true if is_admin?
     return true if is_staff? && group.visibility_level == Group.visibility_levels[:staff]
+    return true if authenticated? && group.visibility_level == Group.visibility_levels[:logged_on_users]
     return false if user.blank?
 
     membership = GroupUser.find_by(group_id: group.id, user_id: user.id)
@@ -213,6 +214,7 @@ class Guardian
     return true if groups.all? { |g| g.visibility_level == Group.visibility_levels[:public] }
     return true if is_admin?
     return true if is_staff? && groups.all? { |g| g.visibility_level == Group.visibility_levels[:staff] }
+    return true if authenticated? && groups.all? { |g| g.visibility_level == Group.visibility_levels[:logged_on_users] }
     return false if user.blank?
 
     memberships = GroupUser.where(group: groups, user_id: user.id).pluck(:owner)
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index 98eeb94..adab91e 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -3027,7 +3027,7 @@ describe Guardian do
   end
 
   describe(:can_see_group) do
-    it 'Correctly handles owner visibile groups' do
+    it 'Correctly handles owner visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:owners])
 
       member = Fabricate(:user)
@@ -3039,13 +3039,14 @@ describe Guardian do
       group.reload
 
       expect(Guardian.new(admin).can_see_group?(group)).to eq(true)
+      expect(Guardian.new(another_user).can_see_group?(group)).to eq(false)
       expect(Guardian.new(moderator).can_see_group?(group)).to eq(false)
       expect(Guardian.new(member).can_see_group?(group)).to eq(false)
       expect(Guardian.new.can_see_group?(group)).to eq(false)
       expect(Guardian.new(owner).can_see_group?(group)).to eq(true)
     end
 
-    it 'Correctly handles staff visibile groups' do
+    it 'Correctly handles staff visible groups' do
       group = Group.new(name: 'group', visibility_level: Group.visibility_levels[:staff])
 
       member = Fabricate(:user)
@@ -3056,6 +3057,7 @@ describe Guardian do
       group.add_owner(owner)
       group.reload
 
+      expect(Guardian.new(another_user).can_see_group?(group)).to eq(false)
       expect(Guardian.new(member).can_see_group?(group)).to eq(false)
       expect(Guardian.new(admin).can_see_group?(group)).to eq(true)
       expect(Guardian.new(moderator).can_see_group?(group)).to eq(true)
@@ -3063,7 +3065,7 @@ describe Guardian do

[... diff too long, it was truncated ...]

GitHub sha: b690fc3d

This commit has been mentioned on Discourse Meta. There might be relevant details there: