Always inherit visibility from root node (#7209)

Author: akatsoulas

kitsune/groups/admin.py

@@ -12,5 +12,34 @@ class GroupProfileAdmin(TreeAdmin):
     raw_id_fields = ["leaders"]
     search_fields = ["slug", "group__name"]
 
+    def get_readonly_fields(self, request, obj=None):
+        """Make visibility read-only for subgroups (non-root nodes)."""
+        readonly = list(super().get_readonly_fields(request, obj))
+
+        if obj and not obj.is_root():
+            if "visibility" not in readonly:
+                readonly.append("visibility")
+
+        return readonly
+
+    def get_form(self, request, obj=None, **kwargs):
+        """Add help text for visibility field based on node type."""
+        form = super().get_form(request, obj, **kwargs)
+
+        if "visibility" in form.base_fields:
+            if obj and not obj.is_root():
+                form.base_fields["visibility"].help_text = (
+                    "Visibility is inherited from parent group and cannot be changed. "
+                    f"This group inherits '{obj.get_parent().visibility}' from its parent. "
+                    "To change visibility, update the root group."
+                )
+            else:
+                form.base_fields["visibility"].help_text = (
+                    "Who can see this group. Children automatically inherit parent's visibility. "
+                    "Changing this will update all descendants in the tree."
+                )
+
+        return form
+
 
 admin.site.register(GroupProfile, GroupProfileAdmin)

kitsune/groups/models.py

@@ -84,13 +84,26 @@ def save(self, *args, **kwargs):
         if not self.pk or update_fields is None or "information" in update_fields:
             self.information_html = wiki_to_html(self.information)
 
+        # update visibility based on parent group
+        old_visibility = None
+        if self.pk:
+            try:
+                old_instance = GroupProfile.objects.get(pk=self.pk)
+                old_visibility = old_instance.visibility
+            except GroupProfile.DoesNotExist:
+                pass
+
         if len(self.path) > self.steplen:
             parent = self.get_parent()
             if parent:
                 self.visibility = parent.visibility
 
         super().save(*args, **kwargs)
 
+        # Propagate visibility changes to all descendants
+        if old_visibility is not None and old_visibility != self.visibility:
+            self.get_descendants().update(visibility=self.visibility)
+
     def update_visibility(self, new_visibility, propagate=True):
         """
         Update visibility for this group and optionally all descendants.

kitsune/groups/tests/test_models.py

@@ -882,6 +882,82 @@ def test_updating_root_propagates_to_descendants(self):
         self.assertEqual(child.visibility, GroupProfile.Visibility.PRIVATE)
         self.assertEqual(grandchild.visibility, GroupProfile.Visibility.PRIVATE)
 
+    def test_save_propagates_visibility_to_descendants(self):
+        """Changing visibility via save() (as in admin) propagates to descendants."""
+        parent_group = Group.objects.create(name="Parent")
+        parent = GroupProfile.add_root(
+            group=parent_group,
+            slug="parent",
+            visibility=GroupProfile.Visibility.PUBLIC,
+        )
+
+        child_group = Group.objects.create(name="Child")
+        child = parent.add_child(
+            group=child_group,
+            slug="child",
+        )
+
+        grandchild_group = Group.objects.create(name="Grandchild")
+        grandchild = child.add_child(
+            group=grandchild_group,
+            slug="grandchild",
+        )
+
+        # All should be PUBLIC initially
+        self.assertEqual(parent.visibility, GroupProfile.Visibility.PUBLIC)
+        child.refresh_from_db()
+        grandchild.refresh_from_db()
+        self.assertEqual(child.visibility, GroupProfile.Visibility.PUBLIC)
+        self.assertEqual(grandchild.visibility, GroupProfile.Visibility.PUBLIC)
+
+        # Change root to PRIVATE via save() (simulates admin form submission)
+        parent.visibility = GroupProfile.Visibility.PRIVATE
+        parent.save()
+
+        # All descendants should now be PRIVATE
+        parent.refresh_from_db()
+        child.refresh_from_db()
+        grandchild.refresh_from_db()
+        self.assertEqual(parent.visibility, GroupProfile.Visibility.PRIVATE)
+        self.assertEqual(child.visibility, GroupProfile.Visibility.PRIVATE)
+        self.assertEqual(grandchild.visibility, GroupProfile.Visibility.PRIVATE)
+
+    def test_cannot_change_subgroup_visibility(self):
+        """Attempting to change subgroup visibility is overridden by parent."""
+        parent_group = Group.objects.create(name="Parent")
+        parent = GroupProfile.add_root(
+            group=parent_group,
+            slug="parent",
+            visibility=GroupProfile.Visibility.PRIVATE,
+        )
+
+        child_group = Group.objects.create(name="Child")
+        child = parent.add_child(
+            group=child_group,
+            slug="child",
+        )
+
+        # Verify child is PRIVATE (inherited)
+        child.refresh_from_db()
+        self.assertEqual(child.visibility, GroupProfile.Visibility.PRIVATE)
+
+        # Try to change child to PUBLIC (simulates admin form submission)
+        child.visibility = GroupProfile.Visibility.PUBLIC
+        child.save()
+
+        # Refresh and verify it's still PRIVATE (overridden by parent)
+        child.refresh_from_db()
+        self.assertEqual(child.visibility, GroupProfile.Visibility.PRIVATE)
+
+        # Verify descendants also remain PRIVATE
+        grandchild_group = Group.objects.create(name="Grandchild")
+        grandchild = child.add_child(
+            group=grandchild_group,
+            slug="grandchild",
+        )
+        grandchild.refresh_from_db()
+        self.assertEqual(grandchild.visibility, GroupProfile.Visibility.PRIVATE)
+
 
 class ModeratedVisibilityTests(TestCase):
     """Test MODERATED visibility with nested groups."""