Fix following_bands returning empty results, add full following support#15
Conversation
- 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): |
There was a problem hiding this comment.
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:
| 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.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)No issues found in unchanged code. Files Reviewed (7 files)
General NotesThis is a well-structured PR. The core logic is sound:
|
|
Hey, thank you for the kind words and your contribution. |
|
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). |
|
crap I forgot to publish a pypi release sorry. I'm on it |
|
As for |
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).