Remove most instances of mark_safe

Author: davidfischer

adserver/admin.py

@@ -8,9 +8,7 @@
 from django.db import models
 from django.template.response import TemplateResponse
 from django.utils import timezone
-from django.utils.html import escape
 from django.utils.html import format_html
-from django.utils.safestring import mark_safe
 from django.utils.translation import gettext_lazy as _
 from djstripe.models import Invoice
 from simple_history.admin import SimpleHistoryAdmin
@@ -190,9 +188,11 @@ def report(self, instance):
         if not instance.pk:
             return ""  # pragma: no cover
 
-        name = escape(instance.name)
-        url = instance.get_absolute_url()
-        return mark_safe(f'<a href="{url}">{name}</a> Report')
+        return format_html(
+            '<a href="{}">{}</a>',
+            instance.get_absolute_url(),
+            f"{instance.name} Report",
+        )
 
 
 class CampaignInline(admin.TabularInline):
@@ -296,10 +296,10 @@ def report(self, instance):
         if not instance.pk:
             return ""  # pragma: no cover
 
-        return mark_safe(
-            '<a href="{url}">{name}</a>'.format(
-                name=escape(instance.name) + " Report", url=instance.get_absolute_url()
-            )
+        return format_html(
+            '<a href="{}">{}</a>',
+            instance.get_absolute_url(),
+            f"{instance.name} Report",
         )
 
     def stripe_customer(self, obj):
@@ -344,8 +344,10 @@ def ad_image(self, obj):
         if not obj.image:
             return ""
 
-        return mark_safe(
-            f'<img src="{obj.image.url}" style="max-width: {self.MAX_IMAGE_WIDTH}px" />'
+        return format_html(
+            '<img src="{}" style="max-width: {}px" />',
+            obj.image.url,
+            self.MAX_IMAGE_WIDTH,
         )
 
     def ctr(self, obj):
@@ -774,11 +776,10 @@ def campaign_report(self, instance):
         if not instance.pk or not instance.advertiser:
             return ""  # pragma: no cover
 
-        return mark_safe(
-            '<a href="{url}">{name}</a>'.format(
-                name=escape(instance.name) + " Report",
-                url=instance.advertiser.get_absolute_url(),
-            )
+        return format_html(
+            '<a href="{}">{}</a>',
+            instance.advertiser.get_absolute_url(),
+            f"{instance.name} Report",
         )
 
     def num_ads(self, obj):
@@ -949,8 +950,10 @@ class AdBaseAdmin(RemoveDeleteMixin, admin.ModelAdmin):
 
     def page_url(self, instance):
         if instance.url:
-            return mark_safe(
-                '<a href="{url}">{url}</a>'.format(url=escape(instance.url))
+            return format_html(
+                '<a href="{}">{}</a>',
+                instance.url,
+                instance.url,
             )
         return None
 

adserver/models.py

@@ -25,6 +25,7 @@
 from django.urls import reverse
 from django.utils import timezone
 from django.utils.functional import cached_property
+from django.utils.html import format_html
 from django.utils.html import mark_safe
 from django.utils.text import slugify
 from django.utils.translation import gettext_lazy as _
@@ -2001,22 +2002,24 @@ def render_links(self, link=None, preview=False):
         url = link or self.link
         if not self.text:
             template = get_template("adserver/advertisement-body.html")
-            ad_html = template.render(
-                {
-                    "ad": self,
-                    "preview": preview,
-                }
-            ).strip()
-        else:
-            ad_html = self.text
-
-        return mark_safe(
-            ad_html.replace(
-                "<a>",
-                '<a href="%s" rel="nofollow noopener sponsored" target="_blank">' % url,
+            return mark_safe(
+                template.render(
+                    {
+                        "url": url,
+                        "ad": self,
+                        "preview": preview,
+                    }
+                ).strip()
             )
+
+        # Old style ads where the text was fully customizable
+        ad_html = self.text
+        a_tag = format_html(
+            '<a href="{}" rel="nofollow noopener sponsored" target="_blank">', url
         )
 
+        return mark_safe(ad_html.replace("<a>", a_tag))
+
     def render_ad(
         self,
         ad_type,

adserver/templates/adserver/advertisement-body.html

@@ -1,5 +1,5 @@
 {% spaceless %}
-<a>
+<a href="{{ url }}" rel="nofollow noopener sponsored" target="_blank">
 {% if ad.headline or preview %}<strong class="ea-headline">{{ ad.headline|default:"" }} </strong>{% endif %}
 <span class="ea-body">{{ ad.content|default:"" }}</span>
 {% if ad.cta or preview %}<strong class="ea-cta"> {{ ad.cta|default:"" }}</strong>{% endif %}