Refine collection visibility and team selection

core/forms.py

@@ -134,6 +134,20 @@ class KanbanForm(forms.ModelForm):
 
 
 class CollectionForm(forms.ModelForm):
+    def __init__(self, *args, user=None, **kwargs):
+        super().__init__(*args, **kwargs)
+        if user and user.is_authenticated:
+            team_slugs = user.team_memberships.values_list('team__slug', flat=True)
+            self.fields['tags'].queryset = Tag.objects.filter(name__in=team_slugs).order_by('name')
+        else:
+            self.fields['tags'].queryset = Tag.objects.none()
+
+    def clean_tags(self):
+        tags = self.cleaned_data['tags']
+        if tags.count() > 1:
+            raise forms.ValidationError(_('Select at most one team.'))
+        return tags
+
     class Meta:
         model = Collection
         fields = ['title', 'description', 'visibility', 'tags']
@@ -141,7 +155,7 @@ class CollectionForm(forms.ModelForm):
             'title': _('Title'),
             'description': _('Description'),
             'visibility': _('Visibility'),
-            'tags': _('Tags'),
+            'tags': _('Team'),
         }
         widgets = {
             'title': forms.TextInput(attrs={'class': 'form-control'}),

core/templates/core/collection_form.html

@@ -10,8 +10,8 @@
     {% if form.non_field_errors %}<div class="alert alert-danger mb-0">{{ form.non_field_errors }}</div>{% endif %}
     <div><label class="form-label">{{ form.title.label }}</label>{{ form.title }}{{ form.title.errors }}</div>
     <div><label class="form-label">{{ form.description.label }}</label>{{ form.description }}{{ form.description.errors }}</div>
-    <div><label class="form-label">{{ form.visibility.label }}</label>{{ form.visibility }}{{ form.visibility.errors }}<div class="form-text">{% trans "Adding one of your team tags turns this into a team collection." %}</div></div>
+    <div><label class="form-label">{{ form.visibility.label }}</label>{{ form.visibility }}{{ form.visibility.errors }}<div class="form-text">{% trans "Optional. Selecting a team makes the collection visible and editable for that team." %}</div></div>
-    <div><label class="form-label">{{ form.tags.label }}</label><div class="border rounded p-3 collection-tags">{{ form.tags }}</div>{{ form.tags.errors }}</div>
+    <div><label class="form-label">{{ form.tags.label }}</label><div class="border rounded p-3 collection-tags">{% if form.tags.field.queryset.exists %}{{ form.tags }}{% else %}<span class="text-muted small">{% trans "You do not belong to a team." %}</span>{% endif %}</div>{{ form.tags.errors }}</div>
     <div class="d-flex gap-2"><button class="btn btn-dark">{% trans "Save" %}</button><a class="btn btn-outline-secondary" href="{% if collection %}{{ collection.get_absolute_url }}{% else %}{% url 'collection_list' %}{% endif %}">{% trans "Cancel" %}</a></div>
   </div></form>
 </div>

core/tests.py

@@ -164,6 +164,16 @@ class CollectionTests(TestCase):
         self.assertRedirects(response, manage_url)
         self.assertTrue(CollectionSection.objects.filter(collection=collection, item=note).exists())
 
+    def test_public_note_can_be_added_to_private_collection_without_conversion(self):
+        collection = Collection.objects.create(owner=self.user, title='Private guide', visibility=Item.Visibility.PRIVATE)
+        note = Item.objects.create(owner=self.user, kind=Item.Kind.NOTE, content='Already public', visibility=Item.Visibility.PUBLIC)
+        manage_url = reverse('collection_manage', args=[collection.id])
+        response = self.client.post(manage_url, {'action': 'add', 'item_id': note.id})
+        self.assertRedirects(response, manage_url)
+        self.assertTrue(collection.sections.filter(item=note).exists())
+        note.refresh_from_db()
+        self.assertEqual(note.visibility, Item.Visibility.PUBLIC)
+
     def test_public_collection_asks_before_copying_private_note(self):
         collection = Collection.objects.create(owner=self.user, title='Public guide', visibility=Item.Visibility.PUBLIC)
         note = Item.objects.create(owner=self.user, kind=Item.Kind.NOTE, content='Private section')
@@ -201,6 +211,21 @@ class CollectionTests(TestCase):
         self.assertEqual(collection.team, team)
         self.assertEqual(collection.visibility, Item.Visibility.TEAM)
 
+    def test_other_users_public_notes_are_not_offered_for_embedding(self):
+        other = get_user_model().objects.create_user(username='other')
+        public_note = Item.objects.create(owner=other, kind=Item.Kind.NOTE, content='External', visibility=Item.Visibility.PUBLIC)
+        collection = Collection.objects.create(owner=self.user, title='Own collection')
+        response = self.client.get(reverse('collection_manage', args=[collection.id]))
+        self.assertNotContains(response, f'name="item_id" value="{public_note.id}"')
+
+    def test_collection_form_only_offers_team_tags(self):
+        team = Team.objects.create(name='Docs team', slug='docs', owner=self.user)
+        TeamMembership.objects.create(team=team, user=self.user, role=TeamMembership.Role.OWNER)
+        Tag.objects.create(name='docs')
+        Tag.objects.create(name='ordinary')
+        response = self.client.get(reverse('collection_create'))
+        self.assertEqual(list(response.context['form'].fields['tags'].queryset.values_list('name', flat=True)), ['docs'])
+
     def test_collection_opens_as_continuous_reading_view(self):
         collection = Collection.objects.create(owner=self.user, title='Reading view')
         first = Item.objects.create(owner=self.user, kind=Item.Kind.NOTE, content='First paragraph')

core/views.py

@@ -173,7 +173,11 @@ def desired_item_scope(item, user, force_private=False):
 
 def item_scope_fits_collections(item, visibility, team):
     sections = item.collection_sections.select_related('collection') if item.pk else CollectionSection.objects.none()
-    return not any(section.collection.visibility != visibility or section.collection.team_id != (team.id if team else None) for section in sections)
+    team_id = team.id if team else None
+    return all(collection_scope_allows_item(
+        section.collection.visibility, section.collection.team_id, section.collection.owner_id,
+        visibility, team_id, item.owner_id,
+    ) for section in sections)
 
 
 def apply_team_from_tags(item, user, force_private=False):
@@ -546,10 +550,22 @@ def user_can_delete_collection(user, collection):
     return user.is_authenticated and (collection.owner_id == user.id or (collection.team_id and TeamMembership.objects.filter(team=collection.team, user=user, role__in=[TeamMembership.Role.OWNER, TeamMembership.Role.ADMIN]).exists()))
 
 
+def collection_scope_allows_item(collection_visibility, collection_team_id, collection_owner_id, item_visibility, item_team_id, item_owner_id):
+    # A collection may reference a note that is already visible to a wider audience.
+    if item_visibility == Item.Visibility.PUBLIC:
+        return True
+    if collection_visibility == Item.Visibility.PUBLIC:
+        return False
+    if collection_visibility == Item.Visibility.TEAM:
+        return item_visibility == Item.Visibility.TEAM and item_team_id == collection_team_id
+    return item_visibility == Item.Visibility.PRIVATE and item_owner_id == collection_owner_id
+
+
 def collection_item_is_compatible(collection, item):
-    if collection.visibility == Item.Visibility.TEAM:
+    return collection_scope_allows_item(
-        return item.visibility == Item.Visibility.TEAM and item.team_id == collection.team_id
+        collection.visibility, collection.team_id, collection.owner_id,
-    return item.visibility == collection.visibility and item.team_id is None
+        item.visibility, item.team_id, item.owner_id,
+    )
 
 
 def collection_target_from_form(form, user):
@@ -564,9 +580,13 @@ def collection_target_from_form(form, user):
 def item_can_change_for_collection(user, item, collection):
     if not user_can_edit_item(user, item):
         return False
+    target_team_id = collection.team_id
     for section in item.collection_sections.select_related('collection').exclude(collection=collection):
         other = section.collection
-        if other.visibility != collection.visibility or other.team_id != collection.team_id:
+        if not collection_scope_allows_item(
+            other.visibility, other.team_id, other.owner_id,
+            collection.visibility, target_team_id, item.owner_id,
+        ):
             return False
     return True
 
@@ -610,12 +630,18 @@ def add_collection_section(collection, item):
     )
 
 
+def editable_collection_notes(user):
+    return Item.objects.select_related('owner', 'team').prefetch_related('tags', 'attachments').filter(
+        kind=Item.Kind.NOTE,
+    ).filter(Q(owner=user) | Q(team_id__in=user_team_ids(user))).distinct()
+
+
 def collection_detail_context(request, collection, pending_item=None):
     can_edit = user_can_edit_collection(request.user, collection)
     q = request.GET.get('q', '').strip()
     candidates = Item.objects.none()
     if can_edit:
-        candidates = visible_items(request.user).filter(kind=Item.Kind.NOTE).exclude(collection_sections__collection=collection)
+        candidates = editable_collection_notes(request.user).exclude(collection_sections__collection=collection)
         if q:
             candidates = candidates.filter(Q(content__icontains=q) | Q(tags__name__icontains=q)).distinct()
         candidates = candidates[:30]
@@ -640,7 +666,7 @@ def collection_list(request):
 @login_required
 def collection_create(request):
     if request.method == 'POST':
-        form = CollectionForm(request.POST)
+        form = CollectionForm(request.POST, user=request.user)
         if form.is_valid():
             visibility, team = collection_target_from_form(form, request.user)
             if visibility == Item.Visibility.TEAM and not team:
@@ -655,7 +681,7 @@ def collection_create(request):
                 messages.success(request, _('Collection created.'))
                 return redirect(collection)
     else:
