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 #18211: Validation of video start and end time in creator dashboard #19153

Closed
wants to merge 6 commits into from

Conversation

yungmulah
Copy link

@yungmulah yungmulah commented Nov 14, 2023

Overview

  1. This PR fixes #[18211].
  2. This PR does the following: Implementation of front end validation of start and end times of videos in the creator dashboard. Allows users to only submit videos if start time < end time or else error will be raised. Handles special case where both start and end time are 0 and allows to submit for playing the whole video. Also update back-end validation to ensure consistency.

Essential Checklist

  • The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...",
    followed by a short, clear summary of the changes.
  • I have followed the instructions for making a code change.
  • The linter/Karma presubmit checks have passed on my local machine.
  • "Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
  • My PR follows the style guide.
  • I have assigned the correct reviewers to this PR (or will leave a
    comment with the phrase "@{{reviewer_username}} PTAL" if I don't have
    permissions to assign reviewers directly).

Proof that changes are correct

275773719-456747d6-c3b0-4a31-91f9-4fc0a201572a.mov

PR Pointers

Copy link

oppiabot bot commented Nov 14, 2023

Assigning @vojtechjelinek for the first pass review of this PR. Thanks!

@yungmulah
Copy link
Author

PTAL @seanlip

@seanlip
Copy link
Member

seanlip commented Nov 17, 2023

Thanks, I'm adding @kevintab95 and @vojtechjelinek who can review this.

Copy link
Member

@vojtechjelinek vojtechjelinek left a comment

Choose a reason for hiding this comment

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

Thanks! Backend changes look good to me. I've added a few frontend comments. Please try to fix them, but deffering to @kevintab95 to check if they are fixed correctly.

core/templates/services/rte-helper-modal.controller.ts Outdated Show resolved Hide resolved
if (start === 0 && end === 0) {
return false;
}
return start >= end;
Copy link
Member

Choose a reason for hiding this comment

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

It is a bit weird that the name of the function is called validateVideoStartEnd but the true return value means this is an invalid video

Copy link
Author

@yungmulah yungmulah Nov 20, 2023

Choose a reason for hiding this comment

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

Thanks, I agree and changed the function name to isVideoStartTimeValid that returns true if the start time < end time. PTAL @kevintab95.

@yungmulah yungmulah closed this Nov 20, 2023
@yungmulah yungmulah deleted the video-start-end-validation branch November 20, 2023 10:28
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.

None yet

4 participants