Skip to content

fix(discogs_importer): use of undefined release.discs[discindex]#271

Merged
arsinclair merged 3 commits intomurdos:masterfrom
antlarr:fix-sakamoto-yearbook
Mar 14, 2026
Merged

fix(discogs_importer): use of undefined release.discs[discindex]#271
arsinclair merged 3 commits intomurdos:masterfrom
antlarr:fix-sakamoto-yearbook

Conversation

@antlarr
Copy link
Contributor

@antlarr antlarr commented Apr 22, 2020

Opening https://www.discogs.com/Ryuichi-Sakamoto-Year-Book-1985-1989/release/11652104
gives a problem because undefined (release.discs[discindex]) doesn't have
a format member in line 721
release.discs[discindex].format.match(/(Vinyl|Cassette)/) ...

This fixes the problem (it leaves the medium empty and stores its tracks
in the previous medium, but at least it doesn't break the script).

Fixes #158
Fixes #275
Fixes #531
Fixes #80
Fixes #431
Fixes #576
Fixes #570
Fixes #567
Fixes #568
Fixes #557

Opening https://www.discogs.com/Ryuichi-Sakamoto-Year-Book-1985-1989/release/11652104
gives a problem because undefined (release.discs[discindex]) doesn't have
a format member in line 721
`release.discs[discindex].format.match(/(Vinyl|Cassette)/) ...`

This fixes the problem (it leaves the medium empty and stores its tracks
in the previous medium, but at least it doesn't break the script).
@antlarr
Copy link
Contributor Author

antlarr commented Apr 22, 2020

This fixes #158

@murdos murdos self-requested a review August 25, 2021 02:35
@arsinclair
Copy link
Collaborator

arsinclair commented Mar 14, 2026

This one is a tricky case, since there's some discrepancy in the assumptions behind mapping Discogs data to MusicBrainz data. The release in question is a 5CD release with 3 index tracks (track group) and some flat tracks at the end of the tracklist which are not associated with any index tracks. What the current script does is it assumes that Discogs index tracks = MB mediums, however this assumption is not correct. Index tracks on Discogs are often used to group the tracks of one medium together, but not always and it is incorrect to map them to MB mediums. Just like on this 5 CD release, the 3rd index track includes the tracks from the 4th CD.

With the proposed solution it creates an empty medium 4 on MB, which is not ideal, so we should come up with a different way to map tracks to mediums.

I don't have a lot of experience with the Discogs script, so I'm hesitant to rewrite it at this moment, maybe it will happen in the future. But since this PR turns a completely unusable script to a script that imports slightly incorrectly grouped tracks, I think this is good to go.

(another thing I don't understand is why the Discogs script combines all track titles of an index track on Discogs into one track on MB, seems incorrect, but that's a separate discussion)

@arsinclair arsinclair self-assigned this Mar 14, 2026
@arsinclair arsinclair changed the title Fix use of undefined release.discs[discindex] fix(discogs_importer): use of undefined release.discs[discindex] Mar 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment