Movies are not sorted correctly if watched on same day

[muhammad-ans]

Description

If we mark as watched two movies on the same day Let's say i watched sicario first and then john wick. In last plays sicario would be shown as latest watched.

Version

0.57.0

Steps to Reproduce

Just add two movies on the same day e.g. sicario first and then john wick. In last plays sicario would be shown as latest watched.

Screenshots

No response

Logs

No response

Server OS

No response

Client Platform

None

Client Device

No response

Client OS

No response

Client Browser

No response

Additional Context

No response

[pbogre]

i noticed this bug as well. it might have something to do with the date/time format that is used for the watch date column in the database

[muhammad-ans]

I think the watch date column just stores date if two movies have same date it sorts them alphabetically

[pbogre]

you're probably right. to fix this issue (and honestly a general improvement) we could use unix millis to store watch dates and then convert it as needed for UI.

Or simply had the time of day at which it was watched.

However i'm just not 100% on how date times are stored so i'm not sure

[leepeuker]

Thanks for reporting this issue @muhammad-ans

The watch date is a simple date, it does not contain time. Currently the last plays are ordered only by this date. If you have watched two different movies on the same day your database decides how they are ordered right now (e.g. ordering by last inserted) because it is not explicitly defined in Movary.

I do not like the idea of adding a time to the watch date because it makes things more difficult to manage for the user and for the development (e.g. timezones differences and stuff). Having to manage only the date is much simpler and I think enough in most cases, it does not happen that often that users watch multiple movies in one day I believe.

But we should definitely find a solution for cases where this does happen.

I am currently thinking about adding a new attribute "position" (or something like that) to movie watch dates, this way you could for example set the "position" for the first watched movie to 1 and the for the second movie to 2. "position" will than be used as secondary order parameter. Newly added watch dates always get the "position" 1 as default, except if there is already one or more watched movies for the date, than we just start adding +1 to the "position" for every new movie added to the date. In case you have added the plays in the wrong order you can manually correct the "position" numbers 🤔

Edit: I just noticed that in mysql we are using the DATETIME type for the watch date at movie_user_watch_dates, so we technically are storing the time, it is just always 00:00:00 because we drop it the code. I think I will change this to DATE to better match how this attribute is defined in the code. sqlite has no date type, so it stays a text. Additionally I noticed we have no created/updated timestamps in the watch dates table, these should probably be added to.

[muhammad-ans]

Sounds good

[leepeuker]

I have added a new attribute position to watch dates and updated the "Edit Watch Date" modal to be a bit more user friendly.

Before:

Now:

Open todos:

• automatically set the correct position if a play is added to a date which already has plays (currently position is always 1 when adding a watch date on default)
• automatically set the correct positions on history imports
• test everything again

[leepeuker]

This part of the nightly tag now, @muhammad-ans @sahinakkaya you are welcome to test it. If you run into any problems please contact me, more testers are always appreciated :D

[sahinakkaya]

I fetched the latest image and restarted the container but now I am getting "500 Internal Server Error" :(

[sahinakkaya]

Let me know if you need any log output and I will try to help.

[leepeuker]

You probably have to run the database migrations?

[sahinakkaya]

Oh, yeah I forgot to run it sorry. After running migrations the error went away but now clicking on "Advanced mode" doesn't do anything. And in the console following error is thrown Uncaught ReferenceError: toggleWatchDateAdvancedMode is not defined

[leepeuker]

I think you probably must force update in your browser, it still uses the old js file from the cache.

[sahinakkaya]

OK, you are right again 🤦‍♂️ Sorry for wasting your precious time twice. I tested the new feature and it works as expected too. Thank you!

[leepeuker]

Ah no problems, at least the migrations should be done automatically anyway, so I feel responsible for this problem anyway :D

[muhammad-ans]

Thanks for this feature. Working as expected but there is no position attribute in export of history. With current implementation it reverses the positions when imported.