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

[Bug] Jamendo audio_set->url contains track ID in query params #1700

Closed
1 task
dhruvkb opened this issue Oct 8, 2021 · 2 comments · Fixed by WordPress/openverse-catalog#239
Closed
1 task
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents
Projects

Comments

@dhruvkb
Copy link
Member

dhruvkb commented Oct 8, 2021

Description

Consider a sample url inside the audio_set JSON field:

https://usercontent.jamendo.com?type=album&id=110159&width=200&trackid=938327

The trackid query param (which is not an attribute of the album) causes the album to appear several times in the data and interferes with the DISTINCT constraint imposed on the SQL.

The trackid param should be cleaned out of this field.

Expectation

The data in the audio_set field should solely be related to the audio set. Extraneous data can affect the ability to uniquely filter out audio tracks from the data.

Screenshots

Screenshot 2021-10-08 at 2 03 36 PM

Resolution

  • 🙋 I would be interested in resolving this bug.
@dhruvkb dhruvkb added good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents 🛠 goal: fix Bug fix 💻 aspect: code Concerns the software code in the repository labels Oct 8, 2021
@obulat
Copy link
Contributor

obulat commented Oct 9, 2021

There is a mixup in the field naming. The actual URL that should be distinct is foreign_landing_url in the table on the screenshot. The url field actually has a URL of the set thumbnail (!), so there is probably no need in removing it. There is a field mixup in the AudioStore, and here is the data that is currently being written for audio_set:

{
  'title': 'Opera I',
  'foreign_landing_url': 'https://www.jamendo.com/album/119/opera-i',
  'url': 'https://usercontent.jamendo.com?type=album&id=119&width=200&trackid=730',
  'creator': 'Haeresis',
  'creator_url': 'https://www.jamendo.com/artist/92/haeresis',
  'foreign_identifier': '119'
}

Could you please confirm, @dhruvkb , if this is the naming that API expects (foreign_landing_url -> url, url -> thumbnail):

{
  'title': 'Opera I',
  'url': 'https://www.jamendo.com/album/119/opera-i',
  'thumbnail': 'https://usercontent.jamendo.com?type=album&id=119&width=200&trackid=730',
  'creator': 'Haeresis',
  'creator_url': 'https://www.jamendo.com/artist/92/haeresis',
  'foreign_identifier': '119'
}

@dhruvkb
Copy link
Member Author

dhruvkb commented Oct 9, 2021

I just meant that the url field inside the audio_set json which contains the album thumbnail should not have the trackid param. The field names are fine.

Internally url is used by the API to refer to the cover art and foreign_landing_url as the place to send a user who intends to see the source. But since the URL pertains to the set as a whole, the trackid param in it is redundant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix good first issue New-contributor friendly help wanted Open to participation from the community 🟧 priority: high Stalls work on the project or its dependents
Projects
Archived in project
Openverse
  
Done!
Development

Successfully merging a pull request may close this issue.

2 participants