Cannot change member_visibility_level of automatic groups

[mdoggydog]

It is not possible to change the member visibility of automatic groups via the groups Manage UI. More specifically: the UI allows selecting a new value, and it cheerfully reports “Saved!” when Save Changes is pressed, but the request is silently ignored by the backend.

This looks like it is a regression that was introduced into the group_params() method of GroupsController in FIX: Add `errors` field if group update confirmation by udan11 · Pull Request #16260 · discourse/discourse · GitHub

[manuel (Manuel Kostka)]

Also just ran into this and find it confusing that I can change the visibility setting, but then it’s silently ignored.

Is there no way to show admins and trust-levels to everyone on the groups page?

[JammyDodger]

It looks like there’s a Contribute > Feature request version of this with some more discussion in:

Respect the visibility settings of all automatic groups

(One person’s feature request is another person’s bug report… ) The “Groups” index page (e.g., /groups or /g), should respect the visibility settings of the automatic groups, and display them accordingly. It does this for the Moderators group, but not the others. This is due to a hard-coded exception in GroupsController.index(), which causes the other automatic groups to never appear on the index page when viewed by non-staff users, no matter what the visibility settings are…

[pmusaraj (Penar Musaraj)]

I just tested this and I can’t reproduce the issue exactly, but we may be testing different things.

Here on meta, I switched the trust_level_4 group’s visibility from Logged on users to Everyone. Before the change, visiting https://meta.discourse.org/g/trust_level_4 returned a 404, after the change it returns the list of members in the group. I have since reversed this edit here so that route is back to a 404 for anons, but changing the visibility works. Also tested the same thing locally.

Is the issue that after changing the group visbility, the group isn’t listed in /groups for regular users? That’s for de-cluttering reasons (it’s not a recent regression, that’s how it was designed in 2017)… regular users can’t join automatic groups, so there’s not much use in listing them in the directory even when the visibility allows it.

[JammyDodger]

@pmusaraj

(it’s not a recent regression, that’s how it was designed in 2017)…

With this in mind I think we can close the Contribute > Bug version of this off and concentrate the conversation in the Respect the visibility settings of all automatic groups Contribute > Feature version.

[JammyDodger]

Re-opening as I’d maybe got confused about what this bug involved…

@mdoggydog

Whoa, there… this bug has not been fixed — it is still present even in 3.2.0.beta5-dev. And, it is a bug on its own — it is independent of the behavior in the noted feature request. (And it crept into the code in 2022, independent of the design decision made in 2017. But, the 2017 decisions likely made it harder to notice this bug.)

This bug is not about the “Who can see this group?” setting (visibility_level); it is about the “Who can see this group’s members?” setting (members_visibility_level).

Try changing “Who can see this group’s members?” of an automatic group… hit Save… then refresh your webpage and see if it really saved the change or not…

My basic repro:

• Go to g/trust_level_1/manage/interaction
• Change ‘Who can see this group’s members?’ to anything other than ‘Everyone’ and save
• Refresh page

Expected - it changes
Actual - it reverts to ‘Everyone’

[pmusaraj (Penar Musaraj)]

Thanks @mdoggydog, I misread the report. Like @JammyDodger, I can repro the issue with the “Who can see this group’s members?” dropdown.

A fix is up for review at

https://github.com/discourse/discourse/pull/25281

[mdoggydog]

Yay, thank you @pmusaraj — the fix even seems to work!

There is an underlying structural bug that still needs to be investigated and fixed: the fact that a user can push “Save” on the frontend and the frontend will report “Saved!”, even when the backend has not saved anything. I’m not sure where/how to file that, but it is tech debt that should be tracked somewhere until fixed.