Preserve page and scroll position when editing tags (#1291)

bookmarks/templates/bundles/preview.html

@@ -3,6 +3,8 @@
   <div>No bookmarks match the current bundle.</div>
 {% else %}
   <div class="mb-4">Found {{ bookmark_list.bookmarks_total }} bookmarks matching this bundle.</div>
-  {% include 'bookmarks/bookmark_list.html' %}
+  {% with pagination_frame="preview" %}
+    {% include 'bookmarks/bookmark_list.html' %}
+  {% endwith %}
 {% endif %}
 </turbo-frame>

bookmarks/templates/shared/pagination.html

@@ -2,7 +2,9 @@
 <ul class="pagination">
   {% if prev_link %}
     <li class="page-item">
-      <a href="{{ prev_link }}" tabindex="-1">Previous</a>
+      <a href="{{ prev_link }}"
+         tabindex="-1"
+         data-turbo-frame="{{ pagination_frame }}">Previous</a>
     </li>
   {% else %}
     <li class="page-item disabled">
@@ -12,7 +14,7 @@
   {% for page_link in page_links %}
     {% if page_link %}
       <li class="page-item {% if page_link.active %}active{% endif %}">
-        <a href="{{ page_link.link }}">{{ page_link.number }}</a>
+        <a href="{{ page_link.link }}" data-turbo-frame="{{ pagination_frame }}">{{ page_link.number }}</a>
       </li>
     {% else %}
       <li class="page-item">
@@ -22,7 +24,9 @@
   {% endfor %}
   {% if next_link %}
     <li class="page-item">
-      <a href="{{ next_link }}" tabindex="-1">Next</a>
+      <a href="{{ next_link }}"
+         tabindex="-1"
+         data-turbo-frame="{{ pagination_frame }}">Next</a>
     </li>
   {% else %}
     <li class="page-item disabled">

bookmarks/templates/tags/edit.html

@@ -1,11 +1,11 @@
 <turbo-frame id="tag-modal">
 <form method="post"
-      action="{% url 'linkding:tags.edit' tag.id %}"
+      action="{% url 'linkding:tags.edit' tag.id %}?{{ request.GET.urlencode }}"
-      data-turbo-frame="_top"
+      data-turbo-frame="tag-main"
       novalidate>
   {% csrf_token %}
   <ld-modal class="modal tag-edit-modal active"
-            data-close-url="{% url 'linkding:tags.index' %}"
+            data-close-url="{% url 'linkding:tags.index' %}?{{ request.GET.urlencode }}"
             data-turbo-frame="tag-modal">
     <div class="modal-overlay" data-close-modal></div>
     <div class="modal-container" role="dialog" aria-modal="true">

bookmarks/templates/tags/index.html

@@ -5,6 +5,7 @@
 {% endblock %}
 {% block content %}
   <div class="tags-page crud-page">
+    <turbo-frame id="tag-main">
     <main aria-labelledby="main-heading">
       <div class="crud-header">
         <h1 id="main-heading">Tags</h1>
@@ -97,7 +98,7 @@
                   </td>
                   <td class="actions">
                     <a class="btn btn-link"
-                       href="{% url 'linkding:tags.edit' tag.id %}"
+                       href="{% url 'linkding:tags.edit' tag.id %}?{{ request.GET.urlencode }}"
                        data-turbo-frame="tag-modal">Edit</a>
                     <button type="submit"
                             name="delete_tag"
@@ -123,6 +124,7 @@
         </div>
       {% endif %}
     </main>
+    <turbo-frame id="tag-modal"></turbo-frame>
+    </turbo-frame>
   </div>
-  <turbo-frame id="tag-modal"></turbo-frame>
 {% endblock %}

bookmarks/templatetags/pagination.py

@@ -12,6 +12,7 @@ register = template.Library()
 @register.inclusion_tag("shared/pagination.html", name="pagination", takes_context=True)
 def pagination(context, page: Page):
     request = context["request"]
+    pagination_frame = context.get("pagination_frame", "_top")
     base_url = request.path
 
     # remove page number and details from query parameters
@@ -51,6 +52,7 @@ def pagination(context, page: Page):
         "prev_link": prev_link,
         "next_link": next_link,
         "page_links": page_links,
+        "pagination_frame": pagination_frame,
     }
 
 

