From 579328365395b891f07fc789e20f0af2291a60da Mon Sep 17 00:00:00 2001 From: rucki Date: Sun, 30 Aug 2026 18:47:35 +0200 Subject: [PATCH] Allow nested collections --- core/admin.py | 1 + ...section_unique_collection_item_and_more.py | 40 +++++++++++ core/models.py | 12 +++- .../core/_collection_section_read.html | 14 ++++ core/templates/core/collection_detail.html | 4 +- core/templates/core/collection_manage.html | 21 +++--- core/templatetags/markdown_extras.py | 22 ++++++- core/tests.py | 47 +++++++++++++ core/views.py | 66 ++++++++++++++++--- 9 files changed, 202 insertions(+), 25 deletions(-) create mode 100644 core/migrations/0019_remove_collectionsection_unique_collection_item_and_more.py create mode 100644 core/templates/core/_collection_section_read.html diff --git a/core/admin.py b/core/admin.py index 1bc4593..f958f7b 100644 --- a/core/admin.py +++ b/core/admin.py @@ -4,6 +4,7 @@ from .models import ApiKey, Attachment, Collection, CollectionSection, Item, Kan class CollectionSectionInline(admin.TabularInline): model = CollectionSection + fk_name = 'collection' extra = 0 diff --git a/core/migrations/0019_remove_collectionsection_unique_collection_item_and_more.py b/core/migrations/0019_remove_collectionsection_unique_collection_item_and_more.py new file mode 100644 index 0000000..3002463 --- /dev/null +++ b/core/migrations/0019_remove_collectionsection_unique_collection_item_and_more.py @@ -0,0 +1,40 @@ +# Generated by Django 5.2.17 on 2026-08-30 11:23 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0018_remove_collection_tags_collection_filter_tags_and_more'), + ] + + operations = [ + migrations.RemoveConstraint( + model_name='collectionsection', + name='unique_collection_item', + ), + migrations.AddField( + model_name='collectionsection', + name='sub_collection', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='containing_sections', to='core.collection'), + ), + migrations.AlterField( + model_name='collectionsection', + name='item', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='collection_sections', to='core.item'), + ), + migrations.AddConstraint( + model_name='collectionsection', + constraint=models.CheckConstraint(condition=models.Q(models.Q(('item__isnull', False), ('sub_collection__isnull', True)), models.Q(('item__isnull', True), ('sub_collection__isnull', False)), _connector='OR'), name='collection_section_exactly_one_target'), + ), + migrations.AddConstraint( + model_name='collectionsection', + constraint=models.UniqueConstraint(condition=models.Q(('item__isnull', False)), fields=('collection', 'item'), name='unique_collection_item'), + ), + migrations.AddConstraint( + model_name='collectionsection', + constraint=models.UniqueConstraint(condition=models.Q(('sub_collection__isnull', False)), fields=('collection', 'sub_collection'), name='unique_collection_sub_collection'), + ), + ] diff --git a/core/models.py b/core/models.py index 0eba004..e1eb0cb 100644 --- a/core/models.py +++ b/core/models.py @@ -222,7 +222,8 @@ class Collection(models.Model): class CollectionSection(models.Model): collection = models.ForeignKey(Collection, on_delete=models.CASCADE, related_name='sections') - item = models.ForeignKey(Item, on_delete=models.PROTECT, related_name='collection_sections') + item = models.ForeignKey(Item, on_delete=models.PROTECT, blank=True, null=True, related_name='collection_sections') + sub_collection = models.ForeignKey(Collection, on_delete=models.PROTECT, blank=True, null=True, related_name='containing_sections') title = models.CharField(max_length=200, blank=True) position = models.PositiveIntegerField(default=0) created_at = models.DateTimeField(auto_now_add=True) @@ -230,12 +231,17 @@ class CollectionSection(models.Model): class Meta: ordering = ['position', 'id'] constraints = [ - models.UniqueConstraint(fields=['collection', 'item'], name='unique_collection_item'), + models.CheckConstraint( + check=(models.Q(item__isnull=False, sub_collection__isnull=True) | models.Q(item__isnull=True, sub_collection__isnull=False)), + name='collection_section_exactly_one_target', + ), + models.UniqueConstraint(fields=['collection', 'item'], condition=models.Q(item__isnull=False), name='unique_collection_item'), + models.UniqueConstraint(fields=['collection', 'sub_collection'], condition=models.Q(sub_collection__isnull=False), name='unique_collection_sub_collection'), models.UniqueConstraint(fields=['collection', 'position'], name='unique_collection_position'), ] def __str__(self): - return f'{self.collection}: {self.title or self.item}' + return f'{self.collection}: {self.title or self.item or self.sub_collection}' class UserPreference(models.Model): diff --git a/core/templates/core/_collection_section_read.html b/core/templates/core/_collection_section_read.html new file mode 100644 index 0000000..95ef55e --- /dev/null +++ b/core/templates/core/_collection_section_read.html @@ -0,0 +1,14 @@ +{% load markdown_extras %} +
+ {% if section.item %} + {% if section.title %}{% if nested %}

