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

Fix music video db update #742

Merged
merged 2 commits into from
Aug 18, 2023

Conversation

GregoireDruant
Copy link
Contributor

  • Artist link to a music video was inserted using "insert or update", passing values that did not constitute an unique key, resulting in duplicate links (and appearing like duplicate videos in db, at least with my kodi settings).
  • In musicvideos.py, obj['LibraryId'] and obj['LibraryName'] was incorrectly set, when they were correctly set a few lines before.
    I removed the problematic lines of code (see diff).
    This fixes automatic updates of the music video library, new videos were not added before this change.

INSERT OR REPLACE does not work when null values are provided as part of the unique index
Which caused new videos not to be added to library on automatic update
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@oddstr13 oddstr13 changed the title Fix/music video db update Fix music video db update Jun 14, 2023
@oddstr13
Copy link
Member

I would like to apologize.

Indecision of what is the best way of solving this has struck me, and I have failed to communicate this fact with you by commenting on this PR.

I did take a look around these parts and made an alternative patch a few weeks back (see #760), but that patch still leaves duplicates if the information is changed Jellyfin side (old ones are not deleted).
The thing is that the same person can have multiple roles, but I don't know how (or if) Jellyfin and the metadata providers handles this.

Your approach here would clobber such information if it does exist, and I haven't managed to decide if that's a good thing or not.

I will say that I'm not a huge fan of adding more SQL queries to maintain if it can be avoided.

@GregoireDruant
Copy link
Contributor Author

GregoireDruant commented Aug 1, 2023

The thing is that the same person can have multiple roles, but I don't know how (or if) Jellyfin and the metadata providers handles this.
Your approach here would clobber such information if it does exist, and I haven't managed to decide if that's a good thing or not.

Hi,
No worries :)

I do not see why my approach would mess with people having different roles.
I just insert new actor_link if this one does not exist.
I do not change any entry in the actor table, thus leaving intact any pre existing actor_link entries with other videos.

I believe I am doing what the previous developer intended to do, but the previous implementation was flawed because the query is not manipulating data that constitute a primary key.

When creating artists links to music videos, jellyfin-kodi does not assign any role.

@oddstr13 oddstr13 merged commit dd4b996 into jellyfin:master Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants