Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature to vote on tags (#240) #241

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions musicbrainzngs/mbxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,20 @@ def make_tag_request(**kwargs):
e_xml = ET.SubElement(e_list, "{%s}%s" % (NS, entity_type.replace('_', '-')))
e_xml.set("{%s}id" % NS, e)
taglist = ET.SubElement(e_xml, "{%s}user-tag-list" % NS)
for tag in tags:
usertag_xml = ET.SubElement(taglist, "{%s}user-tag" % NS)
name_xml = ET.SubElement(usertag_xml, "{%s}name" % NS)
name_xml.text = tag
if isinstance(tags, dict):
for tag, vote in tags.items():
if vote not in ('upvote', 'downvote', 'withdraw'):
raise ValueError("invalid vote: %s" % vote)

usertag_xml = ET.SubElement(taglist, "{%s}user-tag" % NS)
usertag_xml.set('vote', vote)
Copy link
Owner

Choose a reason for hiding this comment

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

we should raise an error if the vote parameter isn't a valid value

Copy link
Author

Choose a reason for hiding this comment

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

I implemented a ValueError

name_xml = ET.SubElement(usertag_xml, "{%s}name" % NS)
name_xml.text = tag
else:
for tag in tags:
usertag_xml = ET.SubElement(taglist, "{%s}user-tag" % NS)
name_xml = ET.SubElement(usertag_xml, "{%s}name" % NS)
name_xml.text = tag
if kwargs.keys():
raise TypeError("make_tag_request() got an unexpected keyword argument '%s'" % kwargs.popitem()[0])

Expand Down
12 changes: 9 additions & 3 deletions musicbrainzngs/musicbrainz.py
Original file line number Diff line number Diff line change
Expand Up @@ -1275,17 +1275,23 @@ def submit_tags(**kwargs):
"""Submit user tags.
Takes parameters named e.g. 'artist_tags', 'recording_tags', etc.,
and of the form:
{entity_id1: {tag1: vote, ...}, ...}
Copy link
Owner

Choose a reason for hiding this comment

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

The documentation should be updated to say what the value of "vote" can be - "upvote", "downvote", or "withdraw". I wonder if it also makes sense to support alternate values for for this? Integers? 1, 0, -1? Booleans? True, False, None?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your comments. I will try to update this branch soon.

And I need to check, if the code still works after all those years.

Copy link
Author

Choose a reason for hiding this comment

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

I improved the documentation now. Is it better now?

For the time being, I would stick with the official values for a vote ("upvote", "downvote", "withdraw"). At the moment, I'm not convinced to include a logic to convert boolean of interger values to this specific strings.


The value of _vote_ can be "upvote", "downvote", or "withdraw".

If you only want to set the tags, you can use a list:
{entity_id1: [tag1, ...], ...}
If you only have one tag for an entity you can use a string instead
of a list.

If you only want to submit one tag, you can use a string:
{entity_id1: tag, ...}

The user's tags for each entity will be set to that list, adding or
removing tags as necessary. Submitting an empty list for an entity
will remove all tags for that entity by the user.
"""
for k, v in kwargs.items():
for id, tags in v.items():
kwargs[k][id] = tags if isinstance(tags, list) else [tags]
kwargs[k][id] = tags if isinstance(tags, (list, dict)) else [tags]

query = mbxml.make_tag_request(**kwargs)
return _do_mb_post("tag", query)
Expand Down
12 changes: 12 additions & 0 deletions test/test_mbxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ def test_make_tag_request(self):
xml = mbxml.make_tag_request(artist_tags={"mbid": ["one", "two"]})
self.assertEqual(expected, xml)

def test_make_tag_request_with_votes(self):
expected = (b'<ns0:metadata xmlns:ns0="http://musicbrainz.org/ns/mmd-2.0#">'
b'<ns0:artist-list><ns0:artist ns0:id="mbid">'
b"<ns0:user-tag-list>"
b'<ns0:user-tag vote="upvote"><ns0:name>one</ns0:name></ns0:user-tag>'
b'<ns0:user-tag vote="downvote"><ns0:name>two</ns0:name></ns0:user-tag>'
b'<ns0:user-tag vote="withdraw"><ns0:name>three</ns0:name></ns0:user-tag>'
b"</ns0:user-tag-list></ns0:artist>"
b"</ns0:artist-list></ns0:metadata>")
xml = mbxml.make_tag_request(artist_tags={"mbid": {"one": "upvote", "two": "downvote", "three": "withdraw"}})
self.assertEqual(expected, xml)

def test_read_error(self):
error = '<?xml version="1.0" encoding="UTF-8"?><error><text>Invalid mbid.</text><text>For usage, please see: http://musicbrainz.org/development/mmd</text></error>'
parts = mbxml.get_error_message(error)
Expand Down
11 changes: 11 additions & 0 deletions test/test_submit.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ def make_xml(**kwargs):
musicbrainz.submit_tags(artist_tags={"mbid": ["one", "two"]})
musicbrainz.mbxml.make_tag_request = oldmake_tag_request

def test_submit_tags_with_votes(self):
self.opener = _common.FakeOpener("<response/>")
musicbrainzngs.compat.build_opener = lambda *args: self.opener
def make_xml(**kwargs):
self.assertEqual({'artist_tags': {'mbid': {"one": "upvote", "two": "downvote", "three": "withdraw"}}}, kwargs)
oldmake_tag_request = musicbrainz.mbxml.make_tag_request
musicbrainz.mbxml.make_tag_request = make_xml

musicbrainz.submit_tags(artist_tags={"mbid": {"one": "upvote", "two": "downvote", "three": "withdraw"}})
musicbrainz.mbxml.make_tag_request = oldmake_tag_request

def test_submit_single_tag(self):
self.opener = _common.FakeOpener("<response/>")
musicbrainzngs.compat.build_opener = lambda *args: self.opener
Expand Down