#1768: status change notifications - MS #3231
Conversation
Matt-Spence
changed the title
|
🥳 Successfully deployed to developer sandbox ms. |
1 similar comment
|
🥳 Successfully deployed to developer sandbox ms. |
|
🥳 Successfully deployed to developer sandbox ms. |
1 similar comment
|
🥳 Successfully deployed to developer sandbox ms. |
| # get change url for domain | ||
| app_label = domain_request.approved_domain._meta.app_label | ||
| model_name = domain._meta.model_name | ||
| obj_id = domain.id | ||
| change_url = reverse("admin:%s_%s_change" % (app_label, model_name), args=[obj_id]) |
There was a problem hiding this comment.
This is really great
| message = f"<li>The status of this domain request cannot be changed because it has been joined to a domain in Ready status: " # noqa | ||
| message += f"<a href='{change_url}'>{domain}</a></li>" | ||
|
|
||
| message_html = mark_safe(message) # nosec |
There was a problem hiding this comment.
| message = f"<li>The status of this domain request cannot be changed because it has been joined to a domain in Ready status: " # noqa | |
| message += f"<a href='{change_url}'>{domain}</a></li>" | |
| message_html = mark_safe(message) # nosec | |
| message = format_html( | |
| "<li>The status of this domain request cannot be changed because it has been joined to a domain in Ready status:" | |
| "<a href='{}'>{}</a></li>", | |
| mark_safe(change_url), | |
| escape(str(domain)) | |
| ) |
(Sorry about the wonky spacing)
(Conditionally blocking) The domain string here technically raises a security issue because when coerced to a string, it defaults to the domain name which is user input. While I don't see the analysts ever approving something like <script>x = totally-real-domain.gov; console.log(x)</script>, but I think its worth patching this just in case.
Your change url is totally fine though. When I've ran into this before I usually just embed escape because of execution order. I've outlined a different approach above, but it doesn't need to look any particular way
Ticket
Resolves #1768
Changes
Context for reviewers
Setup
Code Review Verification Steps
As the original developer, I have
Satisfied acceptance criteria and met development standards
Ensured code standards are met (Original Developer)
Validated user-facing changes (if applicable)
As a code reviewer, I have
Reviewed, tested, and left feedback about the changes
Validated user-facing changes as a developer
Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.
As a designer reviewer, I have
Verified that the changes match the design intention
Validated user-facing changes as a designer
References
Screenshots