Fix #4080

[arogl]

Description

Fixes #4080.

Tested with
https://www.discogs.com/SHOUSE-Love-Tonight-Robin-Schulz-Remix/release/20356324
https://www.discogs.com/release/20356324-SHOUSE-Love-Tonight-Robin-Schulz-Remix

Change REGEX

To Do

• Documentation. (If you've add a new command-line flag, for example, find the appropriate page under docs/ to describe it.)
• Changelog. (Add an entry to docs/changelog.rst near the top of the document.)
• Tests. (Encouraged but not strictly required.)

[wisp3rwind]

Thanks for looking into this 🚀 I have a few concerns regarding the regex, see the inline comment.

[arogl]

I have added the following to the test_discogs.py file, but not sure how to actuqally call the code in discogs.py

    def test_album_for_id(self):
        """Test parsing for a valid Discogs release_id"""
        test_patterns = [('http://www.discogs.com/G%C3%BCnther-Lause-Meru-Ep/release/4354798', 4354798),
                        ('http://www.discogs.com/release/4354798-G%C3%BCnther-Lause-Meru-Ep', 4354798),
                        ('http://www.discogs.com/G%C3%BCnther-4354798Lause-Meru-Ep/release/4354798', 4354798),
                        ('http://www.discogs.com/release/4354798-G%C3%BCnther-4354798Lause-Meru-Ep/', 4354798),
                        ('[r4354798]', 4354798),
                        ('r4354798', 4354798),
                        ('4354798', 4354798),
                        ('yet-another-metadata-provider.org/foo/12345', ''),
                        ('005b84a0-ecd6-39f1-b2f6-6eb48756b268', ''),
                        ]
        for test_pattern, expected in test_patterns:
            for pattern in [
                r'^\[?r?(?P<id>\d+)\]?$',
                r'discogs\.com/release/(?P<id>\d+)-',
                r'discogs\.com/[^/]+/release/(?P<id>\d+)',
                ]:
                match = re.search(pattern, test_pattern)
                if match:
                    match = int(match.group('id'))
                    break
            if not match:
               match = ''
            self.assertEqual(match, expected)

Do I need to redefine the album_for_id method in the test_discogs.py to contain the code required, otherwise I think we need to refactor album_for_id methods across plugins to pull the regex checks as a different method?

[wisp3rwind]

Do I need to redefine the album_for_id method in the test_discogs.py to contain the code required, otherwise I think we need to refactor album_for_id methods across plugins to pull the regex checks as a different method?

I'd say moving the parsing to a separate method in the discogs plugin is easiest, i.e. move the new code to

class DiscogsPlugin(BeetsPlugin):
    # ...
    @staticmethod
    def extract_release_id(album_id):
        # Here goes the new regex parsing, return either an integer id or None

and call that from the new test (thanks by the way for adding one!) and from album_for_id.

[arogl]

Have updated and added tests

[arogl]

@wisp3rwind Thank you for your patience over the last few days.

A learning experience.

[wisp3rwind]

Nice, this is looking good! I have two more stylistic suggestions, the functionality looks fine to me now 👍

[sampsyo]

Yay; this is looking really great. Thanks for working on this, everybody. (Moving the ID extraction to a separate, testable function was a particularly good idea.) 🎉