Add API support for the rich_text field on several resources.
Review Request #4801 — Created Oct. 18, 2013 and submitted
Add API support for the rich_text field on several resources.
rich_text was previously the default for any new objects, and clients
were expected to send text in valid Markdown. This posed a problem for
Review Bot, RBTools, and probably a lot of scripts used in production,
which would suddenly have to know about Markdown.This change gives clients more control. Now, by default, rich text is an
opt-in thing. It's disabled on new objects by default, but callers can
pass rich_text=true in API calls to indicate that the text fields for
that object should be rich text.For resources with multiple affected fields, setting rich_text to true
will set the provided text as-is, and escape any text fields not set
during that API call, ensuring their contents won't accidentally be
interpreted as Markdown.Likewise, if setting rich_text=false, any unspecified text fields will
have any escaping removed, leaving just the text content (meaning that
**This** is a \*test\*
will turn into**This** is a *test*
).The web UI always sets rich_text=true in calls. It also ensures that the
text coming in is Markdown, even if it's not stored as Markdown on the
server. That is, if rbt post provides a description for a review
request, then server-side, rich_text will be false, but the web UI will
then escape it when displaying. This ensures that the user will see
Markdown-formatted text and can act appropriately when they go to save
(which will formally set rich_text=true on the server).Unit tests for the API and UI have been added.
Python and JavaScript unit tests pass.
Tested the following scenarios:
Review requests
- Posted a change for review, with a description that would be otherwise interpreted as Markdown.
Saw that it escaped properly for display, but was internally stored with the original content
andrich_text=false
. - Edited the Description and Testing Done fields individually when a review request had
rich_text=false
. The other fields properly escaped, but the new content went through.
The server hadrich_text=true
after the save. - Did the same test with a change description.
Reviews
- Tested creating a review and saw that it was in Markdown by default.
- Took an old review with
rich_text=false
and edited it. The original text was properly escaped.
Saving it setrich_text=true
.
Comments
- Created a comment for a new review, and saw that it was Markdown by default. Repeated with a new
comment on an existing review. Published these and saw my Markdown was rendered. - Took an old comment with
rich_text=false
and edited it. The original text was properly escaped.
Saving it setrich_text=true
.
Replies
- Made a new reply to a comment and added Markdown. It properly rendered, before and after a reload.
Saw in the database thatrich_text=true
. - Edited an older, unpublished reply with
rich_text=false
. The text was escaped in the input box.
Saved and it saw thatrich_text=true
, and saw it render properly. - Repeated the tests with a reply to a review's body text.
Description | From | Last Updated |
---|---|---|
We repeat (basically) this block in many places. Can it be refactored out? |
david |
- Change Summary:
-
Refactored out the conditional escape/unescape into a new function,
markdown_set_field_escaped
. - Diff:
-
Revision 2 (+1167 -143)