Skip to content

Fix following_bands returning empty results, add full following support#15

Merged
ALERTua merged 2 commits intoALERTua:mainfrom
teancom:fix/following-bands-response-parsing
Mar 3, 2026
Merged

Fix following_bands returning empty results, add full following support#15
ALERTua merged 2 commits intoALERTua:mainfrom
teancom:fix/following-bands-response-parsing

Conversation

@teancom
Copy link
Copy Markdown
Contributor

@teancom teancom commented Mar 3, 2026

I'm working on a PR for adding Browse support to the Music Assistant Bandcamp provider, and I noticed that as-is, the "Following" stuff wasn't returning anything. I poked around and it's a different data structure than the artists/albums/etc. I fleshed it out to support looking up which artists/labels/etc a person follows, if they follow other BC users, and who follows them. And made it work without a person id as well, for un-authed mode. I don't think I'll be using that in the MA provider but it works fine (every function was tested with my toke and unauthenticated agains the live Bandcamp API).

Also did a tiny amount of cleanup (a typo in a function name sorta stuff) but only of the files I was already in.

Thanks tons for this and for the Bandcamp provider itself (I know you're working on both).

  • Parse followeers/more_available response structure for following endpoints
  • Add FanItem model and following_fans/followers endpoint support
  • Allow unauthenticated access when fan_id is provided
  • Add subdomain validation, defensive parsing, and _build_image_url helper
  • Fix _process_json_responce typo

teancom and others added 2 commits March 2, 2026 21:02
- Parse followeers/more_available response structure for following endpoints
- Add FanItem model and following_fans/followers endpoint support
- Allow unauthenticated access when fan_id is provided
- Add subdomain validation, defensive parsing, and _build_image_url helper
- Fix _process_json_responce typo

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@staticmethod
def _build_image_url(image_id: Any) -> str | None:
"""Build image URL from image_id."""
if not isinstance(image_id, int):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: image_id=0 is treated as a valid image ID here, producing https://f4.bcbits.com/img/0_0.jpg. In the Bandcamp API, image_id=0 is a sentinel value meaning "no image". The existing code in parse_artist uses if data.get("bio_image_id") which correctly treats 0 as falsy/absent.

Consider guarding against zero:

Suggested change
if not isinstance(image_id, int):
if not isinstance(image_id, int) or image_id == 0:

The test test_build_image_url_with_zero currently asserts a URL is returned for 0, but that test expectation may be wrong — a zero image ID will likely result in a broken image link.

@kilo-code-bot
Copy link
Copy Markdown

kilo-code-bot bot commented Mar 3, 2026

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
src/bandcamp_async_api/parsers.py 29 _build_image_url treats image_id=0 as valid, producing a broken image URL. Bandcamp uses 0 as a sentinel for "no image". Should guard with or image_id == 0.
Other Observations (not in diff)

No issues found in unchanged code.

Files Reviewed (7 files)
  • src/bandcamp_async_api/__init__.py — no issues
  • src/bandcamp_async_api/client.py — no issues (typo fix + new fan_id param + following-type dispatch look correct)
  • src/bandcamp_async_api/models.py — no issues (FollowingItem, FanItem, updated CollectionSummary union type all look good)
  • src/bandcamp_async_api/parsers.py — 1 issue (see above)
  • tests/conftest.py — no issues
  • tests/test_client.py — no issues
  • tests/test_parsers.py — no issues

General Notes

This is a well-structured PR. The core logic is sound:

  • The followeers/more_available response structure difference is correctly handled.
  • Subdomain validation via _SUBDOMAIN_RE is a nice security touch.
  • The fan_id parameter enabling unauthenticated access is cleanly implemented.
  • The typo fix (_process_json_responce_process_json_response) is welcome.
  • Test coverage is thorough.

Fix these issues in Kilo Cloud

@ALERTua
Copy link
Copy Markdown
Owner

ALERTua commented Mar 3, 2026

Hey, thank you for the kind words and your contribution.
I don't have any suggestions for your PR. I will add real data tests after the merge.
<3

@ALERTua ALERTua merged commit 8c27338 into ALERTua:main Mar 3, 2026
5 checks passed
@ALERTua
Copy link
Copy Markdown
Owner

ALERTua commented Mar 3, 2026

@teancom
Copy link
Copy Markdown
Contributor Author

teancom commented Mar 3, 2026

Oh, thank you for the super-fast review, @ALERTua! I was wondering about the review comment by kilo about image_id == 0, do I need to fix that? And, for the purposes of testing my branch of the provider, what's the general lead-time to getting a new release published to pypi? I have plenty of other work to do in the meantime, no rush! Just curious if it's hours/days/weeks (I haven't published to pypi myself before).

@ALERTua
Copy link
Copy Markdown
Owner

ALERTua commented Mar 3, 2026

crap I forgot to publish a pypi release sorry. I'm on it

@ALERTua
Copy link
Copy Markdown
Owner

ALERTua commented Mar 3, 2026

As for image_id == 0, you uncovered the API endpoints for your PR, you decide :) Nothing critical if the URL leads to 404 I guess, but maybe it is also OK to return None instead of it :)

@ALERTua
Copy link
Copy Markdown
Owner

ALERTua commented Mar 3, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants