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 history view count #8336

Merged
merged 2 commits into from
May 4, 2022
Merged

Fix history view count #8336

merged 2 commits into from
May 4, 2022

Conversation

Mamadou78130
Copy link

@Mamadou78130 Mamadou78130 commented May 2, 2022

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

When watching a video that you have not seen before and going to the history.
Note that the history entry shows 2 views instead of 1 view for said video.

I then modified a function to solve the problem.
So when we go to the video history we see that he has 1 marked view.

Before/After Screenshots/Screen Record

  • Before:
    Capture d’écran du 2022-05-02 15-25-46

  • After:
    Capture d’écran du 2022-05-02 15-24-11

Fixes the following issue(s)

Test Manuel
  • Watch a video you've never seen
  • Go down in history
  • And you see the video shows 1 view

Due diligence

Stypox
Stypox previously requested changes May 2, 2022
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Thank you! What a simple change :-)

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 4, 2022

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@litetex litetex merged commit 47af21d into TeamNewPipe:dev May 4, 2022
@AudricV AudricV added the bug Issue is related to a bug label May 4, 2022
@AudricV AudricV changed the title Fixed viewed counting Fix viewed counting in history May 8, 2022
@AudricV
Copy link
Member

AudricV commented Jun 8, 2022

Unfortunately, this was too easy... Now when contents are enqueued, they are saved in the history, but without any view! The corresponding history entries just say No views.

@Mamadou78130 Do you want to care of this, or should it be fixed by someone else or reverted?

@litetex
Copy link
Member

litetex commented Jun 10, 2022

@TiA4f8R
I think this works as expected: You want to watch something but you didn't watch it yet → 0 views but inside the history.

Anyway, could you open a new issue for that, so we can discuss it there? :)

@AudricV
Copy link
Member

AudricV commented Jun 10, 2022

@litetex Sorry, I think I wasn't clear in my comment, that's why you misunderstood my sentence: this happens when they are enqueued and after they have been played, so it is a regression.

Free feel to open an issue yourself about this regression :)

@Stypox Stypox mentioned this pull request Jun 24, 2022
3 tasks
@Stypox Stypox changed the title Fix viewed counting in history Fix history view count Jul 1, 2022
@Stypox Stypox mentioned this pull request Jul 1, 2022
5 tasks
@Stypox
Copy link
Member

Stypox commented Jul 1, 2022

@Mamadou78130 see #8564 for an actual fix, but thank you anyway for this PR :-)

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

Successfully merging this pull request may close these issues.

NewPipe client counts 1 view as 2 views
4 participants