diff --git a/core/forms.py b/core/forms.py index 196ea61..4474513 100644 --- a/core/forms.py +++ b/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'}), diff --git a/core/templates/core/collection_form.html b/core/templates/core/collection_form.html index eacc44e..52f6007 100644 --- a/core/templates/core/collection_form.html +++ b/core/templates/core/collection_form.html @@ -10,8 +10,8 @@ {% if form.non_field_errors %}
{{ form.non_field_errors }}
{% endif %}
{{ form.title }}{{ form.title.errors }}
{{ form.description }}{{ form.description.errors }}
-
{{ form.visibility }}{{ form.visibility.errors }}
{% trans "Adding one of your team tags turns this into a team collection." %}
-
{{ form.tags }}
{{ form.tags.errors }}
+
{{ form.visibility }}{{ form.visibility.errors }}
{% trans "Optional. Selecting a team makes the collection visible and editable for that team." %}
+
{% if form.tags.field.queryset.exists %}{{ form.tags }}{% else %}{% trans "You do not belong to a team." %}{% endif %}
{{ form.tags.errors }}
{% trans "Cancel" %}
diff --git a/core/tests.py b/core/tests.py index e8ca668..f371fab 100644 --- a/core/tests.py +++ b/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') diff --git a/core/views.py b/core/views.py index 6f5e274..09656ec 100644 --- a/core/views.py +++ b/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 item.visibility == Item.Visibility.TEAM and item.team_id == collection.team_id - return item.visibility == collection.visibility and item.team_id is None + return collection_scope_allows_item( + collection.visibility, collection.team_id, collection.owner_id, + 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) - if team: - incompatible = incompatible | collection.sections.exclude(item__team=team) - else: - incompatible = incompatible | collection.sections.exclude(item__team__isnull=True) - if incompatible.exists(): + incompatible = any(not collection_scope_allows_item( + visibility, team.id if team else None, collection.owner_id, + section.item.visibility, section.item.team_id, section.item.owner_id, + ) for section in collection.sections.select_related('item')) + if incompatible: 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) diff --git a/locale/de/LC_MESSAGES/django.po b/locale/de/LC_MESSAGES/django.po index e0451e1..86ecfe5 100644 --- a/locale/de/LC_MESSAGES/django.po +++ b/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."