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

I worked on this recently, will take a look.

5 Likes

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

2 Likes

I can’t reproduce this @justindirose. 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

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

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

I’ve also run into this problem…

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 @justindirose?

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

@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

Nice find @justindirose, 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.

Hopefully @gdpelican might have time to have a look.

9 Likes

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

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

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

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

5 Likes

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

2 Likes

@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

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

3 Likes

Is this resolved? Should it be closed?

2 Likes

Yep, this should be resolved now.

2 Likes