From ffaaf0521d209143298e3b6d3e15a990e79efafe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sascha=20I=C3=9Fbr=C3=BCcker?= Date: Mon, 16 Sep 2024 12:48:19 +0200 Subject: [PATCH] Speed up response times for certain actions (#829) * return updated HTML from bookmark actions * open details through URL * fix details update * improve modal behavior * use a frame * make behaviors properly destroy themselves * remove page and details params from tag urls * use separate behavior for details and tags * remove separate details view * make it work with other views * add asset actions * remove asset refresh for now * remove details partial * fix tests * remove old partials * update tests * cache and reuse tags * extract search autocomplete behavior * remove details param from pagination * fix tests * only return details modal when navigating in frame * fix link target * remove unused behaviors * use auto submit behavior for user select * fix import --- .../e2e/e2e_test_bookmark_details_modal.py | 7 +- .../e2e/e2e_test_bookmark_details_view.py | 37 -- bookmarks/e2e/e2e_test_tag_cloud_modal.py | 6 +- bookmarks/frontend/api.js | 5 +- bookmarks/frontend/behaviors/bookmark-page.js | 13 +- bookmarks/frontend/behaviors/bulk-edit.js | 29 +- .../frontend/behaviors/confirm-button.js | 20 +- bookmarks/frontend/behaviors/details-modal.js | 62 +++ bookmarks/frontend/behaviors/dropdown.js | 20 +- bookmarks/frontend/behaviors/fetch.js | 48 -- bookmarks/frontend/behaviors/form.js | 81 ++-- bookmarks/frontend/behaviors/index.js | 48 -- bookmarks/frontend/behaviors/modal.js | 51 -- .../frontend/behaviors/search-autocomplete.js | 41 ++ .../frontend/behaviors/tag-autocomplete.js | 6 +- bookmarks/frontend/behaviors/tag-modal.js | 68 +++ bookmarks/frontend/cache.js | 35 ++ .../components/SearchAutoComplete.svelte | 15 +- .../components/TagAutocomplete.svelte | 18 +- bookmarks/frontend/index.js | 9 +- bookmarks/models.py | 2 +- bookmarks/styles/bookmark-details.css | 21 - bookmarks/styles/theme/modals.css | 10 + bookmarks/templates/bookmarks/archive.html | 22 +- .../templates/bookmarks/bookmark_list.html | 5 +- bookmarks/templates/bookmarks/details.html | 13 - .../templates/bookmarks/details/actions.html | 16 - .../templates/bookmarks/details/assets.html | 9 +- .../templates/bookmarks/details/form.html | 8 +- .../templates/bookmarks/details/modal.html | 47 ++ .../templates/bookmarks/details/title.html | 3 - .../templates/bookmarks/details_modal.html | 30 -- bookmarks/templates/bookmarks/head.html | 40 ++ bookmarks/templates/bookmarks/index.html | 23 +- bookmarks/templates/bookmarks/layout.html | 39 +- bookmarks/templates/bookmarks/pagination.html | 16 +- bookmarks/templates/bookmarks/search.html | 48 +- bookmarks/templates/bookmarks/shared.html | 22 +- bookmarks/templates/bookmarks/tag_modal.html | 21 - .../updates/bookmark_view_stream.html | 21 + .../updates/details-modal-frame.html | 10 + .../templates/bookmarks/user_select.html | 9 +- .../templates/settings/integrations.html | 4 +- bookmarks/templatetags/bookmarks.py | 7 +- bookmarks/templatetags/pagination.py | 41 +- bookmarks/templatetags/shared.py | 17 +- bookmarks/tests/helpers.py | 70 +++ bookmarks/tests/test_bookmark_action_view.py | 299 ++++++++++-- .../tests/test_bookmark_archived_view.py | 98 +--- .../tests/test_bookmark_details_modal.py | 453 ++++++------------ bookmarks/tests/test_bookmark_details_view.py | 6 - bookmarks/tests/test_bookmark_index_view.py | 102 +--- bookmarks/tests/test_bookmark_search_tag.py | 26 +- bookmarks/tests/test_bookmark_shared_view.py | 78 +-- .../tests/test_bookmarks_list_template.py | 30 +- bookmarks/tests/test_pagination_tag.py | 9 + bookmarks/tests/test_tag_cloud_template.py | 43 +- bookmarks/urls.py | 62 --- bookmarks/utils.py | 10 + bookmarks/views/bookmarks.py | 203 ++++---- bookmarks/views/{partials => }/contexts.py | 112 +++-- bookmarks/views/partials.py | 40 ++ bookmarks/views/partials/__init__.py | 76 --- bookmarks/views/turbo.py | 19 + siteroot/settings/dev.py | 4 +- 65 files changed, 1419 insertions(+), 1444 deletions(-) delete mode 100644 bookmarks/e2e/e2e_test_bookmark_details_view.py create mode 100644 bookmarks/frontend/behaviors/details-modal.js delete mode 100644 bookmarks/frontend/behaviors/fetch.js delete mode 100644 bookmarks/frontend/behaviors/modal.js create mode 100644 bookmarks/frontend/behaviors/search-autocomplete.js create mode 100644 bookmarks/frontend/behaviors/tag-modal.js create mode 100644 bookmarks/frontend/cache.js delete mode 100644 bookmarks/templates/bookmarks/details.html delete mode 100644 bookmarks/templates/bookmarks/details/actions.html create mode 100644 bookmarks/templates/bookmarks/details/modal.html delete mode 100644 bookmarks/templates/bookmarks/details/title.html delete mode 100644 bookmarks/templates/bookmarks/details_modal.html create mode 100644 bookmarks/templates/bookmarks/head.html delete mode 100644 bookmarks/templates/bookmarks/tag_modal.html create mode 100644 bookmarks/templates/bookmarks/updates/bookmark_view_stream.html create mode 100644 bookmarks/templates/bookmarks/updates/details-modal-frame.html delete mode 100644 bookmarks/tests/test_bookmark_details_view.py rename bookmarks/views/{partials => }/contexts.py (84%) create mode 100644 bookmarks/views/partials.py delete mode 100644 bookmarks/views/partials/__init__.py create mode 100644 bookmarks/views/turbo.py diff --git a/bookmarks/e2e/e2e_test_bookmark_details_modal.py b/bookmarks/e2e/e2e_test_bookmark_details_modal.py index 73872e5..947a48f 100644 --- a/bookmarks/e2e/e2e_test_bookmark_details_modal.py +++ b/bookmarks/e2e/e2e_test_bookmark_details_modal.py @@ -121,8 +121,9 @@ class BookmarkDetailsModalE2ETestCase(LinkdingE2ETestCase): with self.page.expect_navigation(): details_modal.get_by_text("Edit").click() - # Cancel edit, verify return url - with self.page.expect_navigation(url=self.live_server_url + url): + # Cancel edit, verify return to details url + details_url = url + f"&details={bookmark.id}" + with self.page.expect_navigation(url=self.live_server_url + details_url): self.page.get_by_text("Nevermind").click() def test_delete(self): @@ -167,7 +168,7 @@ class BookmarkDetailsModalE2ETestCase(LinkdingE2ETestCase): # Has new snapshots expect(snapshot).to_be_visible() - # Create snapshot + # Remove snapshot asset_list.get_by_text("Remove", exact=False).click() asset_list.get_by_text("Confirm", exact=False).click() diff --git a/bookmarks/e2e/e2e_test_bookmark_details_view.py b/bookmarks/e2e/e2e_test_bookmark_details_view.py deleted file mode 100644 index 8b792ff..0000000 --- a/bookmarks/e2e/e2e_test_bookmark_details_view.py +++ /dev/null @@ -1,37 +0,0 @@ -from django.urls import reverse -from playwright.sync_api import sync_playwright - -from bookmarks.e2e.helpers import LinkdingE2ETestCase - - -class BookmarkDetailsViewE2ETestCase(LinkdingE2ETestCase): - def test_edit_return_url(self): - bookmark = self.setup_bookmark() - - with sync_playwright() as p: - self.open(reverse("bookmarks:details", args=[bookmark.id]), p) - - # Navigate to edit page - with self.page.expect_navigation(): - self.page.get_by_text("Edit").click() - - # Cancel edit, verify return url - with self.page.expect_navigation( - url=self.live_server_url - + reverse("bookmarks:details", args=[bookmark.id]) - ): - self.page.get_by_text("Nevermind").click() - - def test_delete_return_url(self): - bookmark = self.setup_bookmark() - - with sync_playwright() as p: - self.open(reverse("bookmarks:details", args=[bookmark.id]), p) - - # Trigger delete, verify return url - # Should probably return to last bookmark list page, but for now just returns to index - with self.page.expect_navigation( - url=self.live_server_url + reverse("bookmarks:index") - ): - self.page.get_by_text("Delete...").click() - self.page.get_by_text("Confirm").click() diff --git a/bookmarks/e2e/e2e_test_tag_cloud_modal.py b/bookmarks/e2e/e2e_test_tag_cloud_modal.py index 19793eb..7d60dcc 100644 --- a/bookmarks/e2e/e2e_test_tag_cloud_modal.py +++ b/bookmarks/e2e/e2e_test_tag_cloud_modal.py @@ -1,9 +1,7 @@ -from django.test import override_settings from django.urls import reverse -from playwright.sync_api import sync_playwright, expect, Locator +from playwright.sync_api import sync_playwright, expect from bookmarks.e2e.helpers import LinkdingE2ETestCase -from bookmarks.models import Bookmark class TagCloudModalE2ETestCase(LinkdingE2ETestCase): @@ -26,7 +24,7 @@ class TagCloudModalE2ETestCase(LinkdingE2ETestCase): # verify modal is visible modal = page.locator(".modal") expect(modal).to_be_visible() - expect(modal.locator(".modal-title")).to_have_text("Tags") + expect(modal.locator("h2")).to_have_text("Tags") # close with close button modal.locator("button.close").click() diff --git a/bookmarks/frontend/api.js b/bookmarks/frontend/api.js index dfe682f..2a47d88 100644 --- a/bookmarks/frontend/api.js +++ b/bookmarks/frontend/api.js @@ -1,4 +1,4 @@ -export class ApiClient { +export class Api { constructor(baseUrl) { this.baseUrl = baseUrl; } @@ -27,3 +27,6 @@ export class ApiClient { .then((data) => data.results); } } + +const apiBaseUrl = document.documentElement.dataset.apiBaseUrl || ""; +export const api = new Api(apiBaseUrl); diff --git a/bookmarks/frontend/behaviors/bookmark-page.js b/bookmarks/frontend/behaviors/bookmark-page.js index 6f77973..681f5f6 100644 --- a/bookmarks/frontend/behaviors/bookmark-page.js +++ b/bookmarks/frontend/behaviors/bookmark-page.js @@ -5,9 +5,10 @@ class BookmarkItem extends Behavior { super(element); // Toggle notes - const notesToggle = element.querySelector(".toggle-notes"); - if (notesToggle) { - notesToggle.addEventListener("click", this.onToggleNotes.bind(this)); + this.onToggleNotes = this.onToggleNotes.bind(this); + this.notesToggle = element.querySelector(".toggle-notes"); + if (this.notesToggle) { + this.notesToggle.addEventListener("click", this.onToggleNotes); } // Add tooltip to title if it is truncated @@ -20,6 +21,12 @@ class BookmarkItem extends Behavior { }); } + destroy() { + if (this.notesToggle) { + this.notesToggle.removeEventListener("click", this.onToggleNotes); + } + } + onToggleNotes(event) { event.preventDefault(); event.stopPropagation(); diff --git a/bookmarks/frontend/behaviors/bulk-edit.js b/bookmarks/frontend/behaviors/bulk-edit.js index 45ee131..e5b5578 100644 --- a/bookmarks/frontend/behaviors/bulk-edit.js +++ b/bookmarks/frontend/behaviors/bulk-edit.js @@ -13,12 +13,13 @@ class BulkEdit extends Behavior { this.onActionSelected = this.onActionSelected.bind(this); this.init(); - // Reset when bookmarks are refreshed - document.addEventListener("refresh-bookmark-list-done", this.init); + // Reset when bookmarks are updated + document.addEventListener("bookmark-list-updated", this.init); } destroy() { - document.removeEventListener("refresh-bookmark-list-done", this.init); + this.removeListeners(); + document.removeEventListener("bookmark-list-updated", this.init); } init() { @@ -36,13 +37,9 @@ class BulkEdit extends Behavior { this.element.querySelectorAll(".bulk-edit-checkbox:not(.all) input"), ); - // Remove previous listeners if elements are the same - this.activeToggle.removeEventListener("click", this.onToggleActive); - this.actionSelect.removeEventListener("change", this.onActionSelected); - this.allCheckbox.removeEventListener("change", this.onToggleAll); - this.bookmarkCheckboxes.forEach((checkbox) => { - checkbox.removeEventListener("change", this.onToggleBookmark); - }); + // Add listeners, ensure there are no dupes by possibly removing existing listeners + this.removeListeners(); + this.addListeners(); // Reset checkbox states this.reset(); @@ -52,8 +49,9 @@ class BulkEdit extends Behavior { const total = totalHolder?.dataset.bookmarksTotal || 0; const totalSpan = this.selectAcross.querySelector("span.total"); totalSpan.textContent = total; + } - // Add new listeners + addListeners() { this.activeToggle.addEventListener("click", this.onToggleActive); this.actionSelect.addEventListener("change", this.onActionSelected); this.allCheckbox.addEventListener("change", this.onToggleAll); @@ -62,6 +60,15 @@ class BulkEdit extends Behavior { }); } + removeListeners() { + this.activeToggle.removeEventListener("click", this.onToggleActive); + this.actionSelect.removeEventListener("change", this.onActionSelected); + this.allCheckbox.removeEventListener("change", this.onToggleAll); + this.bookmarkCheckboxes.forEach((checkbox) => { + checkbox.removeEventListener("change", this.onToggleBookmark); + }); + } + onToggleActive() { this.active = !this.active; if (this.active) { diff --git a/bookmarks/frontend/behaviors/confirm-button.js b/bookmarks/frontend/behaviors/confirm-button.js index 454c3cf..fb213ee 100644 --- a/bookmarks/frontend/behaviors/confirm-button.js +++ b/bookmarks/frontend/behaviors/confirm-button.js @@ -3,20 +3,14 @@ import { Behavior, registerBehavior } from "./index"; class ConfirmButtonBehavior extends Behavior { constructor(element) { super(element); - element.dataset.type = element.type; - element.dataset.name = element.name; - element.dataset.value = element.value; - element.removeAttribute("type"); - element.removeAttribute("name"); - element.removeAttribute("value"); - element.addEventListener("click", this.onClick.bind(this)); + + this.onClick = this.onClick.bind(this); + element.addEventListener("click", this.onClick); } destroy() { this.reset(); - this.element.setAttribute("type", this.element.dataset.type); - this.element.setAttribute("name", this.element.dataset.name); - this.element.setAttribute("value", this.element.dataset.value); + this.element.removeEventListener("click", this.onClick); } onClick(event) { @@ -56,9 +50,9 @@ class ConfirmButtonBehavior extends Behavior { cancelButton.addEventListener("click", this.reset.bind(this)); const confirmButton = document.createElement(this.element.nodeName); - confirmButton.type = this.element.dataset.type; - confirmButton.name = this.element.dataset.name; - confirmButton.value = this.element.dataset.value; + confirmButton.type = this.element.type; + confirmButton.name = this.element.name; + confirmButton.value = this.element.value; confirmButton.innerText = question ? "Yes" : "Confirm"; confirmButton.className = buttonClasses; confirmButton.addEventListener("click", this.reset.bind(this)); diff --git a/bookmarks/frontend/behaviors/details-modal.js b/bookmarks/frontend/behaviors/details-modal.js new file mode 100644 index 0000000..5646969 --- /dev/null +++ b/bookmarks/frontend/behaviors/details-modal.js @@ -0,0 +1,62 @@ +import { Behavior, registerBehavior } from "./index"; + +class DetailsModalBehavior extends Behavior { + constructor(element) { + super(element); + + this.onClose = this.onClose.bind(this); + this.onKeyDown = this.onKeyDown.bind(this); + + this.overlayLink = element.querySelector("a:has(.modal-overlay)"); + this.buttonLink = element.querySelector("a:has(button.close)"); + + this.overlayLink.addEventListener("click", this.onClose); + this.buttonLink.addEventListener("click", this.onClose); + document.addEventListener("keydown", this.onKeyDown); + } + + destroy() { + this.overlayLink.removeEventListener("click", this.onClose); + this.buttonLink.removeEventListener("click", this.onClose); + document.removeEventListener("keydown", this.onKeyDown); + } + + onKeyDown(event) { + // Skip if event occurred within an input element + const targetNodeName = event.target.nodeName; + const isInputTarget = + targetNodeName === "INPUT" || + targetNodeName === "SELECT" || + targetNodeName === "TEXTAREA"; + + if (isInputTarget) { + return; + } + + if (event.key === "Escape") { + this.onClose(event); + } + } + + onClose(event) { + event.preventDefault(); + this.element.classList.add("closing"); + this.element.addEventListener( + "animationend", + (event) => { + if (event.animationName === "fade-out") { + this.element.remove(); + + const closeUrl = this.overlayLink.href; + Turbo.visit(closeUrl, { + action: "replace", + frame: "details-modal", + }); + } + }, + { once: true }, + ); + } +} + +registerBehavior("ld-details-modal", DetailsModalBehavior); diff --git a/bookmarks/frontend/behaviors/dropdown.js b/bookmarks/frontend/behaviors/dropdown.js index 73b03c2..954ced8 100644 --- a/bookmarks/frontend/behaviors/dropdown.js +++ b/bookmarks/frontend/behaviors/dropdown.js @@ -4,20 +4,16 @@ class DropdownBehavior extends Behavior { constructor(element) { super(element); this.opened = false; + this.onClick = this.onClick.bind(this); this.onOutsideClick = this.onOutsideClick.bind(this); - const toggle = element.querySelector(".dropdown-toggle"); - toggle.addEventListener("click", () => { - if (this.opened) { - this.close(); - } else { - this.open(); - } - }); + this.toggle = element.querySelector(".dropdown-toggle"); + this.toggle.addEventListener("click", this.onClick); } destroy() { this.close(); + this.toggle.removeEventListener("click", this.onClick); } open() { @@ -30,6 +26,14 @@ class DropdownBehavior extends Behavior { document.removeEventListener("click", this.onOutsideClick); } + onClick() { + if (this.opened) { + this.close(); + } else { + this.open(); + } + } + onOutsideClick(event) { if (!this.element.contains(event.target)) { this.close(); diff --git a/bookmarks/frontend/behaviors/fetch.js b/bookmarks/frontend/behaviors/fetch.js deleted file mode 100644 index 32b7184..0000000 --- a/bookmarks/frontend/behaviors/fetch.js +++ /dev/null @@ -1,48 +0,0 @@ -import { Behavior, fireEvents, registerBehavior, swap } from "./index"; - -class FetchBehavior extends Behavior { - constructor(element) { - super(element); - - const eventName = element.getAttribute("ld-on"); - const interval = parseInt(element.getAttribute("ld-interval")) * 1000; - - this.onFetch = this.onFetch.bind(this); - this.onInterval = this.onInterval.bind(this); - - element.addEventListener(eventName, this.onFetch); - if (interval) { - this.intervalId = setInterval(this.onInterval, interval); - } - } - - destroy() { - if (this.intervalId) { - clearInterval(this.intervalId); - } - } - - async onFetch(maybeEvent) { - if (maybeEvent) { - maybeEvent.preventDefault(); - } - const url = this.element.getAttribute("ld-fetch"); - const html = await fetch(url).then((response) => response.text()); - - const target = this.element.getAttribute("ld-target"); - const select = this.element.getAttribute("ld-select"); - swap(this.element, html, { target, select }); - - const events = this.element.getAttribute("ld-fire"); - fireEvents(events); - } - - onInterval() { - if (Behavior.interacting) { - return; - } - this.onFetch(); - } -} - -registerBehavior("ld-fetch", FetchBehavior); diff --git a/bookmarks/frontend/behaviors/form.js b/bookmarks/frontend/behaviors/form.js index 3b6dec2..ca47555 100644 --- a/bookmarks/frontend/behaviors/form.js +++ b/bookmarks/frontend/behaviors/form.js @@ -1,64 +1,55 @@ -import { Behavior, fireEvents, registerBehavior } from "./index"; - -class FormBehavior extends Behavior { - constructor(element) { - super(element); - - element.addEventListener("submit", this.onSubmit.bind(this)); - } - - async onSubmit(event) { - event.preventDefault(); - - const url = this.element.action; - const formData = new FormData(this.element); - if (event.submitter) { - formData.append(event.submitter.name, event.submitter.value); - } - - await fetch(url, { - method: "POST", - body: formData, - redirect: "manual", // ignore redirect - }); - - const events = this.element.getAttribute("ld-fire"); - if (fireEvents) { - fireEvents(events); - } - } -} +import { Behavior, registerBehavior } from "./index"; class AutoSubmitBehavior extends Behavior { constructor(element) { super(element); - element.addEventListener("change", () => { - const form = element.closest("form"); - form.dispatchEvent(new Event("submit", { cancelable: true })); - }); + this.submit = this.submit.bind(this); + element.addEventListener("change", this.submit); + } + + destroy() { + this.element.removeEventListener("change", this.submit); + } + + submit() { + this.element.closest("form").requestSubmit(); } } class UploadButton extends Behavior { constructor(element) { super(element); + this.fileInput = element.nextElementSibling; - const fileInput = element.nextElementSibling; + this.onClick = this.onClick.bind(this); + this.onChange = this.onChange.bind(this); - element.addEventListener("click", () => { - fileInput.click(); - }); + element.addEventListener("click", this.onClick); + this.fileInput.addEventListener("change", this.onChange); + } - fileInput.addEventListener("change", () => { - const form = fileInput.closest("form"); - const event = new Event("submit", { cancelable: true }); - event.submitter = element; - form.dispatchEvent(event); - }); + destroy() { + this.element.removeEventListener("click", this.onClick); + this.fileInput.removeEventListener("change", this.onChange); + } + + onClick(event) { + event.preventDefault(); + this.fileInput.click(); + } + + onChange() { + // Check if the file input has a file selected + if (!this.fileInput.files.length) { + return; + } + const form = this.fileInput.closest("form"); + form.requestSubmit(this.element); + // remove selected file so it doesn't get submitted again + this.fileInput.value = ""; } } -registerBehavior("ld-form", FormBehavior); registerBehavior("ld-auto-submit", AutoSubmitBehavior); registerBehavior("ld-upload-button", UploadButton); diff --git a/bookmarks/frontend/behaviors/index.js b/bookmarks/frontend/behaviors/index.js index 0dfe49c..c973ce2 100644 --- a/bookmarks/frontend/behaviors/index.js +++ b/bookmarks/frontend/behaviors/index.js @@ -103,51 +103,3 @@ export function destroyBehaviors(element) { }); }); } - -export function swap(element, html, options) { - const dom = new DOMParser().parseFromString(html, "text/html"); - - let targetElement = element; - let strategy = "innerHTML"; - if (options.target) { - const parts = options.target.split("|"); - targetElement = - parts[0] === "self" ? element : document.querySelector(parts[0]); - strategy = parts[1] || "innerHTML"; - } - - let contents = Array.from(dom.body.children); - if (options.select) { - contents = Array.from(dom.querySelectorAll(options.select)); - } - - switch (strategy) { - case "append": - targetElement.append(...contents); - break; - case "outerHTML": - targetElement.parentElement.replaceChild(contents[0], targetElement); - break; - case "innerHTML": - default: - Array.from(targetElement.children).forEach((child) => { - child.remove(); - }); - targetElement.append(...contents); - } -} - -export function fireEvents(events) { - if (!events) { - return; - } - events.split(",").forEach((eventName) => { - const targets = Array.from( - document.querySelectorAll(`[ld-on='${eventName}']`), - ); - targets.push(document); - targets.forEach((target) => { - target.dispatchEvent(new CustomEvent(eventName)); - }); - }); -} diff --git a/bookmarks/frontend/behaviors/modal.js b/bookmarks/frontend/behaviors/modal.js deleted file mode 100644 index f22ed24..0000000 --- a/bookmarks/frontend/behaviors/modal.js +++ /dev/null @@ -1,51 +0,0 @@ -import { Behavior, registerBehavior } from "./index"; - -class ModalBehavior extends Behavior { - constructor(element) { - super(element); - - this.onClose = this.onClose.bind(this); - this.onKeyDown = this.onKeyDown.bind(this); - - const modalOverlay = element.querySelector(".modal-overlay"); - const closeButton = element.querySelector("button.close"); - modalOverlay.addEventListener("click", this.onClose); - closeButton.addEventListener("click", this.onClose); - - document.addEventListener("keydown", this.onKeyDown); - } - - destroy() { - document.removeEventListener("keydown", this.onKeyDown); - } - - onKeyDown(event) { - // Skip if event occurred within an input element - const targetNodeName = event.target.nodeName; - const isInputTarget = - targetNodeName === "INPUT" || - targetNodeName === "SELECT" || - targetNodeName === "TEXTAREA"; - - if (isInputTarget) { - return; - } - - if (event.key === "Escape") { - event.preventDefault(); - this.onClose(); - } - } - - onClose() { - document.removeEventListener("keydown", this.onKeyDown); - this.element.classList.add("closing"); - this.element.addEventListener("animationend", (event) => { - if (event.animationName === "fade-out") { - this.element.remove(); - } - }); - } -} - -registerBehavior("ld-modal", ModalBehavior); diff --git a/bookmarks/frontend/behaviors/search-autocomplete.js b/bookmarks/frontend/behaviors/search-autocomplete.js new file mode 100644 index 0000000..d7a686f --- /dev/null +++ b/bookmarks/frontend/behaviors/search-autocomplete.js @@ -0,0 +1,41 @@ +import { Behavior, registerBehavior } from "./index"; +import SearchAutoCompleteComponent from "../components/SearchAutoComplete.svelte"; + +class SearchAutocomplete extends Behavior { + constructor(element) { + super(element); + const input = element.querySelector("input"); + if (!input) { + console.warn("SearchAutocomplete: input element not found"); + return; + } + + const container = document.createElement("div"); + + new SearchAutoCompleteComponent({ + target: container, + props: { + name: "q", + placeholder: input.getAttribute("placeholder") || "", + value: input.value, + linkTarget: input.dataset.linkTarget, + mode: input.dataset.mode, + search: { + user: input.dataset.user, + shared: input.dataset.shared, + unread: input.dataset.unread, + }, + }, + }); + + this.input = input; + this.autocomplete = container.firstElementChild; + input.replaceWith(this.autocomplete); + } + + destroy() { + this.autocomplete.replaceWith(this.input); + } +} + +registerBehavior("ld-search-autocomplete", SearchAutocomplete); diff --git a/bookmarks/frontend/behaviors/tag-autocomplete.js b/bookmarks/frontend/behaviors/tag-autocomplete.js index 755ba3c..7221582 100644 --- a/bookmarks/frontend/behaviors/tag-autocomplete.js +++ b/bookmarks/frontend/behaviors/tag-autocomplete.js @@ -1,19 +1,16 @@ import { Behavior, registerBehavior } from "./index"; import TagAutoCompleteComponent from "../components/TagAutocomplete.svelte"; -import { ApiClient } from "../api"; class TagAutocomplete extends Behavior { constructor(element) { super(element); const input = element.querySelector("input"); if (!input) { - console.warning("TagAutocomplete: input element not found"); + console.warn("TagAutocomplete: input element not found"); return; } const container = document.createElement("div"); - const apiBaseUrl = document.documentElement.dataset.apiBaseUrl || ""; - const apiClient = new ApiClient(apiBaseUrl); new TagAutoCompleteComponent({ target: container, @@ -22,7 +19,6 @@ class TagAutocomplete extends Behavior { name: input.name, value: input.value, placeholder: input.getAttribute("placeholder") || "", - apiClient: apiClient, variant: input.getAttribute("variant"), }, }); diff --git a/bookmarks/frontend/behaviors/tag-modal.js b/bookmarks/frontend/behaviors/tag-modal.js new file mode 100644 index 0000000..9963bff --- /dev/null +++ b/bookmarks/frontend/behaviors/tag-modal.js @@ -0,0 +1,68 @@ +import { Behavior, registerBehavior } from "./index"; + +class TagModalBehavior extends Behavior { + constructor(element) { + super(element); + + this.onClick = this.onClick.bind(this); + this.onClose = this.onClose.bind(this); + + element.addEventListener("click", this.onClick); + } + + destroy() { + this.onClose(); + this.element.removeEventListener("click", this.onClick); + } + + onClick() { + const modal = document.createElement("div"); + modal.classList.add("modal", "active"); + modal.innerHTML = ` +
+
+
+

Tags

+ +
+
+
+
+
+ `; + + const tagCloud = document.querySelector(".tag-cloud"); + const tagCloudContainer = tagCloud.parentElement; + + const content = modal.querySelector(".content"); + content.appendChild(tagCloud); + + const overlay = modal.querySelector(".modal-overlay"); + const closeButton = modal.querySelector(".close"); + overlay.addEventListener("click", this.onClose); + closeButton.addEventListener("click", this.onClose); + + this.modal = modal; + this.tagCloud = tagCloud; + this.tagCloudContainer = tagCloudContainer; + document.body.appendChild(modal); + } + + onClose() { + if (!this.modal) { + return; + } + + this.modal.remove(); + this.tagCloudContainer.appendChild(this.tagCloud); + } +} + +registerBehavior("ld-tag-modal", TagModalBehavior); diff --git a/bookmarks/frontend/cache.js b/bookmarks/frontend/cache.js new file mode 100644 index 0000000..0bcf9e5 --- /dev/null +++ b/bookmarks/frontend/cache.js @@ -0,0 +1,35 @@ +import { api } from "./api.js"; + +class Cache { + constructor(api) { + this.api = api; + + // Reset cached tags after a form submission + document.addEventListener("turbo:submit-end", () => { + this.tagsPromise = null; + }); + } + + getTags() { + if (!this.tagsPromise) { + this.tagsPromise = this.api + .getTags({ + limit: 5000, + offset: 0, + }) + .then((tags) => + tags.sort((left, right) => + left.name.toLowerCase().localeCompare(right.name.toLowerCase()), + ), + ) + .catch((e) => { + console.warn("Cache: Error loading tags", e); + return []; + }); + } + + return this.tagsPromise; + } +} + +export const cache = new Cache(api); diff --git a/bookmarks/frontend/components/SearchAutoComplete.svelte b/bookmarks/frontend/components/SearchAutoComplete.svelte index 61aade0..4a9cec9 100644 --- a/bookmarks/frontend/components/SearchAutoComplete.svelte +++ b/bookmarks/frontend/components/SearchAutoComplete.svelte @@ -1,5 +1,7 @@ + diff --git a/bookmarks/templates/bookmarks/index.html b/bookmarks/templates/bookmarks/index.html index 279fa5e..b73d8e0 100644 --- a/bookmarks/templates/bookmarks/index.html +++ b/bookmarks/templates/bookmarks/index.html @@ -11,24 +11,19 @@

Bookmarks

- {% bookmark_search bookmark_list.search tag_cloud.tags %} + {% bookmark_search bookmark_list.search %} {% include 'bookmarks/bulk_edit/toggle.html' %} - +
-
{% csrf_token %} {% include 'bookmarks/bulk_edit/bar.html' with disable_actions='bulk_unarchive' %} -
+
{% include 'bookmarks/bookmark_list.html' %}
@@ -39,10 +34,16 @@

Tags

-
+
{% include 'bookmarks/tag_cloud.html' %}
+ + {# Bookmark details #} + + {% if details %} + {% include 'bookmarks/details/modal.html' %} + {% endif %} +
{% endblock %} diff --git a/bookmarks/templates/bookmarks/layout.html b/bookmarks/templates/bookmarks/layout.html index c05e221..678da39 100644 --- a/bookmarks/templates/bookmarks/layout.html +++ b/bookmarks/templates/bookmarks/layout.html @@ -3,44 +3,7 @@ {# Use data attributes as storage for access in static scripts #} - - - - - - - - - - - - - linkding - {# Include specific theme variant based on user profile setting #} - {% if request.user_profile.theme == 'light' %} - - - {% elif request.user_profile.theme == 'dark' %} - - - {% else %} - {# Use auto theme as fallback #} - - - - - {% endif %} - {% if request.user_profile.custom_css %} - - {% endif %} - - {% if not request.global_settings.enable_link_prefetch %} - - {% endif %} - - +{% include 'bookmarks/head.html' %}
diff --git a/bookmarks/templates/bookmarks/pagination.html b/bookmarks/templates/bookmarks/pagination.html index 59f853e..62a6394 100644 --- a/bookmarks/templates/bookmarks/pagination.html +++ b/bookmarks/templates/bookmarks/pagination.html @@ -1,9 +1,9 @@ {% load shared %}
    - {% if page.has_previous %} + {% if prev_link %}
  • - Previous + Previous
    • {% else %}
  • @@ -11,10 +11,10 @@
    • {% endif %} - {% for page_number in visible_page_numbers %} - {% if page_number >= 0 %} -
  • - {{ page_number }} + {% for page_link in page_links %} + {% if page_link %} +
  • + {{ page_link.number }}
    • {% else %}
  • @@ -23,9 +23,9 @@ {% endif %} {% endfor %} - {% if page.has_next %} + {% if next_link %}
  • - Next + Next
    • {% else %}
  • diff --git a/bookmarks/templates/bookmarks/search.html b/bookmarks/templates/bookmarks/search.html index 266416f..ecc533e 100644 --- a/bookmarks/templates/bookmarks/search.html +++ b/bookmarks/templates/bookmarks/search.html @@ -1,9 +1,14 @@ {% load widget_tweaks %} -
    +
- - -{# Replace search input with auto-complete component #} - \ No newline at end of file diff --git a/bookmarks/templates/bookmarks/shared.html b/bookmarks/templates/bookmarks/shared.html index d34ed42..385ef98 100644 --- a/bookmarks/templates/bookmarks/shared.html +++ b/bookmarks/templates/bookmarks/shared.html @@ -11,21 +11,17 @@

Shared bookmarks

- {% bookmark_search bookmark_list.search tag_cloud.tags mode='shared' %} -
- {% csrf_token %} -
+
{% include 'bookmarks/bookmark_list.html' %}
@@ -43,10 +39,16 @@

Tags

-
+
{% include 'bookmarks/tag_cloud.html' %}
+ + {# Bookmark details #} + + {% if details %} + {% include 'bookmarks/details/modal.html' %} + {% endif %} +
{% endblock %} diff --git a/bookmarks/templates/bookmarks/tag_modal.html b/bookmarks/templates/bookmarks/tag_modal.html deleted file mode 100644 index 9549f1e..0000000 --- a/bookmarks/templates/bookmarks/tag_modal.html +++ /dev/null @@ -1,21 +0,0 @@ -
-
-
-
-
Tags
- -
-
-
- {% include 'bookmarks/tag_cloud.html' %} -
-
-
-
diff --git a/bookmarks/templates/bookmarks/updates/bookmark_view_stream.html b/bookmarks/templates/bookmarks/updates/bookmark_view_stream.html new file mode 100644 index 0000000..b009915 --- /dev/null +++ b/bookmarks/templates/bookmarks/updates/bookmark_view_stream.html @@ -0,0 +1,21 @@ + + + + + + + + + + diff --git a/bookmarks/templates/bookmarks/updates/details-modal-frame.html b/bookmarks/templates/bookmarks/updates/details-modal-frame.html new file mode 100644 index 0000000..80c51c4 --- /dev/null +++ b/bookmarks/templates/bookmarks/updates/details-modal-frame.html @@ -0,0 +1,10 @@ + +{% include 'bookmarks/head.html' %} + + + {% if details %} + {% include 'bookmarks/details/modal.html' %} + {% endif %} + + + diff --git a/bookmarks/templates/bookmarks/user_select.html b/bookmarks/templates/bookmarks/user_select.html index 1286702..2f16748 100644 --- a/bookmarks/templates/bookmarks/user_select.html +++ b/bookmarks/templates/bookmarks/user_select.html @@ -6,17 +6,10 @@ {% endfor %}
- {{ form.user|add_class:"form-select" }} + {% render_field form.user class+="form-select" ld-auto-submit="" %}
- diff --git a/bookmarks/templates/settings/integrations.html b/bookmarks/templates/settings/integrations.html index 0c925bf..66d9d6d 100644 --- a/bookmarks/templates/settings/integrations.html +++ b/bookmarks/templates/settings/integrations.html @@ -27,7 +27,7 @@
  • After saving the bookmark the linkding window closes and you are back on your website

    • Drag the following bookmarklet to your browser's toolbar:

    - 📎 Add bookmark @@ -43,7 +43,7 @@ Please treat this token as you would any other credential. Any party with access to this token can access and manage all your bookmarks. If you think that a token was compromised you can revoke (delete) it in the admin panel. + target="_blank" href="{% url 'admin:authtoken_tokenproxy_changelist' %}">admin panel. After deleting the token, a new one will be generated when you reload this settings page.

    diff --git a/bookmarks/templatetags/bookmarks.py b/bookmarks/templatetags/bookmarks.py index af7f929..d7706ce 100644 --- a/bookmarks/templatetags/bookmarks.py +++ b/bookmarks/templatetags/bookmarks.py @@ -6,8 +6,6 @@ from bookmarks.models import ( BookmarkForm, BookmarkSearch, BookmarkSearchForm, - Tag, - build_tag_string, User, ) @@ -34,9 +32,7 @@ def bookmark_form( @register.inclusion_tag( "bookmarks/search.html", name="bookmark_search", takes_context=True ) -def bookmark_search(context, search: BookmarkSearch, tags: [Tag], mode: str = ""): - tag_names = [tag.name for tag in tags] - tags_string = build_tag_string(tag_names, " ") +def bookmark_search(context, search: BookmarkSearch, mode: str = ""): search_form = BookmarkSearchForm(search, editable_fields=["q"]) if mode == "shared": @@ -50,7 +46,6 @@ def bookmark_search(context, search: BookmarkSearch, tags: [Tag], mode: str = "" "search": search, "search_form": search_form, "preferences_form": preferences_form, - "tags_string": tags_string, "mode": mode, } diff --git a/bookmarks/templatetags/pagination.py b/bookmarks/templatetags/pagination.py index eff5900..bb46e1e 100644 --- a/bookmarks/templatetags/pagination.py +++ b/bookmarks/templatetags/pagination.py @@ -2,6 +2,7 @@ from functools import reduce from django import template from django.core.paginator import Page +from django.http import QueryDict NUM_ADJACENT_PAGES = 2 @@ -12,11 +13,44 @@ register = template.Library() "bookmarks/pagination.html", name="pagination", takes_context=True ) def pagination(context, page: Page): + # remove page number and details from query parameters + query_params = context["request"].GET.copy() + query_params.pop("page", None) + query_params.pop("details", None) + + prev_link = ( + _generate_link(query_params, page.previous_page_number()) + if page.has_previous() + else None + ) + next_link = ( + _generate_link(query_params, page.next_page_number()) + if page.has_next() + else None + ) + visible_page_numbers = get_visible_page_numbers( page.number, page.paginator.num_pages ) + page_links = [] + for page_number in visible_page_numbers: + if page_number == -1: + page_links.append(None) + else: + link = _generate_link(query_params, page_number) + page_links.append( + { + "active": page_number == page.number, + "number": page_number, + "link": link, + } + ) - return {"page": page, "visible_page_numbers": visible_page_numbers} + return { + "prev_link": prev_link, + "next_link": next_link, + "page_links": page_links, + } def get_visible_page_numbers(current_page_number: int, num_pages: int) -> [int]: @@ -56,3 +90,8 @@ def get_visible_page_numbers(current_page_number: int, num_pages: int) -> [int]: return result return reduce(append_page, visible_pages, []) + + +def _generate_link(query_params: QueryDict, page_number: int) -> str: + query_params["page"] = page_number + return query_params.urlencode() diff --git a/bookmarks/templatetags/shared.py b/bookmarks/templatetags/shared.py index 6524c11..5da3c1c 100644 --- a/bookmarks/templatetags/shared.py +++ b/bookmarks/templatetags/shared.py @@ -28,12 +28,13 @@ def add_tag_to_query(context, tag_name: str): params = context.request.GET.copy() # Append to or create query string - if params.__contains__("q"): - query_string = params.__getitem__("q") + " " - else: - query_string = "" - query_string = query_string + "#" + tag_name - params.__setitem__("q", query_string) + query_string = params.get("q", "") + query_string = (query_string + " #" + tag_name).strip() + params.setlist("q", [query_string]) + + # Remove details ID and page number + params.pop("details", None) + params.pop("page", None) return params.urlencode() @@ -62,6 +63,10 @@ def remove_tag_from_query(context, tag_name: str): query_string = " ".join(query_parts) params.__setitem__("q", query_string) + # Remove details ID and page number + params.pop("details", None) + params.pop("page", None) + return params.urlencode() diff --git a/bookmarks/tests/helpers.py b/bookmarks/tests/helpers.py index dd97f2d..4a5f016 100644 --- a/bookmarks/tests/helpers.py +++ b/bookmarks/tests/helpers.py @@ -2,6 +2,7 @@ import random import logging from datetime import datetime from typing import List +from unittest import TestCase from bs4 import BeautifulSoup from django.contrib.auth.models import User @@ -220,6 +221,75 @@ class HtmlTestMixin: return BeautifulSoup(html, features="html.parser") +class BookmarkListTestMixin(TestCase, HtmlTestMixin): + def assertVisibleBookmarks( + self, response, bookmarks: List[Bookmark], link_target: str = "_blank" + ): + soup = self.make_soup(response.content.decode()) + bookmark_list = soup.select_one( + f'ul.bookmark-list[data-bookmarks-total="{len(bookmarks)}"]' + ) + self.assertIsNotNone(bookmark_list) + + bookmark_items = bookmark_list.select("li[ld-bookmark-item]") + self.assertEqual(len(bookmark_items), len(bookmarks)) + + for bookmark in bookmarks: + bookmark_item = bookmark_list.select_one( + f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' + ) + self.assertIsNotNone(bookmark_item) + + def assertInvisibleBookmarks( + self, response, bookmarks: List[Bookmark], link_target: str = "_blank" + ): + soup = self.make_soup(response.content.decode()) + + for bookmark in bookmarks: + bookmark_item = soup.select_one( + f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' + ) + self.assertIsNone(bookmark_item) + + +class TagCloudTestMixin(TestCase, HtmlTestMixin): + def assertVisibleTags(self, response, tags: List[Tag]): + soup = self.make_soup(response.content.decode()) + tag_cloud = soup.select_one("div.tag-cloud") + self.assertIsNotNone(tag_cloud) + + tag_items = tag_cloud.select("a[data-is-tag-item]") + self.assertEqual(len(tag_items), len(tags)) + + tag_item_names = [tag_item.text.strip() for tag_item in tag_items] + + for tag in tags: + self.assertTrue(tag.name in tag_item_names) + + def assertInvisibleTags(self, response, tags: List[Tag]): + soup = self.make_soup(response.content.decode()) + tag_items = soup.select("a[data-is-tag-item]") + + tag_item_names = [tag_item.text.strip() for tag_item in tag_items] + + for tag in tags: + self.assertFalse(tag.name in tag_item_names) + + def assertSelectedTags(self, response, tags: List[Tag]): + soup = self.make_soup(response.content.decode()) + selected_tags = soup.select_one("p.selected-tags") + self.assertIsNotNone(selected_tags) + + tag_list = selected_tags.select("a") + self.assertEqual(len(tag_list), len(tags)) + + for tag in tags: + self.assertTrue( + tag.name in selected_tags.text, + msg=f"Selected tags do not contain: {tag.name}", + ) + + class LinkdingApiTestCase(APITestCase): def get(self, url, expected_status_code=status.HTTP_200_OK): response = self.client.get(url) diff --git a/bookmarks/tests/test_bookmark_action_view.py b/bookmarks/tests/test_bookmark_action_view.py index 658cb5a..998a825 100644 --- a/bookmarks/tests/test_bookmark_action_view.py +++ b/bookmarks/tests/test_bookmark_action_view.py @@ -1,13 +1,24 @@ +from unittest.mock import patch + from django.contrib.auth.models import User +from django.core.files.uploadedfile import SimpleUploadedFile from django.forms import model_to_dict -from django.test import TestCase +from django.http import HttpResponse +from django.test import TestCase, override_settings from django.urls import reverse -from bookmarks.models import Bookmark -from bookmarks.tests.helpers import BookmarkFactoryMixin +from bookmarks.models import Bookmark, BookmarkAsset +from bookmarks.services import tasks, bookmarks +from bookmarks.tests.helpers import ( + BookmarkFactoryMixin, + BookmarkListTestMixin, + TagCloudTestMixin, +) -class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): +class BookmarkActionViewTestCase( + TestCase, BookmarkFactoryMixin, BookmarkListTestMixin, TagCloudTestMixin +): def setUp(self) -> None: user = self.get_or_create_test_user() @@ -156,6 +167,129 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): self.assertEqual(response.status_code, 404) self.assertTrue(bookmark.shared) + @override_settings(LD_ENABLE_SNAPSHOTS=True) + def test_create_html_snapshot(self): + bookmark = self.setup_bookmark() + with patch.object(tasks, "_create_html_snapshot_task"): + self.client.post( + reverse("bookmarks:index.action"), + { + "create_html_snapshot": [bookmark.id], + }, + ) + self.assertEqual(bookmark.bookmarkasset_set.count(), 1) + asset = bookmark.bookmarkasset_set.first() + self.assertEqual(asset.asset_type, BookmarkAsset.TYPE_SNAPSHOT) + + @override_settings(LD_ENABLE_SNAPSHOTS=True) + def test_can_only_create_html_snapshot_for_own_bookmarks(self): + other_user = self.setup_user() + bookmark = self.setup_bookmark(user=other_user) + with patch.object(tasks, "_create_html_snapshot_task"): + response = self.client.post( + reverse("bookmarks:index.action"), + { + "create_html_snapshot": [bookmark.id], + }, + ) + self.assertEqual(response.status_code, 404) + self.assertEqual(bookmark.bookmarkasset_set.count(), 0) + + def test_upload_asset(self): + bookmark = self.setup_bookmark() + file_content = b"file content" + upload_file = SimpleUploadedFile("test.txt", file_content) + + with patch.object(bookmarks, "upload_asset") as mock_upload_asset: + response = self.client.post( + reverse("bookmarks:index.action"), + {"upload_asset": bookmark.id, "upload_asset_file": upload_file}, + ) + self.assertEqual(response.status_code, 302) + + mock_upload_asset.assert_called_once() + + args, _ = mock_upload_asset.call_args + self.assertEqual(args[0], bookmark) + + upload_file = args[1] + self.assertEqual(upload_file.name, "test.txt") + + def test_can_only_upload_asset_for_own_bookmarks(self): + other_user = self.setup_user() + bookmark = self.setup_bookmark(user=other_user) + file_content = b"file content" + upload_file = SimpleUploadedFile("test.txt", file_content) + + with patch.object(bookmarks, "upload_asset") as mock_upload_asset: + response = self.client.post( + reverse("bookmarks:index.action"), + {"upload_asset": bookmark.id, "upload_asset_file": upload_file}, + ) + self.assertEqual(response.status_code, 404) + + mock_upload_asset.assert_not_called() + + def test_remove_asset(self): + bookmark = self.setup_bookmark() + asset = self.setup_asset(bookmark) + + response = self.client.post( + reverse("bookmarks:index.action"), {"remove_asset": asset.id} + ) + self.assertEqual(response.status_code, 302) + self.assertFalse(BookmarkAsset.objects.filter(id=asset.id).exists()) + + def test_can_only_remove_own_asset(self): + other_user = self.setup_user() + bookmark = self.setup_bookmark(user=other_user) + asset = self.setup_asset(bookmark) + + response = self.client.post( + reverse("bookmarks:index.action"), {"remove_asset": asset.id} + ) + self.assertEqual(response.status_code, 404) + self.assertTrue(BookmarkAsset.objects.filter(id=asset.id).exists()) + + def test_update_state(self): + bookmark = self.setup_bookmark() + + response = self.client.post( + reverse("bookmarks:index.action"), + { + "update_state": bookmark.id, + "is_archived": "on", + "unread": "on", + "shared": "on", + }, + ) + self.assertEqual(response.status_code, 302) + + bookmark.refresh_from_db() + self.assertTrue(bookmark.unread) + self.assertTrue(bookmark.is_archived) + self.assertTrue(bookmark.shared) + + def test_can_only_update_own_bookmark_state(self): + other_user = self.setup_user() + bookmark = self.setup_bookmark(user=other_user) + + response = self.client.post( + reverse("bookmarks:index.action"), + { + "update_state": bookmark.id, + "is_archived": "on", + "unread": "on", + "shared": "on", + }, + ) + self.assertEqual(response.status_code, 404) + + bookmark.refresh_from_db() + self.assertFalse(bookmark.unread) + self.assertFalse(bookmark.is_archived) + self.assertFalse(bookmark.shared) + def test_bulk_archive(self): bookmark1 = self.setup_bookmark() bookmark2 = self.setup_bookmark() @@ -791,58 +925,119 @@ class BookmarkActionViewTestCase(TestCase, BookmarkFactoryMixin): self.assertBookmarksAreUnmodified([bookmark1, bookmark2, bookmark3]) - def test_should_redirect_to_return_url(self): - bookmark1 = self.setup_bookmark() - bookmark2 = self.setup_bookmark() - bookmark3 = self.setup_bookmark() + def test_index_action_redirects_to_index_with_query_params(self): + url = reverse("bookmarks:index.action") + "?q=foo&page=2" + redirect_url = reverse("bookmarks:index") + "?q=foo&page=2" + response = self.client.post(url) - url = ( - reverse("bookmarks:index.action") - + "?return_url=" - + reverse("bookmarks:settings.index") - ) - response = self.client.post( - url, - { - "bulk_action": ["bulk_archive"], - "bulk_execute": [""], - "bookmark_id": [ - str(bookmark1.id), - str(bookmark2.id), - str(bookmark3.id), - ], - }, - ) + self.assertRedirects(response, redirect_url) - self.assertRedirects(response, reverse("bookmarks:settings.index")) + def test_archived_action_redirects_to_archived_with_query_params(self): + url = reverse("bookmarks:archived.action") + "?q=foo&page=2" + redirect_url = reverse("bookmarks:archived") + "?q=foo&page=2" + response = self.client.post(url) - def test_should_not_redirect_to_external_url(self): - bookmark1 = self.setup_bookmark() - bookmark2 = self.setup_bookmark() - bookmark3 = self.setup_bookmark() + self.assertRedirects(response, redirect_url) - def post_with(return_url, follow=None): - url = reverse("bookmarks:index.action") + f"?return_url={return_url}" - return self.client.post( - url, - { - "bulk_action": ["bulk_archive"], - "bulk_execute": [""], - "bookmark_id": [ - str(bookmark1.id), - str(bookmark2.id), - str(bookmark3.id), - ], - }, - follow=follow, + def test_shared_action_redirects_to_shared_with_query_params(self): + url = reverse("bookmarks:shared.action") + "?q=foo&page=2" + redirect_url = reverse("bookmarks:shared") + "?q=foo&page=2" + response = self.client.post(url) + + self.assertRedirects(response, redirect_url) + + def bookmark_update_fixture(self): + user = self.get_or_create_test_user() + profile = user.profile + profile.enable_sharing = True + profile.save() + + return { + "active": self.setup_numbered_bookmarks(3), + "archived": self.setup_numbered_bookmarks(3, archived=True), + "shared": self.setup_numbered_bookmarks(3, shared=True), + } + + def assertBookmarkUpdateResponse(self, response: HttpResponse): + self.assertEqual(response.status_code, 200) + + html = response.content.decode("utf-8") + soup = self.make_soup(html) + + # bookmark list update + self.assertIsNotNone( + soup.select_one( + "turbo-stream[action='update'][target='bookmark-list-container']" ) + ) - response = post_with("https://example.com") - self.assertRedirects(response, reverse("bookmarks:index")) - response = post_with("//example.com") - self.assertRedirects(response, reverse("bookmarks:index")) - response = post_with("://example.com") - self.assertRedirects(response, reverse("bookmarks:index")) + # tag cloud update + self.assertIsNotNone( + soup.select_one( + "turbo-stream[action='update'][target='tag-cloud-container']" + ) + ) - response = post_with("/foo//example.com", follow=True) - self.assertEqual(response.status_code, 404) + # update event + self.assertInHTML( + """ + + """, + html, + ) + + def test_index_action_with_turbo_returns_bookmark_update(self): + fixture = self.bookmark_update_fixture() + response = self.client.post( + reverse("bookmarks:index.action"), + HTTP_ACCEPT="text/vnd.turbo-stream.html", + ) + + visible_tags = self.get_tags_from_bookmarks( + fixture["active"] + fixture["shared"] + ) + invisible_tags = self.get_tags_from_bookmarks(fixture["archived"]) + + self.assertBookmarkUpdateResponse(response) + self.assertVisibleBookmarks(response, fixture["active"] + fixture["shared"]) + self.assertInvisibleBookmarks(response, fixture["archived"]) + self.assertVisibleTags(response, visible_tags) + self.assertInvisibleTags(response, invisible_tags) + + def test_archived_action_with_turbo_returns_bookmark_update(self): + fixture = self.bookmark_update_fixture() + response = self.client.post( + reverse("bookmarks:archived.action"), + HTTP_ACCEPT="text/vnd.turbo-stream.html", + ) + + visible_tags = self.get_tags_from_bookmarks(fixture["archived"]) + invisible_tags = self.get_tags_from_bookmarks( + fixture["active"] + fixture["shared"] + ) + + self.assertBookmarkUpdateResponse(response) + self.assertVisibleBookmarks(response, fixture["archived"]) + self.assertInvisibleBookmarks(response, fixture["active"] + fixture["shared"]) + self.assertVisibleTags(response, visible_tags) + self.assertInvisibleTags(response, invisible_tags) + + def test_shared_action_with_turbo_returns_bookmark_update(self): + fixture = self.bookmark_update_fixture() + response = self.client.post( + reverse("bookmarks:shared.action"), + HTTP_ACCEPT="text/vnd.turbo-stream.html", + ) + + visible_tags = self.get_tags_from_bookmarks(fixture["shared"]) + invisible_tags = self.get_tags_from_bookmarks( + fixture["active"] + fixture["archived"] + ) + + self.assertBookmarkUpdateResponse(response) + self.assertVisibleBookmarks(response, fixture["shared"]) + self.assertInvisibleBookmarks(response, fixture["active"] + fixture["archived"]) + self.assertVisibleTags(response, visible_tags) + self.assertInvisibleTags(response, invisible_tags) diff --git a/bookmarks/tests/test_bookmark_archived_view.py b/bookmarks/tests/test_bookmark_archived_view.py index 4b357b6..040a5f8 100644 --- a/bookmarks/tests/test_bookmark_archived_view.py +++ b/bookmarks/tests/test_bookmark_archived_view.py @@ -1,89 +1,26 @@ import urllib.parse -from typing import List from django.contrib.auth.models import User from django.test import TestCase from django.urls import reverse -from bookmarks.models import Bookmark, BookmarkSearch, Tag, UserProfile +from bookmarks.models import BookmarkSearch, UserProfile from bookmarks.tests.helpers import ( BookmarkFactoryMixin, - HtmlTestMixin, + BookmarkListTestMixin, + TagCloudTestMixin, collapse_whitespace, ) -class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): +class BookmarkArchivedViewTestCase( + TestCase, BookmarkFactoryMixin, BookmarkListTestMixin, TagCloudTestMixin +): def setUp(self) -> None: user = self.get_or_create_test_user() self.client.force_login(user) - def assertVisibleBookmarks( - self, response, bookmarks: List[Bookmark], link_target: str = "_blank" - ): - soup = self.make_soup(response.content.decode()) - bookmark_list = soup.select_one( - f'ul.bookmark-list[data-bookmarks-total="{len(bookmarks)}"]' - ) - self.assertIsNotNone(bookmark_list) - - bookmark_items = bookmark_list.select("li[ld-bookmark-item]") - self.assertEqual(len(bookmark_items), len(bookmarks)) - - for bookmark in bookmarks: - bookmark_item = bookmark_list.select_one( - f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' - ) - self.assertIsNotNone(bookmark_item) - - def assertInvisibleBookmarks( - self, response, bookmarks: List[Bookmark], link_target: str = "_blank" - ): - soup = self.make_soup(response.content.decode()) - - for bookmark in bookmarks: - bookmark_item = soup.select_one( - f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' - ) - self.assertIsNone(bookmark_item) - - def assertVisibleTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - tag_cloud = soup.select_one("div.tag-cloud") - self.assertIsNotNone(tag_cloud) - - tag_items = tag_cloud.select("a[data-is-tag-item]") - self.assertEqual(len(tag_items), len(tags)) - - tag_item_names = [tag_item.text.strip() for tag_item in tag_items] - - for tag in tags: - self.assertTrue(tag.name in tag_item_names) - - def assertInvisibleTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - tag_items = soup.select("a[data-is-tag-item]") - - tag_item_names = [tag_item.text.strip() for tag_item in tag_items] - - for tag in tags: - self.assertFalse(tag.name in tag_item_names) - - def assertSelectedTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - selected_tags = soup.select_one("p.selected-tags") - self.assertIsNotNone(selected_tags) - - tag_list = selected_tags.select("a") - self.assertEqual(len(tag_list), len(tags)) - - for tag in tags: - self.assertTrue( - tag.name in selected_tags.text, - msg=f"Selected tags do not contain: {tag.name}", - ) - def assertEditLink(self, response, url): html = response.content.decode() self.assertInHTML( @@ -307,24 +244,21 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin base_url = reverse("bookmarks:archived") # without params - return_url = urllib.parse.quote_plus(base_url) - url = f"{action_url}?return_url={return_url}" + url = f"{action_url}" response = self.client.get(base_url) self.assertBulkActionForm(response, url) # with query url_params = "?q=foo" - return_url = urllib.parse.quote_plus(base_url + url_params) - url = f"{action_url}?q=foo&return_url={return_url}" + url = f"{action_url}?q=foo" response = self.client.get(base_url + url_params) self.assertBulkActionForm(response, url) # with query and sort url_params = "?q=foo&sort=title_asc" - return_url = urllib.parse.quote_plus(base_url + url_params) - url = f"{action_url}?q=foo&sort=title_asc&return_url={return_url}" + url = f"{action_url}?q=foo&sort=title_asc" response = self.client.get(base_url + url_params) self.assertBulkActionForm(response, url) @@ -527,7 +461,7 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin self.assertEqual( actions_form.attrs["action"], - "/bookmarks/archived/action?q=%23foo&return_url=%2Fbookmarks%2Farchived%3Fq%3D%2523foo", + "/bookmarks/archived/action?q=%23foo", ) def test_encode_search_params(self): @@ -557,3 +491,15 @@ class BookmarkArchivedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin url = reverse("bookmarks:archived") + "?page=alert(%27xss%27)" response = self.client.get(url) self.assertNotContains(response, "alert('xss')") + + def test_turbo_frame_details_modal_renders_details_modal_update(self): + bookmark = self.setup_bookmark() + url = reverse("bookmarks:archived") + f"?bookmark_id={bookmark.id}" + response = self.client.get(url, headers={"Turbo-Frame": "details-modal"}) + + self.assertEqual(200, response.status_code) + + soup = self.make_soup(response.content.decode()) + self.assertIsNotNone(soup.select_one("turbo-frame#details-modal")) + self.assertIsNone(soup.select_one("#bookmark-list-container")) + self.assertIsNone(soup.select_one("#tag-cloud-container")) diff --git a/bookmarks/tests/test_bookmark_details_modal.py b/bookmarks/tests/test_bookmark_details_modal.py index a2ef4ad..291d149 100644 --- a/bookmarks/tests/test_bookmark_details_modal.py +++ b/bookmarks/tests/test_bookmark_details_modal.py @@ -1,14 +1,11 @@ import datetime import re -from unittest.mock import patch -from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TestCase, override_settings from django.urls import reverse from django.utils import formats, timezone from bookmarks.models import BookmarkAsset, UserProfile -from bookmarks.services import bookmarks, tasks from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin @@ -17,23 +14,23 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin user = self.get_or_create_test_user() self.client.force_login(user) - def get_view_name(self): - return "bookmarks:details_modal" - - def get_base_url(self, bookmark): - return reverse(self.get_view_name(), args=[bookmark.id]) - def get_details_form(self, soup, bookmark): - expected_url = reverse("bookmarks:details", args=[bookmark.id]) - return soup.find("form", {"action": expected_url}) + form_url = reverse("bookmarks:index.action") + f"?details={bookmark.id}" + return soup.find("form", {"action": form_url, "enctype": "multipart/form-data"}) - def get_details(self, bookmark, return_url=""): - url = self.get_base_url(bookmark) - if return_url: - url += f"?return_url={return_url}" + def get_index_details_modal(self, bookmark): + url = reverse("bookmarks:index") + f"?details={bookmark.id}" response = self.client.get(url) soup = self.make_soup(response.content) - return soup + modal = soup.find("turbo-frame", {"id": "details-modal"}) + return modal + + def get_shared_details_modal(self, bookmark): + url = reverse("bookmarks:shared") + f"?details={bookmark.id}" + response = self.client.get(url) + soup = self.make_soup(response.content) + modal = soup.find("turbo-frame", {"id": "details-modal"}) + return modal def find_section(self, soup, section_name): dt = soup.find("dt", string=section_name) @@ -54,35 +51,68 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def find_asset(self, soup, asset): return soup.find("div", {"data-asset-id": asset.id}) - def details_route_access_test(self, view_name: str, shareable: bool): + def details_route_access_test(self): # own bookmark bookmark = self.setup_bookmark() - - response = self.client.get(reverse(view_name, args=[bookmark.id])) + response = self.client.get( + reverse("bookmarks:index") + f"?details={bookmark.id}" + ) self.assertEqual(response.status_code, 200) # other user's bookmark other_user = self.setup_user() bookmark = self.setup_bookmark(user=other_user) - - response = self.client.get(reverse(view_name, args=[bookmark.id])) + response = self.client.get( + reverse("bookmarks:index") + f"?details={bookmark.id}" + ) self.assertEqual(response.status_code, 404) - # non-existent bookmark - response = self.client.get(reverse(view_name, args=[9999])) - self.assertEqual(response.status_code, 404) + # non-existent bookmark - just returns without modal in response + response = self.client.get(reverse("bookmarks:index") + "?details=9999") + self.assertEqual(response.status_code, 200) # guest user self.client.logout() - response = self.client.get(reverse(view_name, args=[bookmark.id])) - self.assertEqual(response.status_code, 404 if shareable else 302) + response = self.client.get( + reverse("bookmarks:shared") + f"?details={bookmark.id}" + ) + self.assertEqual(response.status_code, 404) - def details_route_sharing_access_test(self, view_name: str, shareable: bool): + def test_access(self): + # own bookmark + bookmark = self.setup_bookmark() + response = self.client.get( + reverse("bookmarks:index") + f"?details={bookmark.id}" + ) + self.assertEqual(response.status_code, 200) + + # other user's bookmark + other_user = self.setup_user() + bookmark = self.setup_bookmark(user=other_user) + response = self.client.get( + reverse("bookmarks:index") + f"?details={bookmark.id}" + ) + self.assertEqual(response.status_code, 404) + + # non-existent bookmark - just returns without modal in response + response = self.client.get(reverse("bookmarks:index") + "?details=9999") + self.assertEqual(response.status_code, 200) + + # guest user + self.client.logout() + response = self.client.get( + reverse("bookmarks:shared") + f"?details={bookmark.id}" + ) + self.assertEqual(response.status_code, 404) + + def test_access_with_sharing(self): # shared bookmark, sharing disabled other_user = self.setup_user() bookmark = self.setup_bookmark(shared=True, user=other_user) - response = self.client.get(reverse(view_name, args=[bookmark.id])) + response = self.client.get( + reverse("bookmarks:shared") + f"?details={bookmark.id}" + ) self.assertEqual(response.status_code, 404) # shared bookmark, sharing enabled @@ -90,37 +120,31 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin profile.enable_sharing = True profile.save() - response = self.client.get(reverse(view_name, args=[bookmark.id])) - self.assertEqual(response.status_code, 200 if shareable else 404) + response = self.client.get( + reverse("bookmarks:shared") + f"?details={bookmark.id}" + ) + self.assertEqual(response.status_code, 200) # shared bookmark, guest user, no public sharing self.client.logout() - response = self.client.get(reverse(view_name, args=[bookmark.id])) - self.assertEqual(response.status_code, 404 if shareable else 302) + response = self.client.get( + reverse("bookmarks:shared") + f"?details={bookmark.id}" + ) + self.assertEqual(response.status_code, 404) # shared bookmark, guest user, public sharing profile.enable_public_sharing = True profile.save() - response = self.client.get(reverse(view_name, args=[bookmark.id])) - self.assertEqual(response.status_code, 200 if shareable else 302) - - def test_access(self): - self.details_route_access_test(self.get_view_name(), True) - - def test_access_with_sharing(self): - self.details_route_sharing_access_test(self.get_view_name(), True) - - def test_assets_access(self): - self.details_route_access_test("bookmarks:details_assets", True) - - def test_assets_access_with_sharing(self): - self.details_route_sharing_access_test("bookmarks:details_assets", True) + response = self.client.get( + reverse("bookmarks:shared") + f"?details={bookmark.id}" + ) + self.assertEqual(response.status_code, 200) def test_displays_title(self): # with title bookmark = self.setup_bookmark(title="Test title") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) title = soup.find("h2") self.assertIsNotNone(title) @@ -128,7 +152,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # with website title bookmark = self.setup_bookmark(title="", website_title="Website title") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) title = soup.find("h2") self.assertIsNotNone(title) @@ -136,7 +160,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # with URL only bookmark = self.setup_bookmark(title="", website_title="") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) title = soup.find("h2") self.assertIsNotNone(title) @@ -145,7 +169,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_website_link(self): # basics bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.url) self.assertIsNotNone(link) self.assertEqual(link["href"], bookmark.url) @@ -153,7 +177,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # favicons disabled bookmark = self.setup_bookmark(favicon_file="example.png") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.url) image = link.select_one("img") self.assertIsNone(image) @@ -164,14 +188,14 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin profile.save() bookmark = self.setup_bookmark(favicon_file="") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.url) image = link.select_one("img") self.assertIsNone(image) # favicons enabled, favicon present bookmark = self.setup_bookmark(favicon_file="example.png") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.url) image = link.select_one("img") self.assertIsNotNone(image) @@ -180,7 +204,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_reader_mode_link(self): # no latest snapshot bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) self.assertEqual(self.count_weblinks(soup), 2) # snapshot is not complete @@ -194,7 +218,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin asset_type=BookmarkAsset.TYPE_SNAPSHOT, status=BookmarkAsset.STATUS_FAILURE, ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) self.assertEqual(self.count_weblinks(soup), 2) # not a snapshot @@ -203,7 +227,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin asset_type="upload", status=BookmarkAsset.STATUS_COMPLETE, ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) self.assertEqual(self.count_weblinks(soup), 2) # snapshot is complete @@ -212,7 +236,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin asset_type=BookmarkAsset.TYPE_SNAPSHOT, status=BookmarkAsset.STATUS_COMPLETE, ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) self.assertEqual(self.count_weblinks(soup), 3) reader_mode_url = reverse("bookmarks:assets.read", args=[asset.id]) @@ -221,7 +245,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_internet_archive_link_with_snapshot_url(self): bookmark = self.setup_bookmark(web_archive_snapshot_url="https://example.com/") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.web_archive_snapshot_url) self.assertIsNotNone(link) self.assertEqual(link["href"], bookmark.web_archive_snapshot_url) @@ -231,7 +255,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin bookmark = self.setup_bookmark( web_archive_snapshot_url="https://example.com/", favicon_file="example.png" ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.web_archive_snapshot_url) image = link.select_one("svg") self.assertIsNone(image) @@ -244,7 +268,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin bookmark = self.setup_bookmark( web_archive_snapshot_url="https://example.com/", favicon_file="" ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.web_archive_snapshot_url) image = link.select_one("svg") self.assertIsNone(image) @@ -253,7 +277,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin bookmark = self.setup_bookmark( web_archive_snapshot_url="https://example.com/", favicon_file="example.png" ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, bookmark.web_archive_snapshot_url) image = link.select_one("svg") self.assertIsNotNone(image) @@ -267,7 +291,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin "https://web.archive.org/web/20230811214511/https://example.com/" ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) link = self.find_weblink(soup, fallback_web_archive_url) self.assertIsNotNone(link) self.assertEqual(link["href"], fallback_web_archive_url) @@ -281,7 +305,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin profile.bookmark_link_target = UserProfile.BOOKMARK_LINK_TARGET_BLANK profile.save() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) website_link = self.find_weblink(soup, bookmark.url) self.assertIsNotNone(website_link) @@ -297,7 +321,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin profile.bookmark_link_target = UserProfile.BOOKMARK_LINK_TARGET_SELF profile.save() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) website_link = self.find_weblink(soup, bookmark.url) self.assertIsNotNone(website_link) @@ -312,13 +336,13 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_preview_image(self): # without image bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) image = soup.select_one("div.preview-image img") self.assertIsNone(image) # with image bookmark = self.setup_bookmark(preview_image_file="example.png") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) image = soup.select_one("div.preview-image img") self.assertIsNone(image) @@ -328,13 +352,13 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin profile.save() bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) image = soup.select_one("div.preview-image img") self.assertIsNone(image) # preview images enabled, image present bookmark = self.setup_bookmark(preview_image_file="example.png") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) image = soup.select_one("div.preview-image img") self.assertIsNotNone(image) self.assertEqual(image["src"], "/static/example.png") @@ -342,18 +366,15 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_status(self): # renders form bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) form = self.get_details_form(soup, bookmark) self.assertIsNotNone(form) - self.assertEqual( - form["action"], reverse("bookmarks:details", args=[bookmark.id]) - ) self.assertEqual(form["method"], "post") # sharing disabled bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Status") archived = section.find("input", {"type": "checkbox", "name": "is_archived"}) @@ -369,7 +390,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin profile.save() bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Status") archived = section.find("input", {"type": "checkbox", "name": "is_archived"}) @@ -381,7 +402,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # unchecked bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Status") archived = section.find("input", {"type": "checkbox", "name": "is_archived"}) @@ -393,7 +414,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # checked bookmark = self.setup_bookmark(is_archived=True, unread=True, shared=True) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Status") archived = section.find("input", {"type": "checkbox", "name": "is_archived"}) @@ -406,106 +427,29 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_status_visibility(self): # own bookmark bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.find_section(soup, "Status") self.assertIsNotNone(section) # other user's bookmark other_user = self.setup_user(enable_sharing=True) bookmark = self.setup_bookmark(user=other_user, shared=True) - soup = self.get_details(bookmark) + soup = self.get_shared_details_modal(bookmark) section = self.find_section(soup, "Status") self.assertIsNone(section) # guest user self.client.logout() + other_user.profile.enable_public_sharing = True + other_user.profile.save() bookmark = self.setup_bookmark(user=other_user, shared=True) - soup = self.get_details(bookmark) + soup = self.get_shared_details_modal(bookmark) section = self.find_section(soup, "Status") self.assertIsNone(section) - def test_status_update(self): - bookmark = self.setup_bookmark() - - # update status - response = self.client.post( - self.get_base_url(bookmark), - {"is_archived": "on", "unread": "on", "shared": "on"}, - ) - self.assertEqual(response.status_code, 302) - - bookmark.refresh_from_db() - self.assertTrue(bookmark.is_archived) - self.assertTrue(bookmark.unread) - self.assertTrue(bookmark.shared) - - # update individual status - response = self.client.post( - self.get_base_url(bookmark), - {"is_archived": "", "unread": "on", "shared": ""}, - ) - self.assertEqual(response.status_code, 302) - - bookmark.refresh_from_db() - self.assertFalse(bookmark.is_archived) - self.assertTrue(bookmark.unread) - self.assertFalse(bookmark.shared) - - def test_status_update_access(self): - # no sharing - other_user = self.setup_user() - bookmark = self.setup_bookmark(user=other_user) - response = self.client.post( - self.get_base_url(bookmark), - {"is_archived": "on", "unread": "on", "shared": "on"}, - ) - self.assertEqual(response.status_code, 404) - - # shared, sharing disabled - bookmark = self.setup_bookmark(user=other_user, shared=True) - response = self.client.post( - self.get_base_url(bookmark), - {"is_archived": "on", "unread": "on", "shared": "on"}, - ) - self.assertEqual(response.status_code, 404) - - # shared, sharing enabled - bookmark = self.setup_bookmark(user=other_user, shared=True) - profile = other_user.profile - profile.enable_sharing = True - profile.save() - - response = self.client.post( - self.get_base_url(bookmark), - {"is_archived": "on", "unread": "on", "shared": "on"}, - ) - self.assertEqual(response.status_code, 404) - - # shared, public sharing enabled - bookmark = self.setup_bookmark(user=other_user, shared=True) - profile = other_user.profile - profile.enable_public_sharing = True - profile.save() - - response = self.client.post( - self.get_base_url(bookmark), - {"is_archived": "on", "unread": "on", "shared": "on"}, - ) - self.assertEqual(response.status_code, 404) - - # guest user - self.client.logout() - bookmark = self.setup_bookmark(user=other_user, shared=True) - - response = self.client.post( - self.get_base_url(bookmark), - {"is_archived": "on", "unread": "on", "shared": "on"}, - ) - self.assertEqual(response.status_code, 404) - def test_date_added(self): bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Date added") expected_date = formats.date_format(bookmark.date_added, "DATETIME_FORMAT") @@ -515,7 +459,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_tags(self): # without tags bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.find_section(soup, "Tags") self.assertIsNone(section) @@ -523,7 +467,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # with tags bookmark = self.setup_bookmark(tags=[self.setup_tag(), self.setup_tag()]) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Tags") for tag in bookmark.tags.all(): @@ -535,14 +479,14 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_description(self): # without description bookmark = self.setup_bookmark(description="", website_description="") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.find_section(soup, "Description") self.assertIsNone(section) # with description bookmark = self.setup_bookmark(description="Test description") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Description") self.assertEqual(section.text.strip(), bookmark.description) @@ -551,7 +495,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin bookmark = self.setup_bookmark( description="", website_description="Website description" ) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Description") self.assertEqual(section.text.strip(), bookmark.website_description) @@ -559,14 +503,14 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_notes(self): # without notes bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.find_section(soup, "Notes") self.assertIsNone(section) # with notes bookmark = self.setup_bookmark(notes="Test notes") - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Notes") self.assertEqual(section.decode_contents(), "

    Test notes

    ") @@ -575,52 +519,42 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin bookmark = self.setup_bookmark() # with default return URL - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) edit_link = soup.find("a", string="Edit") self.assertIsNotNone(edit_link) - details_url = reverse("bookmarks:details", args=[bookmark.id]) - expected_url = ( - reverse("bookmarks:edit", args=[bookmark.id]) + "?return_url=" + details_url - ) - self.assertEqual(edit_link["href"], expected_url) - - # with custom return URL - soup = self.get_details(bookmark, return_url="/custom") - edit_link = soup.find("a", string="Edit") - self.assertIsNotNone(edit_link) - expected_url = ( - reverse("bookmarks:edit", args=[bookmark.id]) + "?return_url=/custom" - ) - self.assertEqual(edit_link["href"], expected_url) + details_url = reverse("bookmarks:index") + f"?details={bookmark.id}" + expected_url = "/bookmarks/1/edit?return_url=/bookmarks%3Fdetails%3D1" + self.assertEqual(expected_url, edit_link["href"]) def test_delete_button(self): bookmark = self.setup_bookmark() - # basics - soup = self.get_details(bookmark) - delete_button = soup.find("button", {"type": "submit", "name": "remove"}) + modal = self.get_index_details_modal(bookmark) + delete_button = modal.find("button", {"type": "submit", "name": "remove"}) self.assertIsNotNone(delete_button) - self.assertEqual(delete_button.text.strip(), "Delete...") - self.assertEqual(delete_button["value"], str(bookmark.id)) + self.assertEqual("Delete...", delete_button.text.strip()) + self.assertEqual(str(bookmark.id), delete_button["value"]) form = delete_button.find_parent("form") self.assertIsNotNone(form) - expected_url = reverse("bookmarks:index.action") + f"?return_url=/bookmarks" - self.assertEqual(form["action"], expected_url) - - # with custom return URL - soup = self.get_details(bookmark, return_url="/custom") - delete_button = soup.find("button", {"type": "submit", "name": "remove"}) - form = delete_button.find_parent("form") - expected_url = reverse("bookmarks:index.action") + f"?return_url=/custom" - self.assertEqual(form["action"], expected_url) + expected_url = reverse("bookmarks:index.action") + self.assertEqual(expected_url, form["action"]) def test_actions_visibility(self): + # own bookmark + bookmark = self.setup_bookmark() + + soup = self.get_index_details_modal(bookmark) + edit_link = soup.find("a", string="Edit") + delete_button = soup.find("button", {"type": "submit", "name": "remove"}) + self.assertIsNotNone(edit_link) + self.assertIsNotNone(delete_button) + # with sharing other_user = self.setup_user(enable_sharing=True) bookmark = self.setup_bookmark(user=other_user, shared=True) - soup = self.get_details(bookmark) + soup = self.get_shared_details_modal(bookmark) edit_link = soup.find("a", string="Edit") delete_button = soup.find("button", {"type": "submit", "name": "remove"}) self.assertIsNone(edit_link) @@ -632,7 +566,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin profile.save() bookmark = self.setup_bookmark(user=other_user, shared=True) - soup = self.get_details(bookmark) + soup = self.get_shared_details_modal(bookmark) edit_link = soup.find("a", string="Edit") delete_button = soup.find("button", {"type": "submit", "name": "remove"}) self.assertIsNone(edit_link) @@ -642,7 +576,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin self.client.logout() bookmark = self.setup_bookmark(user=other_user, shared=True) - soup = self.get_details(bookmark) + soup = self.get_shared_details_modal(bookmark) edit_link = soup.find("a", string="Edit") delete_button = soup.find("button", {"type": "submit", "name": "remove"}) self.assertIsNone(edit_link) @@ -651,7 +585,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_assets_visibility_no_snapshot_support(self): bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.find_section(soup, "Files") self.assertIsNone(section) @@ -659,7 +593,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin def test_assets_visibility_with_snapshot_support(self): bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.find_section(soup, "Files") self.assertIsNotNone(section) @@ -668,7 +602,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # no assets bookmark = self.setup_bookmark() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Files") asset_list = section.find("div", {"class": "assets"}) self.assertIsNone(asset_list) @@ -677,7 +611,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin bookmark = self.setup_bookmark() self.setup_asset(bookmark) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Files") asset_list = section.find("div", {"class": "assets"}) self.assertIsNotNone(asset_list) @@ -691,7 +625,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin self.setup_asset(bookmark), ] - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) section = self.get_section(soup, "Files") asset_list = section.find("div", {"class": "assets"}) @@ -717,7 +651,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin asset.file = "" asset.save() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) asset_item = self.find_asset(soup, asset) view_url = reverse("bookmarks:assets.view", args=[asset.id]) view_link = asset_item.find("a", {"href": view_url}) @@ -729,7 +663,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin pending_asset = self.setup_asset(bookmark, status=BookmarkAsset.STATUS_PENDING) failed_asset = self.setup_asset(bookmark, status=BookmarkAsset.STATUS_FAILURE) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) asset_item = self.find_asset(soup, pending_asset) asset_text = asset_item.select_one(".asset-text span") @@ -746,7 +680,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin asset2 = self.setup_asset(bookmark, file_size=54639) asset3 = self.setup_asset(bookmark, file_size=11492020) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) asset_item = self.find_asset(soup, asset1) asset_text = asset_item.select_one(".asset-text") @@ -766,7 +700,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # with file asset = self.setup_asset(bookmark) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) asset_item = self.find_asset(soup, asset) view_link = asset_item.find("a", string="View") @@ -779,7 +713,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # without file asset.file = "" asset.save() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) asset_item = self.find_asset(soup, asset) view_link = asset_item.find("a", string="View") @@ -793,7 +727,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin other_user = self.setup_user(enable_sharing=True, enable_public_sharing=True) bookmark = self.setup_bookmark(shared=True, user=other_user) asset = self.setup_asset(bookmark) - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) asset_item = self.find_asset(soup, asset) view_link = asset_item.find("a", string="View") @@ -805,7 +739,7 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin # shared bookmark, guest user self.client.logout() - soup = self.get_details(bookmark) + soup = self.get_shared_details_modal(bookmark) asset_item = self.find_asset(soup, asset) view_link = asset_item.find("a", string="View") @@ -815,77 +749,13 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin self.assertIsNotNone(view_link) self.assertIsNone(delete_button) - def test_remove_asset(self): - # remove asset - bookmark = self.setup_bookmark() - asset = self.setup_asset(bookmark) - - response = self.client.post( - self.get_base_url(bookmark), {"remove_asset": asset.id} - ) - self.assertEqual(response.status_code, 302) - self.assertFalse(BookmarkAsset.objects.filter(id=asset.id).exists()) - - # non-existent asset - response = self.client.post(self.get_base_url(bookmark), {"remove_asset": 9999}) - self.assertEqual(response.status_code, 404) - - # post without asset ID does not remove - asset = self.setup_asset(bookmark) - response = self.client.post(self.get_base_url(bookmark)) - self.assertEqual(response.status_code, 302) - self.assertTrue(BookmarkAsset.objects.filter(id=asset.id).exists()) - - # guest user - asset = self.setup_asset(bookmark) - self.client.logout() - response = self.client.post( - self.get_base_url(bookmark), {"remove_asset": asset.id} - ) - self.assertEqual(response.status_code, 404) - self.assertTrue(BookmarkAsset.objects.filter(id=asset.id).exists()) - - @override_settings(LD_ENABLE_SNAPSHOTS=True) - def test_assets_refresh_when_having_pending_asset(self): - bookmark = self.setup_bookmark() - asset = self.setup_asset(bookmark, status=BookmarkAsset.STATUS_COMPLETE) - fetch_url = reverse("bookmarks:details_assets", args=[bookmark.id]) - - # no pending asset - soup = self.get_details(bookmark) - files_section = self.find_section(soup, "Files") - assets_wrapper = files_section.find("div", {"ld-fetch": fetch_url}) - self.assertIsNone(assets_wrapper) - - # with pending asset - asset.status = BookmarkAsset.STATUS_PENDING - asset.save() - - soup = self.get_details(bookmark) - files_section = self.find_section(soup, "Files") - assets_wrapper = files_section.find("div", {"ld-fetch": fetch_url}) - self.assertIsNotNone(assets_wrapper) - - @override_settings(LD_ENABLE_SNAPSHOTS=True) - def test_create_snapshot(self): - with patch.object( - tasks, "_create_html_snapshot_task" - ) as mock_create_html_snapshot_task: - bookmark = self.setup_bookmark() - response = self.client.post( - self.get_base_url(bookmark), {"create_snapshot": ""} - ) - self.assertEqual(response.status_code, 302) - - self.assertEqual(bookmark.bookmarkasset_set.count(), 1) - @override_settings(LD_ENABLE_SNAPSHOTS=True) def test_create_snapshot_is_disabled_when_having_pending_asset(self): bookmark = self.setup_bookmark() asset = self.setup_asset(bookmark, status=BookmarkAsset.STATUS_COMPLETE) # no pending asset - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) files_section = self.find_section(soup, "Files") create_button = files_section.find( "button", string=re.compile("Create HTML snapshot") @@ -896,40 +766,9 @@ class BookmarkDetailsModalTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin asset.status = BookmarkAsset.STATUS_PENDING asset.save() - soup = self.get_details(bookmark) + soup = self.get_index_details_modal(bookmark) files_section = self.find_section(soup, "Files") create_button = files_section.find( "button", string=re.compile("Create HTML snapshot") ) self.assertTrue(create_button.has_attr("disabled")) - - def test_upload_file(self): - bookmark = self.setup_bookmark() - file_content = b"file content" - upload_file = SimpleUploadedFile("test.txt", file_content) - - with patch.object(bookmarks, "upload_asset") as mock_upload_asset: - response = self.client.post( - self.get_base_url(bookmark), - {"upload_asset": "", "upload_asset_file": upload_file}, - ) - self.assertEqual(response.status_code, 302) - - mock_upload_asset.assert_called_once() - - args, kwargs = mock_upload_asset.call_args - self.assertEqual(args[0], bookmark) - - upload_file = args[1] - self.assertEqual(upload_file.name, "test.txt") - - def test_upload_file_without_file(self): - bookmark = self.setup_bookmark() - - with patch.object(bookmarks, "upload_asset") as mock_upload_asset: - response = self.client.post( - self.get_base_url(bookmark), - {"upload_asset": ""}, - ) - self.assertEqual(response.status_code, 400) - mock_upload_asset.assert_not_called() diff --git a/bookmarks/tests/test_bookmark_details_view.py b/bookmarks/tests/test_bookmark_details_view.py deleted file mode 100644 index 36824de..0000000 --- a/bookmarks/tests/test_bookmark_details_view.py +++ /dev/null @@ -1,6 +0,0 @@ -from bookmarks.tests.test_bookmark_details_modal import BookmarkDetailsModalTestCase - - -class BookmarkDetailsViewTestCase(BookmarkDetailsModalTestCase): - def get_view_name(self): - return "bookmarks:details" diff --git a/bookmarks/tests/test_bookmark_index_view.py b/bookmarks/tests/test_bookmark_index_view.py index 608f281..8a0c680 100644 --- a/bookmarks/tests/test_bookmark_index_view.py +++ b/bookmarks/tests/test_bookmark_index_view.py @@ -1,85 +1,24 @@ import urllib.parse -from typing import List from django.contrib.auth.models import User from django.test import TestCase from django.urls import reverse -from bookmarks.models import Bookmark, BookmarkSearch, Tag, UserProfile -from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin +from bookmarks.models import BookmarkSearch, UserProfile +from bookmarks.tests.helpers import ( + BookmarkFactoryMixin, + BookmarkListTestMixin, + TagCloudTestMixin, +) -class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): - +class BookmarkIndexViewTestCase( + TestCase, BookmarkFactoryMixin, BookmarkListTestMixin, TagCloudTestMixin +): def setUp(self) -> None: user = self.get_or_create_test_user() self.client.force_login(user) - def assertVisibleBookmarks( - self, response, bookmarks: List[Bookmark], link_target: str = "_blank" - ): - soup = self.make_soup(response.content.decode()) - bookmark_list = soup.select_one( - f'ul.bookmark-list[data-bookmarks-total="{len(bookmarks)}"]' - ) - self.assertIsNotNone(bookmark_list) - - bookmark_items = bookmark_list.select("li[ld-bookmark-item]") - self.assertEqual(len(bookmark_items), len(bookmarks)) - - for bookmark in bookmarks: - bookmark_item = bookmark_list.select_one( - f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' - ) - self.assertIsNotNone(bookmark_item) - - def assertInvisibleBookmarks( - self, response, bookmarks: List[Bookmark], link_target: str = "_blank" - ): - soup = self.make_soup(response.content.decode()) - - for bookmark in bookmarks: - bookmark_item = soup.select_one( - f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' - ) - self.assertIsNone(bookmark_item) - - def assertVisibleTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - tag_cloud = soup.select_one("div.tag-cloud") - self.assertIsNotNone(tag_cloud) - - tag_items = tag_cloud.select("a[data-is-tag-item]") - self.assertEqual(len(tag_items), len(tags)) - - tag_item_names = [tag_item.text.strip() for tag_item in tag_items] - - for tag in tags: - self.assertTrue(tag.name in tag_item_names) - - def assertInvisibleTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - tag_items = soup.select("a[data-is-tag-item]") - - tag_item_names = [tag_item.text.strip() for tag_item in tag_items] - - for tag in tags: - self.assertFalse(tag.name in tag_item_names) - - def assertSelectedTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - selected_tags = soup.select_one("p.selected-tags") - self.assertIsNotNone(selected_tags) - - tag_list = selected_tags.select("a") - self.assertEqual(len(tag_list), len(tags)) - - for tag in tags: - self.assertTrue( - tag.name in selected_tags.text, - msg=f"Selected tags do not contain: {tag.name}", - ) - def assertEditLink(self, response, url): html = response.content.decode() self.assertInHTML( @@ -285,24 +224,21 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): base_url = reverse("bookmarks:index") # without params - return_url = urllib.parse.quote_plus(base_url) - url = f"{action_url}?return_url={return_url}" + url = f"{action_url}" response = self.client.get(base_url) self.assertBulkActionForm(response, url) # with query url_params = "?q=foo" - return_url = urllib.parse.quote_plus(base_url + url_params) - url = f"{action_url}?q=foo&return_url={return_url}" + url = f"{action_url}?q=foo" response = self.client.get(base_url + url_params) self.assertBulkActionForm(response, url) # with query and sort url_params = "?q=foo&sort=title_asc" - return_url = urllib.parse.quote_plus(base_url + url_params) - url = f"{action_url}?q=foo&sort=title_asc&return_url={return_url}" + url = f"{action_url}?q=foo&sort=title_asc" response = self.client.get(base_url + url_params) self.assertBulkActionForm(response, url) @@ -503,7 +439,7 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): self.assertEqual( actions_form.attrs["action"], - "/bookmarks/action?q=%23foo&return_url=%2Fbookmarks%3Fq%3D%2523foo", + "/bookmarks/action?q=%23foo", ) def test_encode_search_params(self): @@ -533,3 +469,15 @@ class BookmarkIndexViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): url = reverse("bookmarks:index") + "?page=alert(%27xss%27)" response = self.client.get(url) self.assertNotContains(response, "alert('xss')") + + def test_turbo_frame_details_modal_renders_details_modal_update(self): + bookmark = self.setup_bookmark() + url = reverse("bookmarks:index") + f"?bookmark_id={bookmark.id}" + response = self.client.get(url, headers={"Turbo-Frame": "details-modal"}) + + self.assertEqual(200, response.status_code) + + soup = self.make_soup(response.content.decode()) + self.assertIsNotNone(soup.select_one("turbo-frame#details-modal")) + self.assertIsNone(soup.select_one("#bookmark-list-container")) + self.assertIsNone(soup.select_one("#tag-cloud-container")) diff --git a/bookmarks/tests/test_bookmark_search_tag.py b/bookmarks/tests/test_bookmark_search_tag.py index d40bf47..e8c016f 100644 --- a/bookmarks/tests/test_bookmark_search_tag.py +++ b/bookmarks/tests/test_bookmark_search_tag.py @@ -1,16 +1,13 @@ from bs4 import BeautifulSoup -from django.db.models import QuerySet from django.template import Template, RequestContext from django.test import TestCase, RequestFactory -from bookmarks.models import BookmarkSearch, Tag +from bookmarks.models import BookmarkSearch from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin class BookmarkSearchTagTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): - def render_template( - self, url: str, tags: QuerySet[Tag] = Tag.objects.all(), mode: str = "" - ): + def render_template(self, url: str, mode: str = ""): rf = RequestFactory() request = rf.get(url) request.user = self.get_or_create_test_user() @@ -21,32 +18,31 @@ class BookmarkSearchTagTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): { "request": request, "search": search, - "tags": tags, "mode": mode, }, ) template_to_render = Template( - "{% load bookmarks %}" "{% bookmark_search search tags mode %}" + "{% load bookmarks %} {% bookmark_search search mode %}" ) return template_to_render.render(context) def assertHiddenInput(self, form: BeautifulSoup, name: str, value: str = None): - input = form.select_one(f'input[name="{name}"][type="hidden"]') - self.assertIsNotNone(input) + element = form.select_one(f'input[name="{name}"][type="hidden"]') + self.assertIsNotNone(element) if value is not None: - self.assertEqual(input["value"], value) + self.assertEqual(element["value"], value) def assertNoHiddenInput(self, form: BeautifulSoup, name: str): - input = form.select_one(f'input[name="{name}"][type="hidden"]') - self.assertIsNone(input) + element = form.select_one(f'input[name="{name}"][type="hidden"]') + self.assertIsNone(element) def assertSearchInput(self, form: BeautifulSoup, name: str, value: str = None): - input = form.select_one(f'input[name="{name}"][type="search"]') - self.assertIsNotNone(input) + element = form.select_one(f'input[name="{name}"][type="search"]') + self.assertIsNotNone(element) if value is not None: - self.assertEqual(input["value"], value) + self.assertEqual(element["value"], value) def assertSelect(self, form: BeautifulSoup, name: str, value: str = None): select = form.select_one(f'select[name="{name}"]') diff --git a/bookmarks/tests/test_bookmark_shared_view.py b/bookmarks/tests/test_bookmark_shared_view.py index a20fd2f..c54886b 100644 --- a/bookmarks/tests/test_bookmark_shared_view.py +++ b/bookmarks/tests/test_bookmark_shared_view.py @@ -6,11 +6,16 @@ from django.test import TestCase from django.urls import reverse from bookmarks.models import Bookmark, BookmarkSearch, Tag, UserProfile -from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin +from bookmarks.tests.helpers import ( + BookmarkFactoryMixin, + BookmarkListTestMixin, + TagCloudTestMixin, +) -class BookmarkSharedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): - +class BookmarkSharedViewTestCase( + TestCase, BookmarkFactoryMixin, BookmarkListTestMixin, TagCloudTestMixin +): def authenticate(self) -> None: user = self.get_or_create_test_user() self.client.force_login(user) @@ -24,57 +29,6 @@ class BookmarkSharedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): count=count, ) - def assertVisibleBookmarks( - self, response, bookmarks: List[Bookmark], link_target: str = "_blank" - ): - soup = self.make_soup(response.content.decode()) - bookmark_list = soup.select_one( - f'ul.bookmark-list[data-bookmarks-total="{len(bookmarks)}"]' - ) - self.assertIsNotNone(bookmark_list) - - bookmark_items = bookmark_list.select("li[ld-bookmark-item]") - self.assertEqual(len(bookmark_items), len(bookmarks)) - - for bookmark in bookmarks: - bookmark_item = bookmark_list.select_one( - f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' - ) - self.assertIsNotNone(bookmark_item) - - def assertInvisibleBookmarks( - self, response, bookmarks: List[Bookmark], link_target: str = "_blank" - ): - soup = self.make_soup(response.content.decode()) - - for bookmark in bookmarks: - bookmark_item = soup.select_one( - f'li[ld-bookmark-item] a[href="{bookmark.url}"][target="{link_target}"]' - ) - self.assertIsNone(bookmark_item) - - def assertVisibleTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - tag_cloud = soup.select_one("div.tag-cloud") - self.assertIsNotNone(tag_cloud) - - tag_items = tag_cloud.select("a[data-is-tag-item]") - self.assertEqual(len(tag_items), len(tags)) - - tag_item_names = [tag_item.text.strip() for tag_item in tag_items] - - for tag in tags: - self.assertTrue(tag.name in tag_item_names) - - def assertInvisibleTags(self, response, tags: List[Tag]): - soup = self.make_soup(response.content.decode()) - tag_items = soup.select("a[data-is-tag-item]") - - tag_item_names = [tag_item.text.strip() for tag_item in tag_items] - - for tag in tags: - self.assertFalse(tag.name in tag_item_names) - def assertVisibleUserOptions(self, response, users: List[User]): html = response.content.decode() @@ -84,7 +38,7 @@ class BookmarkSharedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): f'' ) user_select_html = f""" - {''.join(user_options)} """ @@ -593,7 +547,7 @@ class BookmarkSharedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): self.assertEqual( actions_form.attrs["action"], - "/bookmarks/shared/action?q=%23foo&return_url=%2Fbookmarks%2Fshared%3Fq%3D%2523foo", + "/bookmarks/shared/action?q=%23foo", ) def test_encode_search_params(self): @@ -627,3 +581,15 @@ class BookmarkSharedViewTestCase(TestCase, BookmarkFactoryMixin, HtmlTestMixin): url = reverse("bookmarks:shared") + "?page=alert(%27xss%27)" response = self.client.get(url) self.assertNotContains(response, "alert('xss')") + + def test_turbo_frame_details_modal_renders_details_modal_update(self): + bookmark = self.setup_bookmark() + url = reverse("bookmarks:shared") + f"?bookmark_id={bookmark.id}" + response = self.client.get(url, headers={"Turbo-Frame": "details-modal"}) + + self.assertEqual(200, response.status_code) + + soup = self.make_soup(response.content.decode()) + self.assertIsNotNone(soup.select_one("turbo-frame#details-modal")) + self.assertIsNone(soup.select_one("#bookmark-list-container")) + self.assertIsNone(soup.select_one("#tag-cloud-container")) diff --git a/bookmarks/tests/test_bookmarks_list_template.py b/bookmarks/tests/test_bookmarks_list_template.py index c86313e..8a6f5d0 100644 --- a/bookmarks/tests/test_bookmarks_list_template.py +++ b/bookmarks/tests/test_bookmarks_list_template.py @@ -12,7 +12,7 @@ from django.utils import timezone, formats from bookmarks.middlewares import LinkdingMiddleware from bookmarks.models import Bookmark, UserProfile, User from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin -from bookmarks.views.partials import contexts +from bookmarks.views import contexts class BookmarkListTemplateTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): @@ -51,31 +51,25 @@ class BookmarkListTemplateTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): html, ) - def assertViewLink( - self, html: str, bookmark: Bookmark, return_url=reverse("bookmarks:index") - ): - self.assertViewLinkCount(html, bookmark, return_url=return_url) + def assertViewLink(self, html: str, bookmark: Bookmark, base_url=None): + self.assertViewLinkCount(html, bookmark, base_url) - def assertNoViewLink( - self, html: str, bookmark: Bookmark, return_url=reverse("bookmarks:index") - ): - self.assertViewLinkCount(html, bookmark, count=0, return_url=return_url) + def assertNoViewLink(self, html: str, bookmark: Bookmark, base_url=None): + self.assertViewLinkCount(html, bookmark, base_url, count=0) def assertViewLinkCount( self, html: str, bookmark: Bookmark, + base_url: str = None, count=1, - return_url=reverse("bookmarks:index"), ): - details_url = reverse("bookmarks:details", args=[bookmark.id]) - details_modal_url = reverse("bookmarks:details_modal", args=[bookmark.id]) + if base_url is None: + base_url = reverse("bookmarks:index") + details_url = base_url + f"?details={bookmark.id}" self.assertInHTML( f""" - View + View """, html, count=count, @@ -652,7 +646,7 @@ class BookmarkListTemplateTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): bookmark = self.setup_bookmark(user=other_user, shared=True) html = self.render_template(context_type=contexts.SharedBookmarkListContext) - self.assertViewLink(html, bookmark, return_url=reverse("bookmarks:shared")) + self.assertViewLink(html, bookmark, base_url=reverse("bookmarks:shared")) self.assertNoBookmarkActions(html, bookmark) self.assertShareInfo(html, bookmark) @@ -944,7 +938,7 @@ class BookmarkListTemplateTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): self.assertWebArchiveLink( html, "1 week ago", bookmark.web_archive_snapshot_url, link_target="_blank" ) - self.assertViewLink(html, bookmark, return_url=reverse("bookmarks:shared")) + self.assertViewLink(html, bookmark, base_url=reverse("bookmarks:shared")) self.assertNoBookmarkActions(html, bookmark) self.assertShareInfo(html, bookmark) self.assertMarkAsReadButton(html, bookmark, count=0) diff --git a/bookmarks/tests/test_pagination_tag.py b/bookmarks/tests/test_pagination_tag.py index 1d5ee40..19ec728 100644 --- a/bookmarks/tests/test_pagination_tag.py +++ b/bookmarks/tests/test_pagination_tag.py @@ -172,3 +172,12 @@ class PaginationTagTest(TestCase, BookmarkFactoryMixin): rendered_template, 2, True, href="?q=cake&sort=title_asc&page=2" ) self.assertNextLink(rendered_template, 3, href="?q=cake&sort=title_asc&page=3") + + def test_removes_details_parameter(self): + rendered_template = self.render_template( + 100, 10, 2, url="/test?details=1&page=2" + ) + self.assertPrevLink(rendered_template, 1, href="?page=1") + self.assertPageLink(rendered_template, 1, False, href="?page=1") + self.assertPageLink(rendered_template, 2, True, href="?page=2") + self.assertNextLink(rendered_template, 3, href="?page=3") diff --git a/bookmarks/tests/test_tag_cloud_template.py b/bookmarks/tests/test_tag_cloud_template.py index d0e0f0d..04cad07 100644 --- a/bookmarks/tests/test_tag_cloud_template.py +++ b/bookmarks/tests/test_tag_cloud_template.py @@ -8,7 +8,7 @@ from django.test import TestCase, RequestFactory from bookmarks.middlewares import LinkdingMiddleware from bookmarks.models import UserProfile from bookmarks.tests.helpers import BookmarkFactoryMixin, HtmlTestMixin -from bookmarks.views.partials import contexts +from bookmarks.views import contexts class TagCloudTemplateTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): @@ -203,13 +203,28 @@ class TagCloudTemplateTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): tag = self.setup_tag(name="tag1") self.setup_bookmark(tags=[tag], title="term1") + rendered_template = self.render_template(url="/test?q=term1&sort=title_asc") + + self.assertInHTML( + """ + + tag1 + + """, + rendered_template, + ) + + def test_tag_url_removes_page_number_and_details_id(self): + tag = self.setup_tag(name="tag1") + self.setup_bookmark(tags=[tag], title="term1") + rendered_template = self.render_template( - url="/test?q=term1&sort=title_asc&page=2" + url="/test?q=term1&sort=title_asc&page=2&details=5" ) self.assertInHTML( """ - + tag1 """, @@ -347,12 +362,30 @@ class TagCloudTemplateTest(TestCase, BookmarkFactoryMixin, HtmlTestMixin): self.setup_bookmark(tags=[tag], title="term1", description="term2") rendered_template = self.render_template( - url="/test?q=term1 %23tag1 term2&sort=title_asc&page=2" + url="/test?q=term1 %23tag1 term2&sort=title_asc" ) self.assertInHTML( """ - + -tag1 + + """, + rendered_template, + ) + + def test_selected_tag_url_removes_page_number_and_details_id(self): + tag = self.setup_tag(name="tag1") + self.setup_bookmark(tags=[tag], title="term1", description="term2") + + rendered_template = self.render_template( + url="/test?q=term1 %23tag1 term2&sort=title_asc&page=2&details=5" + ) + + self.assertInHTML( + """ + -tag1 diff --git a/bookmarks/urls.py b/bookmarks/urls.py index 74d082f..97b8ab1 100644 --- a/bookmarks/urls.py +++ b/bookmarks/urls.py @@ -9,7 +9,6 @@ from bookmarks.feeds import ( SharedBookmarksFeed, PublicSharedBookmarksFeed, ) -from bookmarks.views import partials app_name = "bookmarks" urlpatterns = [ @@ -31,21 +30,6 @@ urlpatterns = [ path("bookmarks/new", views.bookmarks.new, name="new"), path("bookmarks/close", views.bookmarks.close, name="close"), path("bookmarks//edit", views.bookmarks.edit, name="edit"), - path( - "bookmarks//details", - views.bookmarks.details, - name="details", - ), - path( - "bookmarks//details_modal", - views.bookmarks.details_modal, - name="details_modal", - ), - path( - "bookmarks//details_assets", - views.bookmarks.details_assets, - name="details_assets", - ), # Assets path( "assets/", @@ -57,52 +41,6 @@ urlpatterns = [ views.assets.read, name="assets.read", ), - # Partials - path( - "bookmarks/partials/bookmark-list/active", - partials.active_bookmark_list, - name="partials.bookmark_list.active", - ), - path( - "bookmarks/partials/tag-cloud/active", - partials.active_tag_cloud, - name="partials.tag_cloud.active", - ), - path( - "bookmarks/partials/tag-modal/active", - partials.active_tag_modal, - name="partials.tag_modal.active", - ), - path( - "bookmarks/partials/bookmark-list/archived", - partials.archived_bookmark_list, - name="partials.bookmark_list.archived", - ), - path( - "bookmarks/partials/tag-cloud/archived", - partials.archived_tag_cloud, - name="partials.tag_cloud.archived", - ), - path( - "bookmarks/partials/tag-modal/archived", - partials.archived_tag_modal, - name="partials.tag_modal.archived", - ), - path( - "bookmarks/partials/bookmark-list/shared", - partials.shared_bookmark_list, - name="partials.bookmark_list.shared", - ), - path( - "bookmarks/partials/tag-cloud/shared", - partials.shared_tag_cloud, - name="partials.tag_cloud.shared", - ), - path( - "bookmarks/partials/tag-modal/shared", - partials.shared_tag_modal, - name="partials.tag_modal.shared", - ), # Settings path("settings", views.settings.general, name="settings.index"), path("settings/general", views.settings.general, name="settings.general"), diff --git a/bookmarks/utils.py b/bookmarks/utils.py index 04bb697..b1a38ec 100644 --- a/bookmarks/utils.py +++ b/bookmarks/utils.py @@ -1,10 +1,12 @@ import logging import re import unicodedata +import urllib.parse from datetime import datetime from typing import Optional from dateutil.relativedelta import relativedelta +from django.http import HttpResponseRedirect from django.template.defaultfilters import pluralize from django.utils import timezone, formats @@ -114,6 +116,14 @@ def get_safe_return_url(return_url: str, fallback_url: str): return return_url +def redirect_with_query(request, redirect_url): + query_string = urllib.parse.urlencode(request.GET) + if query_string: + redirect_url += "?" + query_string + + return HttpResponseRedirect(redirect_url) + + def generate_username(email): # taken from mozilla-django-oidc docs :) diff --git a/bookmarks/views/bookmarks.py b/bookmarks/views/bookmarks.py index 0e88281..730741b 100644 --- a/bookmarks/views/bookmarks.py +++ b/bookmarks/views/bookmarks.py @@ -11,7 +11,7 @@ from django.http import ( from django.shortcuts import render from django.urls import reverse -from bookmarks import queries +from bookmarks import queries, utils from bookmarks.models import ( Bookmark, BookmarkAsset, @@ -19,6 +19,7 @@ from bookmarks.models import ( BookmarkSearch, build_tag_string, ) +from bookmarks.services import bookmarks as bookmark_actions, tasks from bookmarks.services.bookmarks import ( create_bookmark, update_bookmark, @@ -34,9 +35,8 @@ from bookmarks.services.bookmarks import ( share_bookmarks, unshare_bookmarks, ) -from bookmarks.services import bookmarks as bookmark_actions, tasks from bookmarks.utils import get_safe_return_url -from bookmarks.views.partials import contexts +from bookmarks.views import contexts, partials, turbo _default_page_size = 30 @@ -48,12 +48,17 @@ def index(request): bookmark_list = contexts.ActiveBookmarkListContext(request) tag_cloud = contexts.ActiveTagCloudContext(request) - return render( + bookmark_details = contexts.get_details_context( + request, contexts.ActiveBookmarkDetailsContext + ) + + return render_bookmarks_view( request, "bookmarks/index.html", { "bookmark_list": bookmark_list, "tag_cloud": tag_cloud, + "details": bookmark_details, }, ) @@ -65,12 +70,17 @@ def archived(request): bookmark_list = contexts.ArchivedBookmarkListContext(request) tag_cloud = contexts.ArchivedTagCloudContext(request) - return render( + bookmark_details = contexts.get_details_context( + request, contexts.ArchivedBookmarkDetailsContext + ) + + return render_bookmarks_view( request, "bookmarks/archive.html", { "bookmark_list": bookmark_list, "tag_cloud": tag_cloud, + "details": bookmark_details, }, ) @@ -81,14 +91,37 @@ def shared(request): bookmark_list = contexts.SharedBookmarkListContext(request) tag_cloud = contexts.SharedTagCloudContext(request) + bookmark_details = contexts.get_details_context( + request, contexts.SharedBookmarkDetailsContext + ) public_only = not request.user.is_authenticated users = queries.query_shared_bookmark_users( request.user_profile, bookmark_list.search, public_only ) - return render( + return render_bookmarks_view( request, "bookmarks/shared.html", - {"bookmark_list": bookmark_list, "tag_cloud": tag_cloud, "users": users}, + { + "bookmark_list": bookmark_list, + "tag_cloud": tag_cloud, + "details": bookmark_details, + "users": users, + }, + ) + + +def render_bookmarks_view(request, template_name, context): + if turbo.is_frame(request, "details-modal"): + return render( + request, + "bookmarks/updates/details-modal-frame.html", + context, + ) + + return render( + request, + template_name, + context, ) @@ -111,76 +144,6 @@ def search_action(request): return HttpResponseRedirect(url) -def _details(request, bookmark_id: int, template: str): - try: - bookmark = Bookmark.objects.get(pk=bookmark_id) - except Bookmark.DoesNotExist: - raise Http404("Bookmark does not exist") - - is_owner = bookmark.owner == request.user - is_shared = ( - request.user.is_authenticated - and bookmark.shared - and bookmark.owner.profile.enable_sharing - ) - is_public_shared = bookmark.shared and bookmark.owner.profile.enable_public_sharing - if not is_owner and not is_shared and not is_public_shared: - raise Http404("Bookmark does not exist") - - if request.method == "POST": - if not is_owner: - raise Http404("Bookmark does not exist") - - return_url = get_safe_return_url( - request.GET.get("return_url"), - reverse("bookmarks:details", args=[bookmark.id]), - ) - - if "remove_asset" in request.POST: - asset_id = request.POST["remove_asset"] - try: - asset = bookmark.bookmarkasset_set.get(pk=asset_id) - except BookmarkAsset.DoesNotExist: - raise Http404("Asset does not exist") - asset.delete() - if "create_snapshot" in request.POST: - tasks.create_html_snapshot(bookmark) - if "upload_asset" in request.POST: - file = request.FILES.get("upload_asset_file") - if not file: - return HttpResponseBadRequest("No file uploaded") - bookmark_actions.upload_asset(bookmark, file) - else: - bookmark.is_archived = request.POST.get("is_archived") == "on" - bookmark.unread = request.POST.get("unread") == "on" - bookmark.shared = request.POST.get("shared") == "on" - bookmark.save() - - return HttpResponseRedirect(return_url) - - details_context = contexts.BookmarkDetailsContext(request, bookmark) - - return render( - request, - template, - { - "details": details_context, - }, - ) - - -def details(request, bookmark_id: int): - return _details(request, bookmark_id, "bookmarks/details.html") - - -def details_modal(request, bookmark_id: int): - return _details(request, bookmark_id, "bookmarks/details_modal.html") - - -def details_assets(request, bookmark_id: int): - return _details(request, bookmark_id, "bookmarks/details/assets.html") - - def convert_tag_string(tag_string: str): # Tag strings coming from inputs are space-separated, however services.bookmarks functions expect comma-separated # strings @@ -307,26 +270,87 @@ def mark_as_read(request, bookmark_id: int): bookmark.save() +def create_html_snapshot(request, bookmark_id: int): + try: + bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) + except Bookmark.DoesNotExist: + raise Http404("Bookmark does not exist") + + tasks.create_html_snapshot(bookmark) + + +def upload_asset(request, bookmark_id: int): + try: + bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) + except Bookmark.DoesNotExist: + raise Http404("Bookmark does not exist") + + file = request.FILES.get("upload_asset_file") + if not file: + raise ValueError("No file uploaded") + + bookmark_actions.upload_asset(bookmark, file) + + +def remove_asset(request, asset_id: int): + try: + asset = BookmarkAsset.objects.get(pk=asset_id, bookmark__owner=request.user) + except BookmarkAsset.DoesNotExist: + raise Http404("Asset does not exist") + + asset.delete() + + +def update_state(request, bookmark_id: int): + try: + bookmark = Bookmark.objects.get(pk=bookmark_id, owner=request.user) + except Bookmark.DoesNotExist: + raise Http404("Bookmark does not exist") + + bookmark.is_archived = request.POST.get("is_archived") == "on" + bookmark.unread = request.POST.get("unread") == "on" + bookmark.shared = request.POST.get("shared") == "on" + bookmark.save() + + @login_required def index_action(request): search = BookmarkSearch.from_request(request.GET) query = queries.query_bookmarks(request.user, request.user_profile, search) - return action(request, query) + handle_action(request, query) + + if turbo.accept(request): + return partials.active_bookmark_update(request) + + return utils.redirect_with_query(request, reverse("bookmarks:index")) @login_required def archived_action(request): search = BookmarkSearch.from_request(request.GET) query = queries.query_archived_bookmarks(request.user, request.user_profile, search) - return action(request, query) + handle_action(request, query) + + if turbo.accept(request): + return partials.archived_bookmark_update(request) + + return utils.redirect_with_query(request, reverse("bookmarks:archived")) @login_required def shared_action(request): - return action(request) + if "bulk_execute" in request.POST: + return HttpResponseBadRequest("View does not support bulk actions") + + handle_action(request) + + if turbo.accept(request): + return partials.shared_bookmark_update(request) + + return utils.redirect_with_query(request, reverse("bookmarks:shared")) -def action(request, query: QuerySet[Bookmark] = None): +def handle_action(request, query: QuerySet[Bookmark] = None): # Single bookmark actions if "archive" in request.POST: archive(request, request.POST["archive"]) @@ -338,11 +362,21 @@ def action(request, query: QuerySet[Bookmark] = None): mark_as_read(request, request.POST["mark_as_read"]) if "unshare" in request.POST: unshare(request, request.POST["unshare"]) + if "create_html_snapshot" in request.POST: + create_html_snapshot(request, request.POST["create_html_snapshot"]) + if "upload_asset" in request.POST: + upload_asset(request, request.POST["upload_asset"]) + if "remove_asset" in request.POST: + remove_asset(request, request.POST["remove_asset"]) + + # State updates + if "update_state" in request.POST: + update_state(request, request.POST["update_state"]) # Bulk actions if "bulk_execute" in request.POST: if query is None: - return HttpResponseBadRequest("View does not support bulk actions") + raise ValueError("Query must be provided for bulk actions") bulk_action = request.POST["bulk_action"] @@ -375,11 +409,6 @@ def action(request, query: QuerySet[Bookmark] = None): if "bulk_unshare" == bulk_action: unshare_bookmarks(bookmark_ids, request.user) - return_url = get_safe_return_url( - request.GET.get("return_url"), reverse("bookmarks:index") - ) - return HttpResponseRedirect(return_url) - @login_required def close(request): diff --git a/bookmarks/views/partials/contexts.py b/bookmarks/views/contexts.py similarity index 84% rename from bookmarks/views/partials/contexts.py rename to bookmarks/views/contexts.py index 701f597..8593451 100644 --- a/bookmarks/views/partials/contexts.py +++ b/bookmarks/views/contexts.py @@ -6,6 +6,7 @@ from django.conf import settings from django.core.handlers.wsgi import WSGIRequest from django.core.paginator import Paginator from django.db import models +from django.http import Http404 from django.urls import reverse from bookmarks import queries @@ -27,17 +28,11 @@ CJK_RE = re.compile(r"[\u4e00-\u9fff]+") class RequestContext: index_view = "bookmarks:index" action_view = "bookmarks:index.action" - bookmark_list_partial_view = "bookmarks:partials.bookmark_list.active" - tag_cloud_partial_view = "bookmarks:partials.tag_cloud.active" - tag_modal_partial_view = "bookmarks:partials.tag_modal.active" def __init__(self, request: WSGIRequest): self.request = request self.index_url = reverse(self.index_view) self.action_url = reverse(self.action_view) - self.bookmark_list_partial_url = reverse(self.bookmark_list_partial_view) - self.tag_cloud_partial_url = reverse(self.tag_cloud_partial_view) - self.tag_modal_partial_url = reverse(self.tag_modal_partial_view) self.query_params = request.GET.copy() self.query_params.pop("details", None) @@ -51,34 +46,25 @@ class RequestContext: encoded_params = query_params.urlencode() return view_url + "?" + encoded_params if encoded_params else view_url - def index(self) -> str: - return self.get_url(self.index_url) + def index(self, add: dict = None, remove: dict = None) -> str: + return self.get_url(self.index_url, add=add, remove=remove) - def action(self, return_url: str) -> str: - return self.get_url(self.action_url, add={"return_url": return_url}) + def action(self, add: dict = None, remove: dict = None) -> str: + return self.get_url(self.action_url, add=add, remove=remove) - def bookmark_list_partial(self) -> str: - return self.get_url(self.bookmark_list_partial_url) - - def tag_cloud_partial(self) -> str: - return self.get_url(self.tag_cloud_partial_url) - - def tag_modal_partial(self) -> str: - return self.get_url(self.tag_modal_partial_url) + def details(self, bookmark_id: int) -> str: + return self.get_url(self.index_url, add={"details": bookmark_id}) def get_bookmark_query_set(self, search: BookmarkSearch): - raise Exception("Must be implemented by subclass") + raise NotImplementedError("Must be implemented by subclass") def get_tag_query_set(self, search: BookmarkSearch): - raise Exception("Must be implemented by subclass") + raise NotImplementedError("Must be implemented by subclass") class ActiveBookmarksContext(RequestContext): index_view = "bookmarks:index" action_view = "bookmarks:index.action" - bookmark_list_partial_view = "bookmarks:partials.bookmark_list.active" - tag_cloud_partial_view = "bookmarks:partials.tag_cloud.active" - tag_modal_partial_view = "bookmarks:partials.tag_modal.active" def get_bookmark_query_set(self, search: BookmarkSearch): return queries.query_bookmarks( @@ -94,9 +80,6 @@ class ActiveBookmarksContext(RequestContext): class ArchivedBookmarksContext(RequestContext): index_view = "bookmarks:archived" action_view = "bookmarks:archived.action" - bookmark_list_partial_view = "bookmarks:partials.bookmark_list.archived" - tag_cloud_partial_view = "bookmarks:partials.tag_cloud.archived" - tag_modal_partial_view = "bookmarks:partials.tag_modal.archived" def get_bookmark_query_set(self, search: BookmarkSearch): return queries.query_archived_bookmarks( @@ -112,9 +95,6 @@ class ArchivedBookmarksContext(RequestContext): class SharedBookmarksContext(RequestContext): index_view = "bookmarks:shared" action_view = "bookmarks:shared.action" - bookmark_list_partial_view = "bookmarks:partials.bookmark_list.shared" - tag_cloud_partial_view = "bookmarks:partials.tag_cloud.shared" - tag_modal_partial_view = "bookmarks:partials.tag_modal.shared" def get_bookmark_query_set(self, search: BookmarkSearch): user = User.objects.filter(username=search.user).first() @@ -132,7 +112,13 @@ class SharedBookmarksContext(RequestContext): class BookmarkItem: - def __init__(self, bookmark: Bookmark, user: User, profile: UserProfile) -> None: + def __init__( + self, + context: RequestContext, + bookmark: Bookmark, + user: User, + profile: UserProfile, + ) -> None: self.bookmark = bookmark is_editable = bookmark.owner == user @@ -154,6 +140,7 @@ class BookmarkItem: self.is_archived = bookmark.is_archived self.unread = bookmark.unread self.owner = bookmark.owner + self.details_url = context.details(bookmark.id) css_classes = [] if bookmark.unread: @@ -200,16 +187,15 @@ class BookmarkListContext: models.prefetch_related_objects(bookmarks_page.object_list, "owner", "tags") self.items = [ - BookmarkItem(bookmark, user, user_profile) for bookmark in bookmarks_page + BookmarkItem(request_context, bookmark, user, user_profile) + for bookmark in bookmarks_page ] self.is_empty = paginator.count == 0 self.bookmarks_page = bookmarks_page self.bookmarks_total = paginator.count self.return_url = request_context.index() - self.action_url = request_context.action(return_url=self.return_url) - self.refresh_url = request_context.bookmark_list_partial() - self.tag_modal_url = request_context.tag_modal_partial() + self.action_url = request_context.action() self.link_target = user_profile.bookmark_link_target self.date_display = user_profile.bookmark_date_display @@ -344,8 +330,6 @@ class TagCloudContext: self.selected_tags = unique_selected_tags self.has_selected_tags = has_selected_tags - self.refresh_url = request_context.tag_cloud_partial() - def get_selected_tags(self, tags: List[Tag]): parsed_query = queries.parse_query_string(self.search.q) tag_names = parsed_query["tag_names"] @@ -396,17 +380,18 @@ class BookmarkAssetItem: class BookmarkDetailsContext: + request_context = RequestContext + def __init__(self, request: WSGIRequest, bookmark: Bookmark): + request_context = self.request_context(request) + user = request.user user_profile = request.user_profile - self.edit_return_url = utils.get_safe_return_url( - request.GET.get("return_url"), - reverse("bookmarks:details", args=[bookmark.id]), - ) - self.delete_return_url = utils.get_safe_return_url( - request.GET.get("return_url"), reverse("bookmarks:index") - ) + self.edit_return_url = request_context.details(bookmark.id) + self.action_url = request_context.action(add={"details": bookmark.id}) + self.delete_url = request_context.action() + self.close_url = request_context.index() self.bookmark = bookmark self.profile = request.user_profile @@ -438,3 +423,44 @@ class BookmarkDetailsContext: ), None, ) + + +class ActiveBookmarkDetailsContext(BookmarkDetailsContext): + request_context = ActiveBookmarksContext + + +class ArchivedBookmarkDetailsContext(BookmarkDetailsContext): + request_context = ArchivedBookmarksContext + + +class SharedBookmarkDetailsContext(BookmarkDetailsContext): + request_context = SharedBookmarksContext + + +def get_details_context( + request: WSGIRequest, context_type +) -> BookmarkDetailsContext | None: + bookmark_id = request.GET.get("details") + if not bookmark_id: + return None + + try: + bookmark = Bookmark.objects.get(pk=int(bookmark_id)) + except Bookmark.DoesNotExist: + # just ignore, might end up in a situation where the bookmark was deleted + # in between navigating back and forth + return None + + is_owner = bookmark.owner == request.user + is_shared = ( + request.user.is_authenticated + and bookmark.shared + and bookmark.owner.profile.enable_sharing + ) + is_public_shared = bookmark.shared and bookmark.owner.profile.enable_public_sharing + if not is_owner and not is_shared and not is_public_shared: + raise Http404("Bookmark does not exist") + if request.method == "POST" and not is_owner: + raise Http404("Bookmark does not exist") + + return context_type(request, bookmark) diff --git a/bookmarks/views/partials.py b/bookmarks/views/partials.py new file mode 100644 index 0000000..8930a66 --- /dev/null +++ b/bookmarks/views/partials.py @@ -0,0 +1,40 @@ +from bookmarks.views import contexts, turbo + + +def render_bookmark_update(request, bookmark_list, tag_cloud, details): + return turbo.stream( + request, + "bookmarks/updates/bookmark_view_stream.html", + { + "bookmark_list": bookmark_list, + "tag_cloud": tag_cloud, + "details": details, + }, + ) + + +def active_bookmark_update(request): + bookmark_list = contexts.ActiveBookmarkListContext(request) + tag_cloud = contexts.ActiveTagCloudContext(request) + details = contexts.get_details_context( + request, contexts.ActiveBookmarkDetailsContext + ) + return render_bookmark_update(request, bookmark_list, tag_cloud, details) + + +def archived_bookmark_update(request): + bookmark_list = contexts.ArchivedBookmarkListContext(request) + tag_cloud = contexts.ArchivedTagCloudContext(request) + details = contexts.get_details_context( + request, contexts.ArchivedBookmarkDetailsContext + ) + return render_bookmark_update(request, bookmark_list, tag_cloud, details) + + +def shared_bookmark_update(request): + bookmark_list = contexts.SharedBookmarkListContext(request) + tag_cloud = contexts.SharedTagCloudContext(request) + details = contexts.get_details_context( + request, contexts.SharedBookmarkDetailsContext + ) + return render_bookmark_update(request, bookmark_list, tag_cloud, details) diff --git a/bookmarks/views/partials/__init__.py b/bookmarks/views/partials/__init__.py deleted file mode 100644 index 2204149..0000000 --- a/bookmarks/views/partials/__init__.py +++ /dev/null @@ -1,76 +0,0 @@ -from django.contrib.auth.decorators import login_required -from django.shortcuts import render - -from bookmarks.views.partials import contexts - - -@login_required -def active_bookmark_list(request): - bookmark_list_context = contexts.ActiveBookmarkListContext(request) - - return render( - request, - "bookmarks/bookmark_list.html", - {"bookmark_list": bookmark_list_context}, - ) - - -@login_required -def active_tag_cloud(request): - tag_cloud_context = contexts.ActiveTagCloudContext(request) - - return render(request, "bookmarks/tag_cloud.html", {"tag_cloud": tag_cloud_context}) - - -@login_required -def active_tag_modal(request): - tag_cloud_context = contexts.ActiveTagCloudContext(request) - - return render(request, "bookmarks/tag_modal.html", {"tag_cloud": tag_cloud_context}) - - -@login_required -def archived_bookmark_list(request): - bookmark_list_context = contexts.ArchivedBookmarkListContext(request) - - return render( - request, - "bookmarks/bookmark_list.html", - {"bookmark_list": bookmark_list_context}, - ) - - -@login_required -def archived_tag_cloud(request): - tag_cloud_context = contexts.ArchivedTagCloudContext(request) - - return render(request, "bookmarks/tag_cloud.html", {"tag_cloud": tag_cloud_context}) - - -@login_required -def archived_tag_modal(request): - tag_cloud_context = contexts.ArchivedTagCloudContext(request) - - return render(request, "bookmarks/tag_modal.html", {"tag_cloud": tag_cloud_context}) - - -def shared_bookmark_list(request): - bookmark_list_context = contexts.SharedBookmarkListContext(request) - - return render( - request, - "bookmarks/bookmark_list.html", - {"bookmark_list": bookmark_list_context}, - ) - - -def shared_tag_cloud(request): - tag_cloud_context = contexts.SharedTagCloudContext(request) - - return render(request, "bookmarks/tag_cloud.html", {"tag_cloud": tag_cloud_context}) - - -def shared_tag_modal(request): - tag_cloud_context = contexts.SharedTagCloudContext(request) - - return render(request, "bookmarks/tag_modal.html", {"tag_cloud": tag_cloud_context}) diff --git a/bookmarks/views/turbo.py b/bookmarks/views/turbo.py new file mode 100644 index 0000000..3ac7dbc --- /dev/null +++ b/bookmarks/views/turbo.py @@ -0,0 +1,19 @@ +from django.http import HttpRequest, HttpResponse +from django.shortcuts import render as django_render + + +def accept(request: HttpRequest): + is_turbo_request = "text/vnd.turbo-stream.html" in request.headers.get("Accept", "") + disable_turbo = request.POST.get("disable_turbo", "false") == "true" + + return is_turbo_request and not disable_turbo + + +def is_frame(request: HttpRequest, frame: str) -> bool: + return request.headers.get("Turbo-Frame") == frame + + +def stream(request: HttpRequest, template_name: str, context: dict) -> HttpResponse: + response = django_render(request, template_name, context) + response["Content-Type"] = "text/vnd.turbo-stream.html" + return response diff --git a/siteroot/settings/dev.py b/siteroot/settings/dev.py index 1c3341f..e40315b 100644 --- a/siteroot/settings/dev.py +++ b/siteroot/settings/dev.py @@ -10,8 +10,8 @@ from .base import * DEBUG = True # Enable debug toolbar -INSTALLED_APPS.append("debug_toolbar") -MIDDLEWARE.append("debug_toolbar.middleware.DebugToolbarMiddleware") +# INSTALLED_APPS.append("debug_toolbar") +# MIDDLEWARE.append("debug_toolbar.middleware.DebugToolbarMiddleware") INTERNAL_IPS = [ "127.0.0.1",