Delete component and logic on deleting series created#12369
Delete component and logic on deleting series created#12369IvanPisquiy06 wants to merge 6 commits intointernetarchive:masterfrom
Conversation
|
Thanks for the PR, @IvanPisquiy06. @cdrini is assigned to this PR and currently has:
PR triage checklist (maintainers / Pam)
Note This comment was automatically generated by Pam, Open Library's Project AI Manager, on behalf of @mekarpeles. Pam is designed to provide status visibility, perform basic project management functions and relevant codebase research, and provide actionable feedback so contributors aren't left waiting. |
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
Adds a reusable Lit “delete with confirmation” button to list/series edit pages and implements server-side series deletion (including cleanup of series edges on linked works) so superlibrarians/admins can delete series from the UI.
Changes:
- Added new Lit web component
ol-delete-btnand exported it via the Lit components entry point. - Updated the edit toolbar macro and list edit template to render and wire up the delete button.
- Extended list/series deletion logic in
lists.py, including removing series edges from linked works before deleting a series.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| openlibrary/templates/type/list/edit.html | Passes list_type into databarEdit and adds a document-level handler to submit deletion via _delete=true. |
| openlibrary/plugins/openlibrary/lists.py | Intercepts _delete on edit POST; expands delete endpoint to support series; removes series edges from works before series deletion. |
| openlibrary/macros/databarEdit.html | Adds a delete button web component into the edit toolbar (currently not restricted to list/series pages). |
| openlibrary/components/lit/index.js | Exports the new OLDeleteBtn component from the Lit bundle entry point. |
| openlibrary/components/lit/OLDeleteBtn.js | Implements the new Lit delete button with confirm dialog + ol-delete-btn-confirm event. |
Comments suppressed due to low confidence (1)
openlibrary/plugins/openlibrary/lists.py:472
lists_delete.pathnow introduces an extra capturing group(lists|series), butPOST(self, key)still only accepts one positional arg. With web.py route matching, this will pass 2 args (key + list_type) and raise aTypeError. Use a non-capturing group(?:lists|series)(or update the handler signature) so the route only captures the full key once.
class lists_delete(delegate.page):
path = r"((?:/people/[^/]+)?/(lists|series)/OL\d+L)/delete"
encoding = "json"
def POST(self, key):
if not (user := get_current_user()):
| <div id="editTools" class="edit" style="align-items: center;"> | ||
| $if ctx.user and (ctx.user.is_admin() or ctx.user.is_super_librarian()): | ||
| <div> | ||
| <form method="post" id="delete-item" name="delete"> | ||
| <ol-delete-btn | ||
| label="$delete_label" | ||
| item-name="$(page.name)" | ||
| item-count="$(len(page.get_seeds()))" | ||
| linked-label="$('works')" | ||
| ></ol-delete-btn> | ||
| </form> |
There was a problem hiding this comment.
databarEdit is used by multiple edit templates (authors, tags, books, etc.). This change renders the new delete component for all admin/super-librarian edit pages and calls page.get_seeds(), which many page types don’t implement, causing template rendering errors for admins. Restrict this block to list/series edit pages (e.g., if list_type in ('list','series')) or guard get_seeds() access before calling it.
| $code: | ||
| if list_type == 'series': | ||
| delete_label = _('Delete Series') | ||
| elif list_type == 'list': | ||
| delete_label = _('Delete List') | ||
| else: | ||
| delete_label = _('Delete') | ||
| linked_label = _('items') | ||
|
|
||
| <div id="editTools" class="edit"> | ||
| <div id="editTools" class="edit" style="align-items: center;"> | ||
| $if ctx.user and (ctx.user.is_admin() or ctx.user.is_super_librarian()): | ||
| <div> | ||
| <form method="post" id="delete-item" name="delete"> | ||
| <ol-delete-btn | ||
| label="$delete_label" | ||
| item-name="$(page.name)" | ||
| item-count="$(len(page.get_seeds()))" | ||
| linked-label="$('works')" | ||
| ></ol-delete-btn> |
There was a problem hiding this comment.
The linked_label variable is only set in the else branch (and never for list/series), but the component hard-codes linked-label="$('works')" and ignores linked_label entirely. This makes the macro’s behavior inconsistent and prevents using the intended list-vs-series wording. Define linked_label for each list_type and pass it through (and make sure it goes through the template i18n function).
| _buildConfirmMessage() { | ||
| const name = this.itemName ? `"${this.itemName}"` : 'this item'; | ||
| if (this.itemCount === 1) { | ||
| return `${name} has 1 ${this.linkedLabel.replace(/s$/, '')}. Deleting it will remove this from that item.\n\nAre you sure you want to delete it?`; | ||
| } else if (this.itemCount > 1) { | ||
| return `${name} has ${this.itemCount} ${this.linkedLabel}. Deleting it will remove this from all of them.\n\nAre you sure you want to delete it?`; | ||
| } | ||
| return `Are you sure you want to delete ${name}? This cannot be undone.`; | ||
| } |
There was a problem hiding this comment.
OLDeleteBtn hard-codes the confirmation dialog text in English. Since this is user-facing UI and the component is intended to be reusable across pages, it should allow callers to provide translated strings (e.g., via attributes/properties for the different message templates) rather than embedding English-only sentences in the component.
| if (!form) return; | ||
| var input = document.createElement('input'); | ||
| input.type = 'hidden'; | ||
| input.name = '_delete'; | ||
| input.value = 'true'; | ||
| form.appendChild(input); |
There was a problem hiding this comment.
This listener appends a new hidden _delete input on every confirmation, so repeated clicks can add duplicate _delete fields to the form. Consider reusing an existing hidden input (create it once) or setting form._delete.value = 'true' to keep the DOM stable.
| if (!form) return; | |
| var input = document.createElement('input'); | |
| input.type = 'hidden'; | |
| input.name = '_delete'; | |
| input.value = 'true'; | |
| form.appendChild(input); | |
| var input; | |
| if (!form) return; | |
| input = form.querySelector('input[name="_delete"]'); | |
| if (!input) { | |
| input = document.createElement('input'); | |
| input.type = 'hidden'; | |
| input.name = '_delete'; | |
| form.appendChild(input); | |
| } | |
| input.value = 'true'; |
Closes #12298
Superlibrarians can now delete series directly from the edit page, without needing database or code access. This helps clean up unused, duplicate, or mistakenly created series (e.g. test entries) without requiring maintainer intervention.
Technical
A new Lit web component
OLDeleteBtn(ol-delete-btn) was added toopenlibrary/components/lit/and exported via the existingindex.jsentry point, so it is automatically included in the Vite bundle atstatic/build/lit-components/production/ol-components.js.The component is fully generic and reusable — the button label, item name, item count, and linked items label are all configurable via attributes, making it usable beyond series (e.g. lists).
databarEdit.htmlwas updated to render the component for both series and list edit pages, gated behind the existing admin/super-librarian permission check. Thelist_typevariable drives a Python code block that sets the appropriate label and linked-label values. The form submission is handled by an existing event listener inedit.htmlthat responds to theol-delete-btn-confirmcustom event.The series deletion logic mirrors the existing list deletion logic, reusing the same patterns to avoid duplication.
Testing
Screenshot
Edit page of a Series:

Confirmation message:

Component reused on edit page of a List:

Stakeholders
@cdrini