Skip to content

Commit 792159c

Browse files
committed
feat: improve album listing and standardize default sort order
- Collapse duplicate albums by title in 'albums list' for cleaner display - Remove 'artist' and 'tracks' columns from default album list view - Fix 'min-plays' filtering bug in album query logic - Change default sort order to 'recent' for all 'list' commands - Update documentation and tests to reflect these changes
1 parent f9280ab commit 792159c

7 files changed

Lines changed: 147 additions & 11 deletions

File tree

plans/CLI_UNIFY.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ Ensure consistent `--limit` defaults:
4242

4343
### B. Sorting & Ordering
4444
Standardize `--sort` and `--order` for `list` commands:
45-
- **Options**: `plays` (default), `name`, `recent`.
45+
- **Options**: `plays`, `name`, `recent`.
46+
- **Default Sort**: `recent` (was `plays`).
4647
- **Order**: `desc` (default), `asc`.
4748
- **Affects**: `albums list`, `artists list`, `tracks list` (new).
4849

src/scrobbledb/commands/albums.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def search_albums(ctx, query, database, limit, artist, format, fields, select):
173173
@database_option
174174
@limit_option(default=50)
175175
@filter_options(artist=True, artist_id=True)
176-
@sort_options(sort_choices=["plays", "name", "recent"], default_sort="plays")
176+
@sort_options(sort_choices=["plays", "name", "recent"], default_sort="recent")
177177
@click.option(
178178
"--min-plays",
179179
type=int,

src/scrobbledb/commands/artists.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ def search_artists(ctx, query, database, limit, format, fields, select):
171171
@artists.command(name="list")
172172
@database_option
173173
@limit_option(default=50)
174-
@sort_options(sort_choices=["plays", "name", "recent"], default_sort="plays")
174+
@sort_options(sort_choices=["plays", "name", "recent"], default_sort="recent")
175175
@click.option(
176176
"--min-plays",
177177
type=int,

src/scrobbledb/commands/tracks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ def search_tracks(ctx, query, database, limit, artist, album, format, fields, se
173173
@database_option
174174
@limit_option(default=50)
175175
@filter_options(artist=True, album=True, artist_id=True, album_id=True)
176-
@sort_options(sort_choices=["plays", "name", "recent"], default_sort="plays")
176+
@sort_options(sort_choices=["plays", "name", "recent"], default_sort="recent")
177177
@click.option(
178178
"--min-plays",
179179
type=int,

src/scrobbledb/domain_format.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ def format_albums_list(albums: list[dict], console: Console, fields: Optional[li
686686
}
687687

688688
if not fields:
689-
fields = ["album", "artist", "tracks", "plays", "last_played"]
689+
fields = ["album", "plays", "last_played"]
690690

691691
valid_fields = [f for f in fields if f in field_config]
692692
if not valid_fields:

src/scrobbledb/domain_queries.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -614,9 +614,6 @@ def get_albums_list(
614614
conditions.append("artists.id = ?")
615615
params.append(artist_id)
616616

617-
if min_plays > 0:
618-
conditions.append("play_count >= ?")
619-
620617
where_clause = "WHERE " + " AND ".join(conditions) if conditions else ""
621618

622619
# Determine sort column
@@ -631,9 +628,9 @@ def get_albums_list(
631628

632629
sql = f"""
633630
SELECT
634-
albums.id as album_id,
631+
MAX(albums.id) as album_id,
635632
albums.title as album_title,
636-
artists.name as artist_name,
633+
MAX(artists.name) as artist_name,
637634
COUNT(DISTINCT tracks.id) as track_count,
638635
COUNT(plays.timestamp) as play_count,
639636
MAX(plays.timestamp) as last_played
@@ -642,7 +639,7 @@ def get_albums_list(
642639
LEFT JOIN tracks ON tracks.album_id = albums.id
643640
LEFT JOIN plays ON plays.track_id = tracks.id
644641
{where_clause}
645-
GROUP BY albums.id, albums.title, artists.name
642+
GROUP BY albums.title COLLATE NOCASE
646643
{"HAVING play_count >= ?" if min_plays > 0 else ""}
647644
ORDER BY {order_by} {order_direction}
648645
LIMIT ?

tests/test_list_sorting.py

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
"""Tests for default sorting in list commands."""
2+
3+
import pytest
4+
import json
5+
import tempfile
6+
import os
7+
import datetime
8+
from click.testing import CliRunner
9+
from scrobbledb.commands import albums, tracks, artists
10+
import sqlite_utils
11+
12+
@pytest.fixture
13+
def runner():
14+
return CliRunner()
15+
16+
@pytest.fixture
17+
def temp_db():
18+
fd, path = tempfile.mkstemp(suffix=".db")
19+
os.close(fd)
20+
db = sqlite_utils.Database(path)
21+
22+
# Setup schema
23+
db.execute("""
24+
CREATE TABLE artists (
25+
id TEXT PRIMARY KEY,
26+
name TEXT
27+
)
28+
""")
29+
db.execute("""
30+
CREATE TABLE albums (
31+
id TEXT PRIMARY KEY,
32+
title TEXT,
33+
artist_id TEXT,
34+
FOREIGN KEY(artist_id) REFERENCES artists(id)
35+
)
36+
""")
37+
db.execute("""
38+
CREATE TABLE tracks (
39+
id TEXT PRIMARY KEY,
40+
title TEXT,
41+
album_id TEXT,
42+
FOREIGN KEY(album_id) REFERENCES albums(id)
43+
)
44+
""")
45+
db.execute("""
46+
CREATE TABLE plays (
47+
track_id TEXT,
48+
timestamp TEXT,
49+
FOREIGN KEY(track_id) REFERENCES tracks(id)
50+
)
51+
""")
52+
53+
# Insert data
54+
# Artist A: 10 plays, last played 2024-01-01
55+
# Artist B: 5 plays, last played 2024-02-01 (More recent, fewer plays)
56+
57+
db["artists"].insert_all([
58+
{"id": "art-a", "name": "Artist A"},
59+
{"id": "art-b", "name": "Artist B"},
60+
])
61+
62+
db["albums"].insert_all([
63+
{"id": "alb-a", "title": "Album A", "artist_id": "art-a"},
64+
{"id": "alb-b", "title": "Album B", "artist_id": "art-b"},
65+
])
66+
67+
db["tracks"].insert_all([
68+
{"id": "trk-a", "title": "Track A", "album_id": "alb-a"},
69+
{"id": "trk-b", "title": "Track B", "album_id": "alb-b"},
70+
])
71+
72+
# Plays for A (10 plays, old)
73+
for i in range(10):
74+
db["plays"].insert({
75+
"track_id": "trk-a",
76+
"timestamp": f"2024-01-01T12:00:0{i}"
77+
})
78+
79+
# Plays for B (5 plays, recent)
80+
for i in range(5):
81+
db["plays"].insert({
82+
"track_id": "trk-b",
83+
"timestamp": f"2024-02-01T12:00:0{i}"
84+
})
85+
86+
yield path
87+
88+
db.close()
89+
if os.path.exists(path):
90+
os.unlink(path)
91+
92+
def test_albums_list_default_sort(runner, temp_db):
93+
"""Test that albums list defaults to sorting by recent (last played)."""
94+
result = runner.invoke(albums.albums, ["list", "--database", temp_db, "--format", "json"])
95+
assert result.exit_code == 0
96+
data = json.loads(result.output)
97+
98+
# Should be sorted by recent: Album B (Feb) then Album A (Jan)
99+
assert len(data) == 2
100+
assert data[0]["album_title"] == "Album B"
101+
assert data[1]["album_title"] == "Album A"
102+
103+
def test_artists_list_default_sort(runner, temp_db):
104+
"""Test that artists list defaults to sorting by recent (last played)."""
105+
result = runner.invoke(artists.artists, ["list", "--database", temp_db, "--format", "json"])
106+
assert result.exit_code == 0
107+
data = json.loads(result.output)
108+
109+
# Should be sorted by recent: Artist B (Feb) then Artist A (Jan)
110+
assert len(data) == 2
111+
assert data[0]["artist_name"] == "Artist B"
112+
assert data[1]["artist_name"] == "Artist A"
113+
114+
def test_tracks_list_default_sort(runner, temp_db):
115+
"""Test that tracks list defaults to sorting by recent (last played)."""
116+
result = runner.invoke(tracks.tracks, ["list", "--database", temp_db, "--format", "json"])
117+
assert result.exit_code == 0
118+
data = json.loads(result.output)
119+
120+
# Should be sorted by recent: Track B (Feb) then Track A (Jan)
121+
assert len(data) == 2
122+
assert data[0]["track_title"] == "Track B"
123+
assert data[1]["track_title"] == "Track A"
124+
125+
def test_plays_list_default_sort(runner, temp_db):
126+
"""Test that plays list defaults to sorting by recent (last played)."""
127+
from scrobbledb.commands import plays
128+
result = runner.invoke(plays.plays, ["list", "--database", temp_db, "--format", "json"])
129+
assert result.exit_code == 0
130+
data = json.loads(result.output)
131+
132+
# Should be sorted by recent: Feb plays then Jan plays
133+
# We inserted 5 Feb plays (Track B) and 10 Jan plays (Track A)
134+
# Total 15 plays.
135+
# The most recent should be from Feb.
136+
assert len(data) == 15
137+
assert "2024-02-01" in data[0]["timestamp"]
138+
assert "2024-01-01" in data[-1]["timestamp"]

0 commit comments

Comments
 (0)