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

support for "date added" and "favourite" fields #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sterling-tenn
Copy link

let me know if there's anything weird with this, but this solves my issue #5 pretty well

there may be a couple edge cases that I might look into at another time but for the most part I'm satisfied with this

@Stampede
Copy link
Owner

It looks mostly OK but I don't have a way to test it. Something I noticed missing is that when something is "loved", then that action gets a timestamp, but you leave it without a timestamp:

annotation_entries.append((generate_annotation_id(), userID, item_id, entry_type, play_count, play_date, rating, loved, None)) (the None at the end sohuld have a timestamp if it's actually loved).

It may not matter, or maybe that is used in Navidrome somewhere non-obvious like for smart playlists, or for ND to make suggestions for favorite songs or something.

If a song is unloved, then leaving that as None is how ND does it already.

Thanks for the PR! Because I can't really test it, I think I will make another branch and then merge your changes into that and people can use it if they want and report issues. But I am not too good at the github thing so gimme a day or 2 to figure out how to do that.

@sterling-tenn
Copy link
Author

Yeah, I didn't put any timestamps for the loved field since that's perfectly fine for how I used it in iTunes. I didn't really care about the timestamp for that, it was just a little indicator for me.

I also just realized that in the case where a song doesn't have a "last played" date (ie never been played before) something fails when updating the "date added" field. I may look into that if I have a time, but it's pretty much a non-issue for me.

One more thing I might change later is just a quick option to specify what path the it_root_music_path points to, because of this #4 (comment), but that's a trivial thing

@bitte-ein-bit
Copy link

I can confirm loved is working for me. Date added not. I had to change it according to the above review.


if playdate > d1[id]['play date']: d1[id].update({'play date': playdate})
if dateadded > d1[id]['date added']: d1[id].update({'date added': dateadded})

Choose a reason for hiding this comment

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

Suggested change
if dateadded > d1[id]['date added']: d1[id].update({'date added': dateadded})
if dateadded > datetime.datetime.fromordinal(1) and dateadded < d1[id]['date added']: d1[id].update({'date added': dateadded})

I think you want to use the earliest date that is defined. All my files had the date I initially imported the files from the disc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants