last.fm settings issue fix (#47)
* last.fm settings issue fix * format + add make format
This commit is contained in:
Harvey
GitHub
@@ -60,8 +60,8 @@ NPM ?= pnpm
|
||||
frontend-test-monitored-artists \
|
||||
frontend-test-playlist-detail \
|
||||
frontend-test-queuehelpers \
|
||||
project-map rebuild \
|
||||
test tests check lint ci
|
||||
rebuild \
|
||||
test tests check lint format ci
|
||||
|
||||
# Help
|
||||
|
||||
@@ -260,9 +260,6 @@ frontend-test-queuehelpers: ## Run queue helper regressions
|
||||
|
||||
# Utilities
|
||||
|
||||
project-map: ## Refresh the project map block
|
||||
cd "$(ROOT_DIR)" && $(PYTHON) scripts/gen-project-map.py
|
||||
|
||||
rebuild: ## Rebuild the application
|
||||
cd "$(ROOT_DIR)" && ./manage.sh --rebuild
|
||||
|
||||
@@ -276,4 +273,8 @@ check: backend-test frontend-check ## Run backend tests and frontend type checks
|
||||
|
||||
lint: backend-lint frontend-lint ## Run linting targets
|
||||
|
||||
format: ## Auto-format backend (ruff --fix) and frontend (prettier)
|
||||
cd "$(ROOT_DIR)" && $(BACKEND_VENV_DIR)/bin/ruff check --fix backend
|
||||
cd "$(FRONTEND_DIR)" && $(NPM) run format
|
||||
|
||||
ci: backend-test backend-lint frontend-check frontend-lint frontend-format-check frontend-test-server ## Run the local CI checks
|
||||
|
||||
@@ -71,8 +71,9 @@ LASTFM_ERROR_MAP: dict[int, tuple[type[Exception], str]] = {
|
||||
9: (ConfigurationError, "Session key expired - please re-authorize with Last.fm"),
|
||||
10: (ConfigurationError, "Invalid API key - check your Last.fm API key"),
|
||||
11: (ExternalServiceError, "Last.fm service is temporarily offline"),
|
||||
26: (ConfigurationError, "API key has been suspended - contact Last.fm support"),
|
||||
14: (TokenNotAuthorizedError, "Token not yet authorized"),
|
||||
17: (ConfigurationError, "Authentication required - re-authorize Last.fm or make your listening history public"),
|
||||
26: (ConfigurationError, "API key has been suspended - contact Last.fm support"),
|
||||
29: (ExternalServiceError, "Rate limit exceeded"),
|
||||
}
|
||||
|
||||
@@ -107,6 +108,10 @@ class LastFmRepository:
|
||||
self._shared_secret = shared_secret
|
||||
self._session_key = session_key
|
||||
|
||||
@property
|
||||
def _can_sign(self) -> bool:
|
||||
return bool(self._shared_secret) and bool(self._session_key)
|
||||
|
||||
def configure(self, api_key: str, shared_secret: str, session_key: str = "") -> None:
|
||||
self._api_key = api_key
|
||||
self._shared_secret = shared_secret
|
||||
@@ -320,6 +325,7 @@ class LastFmRepository:
|
||||
data = await self._request(
|
||||
"user.getTopArtists",
|
||||
params={"user": username, "period": period, "limit": str(limit)},
|
||||
signed=self._can_sign,
|
||||
)
|
||||
artists = [
|
||||
parse_top_artist(item)
|
||||
@@ -340,6 +346,7 @@ class LastFmRepository:
|
||||
data = await self._request(
|
||||
"user.getTopAlbums",
|
||||
params={"user": username, "period": period, "limit": str(limit)},
|
||||
signed=self._can_sign,
|
||||
)
|
||||
albums = [
|
||||
parse_top_album(item)
|
||||
@@ -360,6 +367,7 @@ class LastFmRepository:
|
||||
data = await self._request(
|
||||
"user.getTopTracks",
|
||||
params={"user": username, "period": period, "limit": str(limit)},
|
||||
signed=self._can_sign,
|
||||
)
|
||||
tracks = [
|
||||
parse_top_track(item)
|
||||
@@ -378,6 +386,7 @@ class LastFmRepository:
|
||||
data = await self._request(
|
||||
"user.getRecentTracks",
|
||||
params={"user": username, "limit": str(limit), "extended": "0"},
|
||||
signed=self._can_sign,
|
||||
)
|
||||
tracks = [
|
||||
parse_recent_track(item)
|
||||
@@ -396,6 +405,7 @@ class LastFmRepository:
|
||||
data = await self._request(
|
||||
"user.getLovedTracks",
|
||||
params={"user": username, "limit": str(limit)},
|
||||
signed=self._can_sign,
|
||||
)
|
||||
tracks = [
|
||||
parse_loved_track(item)
|
||||
@@ -414,6 +424,7 @@ class LastFmRepository:
|
||||
data = await self._request(
|
||||
"user.getWeeklyArtistChart",
|
||||
params={"user": username},
|
||||
signed=self._can_sign,
|
||||
)
|
||||
artists = [
|
||||
parse_top_artist(item)
|
||||
@@ -432,6 +443,7 @@ class LastFmRepository:
|
||||
data = await self._request(
|
||||
"user.getWeeklyAlbumChart",
|
||||
params={"user": username},
|
||||
signed=self._can_sign,
|
||||
)
|
||||
albums = [
|
||||
parse_weekly_album_chart_item(item)
|
||||
|
||||
@@ -88,6 +88,14 @@ class TestHandleErrorResponse:
|
||||
with pytest.raises(ExternalServiceError, match="Last.fm error \\(999\\)"):
|
||||
repo._handle_error_response({"error": 999, "message": "weird"})
|
||||
|
||||
def test_error_17_raises_configuration_error(self):
|
||||
repo = _make_repo()
|
||||
with pytest.raises(ConfigurationError, match="re-authorize Last.fm"):
|
||||
repo._handle_error_response({
|
||||
"error": 17,
|
||||
"message": "Login: User required to be logged in",
|
||||
})
|
||||
|
||||
|
||||
class TestConfigureMethod:
|
||||
def test_configure_updates_credentials(self):
|
||||
@@ -107,6 +115,123 @@ class TestConstructorDefaults:
|
||||
assert repo._session_key == ""
|
||||
|
||||
|
||||
class TestCanSignProperty:
|
||||
def test_true_when_both_credentials_present(self):
|
||||
repo = _make_repo(shared_secret="sec", session_key="sk")
|
||||
assert repo._can_sign is True
|
||||
|
||||
def test_false_without_shared_secret(self):
|
||||
repo = _make_repo(shared_secret="", session_key="sk")
|
||||
assert repo._can_sign is False
|
||||
|
||||
def test_false_without_session_key(self):
|
||||
repo = _make_repo(shared_secret="sec", session_key="")
|
||||
assert repo._can_sign is False
|
||||
|
||||
def test_false_with_neither(self):
|
||||
repo = _make_repo(shared_secret="", session_key="")
|
||||
assert repo._can_sign is False
|
||||
|
||||
def test_configure_enables_can_sign(self):
|
||||
repo = _make_repo(shared_secret="", session_key="")
|
||||
assert repo._can_sign is False
|
||||
repo.configure(api_key="k", shared_secret="s", session_key="sk")
|
||||
assert repo._can_sign is True
|
||||
|
||||
|
||||
class TestSignedUserRequests:
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_recent_tracks_sends_signed_request(self):
|
||||
cache = _make_cache()
|
||||
http_client = AsyncMock(spec=httpx.AsyncClient)
|
||||
http_client.get = AsyncMock(
|
||||
return_value=MagicMock(
|
||||
status_code=200,
|
||||
json=lambda: {"recenttracks": {"track": []}},
|
||||
text="",
|
||||
)
|
||||
)
|
||||
repo = LastFmRepository(
|
||||
http_client=http_client,
|
||||
cache=cache,
|
||||
api_key="key",
|
||||
shared_secret="sec",
|
||||
session_key="sk-1",
|
||||
)
|
||||
await repo.get_user_recent_tracks("user1")
|
||||
call_params = http_client.get.call_args.kwargs.get("params", {})
|
||||
assert "api_sig" in call_params
|
||||
assert call_params["sk"] == "sk-1"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_recent_tracks_unsigned_without_session_key(self):
|
||||
cache = _make_cache()
|
||||
http_client = AsyncMock(spec=httpx.AsyncClient)
|
||||
http_client.get = AsyncMock(
|
||||
return_value=MagicMock(
|
||||
status_code=200,
|
||||
json=lambda: {"recenttracks": {"track": []}},
|
||||
text="",
|
||||
)
|
||||
)
|
||||
repo = LastFmRepository(
|
||||
http_client=http_client,
|
||||
cache=cache,
|
||||
api_key="key",
|
||||
)
|
||||
await repo.get_user_recent_tracks("user1")
|
||||
call_params = http_client.get.call_args.kwargs.get("params", {})
|
||||
assert "api_sig" not in call_params
|
||||
assert "sk" not in call_params
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_top_artists_sends_signed_request(self):
|
||||
cache = _make_cache()
|
||||
http_client = AsyncMock(spec=httpx.AsyncClient)
|
||||
http_client.get = AsyncMock(
|
||||
return_value=MagicMock(
|
||||
status_code=200,
|
||||
json=lambda: {"topartists": {"artist": []}},
|
||||
text="",
|
||||
)
|
||||
)
|
||||
repo = LastFmRepository(
|
||||
http_client=http_client,
|
||||
cache=cache,
|
||||
api_key="key",
|
||||
shared_secret="sec",
|
||||
session_key="sk-1",
|
||||
)
|
||||
await repo.get_user_top_artists("user1")
|
||||
call_params = http_client.get.call_args.kwargs.get("params", {})
|
||||
assert "api_sig" in call_params
|
||||
assert call_params["sk"] == "sk-1"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_error_17_through_signed_request_raises_configuration_error(self):
|
||||
cache = _make_cache()
|
||||
http_client = AsyncMock(spec=httpx.AsyncClient)
|
||||
http_client.get = AsyncMock(
|
||||
return_value=MagicMock(
|
||||
status_code=200,
|
||||
json=lambda: {
|
||||
"error": 17,
|
||||
"message": "Login: User required to be logged in",
|
||||
},
|
||||
text="",
|
||||
)
|
||||
)
|
||||
repo = LastFmRepository(
|
||||
http_client=http_client,
|
||||
cache=cache,
|
||||
api_key="key",
|
||||
shared_secret="sec",
|
||||
session_key="sk-1",
|
||||
)
|
||||
with pytest.raises(ConfigurationError, match="re-authorize Last.fm"):
|
||||
await repo.get_user_recent_tracks("user1")
|
||||
|
||||
|
||||
class TestUpdateNowPlaying:
|
||||
@pytest.mark.asyncio
|
||||
async def test_posts_with_required_params(self):
|
||||
|
||||
@@ -46,9 +46,10 @@
|
||||
credsPersisted = !!(form.data?.api_key && form.data?.shared_secret);
|
||||
}
|
||||
|
||||
async function save() {
|
||||
async function save(): Promise<boolean> {
|
||||
const ok = await form.save();
|
||||
if (ok && form.data?.api_key && form.data?.shared_secret) credsPersisted = true;
|
||||
return ok;
|
||||
}
|
||||
|
||||
async function saveAndTest() {
|
||||
@@ -129,7 +130,7 @@
|
||||
<div class="card-body">
|
||||
<h2 class="card-title text-2xl">Last.fm</h2>
|
||||
<p class="text-base-content/70 mb-4">
|
||||
Connect Last.fm to track listens and improve recommendations.
|
||||
Connect Last.fm to scrobble listens and improve recommendations.
|
||||
</p>
|
||||
|
||||
{#if form.loading}
|
||||
@@ -138,7 +139,7 @@
|
||||
</div>
|
||||
{:else if form.data}
|
||||
<ul class="steps steps-horizontal w-full mb-6">
|
||||
<li class="step" class:step-primary={step1Complete}>API Credentials</li>
|
||||
<li class="step" class:step-primary={step1Complete}>API credentials</li>
|
||||
<li class="step" class:step-primary={step2Complete}>Authorize</li>
|
||||
<li class="step" class:step-primary={step3Complete}>Enable</li>
|
||||
</ul>
|
||||
@@ -160,13 +161,11 @@
|
||||
{#if showForm}
|
||||
<div class="space-y-6">
|
||||
<div class="space-y-4">
|
||||
<h3 class="text-lg font-semibold">Step 1 — API Credentials</h3>
|
||||
<h3 class="text-lg font-semibold">Step 1: API credentials</h3>
|
||||
|
||||
{#if !step1Complete}
|
||||
<div class="bg-base-300 rounded-lg p-4 space-y-2">
|
||||
<p class="text-sm font-medium">
|
||||
You will need a Last.fm API key and shared secret:
|
||||
</p>
|
||||
<p class="text-sm font-medium">You'll need a Last.fm API key and shared secret:</p>
|
||||
<ol class="list-decimal list-inside text-sm space-y-1 text-base-content/70">
|
||||
<li>
|
||||
<a
|
||||
@@ -183,7 +182,7 @@
|
||||
<li>
|
||||
Copy the <strong>API Key</strong> and <strong>Shared Secret</strong> from that page
|
||||
</li>
|
||||
<li>Paste them here, then click <strong>Save & Test</strong></li>
|
||||
<li>Paste them here, then select <strong>Save & Test</strong></li>
|
||||
</ol>
|
||||
</div>
|
||||
{/if}
|
||||
@@ -197,7 +196,7 @@
|
||||
type="text"
|
||||
bind:value={form.data.api_key}
|
||||
class="input input-bordered w-full"
|
||||
placeholder="Your Last.fm API key"
|
||||
placeholder="Last.fm API key"
|
||||
/>
|
||||
{#if step1Complete}
|
||||
<div class="label">
|
||||
@@ -224,7 +223,7 @@
|
||||
type={showSecret ? 'text' : 'password'}
|
||||
bind:value={form.data.shared_secret}
|
||||
class="input input-bordered join-item flex-1"
|
||||
placeholder="Your Last.fm shared secret"
|
||||
placeholder="Last.fm shared secret"
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
@@ -278,7 +277,7 @@
|
||||
{#if step1Complete}
|
||||
<div class="divider"></div>
|
||||
<div class="space-y-4">
|
||||
<h3 class="text-lg font-semibold">Step 2 — Authorize</h3>
|
||||
<h3 class="text-lg font-semibold">Step 2: Authorize</h3>
|
||||
|
||||
{#if step2Complete && !pendingToken}
|
||||
<div class="alert alert-success">
|
||||
@@ -298,7 +297,7 @@
|
||||
</button>
|
||||
{:else if !pendingToken}
|
||||
<p class="text-sm text-base-content/70">
|
||||
Open Last.fm in a new tab, approve access, then come back here.
|
||||
Open Last.fm in a new tab, approve access, then return here.
|
||||
</p>
|
||||
<button
|
||||
type="button"
|
||||
@@ -318,7 +317,7 @@
|
||||
<div class="card bg-base-300">
|
||||
<div class="card-body p-4 space-y-3">
|
||||
<p class="text-sm">
|
||||
Once you have approved access in Last.fm, finish the connection here.
|
||||
Once you've approved access in Last.fm, finish connecting it here.
|
||||
</p>
|
||||
<div class="flex gap-2">
|
||||
<button
|
||||
@@ -355,7 +354,7 @@
|
||||
{#if step2Complete || form.data.enabled}
|
||||
<div class="divider"></div>
|
||||
<div class="space-y-4">
|
||||
<h3 class="text-lg font-semibold">Step 3 — Enable</h3>
|
||||
<h3 class="text-lg font-semibold">Step 3: Enable</h3>
|
||||
|
||||
<div class="form-control">
|
||||
<label class="label cursor-pointer justify-start gap-4">
|
||||
@@ -363,6 +362,13 @@
|
||||
type="checkbox"
|
||||
bind:checked={form.data.enabled}
|
||||
class="toggle toggle-primary"
|
||||
onchange={async () => {
|
||||
if (!form.data) return;
|
||||
const prev = !form.data.enabled;
|
||||
const ok = await save();
|
||||
if (!ok && form.data) form.data.enabled = prev;
|
||||
}}
|
||||
disabled={form.saving}
|
||||
/>
|
||||
<div>
|
||||
<span class="label-text font-medium">Enable Last.fm</span>
|
||||
@@ -370,7 +376,7 @@
|
||||
{#if form.data.enabled && !step2Complete}
|
||||
Last.fm is turned on, but the account connection still needs to be finished.
|
||||
{:else}
|
||||
Use Last.fm for scrobbling, recommendations, and extra music details.
|
||||
Use Last.fm for scrobbling, recommendations, and richer music details.
|
||||
{/if}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
@@ -24,9 +24,10 @@
|
||||
const fullyConnected = $derived(step1Complete && step2Complete);
|
||||
const showForm = $derived(!fullyConnected || showDetails);
|
||||
|
||||
async function save() {
|
||||
async function save(): Promise<boolean> {
|
||||
const ok = await form.save();
|
||||
if (ok) dataPersisted = true;
|
||||
return ok;
|
||||
}
|
||||
|
||||
async function test() {
|
||||
@@ -53,7 +54,7 @@
|
||||
<div class="card-body">
|
||||
<h2 class="card-title text-2xl">ListenBrainz</h2>
|
||||
<p class="text-base-content/70 mb-4">
|
||||
Connect to ListenBrainz for personalized recommendations and listening stats.
|
||||
Connect ListenBrainz for tailored recommendations and listening stats.
|
||||
</p>
|
||||
|
||||
{#if form.loading}
|
||||
@@ -83,11 +84,11 @@
|
||||
{#if showForm}
|
||||
<div class="space-y-6">
|
||||
<div class="space-y-4">
|
||||
<h3 class="text-lg font-semibold">Step 1 — Credentials</h3>
|
||||
<h3 class="text-lg font-semibold">Step 1: Credentials</h3>
|
||||
|
||||
{#if !step1Complete}
|
||||
<div class="bg-base-300 rounded-lg p-4 space-y-2 text-sm">
|
||||
<p class="font-medium">To get started:</p>
|
||||
<p class="font-medium">Getting started:</p>
|
||||
<ol class="list-decimal list-inside space-y-1 text-base-content/70">
|
||||
<li>
|
||||
Create a free account at
|
||||
@@ -103,7 +104,7 @@
|
||||
</a>
|
||||
</li>
|
||||
<li>Enter your username below</li>
|
||||
<li>Optionally add your User Token for private statistics</li>
|
||||
<li>Add a User Token if you want private listening stats</li>
|
||||
</ol>
|
||||
</div>
|
||||
{/if}
|
||||
@@ -117,7 +118,7 @@
|
||||
type="text"
|
||||
bind:value={form.data.username}
|
||||
class="input input-bordered w-full"
|
||||
placeholder="Your ListenBrainz username"
|
||||
placeholder="ListenBrainz username"
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -131,7 +132,7 @@
|
||||
type={showToken ? 'text' : 'password'}
|
||||
bind:value={form.data.user_token}
|
||||
class="input input-bordered join-item flex-1"
|
||||
placeholder="For private statistics"
|
||||
placeholder="Needed for private stats"
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
@@ -143,7 +144,7 @@
|
||||
</div>
|
||||
<label class="label">
|
||||
<span class="label-text-alt text-base-content/50">
|
||||
Find your token at
|
||||
Get your token from
|
||||
<a
|
||||
href="https://listenbrainz.org/settings/"
|
||||
target="_blank"
|
||||
@@ -197,7 +198,7 @@
|
||||
{#if step1Complete || form.data.enabled}
|
||||
<div class="divider"></div>
|
||||
<div class="space-y-4">
|
||||
<h3 class="text-lg font-semibold">Step 2 — Enable</h3>
|
||||
<h3 class="text-lg font-semibold">Step 2: Enable</h3>
|
||||
|
||||
<div class="form-control">
|
||||
<label class="label cursor-pointer justify-start gap-4">
|
||||
@@ -205,15 +206,22 @@
|
||||
type="checkbox"
|
||||
bind:checked={form.data.enabled}
|
||||
class="toggle toggle-primary"
|
||||
onchange={async () => {
|
||||
if (!form.data) return;
|
||||
const prev = !form.data.enabled;
|
||||
const ok = await save();
|
||||
if (!ok && form.data) form.data.enabled = prev;
|
||||
}}
|
||||
disabled={form.saving}
|
||||
/>
|
||||
<div>
|
||||
<span class="label-text font-medium">Enable ListenBrainz Integration</span>
|
||||
<span class="label-text font-medium">Enable ListenBrainz</span>
|
||||
<p class="text-xs text-base-content/50">
|
||||
{#if form.data.enabled && !step1Complete}
|
||||
Integration is enabled but credentials are incomplete. Consider updating
|
||||
your username above.
|
||||
ListenBrainz is on, but your username is still missing. Add it above to
|
||||
finish setup.
|
||||
{:else}
|
||||
Show personalized recommendations and listening stats on the home page.
|
||||
Show tailored recommendations and listening stats on the home page.
|
||||
{/if}
|
||||
</p>
|
||||
</div>
|
||||
@@ -235,9 +243,9 @@
|
||||
<div class="divider"></div>
|
||||
<div class="alert alert-info">
|
||||
<span>
|
||||
To scrobble your listening activity to ListenBrainz,
|
||||
To scrobble your listening to ListenBrainz,
|
||||
<a href="/settings?tab=scrobbling" class="link font-medium"
|
||||
>enable it in the Scrobbling tab →</a
|
||||
>turn it on in the Scrobbling tab</a
|
||||
>
|
||||
</span>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user