Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
25 changes: 25 additions & 0 deletions src-tauri/src/commands/playlist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,31 @@ pub async fn list_playlist_tracks(
Ok(tracks)
}

/// Return the IDs of every user playlist that currently contains `track_id`.
/// Smart playlists are excluded — their membership is computed on the fly
/// from rules and would be misleading to expose as a toggle target.
///
/// Used by the `+` popover to render a checkmark on rows the track is
/// already in (and to flip the click handler from "add" to "remove").
#[tauri::command]
pub async fn list_playlists_containing_track(
state: tauri::State<'_, AppState>,
track_id: i64,
) -> AppResult<Vec<i64>> {
let pool = state.require_profile_pool().await?;
let rows: Vec<(i64,)> = sqlx::query_as(
"SELECT pt.playlist_id
FROM playlist_track pt
JOIN playlist p ON p.id = pt.playlist_id
WHERE pt.track_id = ?
AND p.is_smart = 0",
)
.bind(track_id)
.fetch_all(&pool)
.await?;
Ok(rows.into_iter().map(|(id,)| id).collect())
}

/// Append a single track to the end of a playlist. Idempotent — if the track
/// is already in the playlist the existing row is preserved and `updated_at`
/// is still bumped so the UI reflects the user's intent.
Expand Down
1 change: 1 addition & 0 deletions src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ pub fn run() {
commands::playlist::update_playlist,
commands::playlist::delete_playlist,
commands::playlist::list_playlist_tracks,
commands::playlist::list_playlists_containing_track,
commands::playlist::add_track_to_playlist,
commands::playlist::add_tracks_to_playlist,
commands::playlist::remove_track_from_playlist,
Expand Down
100 changes: 95 additions & 5 deletions src/components/views/LibraryView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ import { resolvePlaylistColor } from "../../lib/playlistVisuals";
import { resolveArtwork } from "../../lib/tauri/artwork";
import { FadeInImage } from "../common/FadeInImage";
import { PlaylistIcon } from "../../lib/PlaylistIcon";
import type { Playlist } from "../../lib/tauri/playlist";
import {
listPlaylistsContainingTrack,
type Playlist,
} from "../../lib/tauri/playlist";
import { pickFolder } from "../../lib/tauri/dialog";
import {
removeFolderFromLibrary,
Expand Down Expand Up @@ -137,6 +140,7 @@ export function LibraryView({
const {
playlists,
addTracksToPlaylist,
removeTrackFromPlaylist,
addSourceToPlaylist,
createPlaylist,
} = usePlaylist();
Expand Down Expand Up @@ -541,6 +545,16 @@ export function LibraryView({
console.error("[LibraryView] add to playlist failed", err);
}
}}
onRemoveFromPlaylist={async (playlistId, trackId) => {
try {
await removeTrackFromPlaylist(playlistId, trackId);
} catch (err) {
console.error(
"[LibraryView] remove from playlist failed",
err,
);
}
}}
onCreatePlaylist={(trackId) => {
setPendingSourceForCreate({ kind: "tracks", ids: [trackId] });
setIsCreatePlaylistModalOpen(true);
Expand Down Expand Up @@ -945,7 +959,11 @@ interface TrackTableProps {
likedIds: Set<number>;
onToggleLike: (trackId: number) => void;
playlists: Playlist[];
onAddToPlaylist: (playlistId: number, trackId: number) => void;
onAddToPlaylist: (playlistId: number, trackId: number) => Promise<void> | void;
onRemoveFromPlaylist: (
playlistId: number,
trackId: number,
) => Promise<void> | void;
onCreatePlaylist: (trackId: number) => void;
onNavigateToAlbum: (albumId: number) => void;
onNavigateToArtist: (artistId: number) => void;
Expand All @@ -966,6 +984,7 @@ function TrackTable({
onToggleLike,
playlists,
onAddToPlaylist,
onRemoveFromPlaylist,
onCreatePlaylist,
onNavigateToAlbum,
onNavigateToArtist,
Expand All @@ -976,6 +995,13 @@ function TrackTable({
"use no memo";
const unknown = t("library.table.unknown");
const [openMenuTrackId, setOpenMenuTrackId] = useState<number | null>(null);
// Per-track playlist membership snapshot, fetched the first time the
// user opens the `+` popover for a given track. Entry stays cached for
// the lifetime of the table so reopening the menu is instant. Optimistic
// updates flip the set on toggle.
const [trackMembership, setTrackMembership] = useState<
Map<number, Set<number>>
>(new Map());
const [ratingOverrides, setRatingOverrides] = useState<
Map<number, number | null>
>(new Map());
Expand Down Expand Up @@ -1208,7 +1234,27 @@ function TrackTable({
data-add-to-playlist-trigger
onClick={(e) => {
e.stopPropagation();
setOpenMenuTrackId(isMenuOpen ? null : track.id);
const opening = !isMenuOpen;
setOpenMenuTrackId(opening ? track.id : null);
// Lazy-fetch membership the first time this track's
// popover is opened. Subsequent opens reuse the cached
// set (kept in sync via optimistic updates on toggle).
if (opening && !trackMembership.has(track.id)) {
listPlaylistsContainingTrack(track.id)
.then((ids) => {
setTrackMembership((prev) => {
const next = new Map(prev);
next.set(track.id, new Set(ids));
return next;
});
Comment on lines +1230 to +1242
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Stale fetch clobbers toggles 🐞 Bug ≡ Correctness

In TrackTable, listPlaylistsContainingTrack() resolves asynchronously and unconditionally overwrites
trackMembership for the track, so a late response can erase optimistic add/remove flips (or race
with multiple in-flight fetches) and render incorrect checkmarks/toggle behavior.
Agent Prompt
## Issue description
`listPlaylistsContainingTrack(track.id)` writes `next.set(track.id, new Set(ids))` when it resolves, regardless of whether the user already toggled membership for that track while the request was in flight. This can clobber optimistic updates and/or produce an incorrect cached membership set.

## Issue Context
The race is possible whenever the user opens the `+` popover and clicks a playlist before the membership fetch returns, or when the popover is opened/closed/reopened quickly (triggering multiple concurrent fetches).

## Fix Focus Areas
- src/components/views/LibraryView.tsx[1224-1290]

## Implementation direction
- Track per-track in-flight requests (e.g., `membershipFetchVersion: Map<trackId, number>` or `inFlight: Set<trackId>`).
- When a fetch resolves, update state only if it’s still the latest request for that track.
- Merge fetched IDs with any optimistic mutations that occurred during the request (e.g., record pending ops per track while loading, then apply them to the fetched `Set` before committing).
- Prevent duplicate fetches for the same track while one is already in flight.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

})
.catch((err) => {
console.error(
"[LibraryView] load membership failed",
err,
);
});
}
}}
aria-label={t("trackActions.addToPlaylist")}
aria-haspopup="menu"
Expand All @@ -1225,8 +1271,28 @@ function TrackTable({
<AddToPlaylistPopover
playlists={playlists}
trackId={track.id}
memberPlaylistIds={trackMembership.get(track.id)}
onPick={(playlistId) => {
onAddToPlaylist(playlistId, track.id);
const members = trackMembership.get(track.id);
const isMember = members?.has(playlistId) ?? false;
// Optimistic membership flip — the underlying mutations
// are idempotent on the backend, so a failed RPC just
// means the visual state will drift until the next
// popover open, which is the worst-case loss for a
// single click.
setTrackMembership((prev) => {
const next = new Map(prev);
const set = new Set(next.get(track.id) ?? []);
if (isMember) set.delete(playlistId);
else set.add(playlistId);
next.set(track.id, set);
return next;
});
if (isMember) {
void onRemoveFromPlaylist(playlistId, track.id);
} else {
void onAddToPlaylist(playlistId, track.id);
}
Comment on lines +1269 to +1288
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Smart playlists become toggleable 🐞 Bug ≡ Correctness

Although the backend membership API excludes smart playlists, the frontend optimistic membership set
can still add smart playlist IDs, causing smart playlists to show checkmarks and later route clicks
to removeTrackFromPlaylist, contradicting the intended “non-toggle target” behavior.
Agent Prompt
## Issue description
Smart playlists are excluded server-side from `list_playlists_containing_track`, but the client-side optimistic update adds/removes *any* `playlistId` into `trackMembership`. This allows smart playlists to gain checkmarks and be treated as toggle targets (including calling remove), which contradicts the feature intent.

## Issue Context
- Backend explicitly filters `p.is_smart = 0` for the membership list.
- Frontend uses `memberPlaylistIds` both to render checkmarks and to decide add vs remove.
- Smart playlists are re-materialized by deleting and recreating `playlist_track` rows, so manual toggles are not durable.

## Fix Focus Areas
- src/components/views/LibraryView.tsx[1264-1387]
- src-tauri/src/commands/playlist.rs[419-442]
- src/lib/tauri/playlist.ts[9-34]

## Implementation direction
- Ensure smart playlists never render a membership checkmark: in `AddToPlaylistPopover`, compute `isMember` as `!pl.is_smart && (memberPlaylistIds?.has(pl.id) ?? false)`.
- In `TrackTable.onPick`, look up the playlist by id (from `playlists`) and:
  - If `pl.is_smart`, always call the add handler and **do not** mutate `trackMembership` (so smart playlists can’t become “members” in the local Set).
  - Otherwise keep current toggle/optimistic behavior.
- Optionally: consider filtering smart playlists out of the list entirely when `memberPlaylistIds` is provided, if the product intent is “not a target at all.”

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

setOpenMenuTrackId(null);
}}
onCreate={() => {
Expand All @@ -1251,6 +1317,14 @@ interface AddToPlaylistPopoverProps {
onPick: (playlistId: number) => void;
onCreate: () => void;
t: Translator;
/**
* Optional set of playlist IDs the target is already in. Only meaningful
* for the track popover — when provided, matching rows render a green
* checkmark and the caller is expected to toggle (remove) rather than
* add on click. Albums/artists/folders skip this prop because their
* "+ to playlist" action is a bulk add with no symmetric remove.
*/
memberPlaylistIds?: ReadonlySet<number>;
}

/**
Expand All @@ -1266,6 +1340,7 @@ function AddToPlaylistPopover({
onPick,
onCreate,
t,
memberPlaylistIds,
}: AddToPlaylistPopoverProps) {
return (
<div
Expand All @@ -1285,11 +1360,19 @@ function AddToPlaylistPopover({
) : (
playlists.map((pl) => {
const color = resolvePlaylistColor(pl.color_id);
const isMember = memberPlaylistIds?.has(pl.id) ?? false;
return (
<button
key={pl.id}
type="button"
role="menuitem"
aria-label={
isMember
? t("trackActions.removeFromPlaylistNamed", {
name: pl.name,
})
: t("trackActions.addToPlaylistNamed", { name: pl.name })
}
onClick={() => onPick(pl.id)}
className="w-full flex items-center space-x-2 p-2 rounded-lg text-left hover:bg-zinc-50 dark:hover:bg-zinc-700/30 transition-colors"
>
Expand All @@ -1298,9 +1381,16 @@ function AddToPlaylistPopover({
>
<PlaylistIcon iconId={pl.icon_id} size={14} />
</div>
<span className="text-sm font-medium text-zinc-800 dark:text-zinc-200 truncate">
<span className="flex-1 text-sm font-medium text-zinc-800 dark:text-zinc-200 truncate">
{pl.name}
</span>
{isMember && (
<Check
size={14}
className="shrink-0 text-emerald-500"
aria-hidden="true"
/>
)}
</button>
);
})
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/ar.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "تشغيل الراديو",
"rating": "التقييم",
"ratingNone": "بدون تقييم",
"batchEdit": "تعديل علامات {{count}} مقطوعات…"
"batchEdit": "تعديل علامات {{count}} مقطوعات…",
"addToPlaylistNamed": "إضافة إلى \"{{name}}\"",
"removeFromPlaylistNamed": "إزالة من \"{{name}}\""
},
"batchTagEdit": {
"title": "تحرير العلامات بشكل جماعي",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Radio starten",
"rating": "Bewertung",
"ratingNone": "Keine Bewertung",
"batchEdit": "Tags für {{count}} Titel bearbeiten…"
"batchEdit": "Tags für {{count}} Titel bearbeiten…",
"addToPlaylistNamed": "Zu \"{{name}}\" hinzufügen",
"removeFromPlaylistNamed": "Aus \"{{name}}\" entfernen"
},
"batchTagEdit": {
"title": "Tags im Stapel bearbeiten",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,9 @@
"startRadio": "Start radio",
"rating": "Rating",
"ratingNone": "No rating",
"batchEdit": "Edit tags for {{count}} tracks…"
"batchEdit": "Edit tags for {{count}} tracks…",
"addToPlaylistNamed": "Add to \"{{name}}\"",
"removeFromPlaylistNamed": "Remove from \"{{name}}\""
},
"batchTagEdit": {
"title": "Batch edit tags",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/es.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Iniciar la radio",
"rating": "Valoración",
"ratingNone": "Sin valoración",
"batchEdit": "Editar etiquetas de {{count}} canciones…"
"batchEdit": "Editar etiquetas de {{count}} canciones…",
"addToPlaylistNamed": "Añadir a \"{{name}}\"",
"removeFromPlaylistNamed": "Quitar de \"{{name}}\""
},
"batchTagEdit": {
"title": "Editar etiquetas en lote",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,9 @@
"startRadio": "Démarrer la radio",
"rating": "Note",
"ratingNone": "Aucune note",
"batchEdit": "Modifier les tags de {{count}} morceaux…"
"batchEdit": "Modifier les tags de {{count}} morceaux…",
"addToPlaylistNamed": "Ajouter \"{{name}}\" à cette playlist",
"removeFromPlaylistNamed": "Retirer \"{{name}}\" de cette playlist"
},
"batchTagEdit": {
"title": "Modifier les tags en lot",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/hi.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "रेडियो शुरू करें",
"rating": "रेटिंग",
"ratingNone": "कोई रेटिंग नहीं",
"batchEdit": "{{count}} ट्रैक के टैग संपादित करें…"
"batchEdit": "{{count}} ट्रैक के टैग संपादित करें…",
"addToPlaylistNamed": "\"{{name}}\" में जोड़ें",
"removeFromPlaylistNamed": "\"{{name}}\" से हटाएं"
},
"batchTagEdit": {
"title": "एकसाथ टैग संपादन",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/id.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Mulai radio",
"rating": "Peringkat",
"ratingNone": "Tanpa peringkat",
"batchEdit": "Edit tag untuk {{count}} lagu…"
"batchEdit": "Edit tag untuk {{count}} lagu…",
"addToPlaylistNamed": "Tambahkan ke \"{{name}}\"",
"removeFromPlaylistNamed": "Hapus dari \"{{name}}\""
},
"batchTagEdit": {
"title": "Edit tag massal",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/it.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Avvia radio",
"rating": "Valutazione",
"ratingNone": "Nessuna valutazione",
"batchEdit": "Modifica tag per {{count}} brani…"
"batchEdit": "Modifica tag per {{count}} brani…",
"addToPlaylistNamed": "Aggiungi a \"{{name}}\"",
"removeFromPlaylistNamed": "Rimuovi da \"{{name}}\""
},
"batchTagEdit": {
"title": "Modifica tag in blocco",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/ja.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "ラジオを開始",
"rating": "評価",
"ratingNone": "評価なし",
"batchEdit": "{{count}}曲のタグを編集…"
"batchEdit": "{{count}}曲のタグを編集…",
"addToPlaylistNamed": "「{{name}}」に追加",
"removeFromPlaylistNamed": "「{{name}}」から削除"
},
"batchTagEdit": {
"title": "タグの一括編集",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/kr.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "라디오 시작",
"rating": "평점",
"ratingNone": "평점 없음",
"batchEdit": "{{count}}곡의 태그 편집…"
"batchEdit": "{{count}}곡의 태그 편집…",
"addToPlaylistNamed": "\"{{name}}\"에 추가",
"removeFromPlaylistNamed": "\"{{name}}\"에서 제거"
},
"batchTagEdit": {
"title": "태그 일괄 편집",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/nl.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Start radio",
"rating": "Beoordeling",
"ratingNone": "Geen beoordeling",
"batchEdit": "Tags voor {{count}} nummers bewerken…"
"batchEdit": "Tags voor {{count}} nummers bewerken…",
"addToPlaylistNamed": "Toevoegen aan \"{{name}}\"",
"removeFromPlaylistNamed": "Verwijderen uit \"{{name}}\""
},
"batchTagEdit": {
"title": "Tags in batch bewerken",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/pt-BR.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Iniciar rádio",
"rating": "Classificação",
"ratingNone": "Sem classificação",
"batchEdit": "Editar tags de {{count}} faixas…"
"batchEdit": "Editar tags de {{count}} faixas…",
"addToPlaylistNamed": "Adicionar a \"{{name}}\"",
"removeFromPlaylistNamed": "Remover de \"{{name}}\""
},
"batchTagEdit": {
"title": "Editar tags em lote",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/pt.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Iniciar rádio",
"rating": "Classificação",
"ratingNone": "Sem classificação",
"batchEdit": "Editar tags de {{count}} faixas…"
"batchEdit": "Editar tags de {{count}} faixas…",
"addToPlaylistNamed": "Adicionar a \"{{name}}\"",
"removeFromPlaylistNamed": "Remover de \"{{name}}\""
},
"batchTagEdit": {
"title": "Editar etiquetas em lote",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/ru.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Включить радио",
"rating": "Оценка",
"ratingNone": "Без оценки",
"batchEdit": "Изменить теги для {{count}} треков…"
"batchEdit": "Изменить теги для {{count}} треков…",
"addToPlaylistNamed": "Добавить в \"{{name}}\"",
"removeFromPlaylistNamed": "Удалить из \"{{name}}\""
},
"batchTagEdit": {
"title": "Массовое редактирование тегов",
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/locales/tr.json
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,9 @@
"startRadio": "Radyoyu başlat",
"rating": "Puan",
"ratingNone": "Puan yok",
"batchEdit": "{{count}} parça için etiketleri düzenle…"
"batchEdit": "{{count}} parça için etiketleri düzenle…",
"addToPlaylistNamed": "\"{{name}}\" listesine ekle",
"removeFromPlaylistNamed": "\"{{name}}\" listesinden çıkar"
},
"batchTagEdit": {
"title": "Toplu etiket düzenleme",
Expand Down
Loading
Loading