Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/lists/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,6 @@
path("list/edit", views.edit, name="list_edit"),
path("list/delete", views.delete, name="list_delete"),
path("list_item_toggle", views.list_item_toggle, name="list_item_toggle"),
path("bulk_lists_modal", views.bulk_lists_modal, name="bulk_lists_modal"),
path("bulk_list_add", views.bulk_list_add, name="bulk_list_add"),
]
36 changes: 36 additions & 0 deletions src/lists/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,39 @@ def list_item_toggle(request):
"lists/components/list_item_button.html",
{"custom_list": custom_list, "item": item, "has_item": has_item},
)


@require_GET
def bulk_lists_modal(request):
"""Return the modal showing all custom lists for bulk adding items."""
custom_lists = (
CustomList.objects.filter(Q(owner=request.user) | Q(collaborators=request.user))
.distinct()
.order_by("name")
)
return render(
request,
"lists/components/bulk_fill_lists.html",
{"custom_lists": custom_lists},
)


@require_POST
def bulk_list_add(request):
"""Add multiple items to a custom list at once."""
item_ids = request.POST.getlist("item_ids")
custom_list_id = request.POST["custom_list_id"]

custom_list = get_object_or_404(CustomList, id=custom_list_id)

if custom_list.user_can_edit(request.user):
CustomListItem.objects.bulk_create(
[CustomListItem(custom_list=custom_list, item_id=i) for i in item_ids],
ignore_conflicts=True,
)
logger.info("%d items bulk added to %s.", len(item_ids), custom_list)

return render(request, "lists/components/bulk_fill_lists_success.html")

messages.error(request, "You do not have permission to edit this list.")
return helpers.redirect_back(request)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation for fetching the CustomList and checking for edit permissions can be improved for security and efficiency.

  1. Accessing request.POST["custom_list_id"] directly will raise a MultiValueDictKeyError if the key is missing. It's safer to use request.POST.get() and handle the case where it's not provided.
  2. Fetching the object and then checking for permissions separately can leak information about the existence of a list a user doesn't have access to. It's better to incorporate the permission check into the get_object_or_404 query. This also makes the view consistent with list_item_toggle.

Here's a suggested refactoring that addresses both points and simplifies the code by removing the if/else block for permissions.

Suggested change
custom_list_id = request.POST["custom_list_id"]
custom_list = get_object_or_404(CustomList, id=custom_list_id)
if custom_list.user_can_edit(request.user):
CustomListItem.objects.bulk_create(
[CustomListItem(custom_list=custom_list, item_id=i) for i in item_ids],
ignore_conflicts=True,
)
logger.info("%d items bulk added to %s.", len(item_ids), custom_list)
return render(request, "lists/components/bulk_fill_lists_success.html")
messages.error(request, "You do not have permission to edit this list.")
return helpers.redirect_back(request)
custom_list_id = request.POST.get("custom_list_id")
if not custom_list_id:
messages.error(request, "No list was selected.")
return helpers.redirect_back(request)
custom_list = get_object_or_404(
CustomList.objects.filter(
Q(owner=request.user) | Q(collaborators=request.user), id=custom_list_id
).distinct()
)
CustomListItem.objects.bulk_create(
[CustomListItem(custom_list=custom_list, item_id=i) for i in item_ids],
ignore_conflicts=True,
)
logger.info("%d items bulk added to %s.", len(item_ids), custom_list)
return render(request, "lists/components/bulk_fill_lists_success.html")

20 changes: 18 additions & 2 deletions src/templates/app/components/media_card.html
Original file line number Diff line number Diff line change
@@ -1,8 +1,23 @@
{% load app_tags %}

