FIX: Unable to set allowed groups to everyone group #5
Conversation
This update fixes an issue where the copy post button does not show up when it is set to the "everyone" group. This is because the "everyone" group is not a `visible_group`, and only visible groups are attached to the current user's serializer. Hence, we must explicitly check for the everyone group when comparing against the current user's groups.
| @@ -48,7 +48,7 @@ | |||
| before do | |||
| theme_component.update_setting( | |||
| :copy_button_allowed_groups, | |||
| [Group::AUTO_GROUPS[:trust_level_4]], | |||
| Group::AUTO_GROUPS[:trust_level_4].to_s, | |||
There was a problem hiding this comment.
Sneaking in this fix to the spec. This was incorrectly set before and thus not working accurately. The theme setting expects a string based format where group id's are separated by pipes (i.e. "1|5|19"), however we were passing an array of ID's instead.
There was a problem hiding this comment.
The fix as it is looks good. 👍
What I'm wondering is if this potentially affects more places in the front-end, and we should instead explicitly add the "everyone" group in the serializer. Although I realize that might break assumptions in other parts of the front-end that "everyone" is not included. 😅
Yes, I was thinking the same 😅, maybe something we could discuss internally before deciding, so I think I'll merge this as a solution for now. |
🔍 Overview
This update fixes an issue where the copy post button does not show up when it is set to the "everyone" group. This is because the "everyone" group is not a
visible_group, and only visible groups are attached to the current user's serializer. Hence, we must explicitly check for the everyone group when comparing against the current user's groups.🔗 Related links
https://meta.discourse.org/t/copy-post-component/218883/22?u=keegan