feat(np): Add custom blocks for formatting and styling to BE #102975
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
leeandher
left a comment
leeandher
left a comment
There was a problem hiding this comment.
this is awesome 🔥 my only nit is that i prefer paragraph to section because <p> is the html equivalent, <section> is a bit different, but the code looks excellent!
| texts.append(f"<strong>{block.text}</strong>") | ||
| elif block.type == NotificationBodyTextBlockType.CODE: | ||
| texts.append(f"<code>{block.text}</code>") | ||
| return mark_safe(" ".join(texts)) |
There was a problem hiding this comment.
is mark_safe necessary here? I thought this was mainly for Django views.
There was a problem hiding this comment.
w/o mark_safe the html tags will get escaped when inputted so they wont get rendered/used. mark_safe was suggested by le claude, lmk if there's a better way tho since I do not know of one :hide:
| texts.append(f"**{block.text}**") | ||
| elif block.type == NotificationBodyTextBlockType.CODE: | ||
| texts.append(f"`{block.text}`") | ||
| return " ".join(texts) |
There was a problem hiding this comment.
Bug: Markdown Formatting Vulnerable to User Input
The render_text_blocks method wraps user text with markdown formatting (** for bold, ` for code) but doesn't escape markdown special characters in the user text first. When the concatenated string is passed to create_text_block or create_code_block, escape_markdown_special_chars only escapes &, <, >, _ but not * or `. If user-supplied text like error_message contains these characters, it will break markdown rendering or cause unintended formatting injection.
There was a problem hiding this comment.
I've decided this a fine sacrifice, since the alternative is a lot of selective formatting. If it's a big problem I'll come back and do a patch
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #102975 +/- ##
===========================================
+ Coverage 78.93% 80.56% +1.63%
===========================================
Files 9137 9139 +2
Lines 392766 394579 +1813
Branches 24966 24966
===========================================
+ Hits 310027 317902 +7875
+ Misses 82338 76276 -6062
Partials 401 401 |
This PR adds two new "Block" types to the platform (currently just for the rendered template body)
NotificationFormattingBlockNotificationTextBlockNotificationBodyFormattingBlock- Currently is only used for "Section" blocks which are just to specify the enclosed Text blocks should be on a newlineNotificationBodyTextBlock- Specifies the enclosed text should be formatted in some way. Currently we haveCODE,BOLD,PLAINFor simplicity we only go 1 layer deep, meaning formatting blocks can only contain text blocks and text blocks can only contain text (so there isn't any recursive navigating).
relies on #102976