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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added "total views" in the my channels list #5007

Merged
merged 7 commits into from May 31, 2022
Merged

Added "total views" in the my channels list #5007

merged 7 commits into from May 31, 2022

Conversation

Poslovitch
Copy link
Contributor

@Poslovitch Poslovitch commented May 27, 2022 •
edited

Description

I added a "total views" counter for each channel in the "my-channels" list.

As requested in #4484, I added a test 馃槈

Related issues

Implements #4331, completes #4484.

Has this been tested?

  • 馃憤 yes, I added tests to the test suite

Screenshots

@Poslovitch
Copy link
Contributor Author

The test fails, seems like it's the same issue I encountered with #5004 : it sends a "null"/"undefined" value.

Chocobozzz
Chocobozzz reviewed May 30, 2022
View reviewed changes
Copy link
Owner

@Chocobozzz Chocobozzz left a comment

Choose a reason for hiding this comment

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

Thanks!

Could you also update OpenAPI documentation please?

server/tests/api/videos/video-channels.ts Outdated Show resolved Hide resolved
@Poslovitch
Copy link
Contributor Author

Done!

Chocobozzz
Chocobozzz reviewed May 30, 2022
View reviewed changes
Copy link
Owner

@Chocobozzz Chocobozzz left a comment

Choose a reason for hiding this comment

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

Tests seem to fail

client/src/app/shared/shared-main/video-channel/video-channel.model.ts Show resolved Hide resolved
server/models/video/video-channel.ts Outdated Show resolved Hide resolved
@Poslovitch
Copy link
Contributor Author

The same test is failing again

@Poslovitch
Copy link
Contributor Author

Oh, it's failing with another "cause", actually :

      AssertionError: expected 4 to equal 2
      + expected - actual

      -4
      +2

But I don't understand why the value is 4 instead of 2.

@Chocobozzz
Copy link
Owner

But I don't understand why the value is 4 instead of 2.

You use videoUUID that is already used in the previous test. In fact I don't think you need to simulate more views, just test that totalViews === 2

@Chocobozzz Chocobozzz merged commit c6f8ca4 into Chocobozzz:develop May 31, 2022
@Chocobozzz
Copy link
Owner

Thanks!

@Chocobozzz Chocobozzz mentioned this pull request May 31, 2022
Closed
@Poslovitch Poslovitch deleted the poslovitch/add-total-views-in-the-4331 branch May 31, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Chocobozzz Chocobozzz Chocobozzz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Poslovitch @Chocobozzz