{{ section.title }}

{% else %}

{{ section.title }}

{% endif %}{% endif %} + {% if nested %}{{ section.item|collection_sub_item_markdown }}{% else %}{{ section.item|collection_item_markdown }}{% endif %} + {% if section.item.comment %}
{{ section.item.comment|markdown }}
{% endif %} + {% elif section.sub_collection %} + {% if nested %}

{{ section.title|default:section.sub_collection.title }}

{% else %}

{{ section.title|default:section.sub_collection.title }}

{% endif %} + {% if section.sub_collection.description %}
{{ section.sub_collection.description|markdown }}
{% endif %} + {% for child_section in section.sub_collection.sections.all %} + {% include 'core/_collection_section_read.html' with section=child_section nested=True %} + {% endfor %} + {% endif %} +
diff --git a/core/templates/core/collection_detail.html b/core/templates/core/collection_detail.html index 0fa9008..55db04a 100644 --- a/core/templates/core/collection_detail.html +++ b/core/templates/core/collection_detail.html @@ -22,7 +22,7 @@ {% if sections or tag_items %} {% endif %} @@ -30,7 +30,7 @@ {% if collection.mode == 'tags' %} {% for item in tag_items %}
{{ item|collection_item_markdown }}{% if item.comment %}
{{ item.comment|markdown }}
{% endif %}
{% empty %}
{% trans "No notes match the selected tags yet." %}
{% endfor %} {% else %} - {% for section in sections %}
{% if section.title %}

{{ section.title }}

{% endif %}{{ section.item|collection_item_markdown }}{% if section.item.comment %}
{{ section.item.comment|markdown }}
{% endif %}
{% empty %}
{% trans "This collection has no sections yet." %}
{% endfor %} + {% for section in sections %}{% include 'core/_collection_section_read.html' with section=section %}{% empty %}
{% trans "This collection has no sections yet." %}
{% endfor %} {% endif %} diff --git a/core/templates/core/collection_manage.html b/core/templates/core/collection_manage.html index 5b1a529..0aee12f 100644 --- a/core/templates/core/collection_manage.html +++ b/core/templates/core/collection_manage.html @@ -18,7 +18,7 @@ {% if collection.description %}
{{ collection.description|markdown }}
{% endif %} {% if sections %} -
{% trans "Contents" %}
    {% for section in sections %}
  1. {{ section.title|default:section.item.content|markdown_title|truncatechars:90 }}
    2. {% endfor %}
+
{% trans "Contents" %}
    {% for section in sections %}
  1. {% if section.item %}{{ section.title|default:section.item.content|markdown_title|truncatechars:90 }}{% else %}{{ section.title|default:section.sub_collection.title|truncatechars:90 }}{% endif %}
    2. {% endfor %}
{% endif %} {% if pending_item %} @@ -39,11 +39,11 @@ {% for section in sections %}
-

{{ section.title|default:section.item.content|markdown_title|truncatechars:100 }}

-
{% trans "Edit note" %}#{{ section.item.id }}
+

{% if section.item %}{{ section.title|default:section.item.content|markdown_title|truncatechars:100 }}{% else %}{{ section.title|default:section.sub_collection.title|truncatechars:100 }}{% endif %}

+
{% if section.item %}{% trans "Edit note" %}#{{ section.item.id }}{% else %}{% trans "Edit collection" %}#{{ section.sub_collection.id }}{% endif %}
-
{{ section.item|item_markdown }}
- {% if section.item.comment %}
{{ section.item.comment|markdown }}
{% endif %} + {% if section.item %}
{{ section.item|item_markdown }}
+ {% if section.item.comment %}
{{ section.item.comment|markdown }}
{% endif %}{% else %}
{% trans "Nested collection" %}: {{ section.sub_collection.title }}
{% endif %} {% if can_edit %}
{% csrf_token %}
{% csrf_token %}
@@ -56,11 +56,16 @@ {% if can_edit %}
-

{% trans "Add existing note" %}

-
-
+

{% trans "Add existing note or collection" %}

+
+

{% trans "Notes" %}

+
{% for item in candidates %}
{{ item.content|truncatechars:180 }}
#{{ item.id }} · {{ item.get_visibility_display }}{% if item.team %} · {{ item.team.name }}{% endif %}
{% csrf_token %}
{% empty %}
{% trans "No matching notes." %}
{% endfor %}
+

{% trans "Collections" %}

