Fix attachment editing and return URLs

core/templates/core/_edit_form.html

@@ -26,6 +26,12 @@
       <div class="col-md-4"><input class="form-control" type="file" name="files" multiple x-ref="fileInput" @change="previewFile($event)"></div>
     </div>
     {% if item.kind == 'link' %}<input class="form-control" type="url" name="url" value="{{ item.url }}" placeholder="URL">{% endif %}
+    {% if item.attachments.all %}
+      <div class="border rounded p-2 small">
+        <div class="text-muted mb-1">{% trans "Existing attachments" %}</div>
+        {% for a in item.attachments.all %}<label class="d-flex gap-2 align-items-center mb-1"><input type="checkbox" name="delete_attachments" value="{{ a.id }}"> <span>{{ a.file.name }}</span> <span class="text-danger">{% trans "delete" %}</span></label>{% endfor %}
+      </div>
+    {% endif %}
     <template x-if="preview"><div class="small"><span class="text-muted">{% trans "Image from clipboard:" %}</span><br><img :src="preview" class="img-thumbnail mt-1" style="max-height:160px"></div></template>
     <div class="d-flex gap-2">
       <button class="btn btn-dark btn-sm">{% trans "Save" %}</button>

core/templates/core/_item.html

@@ -1,5 +1,6 @@
 {% load markdown_extras i18n %}
-<article id="item-{{ item.id }}" class="card item-card shadow-sm {% if item.is_done %}done{% elif item.is_overdue %}border-danger border-2{% elif item.is_due_today %}border-warning border-2{% endif %}" ondblclick="if(!event.target.closest('a,button,input,textarea,select,label')) window.location.href='{{ item.get_absolute_url }}?next={{ request.get_full_path|urlencode }}'">
+{% firstof request.GET.next request.get_full_path as return_url %}
+<article id="item-{{ item.id }}" class="card item-card shadow-sm {% if item.is_done %}done{% elif item.is_overdue %}border-danger border-2{% elif item.is_due_today %}border-warning border-2{% endif %}" ondblclick="if(!event.target.closest('a,button,input,textarea,select,label')) window.location.href='{{ item.get_absolute_url }}?next={{ return_url|urlencode }}'">
   <div class="card-body">
     <div class="item-row d-flex justify-content-between align-items-start gap-3">
       <div class="item-main flex-grow-1 min-w-0" style="min-width:0">
@@ -27,10 +28,10 @@
       <div class="item-actions btn-group btn-group-sm">
         {% if item.kind == 'todo' %}<button class="btn btn-outline-primary item-action-btn" title="{% trans 'Done' %}" aria-label="{% trans 'Done' %}" hx-post="{% url 'toggle_done' item.id %}{% if request.GET.urlencode %}?{{ request.GET.urlencode }}{% endif %}" hx-target="#item-{{ item.id }}" hx-swap="outerHTML">✓ <span class="d-md-none">{% trans "Done" %}</span></button>{% endif %}
         {% if request.resolver_match.url_name == 'item_detail' %}
-          <a class="btn btn-outline-secondary item-action-btn" title="{% trans 'Edit' %}" aria-label="{% trans 'Edit' %}" href="{% url 'item_editor' item.id %}?next={{ request.GET.next|default:'/'|urlencode }}">✎ <span class="d-md-none">{% trans "Edit" %}</span></a>
+          <a class="btn btn-outline-secondary item-action-btn" title="{% trans 'Edit' %}" aria-label="{% trans 'Edit' %}" href="{% url 'item_editor' item.id %}?next={{ return_url|urlencode }}">✎ <span class="d-md-none">{% trans "Edit" %}</span></a>
         {% else %}
-          <a class="btn btn-outline-secondary item-action-btn d-md-none" title="{% trans 'Edit' %}" aria-label="{% trans 'Edit' %}" href="{% url 'item_editor' item.id %}?next={{ request.get_full_path|urlencode }}">✎ <span>{% trans "Edit" %}</span></a>
+          <a class="btn btn-outline-secondary item-action-btn d-md-none" title="{% trans 'Edit' %}" aria-label="{% trans 'Edit' %}" href="{% url 'item_editor' item.id %}?next={{ return_url|urlencode }}">✎ <span>{% trans "Edit" %}</span></a>
-          <button class="btn btn-outline-secondary item-action-btn d-none d-md-inline-block" title="{% trans 'Edit' %}" aria-label="{% trans 'Edit' %}" hx-get="{% url 'edit_item' item.id %}{% if request.GET.urlencode %}?{{ request.GET.urlencode }}{% endif %}" hx-target="#item-{{ item.id }}" hx-swap="outerHTML">✎</button>
+          <button class="btn btn-outline-secondary item-action-btn d-none d-md-inline-block" title="{% trans 'Edit' %}" aria-label="{% trans 'Edit' %}" hx-get="{% url 'edit_item' item.id %}?next={{ return_url|urlencode }}" hx-target="#item-{{ item.id }}" hx-swap="outerHTML">✎</button>
         {% endif %}
         <button class="btn btn-outline-danger item-action-btn" title="{% trans 'Delete' %}" aria-label="{% trans 'Delete' %}" hx-post="{% url 'delete_item' item.id %}" hx-target="#item-{{ item.id }}" hx-swap="delete">× <span class="d-md-none">{% trans "Delete" %}</span></button>
       </div>

