search

jellyfin / jellyfin-plugin-reports

Reports Plugin for Jellyfin
https://jellyfin.org
MIT License
30 stars 19 forks source link

Attempt port #1

Closed hawken93 closed 5 years ago

hawken93 commented 5 years ago

This has been very demotivating to fix lol..

Lot of ugly practices relating to the database, and especially hasAnyProviderId looks very ugly in the backend and is prone to match a lot of false positives. the backend code has a commented "suggestion" to create another table instead of having a one-to-many relation inside of this table. They also have removed the dictionary in order to only match on the key, probably because it was not necessary or correct to match on the id itself. Ideally I want to change jellyfin instead because this is stupid. If there's an issue for the database work I'd like to reference it from here so they can see that this needs to be fixed :) By the way, the file in question in Jellyfin is SqliteItemRepository.cs with over 6400 lines!

There's also another error I couldn't really figure out relating to InternalId stuff in ILibraryManager which seems like this module should do something completely different instead.

Finally, there's a missing cast in MusicAlbum.cs in jellyfin which I put there instead of here, so this will error out for now but I'll link a PR to fix this in jellyfin.

JustAMan commented 5 years ago

I think @sparky8251 has the effort on sanitizing the DB going.

hawken93 commented 5 years ago

@nvllsvm it seems like you left a review that got lost by me pushing things, was this implemented or might it have gotten lost by me force pushing?

joshuaboniface commented 5 years ago

@cvium Agreed, better to just revert and go from there. If we know which commit is the basis I can git reset and force-push it up.

cvium commented 5 years ago

@joshuaboniface The last compatible commit is 26905e18b47aa661e6b012138f198fd355ad3d52

joshuaboniface commented 5 years ago

Superseded by #2 - feel free to close @hawken93 if you wish, or if there's other valuable things in here please rebase your new work off the updated master.

hawken93 commented 5 years ago

Thanks. I'll check if I have anything to add to #2 :)