+
+ {% for candidate in collection_candidates %}
{{ candidate.title }}
#{{ candidate.id }} · {{ candidate.get_visibility_display }}{% if candidate.team %} · {{ candidate.team.name }}{% endif %}
{% csrf_token %}
{% empty %}
{% trans "No matching collections." %}
{% endfor %} +
{% endif %}
diff --git a/core/templatetags/markdown_extras.py b/core/templatetags/markdown_extras.py index 45950fc..a430f81 100644 --- a/core/templatetags/markdown_extras.py +++ b/core/templatetags/markdown_extras.py @@ -7,7 +7,7 @@ from django.utils.safestring import mark_safe register = template.Library() ALLOWED_TAGS = set(bleach.sanitizer.ALLOWED_TAGS) | { - 'p', 'pre', 'code', 'blockquote', 'h1', 'h2', 'h3', 'h4', 'ul', 'ol', 'li', + 'p', 'pre', 'code', 'blockquote', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'ul', 'ol', 'li', 'strong', 'em', 'u', 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'hr', 'br', 'img' } ALLOWED_ATTRS = {'a': ['href', 'title', 'rel'], 'code': ['class'], 'img': ['src', 'alt', 'title', 'class']} @@ -71,7 +71,18 @@ def markdown_title(text): return '' -def render_item_markdown(item, hide_tags=False): +def offset_heading_tags(html, offset=0): + if not offset: + return html + + def replace(match): + slash, level = match.groups() + return f'<{slash}h{min(6, int(level) + offset)}>' + + return re.sub(r'<(/?)h([1-6])>', replace, str(html)) + + +def render_item_markdown(item, hide_tags=False, heading_offset=0): text = strip_tags_outside_code(item.content) if hide_tags else (item.content or '') attachments = {str(a.id): a for a in item.attachments.all()} @@ -81,7 +92,7 @@ def render_item_markdown(item, hide_tags=False): return match.group(0) return f'![{attachment.file.name}]({attachment.file.url})' - return markdown(FILE_RE.sub(replace, text)) + return mark_safe(offset_heading_tags(markdown(FILE_RE.sub(replace, text)), heading_offset)) @register.filter @@ -94,6 +105,11 @@ def collection_item_markdown(item): return render_item_markdown(item, hide_tags=True) +@register.filter +def collection_sub_item_markdown(item): + return render_item_markdown(item, hide_tags=True, heading_offset=1) + + @register.filter def is_image(file_name): return str(file_name).lower().split('?')[0].endswith(('.png', '.jpg', '.jpeg', '.gif', '.webp', '.bmp', '.svg')) diff --git a/core/tests.py b/core/tests.py index 13b66e6..7323c80 100644 --- a/core/tests.py +++ b/core/tests.py @@ -341,6 +341,53 @@ class CollectionTests(TestCase): self.assertContains(response, '>Section## SectionChild', html=True) + self.assertContains(response, '

Child note

', html=True) + self.assertNotContains(response, '#internal') + + def test_collection_loop_is_rejected(self): + parent = Collection.objects.create(owner=self.user, title='Parent') + child = Collection.objects.create(owner=self.user, title='Child') + CollectionSection.objects.create(collection=parent, sub_collection=child, position=0) + manage_url = reverse('collection_manage', args=[child.id]) + + response = self.client.post(manage_url, {'action': 'add_collection', 'collection_id': parent.id}, follow=True) + + self.assertContains(response, 'collection loop') + self.assertFalse(child.sections.filter(sub_collection=parent).exists()) + + def test_public_collection_cannot_include_private_collection(self): + parent = Collection.objects.create(owner=self.user, title='Parent', visibility=Item.Visibility.PUBLIC) + child = Collection.objects.create(owner=self.user, title='Child', visibility=Item.Visibility.PRIVATE) + manage_url = reverse('collection_manage', args=[parent.id]) + + response = self.client.post(manage_url, {'action': 'add_collection', 'collection_id': child.id}, follow=True) + + self.assertContains(response, 'incompatible visibility') + self.assertFalse(parent.sections.filter(sub_collection=child).exists()) + + def test_private_collection_can_include_public_collection(self): + parent = Collection.objects.create(owner=self.user, title='Parent', visibility=Item.Visibility.PRIVATE) + child = Collection.objects.create(owner=self.user, title='Child', visibility=Item.Visibility.PUBLIC) + manage_url = reverse('collection_manage', args=[parent.id]) + + response = self.client.post(manage_url, {'action': 'add_collection', 'collection_id': child.id}) + + self.assertRedirects(response, manage_url) + self.assertTrue(parent.sections.filter(sub_collection=child).exists()) + def test_public_collection_is_visible_without_login(self): collection = Collection.objects.create(owner=self.user, title='Public', visibility=Item.Visibility.PUBLIC) self.client.logout() diff --git a/core/views.py b/core/views.py index e1c8b28..fad7ed8 100644 --- a/core/views.py +++ b/core/views.py @@ -542,7 +542,7 @@ def team_accept_invite(request, token): def visible_collections(user): - qs = Collection.objects.select_related('owner', 'team').prefetch_related('filter_tags', 'sections__item') + qs = Collection.objects.select_related('owner', 'team').prefetch_related('filter_tags', 'sections__item', 'sections__sub_collection') if user.is_authenticated: return qs.filter(Q(owner=user) | Q(team_id__in=user_team_ids(user)) | Q(visibility=Item.Visibility.PUBLIC)).distinct() return qs.filter(visibility=Item.Visibility.PUBLIC) @@ -574,6 +574,22 @@ def collection_item_is_compatible(collection, item): ) +def collection_subcollection_is_compatible(collection, sub_collection): + return collection_scope_allows_item( + collection.visibility, collection.team_id, collection.owner_id, + sub_collection.visibility, sub_collection.team_id, sub_collection.owner_id, + ) + + +def collection_contains_collection(collection, target): + if collection.pk == target.pk: + return True + for section in collection.sections.select_related('sub_collection'): + if section.sub_collection and collection_contains_collection(section.sub_collection, target): + return True + return False + + def collection_target_from_form(form, user): team = form.cleaned_data.get('team') if team: @@ -639,11 +655,22 @@ def copy_item_for_collection(item, collection, owner): return adapt_item_for_collection(copied, collection) -def add_collection_section(collection, item): +def next_collection_section_position(collection): last_position = collection.sections.order_by('-position').values_list('position', flat=True).first() + return (last_position + 1) if last_position is not None else 0 + + +def add_collection_section(collection, item): return CollectionSection.objects.get_or_create( collection=collection, item=item, - defaults={'position': (last_position + 1) if last_position is not None else 0}, + defaults={'position': next_collection_section_position(collection)}, + ) + + +def add_collection_subcollection(collection, sub_collection): + return CollectionSection.objects.get_or_create( + collection=collection, sub_collection=sub_collection, + defaults={'position': next_collection_section_position(collection)}, ) @@ -657,17 +684,23 @@ 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() + collection_candidates = Collection.objects.none() if can_edit: candidates = editable_collection_notes(request.user).exclude(collection_sections__collection=collection) + collection_candidates = visible_collections(request.user).filter(mode=Collection.Mode.MANUAL).exclude(pk=collection.pk).exclude(containing_sections__collection=collection) + collection_candidates = [candidate for candidate in collection_candidates if collection_subcollection_is_compatible(collection, candidate) and not collection_contains_collection(candidate, collection)] if q: candidates = candidates.filter(Q(content__icontains=q) | Q(tags__name__icontains=q)).distinct() + collection_candidates = [candidate for candidate in collection_candidates if q.lower() in candidate.title.lower()] candidates = candidates[:30] + collection_candidates = collection_candidates[:30] return { 'collection': collection, - 'sections': collection.sections.select_related('item').prefetch_related('item__tags', 'item__attachments'), + 'sections': collection.sections.select_related('item', 'sub_collection').prefetch_related('item__tags', 'item__attachments', 'sub_collection__sections__item', 'sub_collection__sections__sub_collection'), 'can_edit': can_edit, 'can_delete': user_can_delete_collection(request.user, collection), 'candidates': candidates, + 'collection_candidates': collection_candidates, 'q': q, 'pending_item': pending_item, 'pending_can_convert': pending_item and item_can_change_for_collection(request.user, pending_item, collection), @@ -718,10 +751,16 @@ def collection_edit(request, pk): visibility, team = collection_target_from_form(form, request.user) target_mode = form.cleaned_data['mode'] has_manual_sections = collection.mode == Collection.Mode.MANUAL and collection.sections.exists() - incompatible = target_mode == Collection.Mode.MANUAL and 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')) + incompatible = target_mode == Collection.Mode.MANUAL and 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, + ) if section.item else not collection_scope_allows_item( + visibility, team.id if team else None, collection.owner_id, + section.sub_collection.visibility, section.sub_collection.team_id, section.sub_collection.owner_id, + ) + for section in collection.sections.select_related('item', 'sub_collection') + ) if target_mode == Collection.Mode.TAGS and has_manual_sections: form.add_error('mode', _('Remove all manual sections before changing to a tag collection.')) elif incompatible: @@ -747,7 +786,7 @@ def collection_detail(request, pk): collection = get_object_or_404(visible_collections(request.user), pk=pk) return render(request, 'core/collection_detail.html', { 'collection': collection, - 'sections': collection.sections.select_related('item').prefetch_related('item__tags', 'item__attachments') if collection.mode == Collection.Mode.MANUAL else [], + 'sections': collection.sections.select_related('item', 'sub_collection').prefetch_related('item__tags', 'item__attachments', 'sub_collection__sections__item', 'sub_collection__sections__sub_collection') if collection.mode == Collection.Mode.MANUAL else [], 'tag_items': collection_tag_items(collection) if collection.mode == Collection.Mode.TAGS else [], 'can_edit': user_can_edit_collection(request.user, collection), }) @@ -782,6 +821,15 @@ def collection_manage(request, pk): return redirect(manage_url) add_collection_section(collection, item) return redirect(manage_url) + if action == 'add_collection': + sub_collection = get_object_or_404(visible_collections(request.user), pk=request.POST.get('collection_id'), mode=Collection.Mode.MANUAL) + if sub_collection.pk == collection.pk or collection_contains_collection(sub_collection, collection): + messages.error(request, _('This would create a collection loop.')) + elif not collection_subcollection_is_compatible(collection, sub_collection): + messages.error(request, _('This collection has an incompatible visibility.')) + else: + add_collection_subcollection(collection, sub_collection) + return redirect(manage_url) section = get_object_or_404(CollectionSection, pk=request.POST.get('section_id'), collection=collection) if action == 'remove': section.delete()