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

[#11853] Add stricter checks for potentially destructive question edits #12030

Conversation

cedricongjh
Copy link
Contributor

Fixes #11853

Outline of Solution

Added a check to make sure that if the question is in a session that is published, we show the warning message when the user makes edits.

@cedricongjh cedricongjh force-pushed the 11853-warning-message-not-shown-when-editing-questions branch from a5d32b4 to 293d6d6 Compare January 20, 2023 10:04
@zhaojj2209 zhaojj2209 added the s.ToReview The PR is waiting for review(s) label Jan 25, 2023
@zhaojj2209
Copy link
Contributor

InstructorFeedbackEditPageE2ETest is failing consistently; seems that it might be failing not because of E2E test instability but because of changes made in this PR. Do investigate further and let me know if you run into any issues debugging E2E tests.

@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@cedricongjh cedricongjh force-pushed the 11853-warning-message-not-shown-when-editing-questions branch from 16da6c6 to 710da25 Compare February 2, 2023 08:57
Copy link
Contributor

@zhaojj2209 zhaojj2209 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes!

E2E tests now pass, so they probably failed previously due to stability issues.

@zhaojj2209 zhaojj2209 added s.FinalReview The PR is ready for final review and removed s.ToReview The PR is waiting for review(s) labels Feb 6, 2023
Copy link
Member

@hhdqirui hhdqirui left a comment

Choose a reason for hiding this comment

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

LGTM!

@hhdqirui hhdqirui added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.FinalReview The PR is ready for final review labels Feb 7, 2023
@samuelfangjw samuelfangjw added the c.Task Other non-user-facing works, e.g. refactoring, adding tests label Feb 7, 2023
@samuelfangjw samuelfangjw added this to the V8.22.0 milestone Feb 7, 2023
Copy link
Member

@samuelfangjw samuelfangjw left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the contribution :)

@zhaojj2209 zhaojj2209 merged commit afff7a3 into TEAMMATES:master Feb 7, 2023
weiquu added a commit to weiquu/teammates that referenced this pull request Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning message for potentially destructive edits of questions not shown under specific conditions
5 participants