-        form = CollectionForm()
+        form = CollectionForm(user=request.user)
     return render(request, 'core/collection_form.html', {'form': form, 'heading': _('Create collection')})
 
 
@@ -665,15 +691,14 @@ def collection_edit(request, pk):
     if not user_can_edit_collection(request.user, collection):
         return JsonResponse({'error': 'forbidden'}, status=403)
     if request.method == 'POST':
-        form = CollectionForm(request.POST, instance=collection)
+        form = CollectionForm(request.POST, instance=collection, user=request.user)
         if form.is_valid():
             visibility, team = collection_target_from_form(form, request.user)
-            incompatible = collection.sections.exclude(item__visibility=visibility)
+            incompatible = any(not collection_scope_allows_item(
-            if team:
+                visibility, team.id if team else None, collection.owner_id,
-                incompatible = incompatible | collection.sections.exclude(item__team=team)
+                section.item.visibility, section.item.team_id, section.item.owner_id,
-            else:
+            ) for section in collection.sections.select_related('item'))
-                incompatible = incompatible | collection.sections.exclude(item__team__isnull=True)
+            if incompatible:
-            if incompatible.exists():
                 form.add_error(None, _('The collection visibility cannot change while it contains incompatible notes.'))
             elif visibility == Item.Visibility.TEAM and not team:
                 form.add_error('visibility', _('Select a team tag for a team collection.'))
@@ -686,7 +711,7 @@ def collection_edit(request, pk):
                 messages.success(request, _('Collection saved.'))
                 return redirect(collection)
     else:
-        form = CollectionForm(instance=collection)
+        form = CollectionForm(instance=collection, user=request.user)
     return render(request, 'core/collection_form.html', {'form': form, 'collection': collection, 'heading': _('Edit collection')})
 
 
@@ -708,7 +733,7 @@ def collection_manage(request, pk):
     if request.method == 'POST':
         action = request.POST.get('action')
         if action in {'add', 'confirm_add'}:
-            item = get_object_or_404(visible_items(request.user), pk=request.POST.get('item_id'), kind=Item.Kind.NOTE)
+            item = get_object_or_404(editable_collection_notes(request.user), pk=request.POST.get('item_id'))
             if collection_item_is_compatible(collection, item):
                 add_collection_section(collection, item)
                 return redirect(manage_url)

locale/de/LC_MESSAGES/django.po

@@ -756,6 +756,18 @@ msgstr "Keine passenden Notizen."
 msgid "Adding one of your team tags turns this into a team collection."
 msgstr "Durch Hinzufügen eines deiner Team-Tags wird daraus eine Team-Sammlung."
 
+#: core/templates/core/collection_form.html
+msgid "Optional. Selecting a team makes the collection visible and editable for that team."
+msgstr "Optional. Durch Auswahl eines Teams wird die Sammlung für dieses Team sichtbar und bearbeitbar."
+
+#: core/templates/core/collection_form.html
+msgid "You do not belong to a team."
+msgstr "Du gehörst keinem Team an."
+
+#: core/forms.py
+msgid "Select at most one team."
+msgstr "Wähle höchstens ein Team aus."
+
 #: core/templates/core/collection_list.html
 msgid "Combine existing notes into ordered documents."
 msgstr "Fasse bestehende Notizen zu geordneten Dokumenten zusammen."