bookmarks/tests/test_pagination_tag.py

@@ -7,7 +7,12 @@ from bookmarks.tests.helpers import BookmarkFactoryMixin
 
 class PaginationTagTest(TestCase, BookmarkFactoryMixin):
     def render_template(
-        self, num_items: int, page_size: int, current_page: int, url: str = "/test"
+        self,
+        num_items: int,
+        page_size: int,
+        current_page: int,
+        url: str = "/test",
+        frame: str = None,
     ) -> str:
         rf = RequestFactory()
         request = rf.get(url)
@@ -16,7 +21,11 @@ class PaginationTagTest(TestCase, BookmarkFactoryMixin):
         paginator = Paginator(range(0, num_items), page_size)
         page = paginator.page(current_page)
 
-        context = RequestContext(request, {"page": page})
+        context_dict = {"page": page}
+        if frame:
+            context_dict["pagination_frame"] = frame
+        context = RequestContext(request, context_dict)
+
         template_to_render = Template("{% load pagination %}{% pagination page %}")
         return template_to_render.render(context)
 
@@ -30,12 +39,14 @@ class PaginationTagTest(TestCase, BookmarkFactoryMixin):
             html,
         )
 
-    def assertPrevLink(self, html: str, page_number: int, href: str = None):
+    def assertPrevLink(
+        self, html: str, page_number: int, href: str = None, frame: str = "_top"
+    ):
         href = href if href else f"/test?page={page_number}"
         self.assertInHTML(
             f"""
             <li class="page-item">
-                <a href="{href}" tabindex="-1">Previous</a>
+                <a href="{href}" tabindex="-1" data-turbo-frame="{frame}">Previous</a>
             </li>
             """,
             html,
@@ -51,12 +62,14 @@ class PaginationTagTest(TestCase, BookmarkFactoryMixin):
             html,
         )
 
-    def assertNextLink(self, html: str, page_number: int, href: str = None):
+    def assertNextLink(
+        self, html: str, page_number: int, href: str = None, frame: str = "_top"
+    ):
         href = href if href else f"/test?page={page_number}"
         self.assertInHTML(
             f"""
             <li class="page-item">
-                <a href="{href}" tabindex="-1">Next</a>
+                <a href="{href}" tabindex="-1" data-turbo-frame="{frame}">Next</a>
             </li>
             """,
             html,
@@ -69,13 +82,14 @@ class PaginationTagTest(TestCase, BookmarkFactoryMixin):
         active: bool,
         count: int = 1,
         href: str = None,
+        frame: str = "_top",
     ):
         active_class = "active" if active else ""
         href = href if href else f"/test?page={page_number}"
         self.assertInHTML(
             f"""
             <li class="page-item {active_class}">
-                <a href="{href}">{page_number}</a>
+                <a href="{href}" data-turbo-frame="{frame}">{page_number}</a>
             </li>
             """,
             html,
@@ -188,3 +202,10 @@ class PaginationTagTest(TestCase, BookmarkFactoryMixin):
         self.assertPageLink(rendered_template, 1, False, href="/test?page=1")
         self.assertPageLink(rendered_template, 2, True, href="/test?page=2")
         self.assertNextLink(rendered_template, 3, href="/test?page=3")
+
+    def test_respects_pagination_frame(self):
+        rendered_template = self.render_template(100, 10, 2, frame="my_frame")
+        self.assertPrevLink(rendered_template, 1, frame="my_frame")
+        self.assertPageLink(rendered_template, 1, False, frame="my_frame")
+        self.assertPageLink(rendered_template, 2, True, frame="my_frame")
+        self.assertNextLink(rendered_template, 3, frame="my_frame")

bookmarks/tests/test_tags_edit_view.py

@@ -90,20 +90,17 @@ class TagsEditViewTestCase(TestCase, BookmarkFactoryMixin):
         tag2.refresh_from_db()
         self.assertEqual(tag2.name, "tag1")
 
-    def test_update_shows_success_message(self):
+    def test_update_tag_preserves_query_parameters(self):
         tag = self.setup_tag(name="old_name")
 
-        response = self.client.post(
+        url = (
-            reverse("linkding:tags.edit", args=[tag.id]),
+            reverse("linkding:tags.edit", args=[tag.id])
-            {"name": "new_name"},
+            + "?search=search&unused=true&page=2&sort=name-desc"
-            follow=True,
         )
+        response = self.client.post(url, {"name": "new_name"})
 
-        self.assertInHTML(
+        expected_redirect = (
-            """
+            reverse("linkding:tags.index")
-            <div class="toast toast-success" role="alert">
+            + "?search=search&unused=true&page=2&sort=name-desc"
-                Tag "new_name" updated successfully.
-            </div>
-        """,
-            response.content.decode(),
         )
+        self.assertRedirects(response, expected_redirect)

bookmarks/tests/test_tags_index_view.py

@@ -153,24 +153,6 @@ class TagsIndexViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin):
         self.assertRedirects(response, reverse("linkding:tags.index"))
         self.assertFalse(Tag.objects.filter(id=tag.id).exists())
 
-    def test_tag_delete_action_shows_success_message(self):
-        tag = self.setup_tag(name="tag_to_delete")
-
-        response = self.client.post(
-            reverse("linkding:tags.index"), {"delete_tag": tag.id}, follow=True
-        )
-
-        self.assertEqual(response.status_code, 200)
-
-        self.assertInHTML(
-            """
-            <div class="toast toast-success" role="alert">
-                Tag "tag_to_delete" deleted successfully.
-            </div>
-        """,
-            response.content.decode(),
-        )
-
     def test_tag_delete_action_preserves_query_parameters(self):
         tag = self.setup_tag(name="search_tag")
 

bookmarks/tests_e2e/e2e_test_tag_management.py

@@ -113,9 +113,6 @@ class TagManagementE2ETestCase(LinkdingE2ETestCase):
         # Verify modal is closed
         expect(modal).not_to_be_visible()
 
-        # Verify the success message is shown
-        self.verify_success_message('Tag "new-name" updated successfully.')
-
         # Verify the updated tag is shown in the list
         expect(self.locate_tag_row("new-name")).to_be_visible()
         expect(self.locate_tag_row("old-name")).not_to_be_visible()
@@ -157,6 +154,89 @@ class TagManagementE2ETestCase(LinkdingE2ETestCase):
         tag.refresh_from_db()
         self.assertEqual(tag.name, "tag-to-edit")
 
+    def test_edit_tag_preserves_query_and_scroll_position(self):
+        # Create enough tags to have multiple pages (50 per page)
+        for i in range(70):
+            self.setup_tag(name=f"test-tag-{i:02d}")
+
+        # Open tags page 2 with search query
+        url = reverse("linkding:tags.index") + "?search=test&page=2"
+        self.open(url)
+
+        # Verify we're on page 2
+        expect(self.locate_tag_row("test-tag-00")).not_to_be_visible()
+        expect(self.locate_tag_row("test-tag-50")).to_be_visible()
+        expect(self.locate_tag_row("test-tag-60")).to_be_visible()
+
+        # Scroll down
+        self.page.evaluate("window.scrollTo(0, 300)")
+        initial_scroll = self.page.evaluate("window.scrollY")
+        self.assertGreater(initial_scroll, 0)
+
+        # Edit tag
+        tag_row = self.locate_tag_row("test-tag-55")
+        tag_row.get_by_role("link", name="Edit").click()
+
+        modal = self.locate_tag_modal()
+
+        name_input = modal.get_by_label("Name")
+        name_input.fill("test-tag-55-edited")
+
+        modal.get_by_text("Save").click()
+
+        expect(modal).not_to_be_visible()
+
+        # Verify query parameters and scroll position are preserved
+        current_url = self.page.url
+        self.assertIn("search=test", current_url)
+        self.assertIn("page=2", current_url)
+
+        expect(self.locate_tag_row("test-tag-00")).not_to_be_visible()
+        expect(self.locate_tag_row("test-tag-50")).to_be_visible()
+        expect(self.locate_tag_row("test-tag-55-edited")).to_be_visible()
+        expect(self.locate_tag_row("test-tag-60")).to_be_visible()
+
+        final_scroll = self.page.evaluate("window.scrollY")
+        self.assertEqual(initial_scroll, final_scroll)
+
+    def test_delete_tag_preserves_query_and_scroll_position(self):
+        # Create enough tags to have multiple pages (50 per page)
+        for i in range(70):
+            self.setup_tag(name=f"test-tag-{i:02d}")
+
+        # Open tags page 2 with search query
+        url = reverse("linkding:tags.index") + "?search=test&page=2"
+        self.open(url)
+
+        # Verify we're on page 2
+        expect(self.locate_tag_row("test-tag-00")).not_to_be_visible()
+        expect(self.locate_tag_row("test-tag-50")).to_be_visible()
+        expect(self.locate_tag_row("test-tag-55")).to_be_visible()
+        expect(self.locate_tag_row("test-tag-60")).to_be_visible()
+
+        # Scroll down
+        self.page.evaluate("window.scrollTo(0, 300)")
+        initial_scroll = self.page.evaluate("window.scrollY")
+        self.assertGreater(initial_scroll, 0)
+
+        # Delete tag
+        tag_row = self.locate_tag_row("test-tag-55")
+        tag_row.get_by_role("button", name="Remove").click()
+        self.locate_confirm_dialog().get_by_text("Confirm").click()
+
+        # Verify query parameters and scroll position are preserved
+        current_url = self.page.url
+        self.assertIn("search=test", current_url)
+        self.assertIn("page=2", current_url)
+
+        expect(self.locate_tag_row("test-tag-00")).not_to_be_visible()
+        expect(self.locate_tag_row("test-tag-50")).to_be_visible()
+        expect(self.locate_tag_row("test-tag-55")).not_to_be_visible()
+        expect(self.locate_tag_row("test-tag-60")).to_be_visible()
+
+        final_scroll = self.page.evaluate("window.scrollY")
+        self.assertEqual(initial_scroll, final_scroll)
+
     def test_merge_tags(self):
         target_tag = self.setup_tag(name="target-tag")
         merge_tag1 = self.setup_tag(name="merge-tag1")

bookmarks/views/tags.py

@@ -10,6 +10,7 @@ from django.urls import reverse
 from bookmarks.forms import TagForm, TagMergeForm
 from bookmarks.models import Bookmark, Tag
 from bookmarks.type_defs import HttpRequest
+from bookmarks.utils import redirect_with_query
 from bookmarks.views import turbo
 
 
@@ -18,15 +19,8 @@ def tags_index(request: HttpRequest):
     if request.method == "POST" and "delete_tag" in request.POST:
         tag_id = request.POST.get("delete_tag")
         tag = get_object_or_404(Tag, id=tag_id, owner=request.user)
-        tag_name = tag.name
         tag.delete()
-        messages.success(request, f'Tag "{tag_name}" deleted successfully.')
+        return redirect_with_query(request, reverse("linkding:tags.index"))
-
-        redirect_url = reverse("linkding:tags.index")
-        if request.GET:
-            redirect_url += "?" + request.GET.urlencode()
-
-        return HttpResponseRedirect(redirect_url)
 
     search = request.GET.get("search", "").strip()
     unused_only = request.GET.get("unused", "") == "true"
@@ -100,8 +94,7 @@ def tag_edit(request: HttpRequest, tag_id: int):
     if request.method == "POST":
         if form.is_valid():
             form.save()
-            messages.success(request, f'Tag "{tag.name}" updated successfully.')
+            return redirect_with_query(request, reverse("linkding:tags.index"))
-            return HttpResponseRedirect(reverse("linkding:tags.index"))
         else:
             return turbo.stream(
                 turbo.replace(