Babble: Admins can't see a group set to `group owner` only visibility

Support
1

I just ran into this on a forum I run. Repro steps:

  1. Create a new group as an admin
  2. Set visibility to “group owners”
  3. Create group
  4. Return to /g
  5. Group will not show up

If you check the console by using Group.find_by(name: <name>) it returns correctly. Changing the visibility to “Group owners and staff” does return the visibility.

Unless this is by design, my assumption is that even admins should see groups at this visibility level (though not mods).

I didn’t notice this behavior until upgrading to 2.4.0.beta2 yesterday.

9 Likes
2

Does reloading help?

3

No, and even pulling the JSON endpoint the group doesn’t show up as an admin.

2 Likes
5

I worked on this recently, will take a look.

5 Likes
6

Yeah. It didn’t seem like the kind of thing you’d have overlooked, but it was all I had. :wink:

2 Likes
7

I can’t reproduce this @justin. I tried locally and on a DO hosted instance, and I tried creating a group from two different admin accounts. Admins in all cases see all groups.

3 Likes
8

Odd - I’ll have to try to do it on a fresh install. I can for sure repro it on a production instance I have. I’ll try updating it and go from there.

4 Likes
9

I tried on a fresh install and got the same results as you @pmusaraj. My guess is it’s a plugin I’m using. If I find out what’s up I’ll report back.

5 Likes
10

I’ve also run into this problem…

11

Are you also using plugins? Which ones (focusing on the ones Discourse does not create & ship)… or are you saying this is a new regression @justin?

12

Hi, I just changed a group’s visibility from “Group owners” to “Group owners, members” and it became visible again. I wonder if there is something quirky with the code for the setting for a group when its visibility is restricted to “Group owners”?

2 Likes
13

@outofthebox Do you by chance have the Babble chat plugin installed? Seems like it’s a regression relating to that after I removed it from my instance. Uninstalling that plug-in and rebuilding brought the group visibility back.

6 Likes
14

Nice find @justin, I had a quick look at the Babble source code, and there does seem to be a conflict with my updates to groups in core. Babble adds a hardcoded rule for visibility_level: 4, which now clashes with the owners group in core.

https://github.com/gdpelican/babble/blob/b5500411a6d322478426e9e86eadbfdb51924a36/app/models/group.rb#L4

Hopefully @gdpelican might have time to have a look.

9 Likes
15

How to fix this for the current version of the discourse?
Should I delete this code? .where.not(visibility_level: 4)

  @@visible_group_scope = method(:visible_groups).clone
  scope :visible_groups, ->(user, order = nil, opts = {}) {
    @@visible_group_scope.call(user, order, opts)
  }
end
16

Ok, so I have dived into this issue a little deeper.

It seems like babble had assigned its own visibility_level for its own use, and took the next unused constant (4).

A few weeks ago, the Discourse core code added a new visiblity_level as well, and took the next unused constant, which was 4 as well. This caused a duplicate use of this constant.

So a fix would consist of two parts:

  • change the visibility_level used by babble. That would be easy. If we wanted to do this really neatly we would have each plugin register its own BASE_VISIBILITY_LEVEL in order to avoid future clashes. But for now we could just pick something (for instance 1001).
  • change the groups with visibility_level 4 to the new constant - but only the groups that were used by babble.
    Would this query select the right groups @gdpelican ?
UPDATE groups 
SET visibility_level = 1001 
WHERE id IN (
  SELECT g.id 
  FROM topics t 
  LEFT JOIN topic_allowed_groups tag ON tag.topic_id = t.id 
  LEFT JOIN groups g ON g.id = tag.group_id 
  WHERE t.archetype='chat');`
8 Likes
17

Hi sorry, I will take a look at this in the coming days, but your general outline of a fix is spot on. The back port will be the tricky part, I’ll have a closer look soon.

5 Likes
18

Alright, I have pushed a fix for this. Let me know if you experience any additional oddness?

5 Likes
19

I found one issue: it looks like the Backfiller functionality is lacking multisite support (it needs to loop through the available connections)

2 Likes
20

@RGJ Would you be up for putting up a PR for that one? I can merge it but will have limited connectivity for the next few days.

3 Likes
21

Sure, I’ll be able to do that this weekend.

3 Likes