core/templates/core/item_editor.html

@@ -28,6 +28,12 @@
       <div class="col-md-4"><input class="form-control" type="file" name="files" multiple x-ref="fileInput" @change="previewFile($event)"></div>
     </div>
     {% if item.kind == 'link' %}<input class="form-control" type="url" name="url" value="{{ item.url }}" placeholder="URL">{% endif %}
+    {% if item.attachments.all %}
+      <div class="border rounded p-2 small">
+        <div class="text-muted mb-1">{% trans "Existing attachments" %}</div>
+        {% for a in item.attachments.all %}<label class="d-flex gap-2 align-items-center mb-1"><input type="checkbox" name="delete_attachments" value="{{ a.id }}"> <span>{{ a.file.name }}</span> <span class="text-danger">{% trans "delete" %}</span></label>{% endfor %}
+      </div>
+    {% endif %}
     <template x-if="preview"><div class="small"><span class="text-muted">{% trans "Image preview:" %}</span><br><img :src="preview" class="img-thumbnail mt-1" style="max-height:180px"></div></template>
     <div class="d-flex gap-2 d-none d-md-flex"><button class="btn btn-dark">{% trans "Save" %}</button><a class="btn btn-outline-secondary" href="{{ next_url }}">{% trans "Cancel" %}</a></div>
   </div>

core/tests.py

@@ -7,7 +7,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile
 from django.test import TestCase, override_settings
 from django.urls import reverse
 
-from .models import ApiKey, Collection, CollectionSection, Item, Tag, Team, TeamMembership, UserPreference
+from .models import ApiKey, Attachment, Collection, CollectionSection, Item, Tag, Team, TeamMembership, UserPreference
 from .templatetags.markdown_extras import markdown, markdown_title
 from .views import parse_due_command, parse_quick_content
 
@@ -214,6 +214,28 @@ class AttachmentEditTests(TestCase):
         self.assertEqual(item.attachments.count(), 1)
         self.assertContains(response, 'hello.txt')
 
+    def test_existing_attachments_are_shown_and_can_be_deleted_in_editor(self):
+        item = Item.objects.create(owner=self.user, content='Original')
+        attachment = Attachment.objects.create(item=item, file=SimpleUploadedFile('duplicate.txt', b'duplicate', content_type='text/plain'))
+
+        response = self.client.get(reverse('item_editor', args=[item.id]))
+        self.assertContains(response, 'duplicate.txt')
+        self.assertContains(response, 'name="delete_attachments"')
+
+        response = self.client.post(reverse('item_editor', args=[item.id]), {
+            'content': 'Original', 'comment': '', 'visibility': Item.Visibility.PRIVATE, 'url': '', 'delete_attachments': [str(attachment.id)],
+        })
+
+        self.assertRedirects(response, item.get_absolute_url())
+        self.assertFalse(item.attachments.exists())
+
+    def test_detail_editor_uses_original_return_url_not_partial_endpoint(self):
+        item = Item.objects.create(owner=self.user, content='Original')
+        response = self.client.get(f'{item.get_absolute_url()}?next=/')
+
+        self.assertContains(response, f'{reverse("item_editor", args=[item.id])}?next=/')
+        self.assertNotContains(response, reverse('item_card', args=[item.id]))
+
 
 class TeamItemEditTests(TestCase):
     def setUp(self):

core/views.py

@@ -157,6 +157,14 @@ def parse_due_at_from_post(post):
         return None
 
 
+def delete_selected_attachments(item, attachment_ids):
+    for attachment in item.attachments.filter(pk__in=attachment_ids):
+        attachment.file.delete(save=False)
+        attachment.delete()
+    if hasattr(item, '_prefetched_objects_cache'):
+        item._prefetched_objects_cache.pop('attachments', None)
+
+
 def attach_files_and_replace_tokens(item, files):
     replacements = {}
     attached = False
@@ -1013,6 +1021,7 @@ def edit_item(request, pk):
             item.visibility, item.team = target_visibility, target_team
             item.save()
             form.save_m2m()
+            delete_selected_attachments(item, request.POST.getlist('delete_attachments'))
             attach_files_and_replace_tokens(item, request.FILES.getlist('files'))
             item.sync_metadata()
             apply_team_from_tags(item, request.user, force_private=item.visibility == Item.Visibility.PRIVATE)
@@ -1042,6 +1051,7 @@ def item_editor(request, pk):
             item.visibility, item.team = target_visibility, target_team
             item.save()
             form.save_m2m()
+            delete_selected_attachments(item, request.POST.getlist('delete_attachments'))
             attach_files_and_replace_tokens(item, request.FILES.getlist('files'))
             item.sync_metadata()
             apply_team_from_tags(item, request.user, force_private=item.visibility == Item.Visibility.PRIVATE)