<div class="{% if secondary_color %}bg-[#39404b]{% else %}bg-[#2a2f35]{% endif %} rounded-lg overflow-hidden shadow-lg relative {% if active %}ring-[1.5px] ring-slate-400{% endif %}"
x-data="{ trackOpen: false, listsOpen: false, historyOpen: false }">
data-bulk-id="{{ item.id }}"
x-data="{ trackOpen: false, listsOpen: false, historyOpen: false }"
:class="$store.bulk.active && $store.bulk.ids.includes({{ item.id }}) ? 'ring-2 ring-indigo-500' : ''">
<div class="relative">
{# Bulk select overlay — covers the card when bulk mode is active #}
<div x-show="$store.bulk.active"
class="absolute inset-0 z-20 cursor-pointer"
@click="$store.bulk.toggle({{ item.id }})">
<div class="absolute top-2 right-2">
<input type="checkbox"
:checked="$store.bulk.ids.includes({{ item.id }})"
@click.stop
@change="$store.bulk.toggle({{ item.id }})"
class="w-5 h-5 rounded accent-indigo-600 cursor-pointer">
</div>
</div>

<a href="{{ item|media_url }}">
<img alt="{{ title }}"
class="lazyload w-full {% if from_grid %}aspect-2/3{% else %}h-48{% endif %} bg-[#3e454d] {% if item.image != IMG_NONE %}object-cover{% endif %}"
Expand Down Expand Up @@ -69,7 +84,8 @@
</div>
{% endif %}

<div class="absolute inset-0 bg-black/50 flex items-center justify-center opacity-0 hover-tap:opacity-100{% if user.clickable_media_cards %} pointer-coarse:hidden{% endif %}">
<div class="absolute inset-0 bg-black/50 flex items-center justify-center opacity-0 hover-tap:opacity-100{% if user.clickable_media_cards %} pointer-coarse:hidden{% endif %}"
:class="$store.bulk.active ? '!opacity-0 !pointer-events-none' : ''">
<a href="{{ item|media_url }}" class="absolute inset-0"></a>

<div class="relative flex items-center justify-center space-x-2.5">
Expand Down
9 changes: 9 additions & 0 deletions src/templates/app/components/media_table_items.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,17 @@

{% for media in media_list %}
<tr class="hover:bg-[#39404b] transition-colors cursor-pointer hover-tap"
data-bulk-id="{{ media.item.id }}"
x-data="{ trackOpen: false }"
:class="$store.bulk.active && $store.bulk.ids.includes({{ media.item.id }}) ? 'bg-indigo-900/30' : ''"
{% if forloop.last and media_list.has_next %} hx-get="{% url 'medialist' media_type %}?page={{ media_list.next_page_number }}" hx-trigger="revealed threshold:200px" hx-swap="afterend" hx-include="#filter-form" hx-indicator="#loading-indicator" {% endif %}>
<td x-show="$store.bulk.active" class="p-2">
<input type="checkbox"
:checked="$store.bulk.ids.includes({{ media.item.id }})"
@click.stop
@change="$store.bulk.toggle({{ media.item.id }})"
class="w-4 h-4 rounded accent-indigo-600 cursor-pointer">
</td>
<td class="p-2 relative">
<img alt="{{ media.item }}"
class="lazyload min-w-10 w-10 h-10 object-cover rounded-md parent-hover-tap:hidden"
Expand Down
74 changes: 73 additions & 1 deletion src/templates/app/media_list.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,27 @@
{% block content %}
<h1 class="text-3xl font-bold mb-6">{{ media_type|media_type_readable_plural }}</h1>

<div x-data="{ sort: '{{ current_sort }}', status: '{{ current_status }}', layout: '{{ current_layout }}', search: '{{ request.GET.search|default:'' }}', statusLabels: {
<script>
document.addEventListener('alpine:init', () => {
Alpine.store('bulk', {
active: false,
ids: [],
toggle(id) {
const i = this.ids.indexOf(id);
i === -1 ? this.ids.push(id) : this.ids.splice(i, 1);
},
selectAll() {
document.querySelectorAll('[data-bulk-id]').forEach(function(el) {
const id = parseInt(el.dataset.bulkId);
if (Alpine.store('bulk').ids.indexOf(id) === -1) Alpine.store('bulk').ids.push(id);
});
},
Comment on lines +21 to +26
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The selectAll function can be implemented more efficiently. The current implementation iterates over all visible items and checks for existence in the ids array one by one, which can be slow if there are many items on the page or many items are already selected.

A more performant approach would be to get all visible IDs, merge them with the already selected IDs using a Set to handle duplicates, and then update the ids array.

        selectAll() {
          const allVisibleIds = Array.from(document.querySelectorAll('[data-bulk-id]'), el => parseInt(el.dataset.bulkId));
          this.ids = Array.from(new Set([...this.ids, ...allVisibleIds]));
        },

clear() { this.ids = []; this.active = false; },
});
});
</script>

<div x-data="{ sort: '{{ current_sort }}', status: '{{ current_status }}', layout: '{{ current_layout }}', search: '{{ request.GET.search|default:'' }}', bulkListsOpen: false, statusLabels: {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

The search parameter from the URL, as well as current_sort, current_status, and current_layout, are rendered directly into an Alpine.js x-data attribute. Because these values are enclosed in single quotes and not properly escaped for a JavaScript context, an attacker can provide a malicious value (e.g., in the search query parameter) containing single quotes to break out of the string and execute arbitrary JavaScript (Reflected XSS).

{% for value, label in status_choices %}
'{{ value }}': '{{ label }}'
{% if not forloop.last %},{% endif %}
Expand Down Expand Up @@ -117,6 +137,16 @@ <h1 class="text-3xl font-bold mb-6">{{ media_type|media_type_readable_plural }}<
</div>
</div>

<!-- Bulk Select Toggle -->
<button class="flex items-center px-4 py-2 rounded-md transition-colors cursor-pointer"
:class="$store.bulk.active ? 'bg-indigo-600 text-white' : 'bg-[#39404b] hover:bg-[#454d5a]'"
@click="$store.bulk.active = !$store.bulk.active; if (!$store.bulk.active) $store.bulk.ids = []"
type="button"
title="Select items">
{% include "app/icons/circle-check.svg" with classes="w-4 h-4 mr-2" %}
Select
</button>

<!-- Layout Toggle -->
<div class="flex rounded-md overflow-hidden border border-gray-700">
<a :href="`{% url 'medialist' media_type %}?${search ? 'search=' + search + '&' : ''}${status !== 'all' ? 'status=' + status + '&' : ''}${sort !== 'score' ? 'sort=' + sort + '&' : ''}layout=grid`"
Expand All @@ -133,6 +163,47 @@ <h1 class="text-3xl font-bold mb-6">{{ media_type|media_type_readable_plural }}<
</div>
</div>

<!-- Hidden form keeping selected IDs and CSRF token in sync for hx-include -->
<form id="bulk-ids-form" class="hidden">
<input type="hidden" name="csrfmiddlewaretoken" value="{{ csrf_token }}">
<template x-for="id in $store.bulk.ids" :key="id">
<input type="hidden" name="item_ids" :value="id">
</template>
</form>

<!-- Bulk action bar -->
<div x-show="$store.bulk.active"
x-cloak
class="sticky top-2 z-40 flex items-center gap-3 mb-4 p-3 bg-[#2a2f35] border border-gray-700 rounded-lg shadow-lg">
<span class="text-sm text-gray-300">
<span x-text="$store.bulk.ids.length"></span> selected
</span>
<button class="px-3 py-1.5 text-sm bg-[#39404b] hover:bg-[#454d5a] text-white rounded-md transition-colors cursor-pointer"
type="button"
@click="$store.bulk.selectAll()">Select all visible</button>
<button class="px-3 py-1.5 text-sm bg-emerald-600 hover:bg-emerald-500 text-white rounded-md transition-colors cursor-pointer"
type="button"
x-show="$store.bulk.ids.length > 0"
hx-get="{% url 'bulk_lists_modal' %}"
hx-target="#bulk-lists-modal-content"
hx-trigger="click"
@click="bulkListsOpen = true">Add to list</button>
<button class="ml-auto px-3 py-1.5 text-sm text-gray-400 hover:text-white transition-colors cursor-pointer"
type="button"
@click="$store.bulk.clear()">Cancel</button>
</div>

<!-- Bulk lists modal -->
<div x-show="bulkListsOpen"
x-cloak
@keydown.escape.window="bulkListsOpen = false"
class="fixed inset-0 bg-black/50 flex items-center justify-center z-50">
<div class="w-96 max-h-[90vh] px-4 md:px-0 relative z-60"
@click.outside="bulkListsOpen = false">
<div id="bulk-lists-modal-content"></div>
</div>
</div>

{% if not media_list %}
<div id="empty_list"
class="flex flex-col items-center justify-center py-16 bg-[#2a2f35] rounded-lg">
Expand All @@ -157,6 +228,7 @@ <h3 class="text-xl font-semibold mb-2">No {{ media_type|media_type_readable_plur
<table class="w-full bg-[#2a2f35]">
<thead class="text-left text-gray-400 text-sm">
<tr>
<th x-show="$store.bulk.active" class="p-2 w-10"></th>
<th class="p-2 w-15"></th>
<th class="p-2 pe-8 w-2/5">Title</th>
<th class="p-2 text-center">Score</th>
Expand Down
25 changes: 25 additions & 0 deletions src/templates/lists/components/bulk_fill_lists.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@

<div class="bg-[#2a2f35] p-6 rounded-lg max-h-[70svh] overflow-y-auto">
<div class="flex items-center justify-between mb-6">
<h2 class="text-2xl font-bold text-white">
Add <span x-text="$store.bulk.ids.length"></span> items to list
</h2>
<button class="text-gray-400 hover:text-white cursor-pointer"
@click="bulkListsOpen = false">{% include "app/icons/x.svg" with classes="w-6 h-6" %}</button>
</div>
<ul class="space-y-3">
{% for custom_list in custom_lists %}
<li class="flex items-center justify-between bg-[#39404b] p-3 rounded-md gap-1">
<span class="text-white">{{ custom_list.name }}</span>
<button class="px-3 py-1.5 bg-emerald-600 hover:bg-emerald-500 text-white text-sm rounded-md transition-colors cursor-pointer"
hx-post="{% url 'bulk_list_add' %}"
hx-include="#bulk-ids-form"
hx-vals='{"custom_list_id": "{{ custom_list.id }}"}'
hx-target="#bulk-lists-modal-content"
hx-swap="innerHTML">Add</button>
</li>
{% empty %}
<li class="text-gray-200">You haven't created any lists yet.</li>
{% endfor %}
</ul>
</div>
7 changes: 7 additions & 0 deletions src/templates/lists/components/bulk_fill_lists_success.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<div class="bg-[#2a2f35] p-6 rounded-lg text-center"
x-init="setTimeout(() => { bulkListsOpen = false; Alpine.store('bulk').clear() }, 1500)">
<div class="flex flex-col items-center gap-3 py-4">
{% include "app/icons/states/completed.svg" with classes="w-10 h-10 text-emerald-400" %}
<p class="text-white font-medium">Added to list!</p>
</div>
</div>