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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds feature flag to hide voiceover regeneration feature from exp editor page and voice artist accent labeling feature from voiceover admin page #21339

Merged
merged 1 commit into from
Dec 8, 2024
Merged

Adds feature flag to hide voiceover regeneration feature from exp editor page and voice artist accent labeling feature from voiceover admin page #21339

merged 1 commit into from
Dec 8, 2024
+69 −3

Conversation

Nik-09
Copy link
Member

@Nik-09 Nik-09 commented Nov 25, 2024 •
edited
Loading

Overview

The PR adds two feature flags:
First, to hide the functionality of automatic voiceover regeneration for a single content from the exploration editor page (voiceover tab).
Second, to hide accent labeling to voice artist functionality from the voiceover admin page. (Fixes: #20951)
The feature to label accents to voice artists is already present in the PROD, since we wanted to gate this functionality hence the feature flag is directly added in the PROD mode. PTAL at this issue: #20951.

Essential Checklist

Please follow the instructions for making a code change.

  • I have linked the issue that this PR fixes in the "Development" section of the sidebar.
  • I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
  • I have written tests for my code.
  • The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
  • I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Testing doc (for PRs with Beam jobs that modify production server data)

  • A testing doc has been written: ... (ADD LINK) ...
  • (To be confirmed by the server admin) All jobs in this PR have been verified on a live server, and the PR is safe to merge.

Proof that changes are correct

2024-11-25.18-50-54.mp4

Proof of changes on desktop with slow/throttled network

Proof of changes on mobile phone

Proof of changes in Arabic language

PR Pointers

@Nik-09 Nik-09 requested review from a team as code owners November 25, 2024 13:15
@oppiabot OppiaBot
Copy link

oppiabot bot commented Nov 25, 2024

Hi @Nik-09 please assign the required reviewer(s) for this PR. Thanks!

@Nik-09 Nik-09 assigned kevintab95 and adhiamboperes and unassigned Nik-09 Nov 25, 2024
@seanlip
Copy link
Member

seanlip commented Nov 27, 2024 •
edited
Loading

@Nik-09 One question, at which point do we plan to remove the code that's gated by the 2nd feature flag here?

@Nik-09
Copy link
Member Author

Nik-09 commented Nov 27, 2024

@Nik-09 One question, at which point do we plan to remove the code that's gated by the 2nd feature flag here?

We will be removing the 2nd feature flag in PR (#20675). The reason I am not speeding up the merge process of the mentioned PR is because firstly I wanted to ensure the successful voiceover migration from ExplorationModel to EntityVoiceoversModel.

Let me know if you have further questions.

Thank you!

@Nik-09 Nik-09 assigned seanlip and unassigned Nik-09 Nov 27, 2024
Nik-09
Nik-09 commented Nov 27, 2024
View reviewed changes
Copy link
Member Author

@Nik-09 Nik-09 left a comment

Choose a reason for hiding this comment

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

Hi @kevintab95 & @adhiamboperes PTAL at this PR once you are free.
Thank you!

core/feature_flag_list.py Show resolved Hide resolved
@seanlip seanlip removed their assignment Dec 2, 2024
adhiamboperes
adhiamboperes approved these changes Dec 5, 2024
View reviewed changes
Copy link

@adhiamboperes adhiamboperes left a comment

Choose a reason for hiding this comment

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

Thanks @Nik-09! LGTM for @oppia/web-android-compatibility-reviewers. Only had one comment.

core/feature_flag_list.py Show resolved Hide resolved
@adhiamboperes adhiamboperes assigned Nik-09 and unassigned adhiamboperes Dec 5, 2024
@oppiabot OppiaBot
Copy link

oppiabot bot commented Dec 5, 2024

Unassigning @kevintab95 since they have already approved the PR.

@oppiabot oppiabot bot added the PR: LGTM label Dec 5, 2024
@oppiabot OppiaBot
Copy link

oppiabot bot commented Dec 5, 2024

Hi @Nik-09, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

@Nik-09 Nik-09 added this pull request to the merge queue Dec 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 6, 2024
@Nik-09 Nik-09 added this pull request to the merge queue Dec 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 6, 2024
@Nik-09 Nik-09 added this pull request to the merge queue Dec 8, 2024
Merged via the queue into oppia:develop with commit cac148a Dec 8, 2024
130 checks passed
@Nik-09 Nik-09 deleted the feature_flag_for_automatic_voiceover_generation_from_exp branch December 8, 2024 05:17
@Nik-09 Nik-09 mentioned this pull request Dec 9, 2024
Open
mon4our pushed a commit to mon4our/oppia that referenced this pull request Dec 10, 2024
@Nik-09 @mon4our
Adds feature flag to hide voiceover regeneration feature from exp edi…
bd2cf25 
…tor page and voice artist accent labeling feature from voiceover admin page (oppia#21339)

Adds feature flag
github-merge-queue bot pushed a commit that referenced this pull request Dec 30, 2024
@mon4our @Nik-09 @seanlip
Fix part of #19435: Migrate Skill Editor Page to Angular (#20770)
a52eea8 
* Made necessary changes to migrate skill editor page to angular.

* Fixed linting issues.

* Fixed linting issues.

* Fixed failing frontend test.

* Fixed failing frontend test.

* Fixed 404 error

* Made "MANUALLY_REGISTERED_WITH_BACKEND": true for skill_editor

* Fixed console error

* Removed remaining downgradeComponents

* Removed remaining downgradeInjectables

* Removed remaining downgradeInjectables

* Removed remaining downgradeInjectables

* Fixed issue with unsaved changes modal

* Removed unnecessary script

* Adds feature flag to hide voiceover regeneration feature from exp editor page and voice artist accent labeling feature from voiceover admin page (#21339)

Adds feature flag

* Removed file from webpack

* Removed unnecessary code

* Fixed navbar on mobile

* Fixed modal issue and removed sharedcompmodule from app.module

---------

Co-authored-by: Nikhil <nikhil.agarwal.2019@gmail.com>
Co-authored-by: Sean Lip <sean@seanlip.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adhiamboperes adhiamboperes adhiamboperes approved these changes

@kevintab95 kevintab95 kevintab95 approved these changes

Assignees

@Nik-09 Nik-09

Labels
PR: LGTM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: Add feature flag to the accent label card in voiceover admin page
4 participants
@Nik-09 @seanlip @kevintab95 @adhiamboperes