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 handling of video published date in video lists #4752

Merged
merged 5 commits into from
Mar 13, 2024

Conversation

absidue
Copy link
Member

@absidue absidue commented Mar 8, 2024

Fix handling of video published date in video lists

Pull Request Type

  • Bugfix
  • Feature Implementation

Related issue

Fixes #4541 (not sure if GitHub automatically closes discussions with pull requests but I'm listing it here anyway)
Fixes #2579
Closes #3736

Description

This started as an investigation into #4541 (FreeTube showing published dates in German, despite the display language being set to English) and in the process of fixing that, I ended up solving #2579 and implementing #3736.

The issue with the translated published strings was because we were using the publishedText property in Invidious' API response, which it turns out is a translatable string on the Invidious side, so when an instance like invidious.nerdvpn.de decides to randomly return German translations for no apparent reason, FreeTube bugs out and shows that to the user. The solution to that is using the published property instead, which contains Invidious' parsing of the publishedText into a unix timestamp in milliseconds.

As I was switching to the published field for Invidious anyway, I decided I might as well do it for the local API too, that means parsing the relative date strings during the local API parsing, so that ft-list-video only has to deal with unix timestamps. Converting it back to a relative timestamp is then done in ft-list-video based on the current date and time. One major benefit of doing that, is that when we save videos in the subscription cache, they are no longer saved with 10 minutes ago, instead they have the unix timestamp, so instead of only being able to show 10 minutes ago when it is restored from the cache 3 hours later, it will now show 3 hours ago.
Please note that as these timestamps are calculated based on relative timestamps, they might not match up with YouTube's, as they have access to the absolute timestamp in their databases. However showing a potentially slightly off relative timestamp is a lot better than still showing 10 minutes ago 3 hours after the video was published.

The mark as watched invalid date issue was caused by us saving the text "10 minutes ago" in the watch history, but as we save the unix timestamp on the watch page, the code that read the date from the history was expecting a unix timestamp, which "10 mins ago" most certainly is not. The solution to that was quite easy now that the rest of the ft-list-video component was already using unix timestamps.

Testing

  1. Test various video lists (e.g. subscriptions, search, trending) with both the local and Invidious API and make sure that published dates show up correctly.
  2. Mark a video as watched from the subscription page (3 dots menu -> Mark as Watched) with the local or Invidious API without RSS and check the history tab to confirm that it shows an actual date and time instead of Invalid date
  3. Refresh your subscriptions without RSS, wait a while and then come back later, switch to a different page (the timestamps don't update live while you are on the same page, as that would be quite expensive to do every second) and then back to the subscriptions page and it should show updated timestamps without you refreshing the subscriptions.

This pull request doesn't change how published dates are handled for community posts, but I will switch that to the published field in a follow up pull request, so we get the same updated timestamps when we restore it from the cache.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: latest nightly

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Mar 8, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) March 8, 2024 23:07
@PikachuEXE
Copy link
Collaborator

Date format seems changed for some reason

Before this PR (history page)
image

This PR
image

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented Mar 9, 2024

Seeing the same as Pika

Also

Something is wrong bc this podcast is live once a week on YT and you will see here that the published date for multiple streams is marked as 30 days ago

VirtualBoxVM_ucMWIJgmuK.mp4

@absidue
Copy link
Member Author

absidue commented Mar 9, 2024

I had based it on the existing formatting code, which seems to only switch to months when it's above 31 days and doesn't support weeks, so I'll need to change that.

@absidue
Copy link
Member Author

absidue commented Mar 9, 2024

@PikachuEXE @efb4f5ff-1298-471a-8973-3d47447115dc Those issues should be fixed now :).

Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

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

Marked watched date reflects date of vid being published when viewing it in History

VirtualBoxVM_RpmIppbh8C.mp4

Edit: Most popular tab has interesting publish dates

Capture

@absidue
Copy link
Member Author

absidue commented Mar 9, 2024

Marked watched date reflects date of vid being published when viewing it in History

That's the same as if you watch it on the watch page.

@efb4f5ff-1298-471a-8973-3d47447115dc

Wow im not sure i ever noticed that. To me that doesnt make sense but that is probably out of scope of this PR

@absidue
Copy link
Member Author

absidue commented Mar 9, 2024

Fixed now (Invidious gives all videos on the popular page the type shortVideo, even though they are clearly normal videos).

@PikachuEXE
Copy link
Collaborator

I still see date format change
image

@absidue
Copy link
Member Author

absidue commented Mar 9, 2024

It respects the users chosen locale now, so as you presumably selected en-US as your display language, you get month/day/year.

@FreeTubeBot FreeTubeBot merged commit 98aded9 into FreeTubeApp:development Mar 13, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Mar 13, 2024
@absidue absidue deleted the video-published-fix branch March 13, 2024 06:30
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Mar 13, 2024
…h-matching-videos

* development:
  Fix handling of video published date in video lists (FreeTubeApp#4752)
  Translated using Weblate (Dutch)
  Translated using Weblate (Chinese (Simplified))
  Bump lefthook from 1.6.4 to 1.6.5 (FreeTubeApp#4758)
  Bump marked from 12.0.0 to 12.0.1 (FreeTubeApp#4757)
  Bump electron from 29.1.0 to 29.1.1 (FreeTubeApp#4756)
  Translated using Weblate (Portuguese)
  Translated using Weblate (Portuguese (Brazil))
  Improve touch controls for dash quality selector (FreeTubeApp#4750)
  Translated using Weblate (Serbian)
  Translated using Weblate (Spanish)
  Translated using Weblate (French)
  Translated using Weblate (German)
  Translated using Weblate (Estonian)
PikachuEXE added a commit to PikachuEXE/FreeTube that referenced this pull request Mar 13, 2024
* development: (28 commits)
  Fix handling of video published date in video lists (FreeTubeApp#4752)
  Translated using Weblate (Dutch)
  Translated using Weblate (Chinese (Simplified))
  Bump lefthook from 1.6.4 to 1.6.5 (FreeTubeApp#4758)
  Bump marked from 12.0.0 to 12.0.1 (FreeTubeApp#4757)
  Bump electron from 29.1.0 to 29.1.1 (FreeTubeApp#4756)
  Translated using Weblate (Portuguese)
  Translated using Weblate (Portuguese (Brazil))
  Improve touch controls for dash quality selector (FreeTubeApp#4750)
  Translated using Weblate (Serbian)
  Translated using Weblate (Spanish)
  Translated using Weblate (French)
  Translated using Weblate (German)
  Translated using Weblate (Estonian)
  Translated using Weblate (Polish)
  Translated using Weblate (Basque)
  Translated using Weblate (Portuguese (Brazil))
  Translated using Weblate (Portuguese (Brazil))
  Translated using Weblate (Portuguese (Brazil))
  Fix playlists database import and export not using the actual database format (FreeTubeApp#4664)
  ...

# Conflicts:
#	src/renderer/views/Search/Search.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants