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

InstructorAttributes: remove HTML sanitization before saving for role, displayedName #7726 #8596

Merged
merged 56 commits into from Apr 13, 2018
Merged

InstructorAttributes: remove HTML sanitization before saving for role, displayedName #7726 #8596

merged 56 commits into from Apr 13, 2018

Conversation

shaharyar-shamshi
Copy link
Contributor

@shaharyar-shamshi shaharyar-shamshi commented Mar 6, 2018 •
edited
Loading

Fixes #7726
Removal of html sanitization before saving the role and displayedName of instructor.

@shaharyar-shamshi
Copy link
Contributor Author

Ongoing

@shaharyar-shamshi shaharyar-shamshi changed the title InstructorAttributes: Remove html sanitization before saving for role, displayedName #7726 InstructorAttributes: Remove html sanitization before saving for role, displayedName #7746 Mar 6, 2018
@shaharyar-shamshi shaharyar-shamshi changed the title InstructorAttributes: Remove html sanitization before saving for role, displayedName #7746 InstructorAttributes: Remove html sanitization before saving for role, displayedName #7726 Mar 6, 2018
@whipermr5 whipermr5 added the s.Ongoing The PR is being worked on by the author(s) label Mar 6, 2018
@tran-tien-dat tran-tien-dat requested review from leeyimin and removed request for leeyimin March 8, 2018 14:01
@shaharyar-shamshi
Copy link
Contributor Author

shaharyar-shamshi commented Mar 10, 2018 •
edited
Loading

@wkurniawan07 sir I have made all the necessary changes kindly review it and suggest any further changes if required

@shaharyar-shamshi
Copy link
Contributor Author

@wkurniawan07 Sir it is ready for review

@shaharyar-shamshi
Copy link
Contributor Author

@wkurniawan07 sir it is ready for review kindly have an look

@shaharyar-shamshi
Copy link
Contributor Author

@wkurniawan07 Sir ready for review

@tshradheya tshradheya added s.ToReview The PR is waiting for review(s) and removed s.Ongoing The PR is being worked on by the author(s) labels Mar 14, 2018
@shaharyar-shamshi
Copy link
Contributor Author

@whipermr5 sir I have made the changes kindly review it

whipermr5
whipermr5 approved these changes Apr 10, 2018
View reviewed changes
Copy link
Member

@whipermr5 whipermr5 left a comment

Choose a reason for hiding this comment

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

Script looks good 👍

@whipermr5 whipermr5 self-assigned this Apr 10, 2018
@whipermr5 whipermr5 added s.FinalReview The PR is ready for final review and removed s.Ongoing The PR is being worked on by the author(s) labels Apr 10, 2018
@whipermr5 whipermr5 added this to the V6.5.0 milestone Apr 10, 2018
@whipermr5 whipermr5 requested a review from damithc April 10, 2018 17:48
@damithc damithc 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 Apr 11, 2018
@damithc
Copy link
Contributor

damithc commented Apr 11, 2018

@whipermr5 as usual, please create an issue for me to run the script.

@shaharyar-shamshi
Copy link
Contributor Author

shaharyar-shamshi commented Apr 12, 2018 •
edited
Loading

@whipermr5 sir when I run the script on my local enviroment
it's output was this is this correct output.

@whipermr5
Copy link
Member

@shaharyar-shamshi Looks correct! You'll need to set isPreview to false (but don't commit the change) and run it again for it to actually update the entities.

@shaharyar-shamshi
Copy link
Contributor Author

shaharyar-shamshi commented Apr 13, 2018 •
edited
Loading

@whipermr5 thanks sir , Sir should I open up the issue to run the script ?

@whipermr5 whipermr5 added the e.2 label Apr 13, 2018
@whipermr5
Copy link
Member

@shaharyar-shamshi No need; I'll open it once the script is completed in your next PR (#8760).

@whipermr5 whipermr5 changed the title InstructorAttributes: Remove html sanitization before saving for role, displayedName #7726 InstructorAttributes: remove HTML sanitization before saving for role, displayedName #7726 Apr 13, 2018
@whipermr5 whipermr5 merged commit c2f8770 into TEAMMATES:master Apr 13, 2018
@shaharyar-shamshi shaharyar-shamshi deleted the issue_7726 branch April 13, 2018 16:15
@whipermr5 whipermr5 added the c.Task Other non-user-facing works, e.g. refactoring, adding tests label Apr 14, 2018
jacoblipech pushed a commit to jacoblipech/teammates that referenced this pull request May 14, 2018
@shaharyar-shamshi
[TEAMMATES#7726] InstructorAttributes: remove HTML sanitization befor…
9a7103d 
…e saving for role, displayedName (TEAMMATES#8596)
jacoblipech pushed a commit to jacoblipech/teammates that referenced this pull request Feb 14, 2019
@shaharyar-shamshi @jacoblipech
[TEAMMATES#7726] InstructorAttributes: remove HTML sanitization befor…
f6a3e96 
…e saving for role, displayedName (TEAMMATES#8596)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xpdavid xpdavid xpdavid approved these changes

@whipermr5 whipermr5 whipermr5 approved these changes

@damithc damithc damithc approved these changes

Assignees

@whipermr5 whipermr5

@xpdavid xpdavid

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
Milestone
V6.5.0
Development

Successfully merging this pull request may close these issues.
6 participants
@shaharyar-shamshi @xpdavid @whipermr5 @damithc @wkurniawan07 @tshradheya