diff --git a/adserver/forms.py b/adserver/forms.py index 829cc3c5..4f1b6a8a 100644 --- a/adserver/forms.py +++ b/adserver/forms.py @@ -1,9 +1,13 @@ """Forms for the ad server.""" +import csv import logging from datetime import timedelta +from io import BytesIO +from io import TextIOWrapper import bleach +import requests import stripe from crispy_forms.bootstrap import PrependedText from crispy_forms.helper import FormHelper @@ -17,8 +21,11 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.sites.shortcuts import get_current_site +from django.core.exceptions import ValidationError from django.core.files.images import get_image_dimensions +from django.core.files.storage import default_storage from django.core.mail import EmailMessage +from django.core.validators import URLValidator from django.db.models import Q from django.template.loader import render_to_string from django.urls import reverse @@ -936,24 +943,10 @@ def clean(self): ), ) - # Check image size - allow @2x images (double height, double width) if ad_type.has_image and image: width, height = get_image_dimensions(image) - if all( - ( - ad_type.image_width, - ad_type.image_height, - ( - width != ad_type.image_width - or height != ad_type.image_height - ), - ( - width // 2 != ad_type.image_width - or height // 2 != ad_type.image_height - ), - ) - ): + if not ad_type.validate_image(image): self.add_error( "image", forms.ValidationError( @@ -975,7 +968,7 @@ def clean(self): else: stripped_text = f"{headline}{content}{cta}" - if len(stripped_text) > ad_type.max_text_length: + if not ad_type.validate_text(stripped_text): self.add_error( "text" if text else "content", forms.ValidationError( @@ -1101,24 +1094,12 @@ def __init__(self, *args, **kwargs): Submit("submit", _("Save advertisement")), ) - def generate_slug(self): - campaign_slug = self.flight.campaign.slug - slug = slugify(self.instance.name) - if not slug.startswith(campaign_slug): - slug = slugify(f"{campaign_slug}-{slug}") - - while Advertisement.objects.filter(slug=slug).exists(): - random_chars = get_random_string(3) - slug = slugify(f"{slug}-{random_chars}") - - return slug - def save(self, commit=True): if not self.instance.flight_id: self.instance.flight = self.flight if not self.instance.slug: # Only needed on create - self.instance.slug = self.generate_slug() + self.instance.slug = Advertisement.generate_slug(self.instance.name) new_instance = super().save(commit) @@ -1150,6 +1131,170 @@ class Meta: } +class BulkAdvertisementUploadCSVForm(forms.Form): + """ + Used by advertisers to upload ads in bulk. + + The actual saving of bulk ads is done by the BulkAdvertisementPreviewForm. + """ + + REQUIRED_FIELD_NAMES = [ + "Name", + "Live", + "Link URL", + "Image URL", + "Headline", + "Content", + "Call to Action", + ] + + advertisements = forms.FileField( + label=_("Advertisements"), help_text=_("Upload a CSV using our ad template") + ) + + def __init__(self, *args, **kwargs): + """Add the form helper and customize the look of the form.""" + if "flight" in kwargs: + self.flight = kwargs.pop("flight") + else: + raise RuntimeError("'flight' is required for the bulk ad form") + + super().__init__(*args, **kwargs) + + self.fields["advertisements"].widget.attrs["accept"] = "text/csv" + + self.helper = FormHelper() + self.helper.attrs = { + "id": "advertisements-bulk-upload", + "enctype": "multipart/form-data", + } + + self.helper.layout = Layout( + Fieldset( + "", + Field("advertisements", placeholder="Upload a CSV file"), + css_class="my-3", + ), + Submit("submit", _("Preview ads")), + ) + + def clean_advertisements(self): + """Verify the CSV can be opened and has all the right fields.""" + csvfile = self.cleaned_data["advertisements"] + try: + reader = csv.DictReader( + TextIOWrapper(csvfile, encoding="utf-8", newline="") + ) + except Exception: + raise forms.ValidationError(_("Could not open the CSV file.")) + + for fieldname in self.REQUIRED_FIELD_NAMES: + if fieldname not in reader.fieldnames: + raise forms.ValidationError( + _("Missing required field %(fieldname)s."), + params={"fieldname": fieldname}, + ) + + ads = [] + url_validator = URLValidator(schemes=("http", "https")) + for row in reader: + image_url = row["Image URL"].strip() + link_url = row["Link URL"].strip() + name = row["Name"].strip() + headline = row["Headline"].strip() + content = row["Content"].strip() + cta = row["Call to Action"].strip() + + for url in (image_url, link_url): + try: + url_validator(url) + except ValidationError: + raise forms.ValidationError( + _("'%(url)s' is an invalid URL."), params={"url": url} + ) + + image_resp = None + try: + image_resp = requests.get(image_url, timeout=3, stream=True) + except Exception: + pass + + if not image_resp or not image_resp.ok: + raise forms.ValidationError( + _("Could not retrieve image '%(image)s'."), + params={"image": image_url}, + ) + + image = BytesIO(image_resp.raw.read()) + width, height = get_image_dimensions(image) + + if width is None or height is None: + raise forms.ValidationError( + _("Image for %(name)s isn't a valid image"), + params={ + "name": name, + }, + ) + + ad_text = f"{headline}{content}{cta}" + + for ad_type in self.flight.campaign.allowed_ad_types( + exclude_deprecated=True + ): + if not ad_type.validate_text(ad_text): + raise forms.ValidationError( + _( + "Total text for '%(ad)s' must be %(max_chars)s or less (it is %(text_len)s)" + ), + params={ + "ad": name, + "max_chars": ad_type.max_text_length, + "text_len": len(ad_text), + }, + ) + + if not ad_type.validate_image(image): + raise forms.ValidationError( + _( + "Images must be %(required_width)s * %(required_height)s " + "(for %(name)s it is %(width)s * %(height)s)" + ), + params={ + "name": name, + "required_width": ad_type.image_width, + "required_height": ad_type.image_height, + "width": width, + "height": height, + }, + ) + + image_name = image_url[image_url.rfind("/") + 1 :] + image_path = f"images/{timezone.now():%Y}/{timezone.now():%m}/{image_name}" + default_storage.save(image_path, image) + + ads.append( + { + "name": name, + "image_path": image_path, + "image_name": image_name, + "image_url": default_storage.url(image_path), + "live": row["Live"].strip().lower() == "true", + "link": link_url, + "headline": headline, + "content": content, + "cta": cta, + } + ) + + return ads + + def get_ads(self): + if not self.is_valid(): + raise RuntimeError("Form must be valid and bound to get the ads") + + return self.cleaned_data["advertisements"] + + class AdvertisementCopyForm(forms.Form): """Used by advertisers to re-use their ads.""" diff --git a/adserver/models.py b/adserver/models.py index 4bbddc18..714fb145 100644 --- a/adserver/models.py +++ b/adserver/models.py @@ -14,6 +14,7 @@ from django.conf import settings from django.core.cache import cache from django.core.cache import caches +from django.core.files.images import get_image_dimensions from django.core.validators import MaxValueValidator from django.core.validators import MinValueValidator from django.db import IntegrityError @@ -25,6 +26,7 @@ from django.templatetags.static import static from django.urls import reverse from django.utils import timezone +from django.utils.crypto import get_random_string from django.utils.functional import cached_property from django.utils.html import format_html from django.utils.html import mark_safe @@ -1511,6 +1513,41 @@ def __str__(self): """Simple override.""" return self.name + def validate_text(self, text): + """Return true if this text is valid for this ad type and False otherwise.""" + text_length = len(text) + if ( + self.has_text + and self.max_text_length + and text_length > self.max_text_length + ): + return False + + # Default is to pass validation if there is no text on this ad type + return True + + def validate_image(self, image): + """Return true if this image is valid for this ad type and False otherwise.""" + if self.has_image: + width, height = get_image_dimensions(image) + + if not width or not height: + return False + + # Check image size - allow @2x images (double height, double width) + if all( + ( + self.image_width, # If these are none, ad type accepts all sizes + self.image_height, + width != self.image_width or height != self.image_height, + width // 2 != self.image_width or height // 2 != self.image_height, + ) + ): + return False + + # If there's no image on this ad type -- always pass validation + return True + class Advertisement(TimeStampedModel, IndestructibleModel): """ @@ -1622,32 +1659,18 @@ def __copy__(self): ad = Advertisement.objects.get(pk=self.pk) new_name = ad.name - new_slug = ad.slug # Fix up names/slugs of ads that have been copied before # Remove dates and (" Copy") from the end of the name/slug new_name = re.sub(" \d{4}-\d{2}-\d{2}$", "", new_name) while new_name.endswith(" Copy"): new_name = new_name[:-5] - new_slug = re.sub("-copy\d*$", "", new_slug) - new_slug = re.sub("-\d{8}(-\d+)?$", "", new_slug) - - # Get a slug that doesn't already exist - # This tries -20230501, then -20230501-1, etc. - new_slug += "-{}".format(timezone.now().strftime("%Y%m%d")) - digit = 0 - while Advertisement.objects.filter(slug=new_slug).exists(): - ending = f"-{digit}" - if new_slug.endswith(ending): - new_slug = new_slug[: -len(ending)] - digit += 1 - new_slug += f"-{digit}" ad_types = ad.ad_types.all() ad.pk = None ad.name = new_name + " {}".format(timezone.now().strftime("%Y-%m-%d")) - ad.slug = new_slug + ad.slug = Advertisement.generate_slug(new_name) ad.live = False # The new ad should always be non-live ad.save() @@ -1668,6 +1691,17 @@ def get_absolute_url(self): }, ) + @classmethod + def generate_slug(cls, name): + """Generates an available slug -- involves database lookup(s).""" + slug = slugify(f"{name}-{timezone.now():%Y%m%d}") + + while Advertisement.objects.filter(slug=slug).exists(): + random_chars = get_random_string(8) + slug = slugify(f"{name}-{random_chars}") + + return slug + @property def advertiser(self): return self.flight.campaign.advertiser diff --git a/adserver/templates/adserver/advertiser/advertisement-bulk-create.html b/adserver/templates/adserver/advertiser/advertisement-bulk-create.html new file mode 100644 index 00000000..414428b3 --- /dev/null +++ b/adserver/templates/adserver/advertiser/advertisement-bulk-create.html @@ -0,0 +1,57 @@ +{% extends "adserver/advertiser/overview.html" %} +{% load i18n %} +{% load static %} +{% load humanize %} +{% load crispy_forms_tags %} + + +{% block title %}{% trans 'Bulk create ads' %}{% endblock %} + + +{% block breadcrumbs %} + {{ block.super }} + + + +{% endblock breadcrumbs %} + + +{% block content_container %} + +

{% block heading %}{% trans 'Bulk create ads' %}{% endblock heading %}

+ +
+ +
+ + {% if not preview_ads %} + {% static 'advertisement-bulk-create-template.csv' as bulk_create_template_url %} +

{% blocktrans %}Create multiple ads for your flight by uploading a CSV file. Download the CSV template and upload it with your ads.{% endblocktrans %}

+ +

{% trans 'For tips on crafting high-performing ads across EthicalAds, see our "creatives that convert" guide.' %}

+ + {% crispy form form.helper %} + {% else %} + {% url 'advertisement_bulk_create' advertiser flight as bulk_create_url %} +

{% blocktrans %}Preview and save your ads or update your CSV and upload again.{% endblocktrans %}

+ +
+ {% for advertisement in preview_ads %} +
{{ advertisement.name }}
+ {% with ad_type=preview_ad_type %} + {% include "adserver/includes/ad-preview.html" %} + {% endwith %} + {% endfor %} +
+ +
+ {% csrf_token %} + + +
+ {% endif %} +
+ +
+ +{% endblock content_container %} diff --git a/adserver/templates/adserver/advertiser/flight-detail.html b/adserver/templates/adserver/advertiser/flight-detail.html index 2cfdab0d..f62d2ba2 100644 --- a/adserver/templates/adserver/advertiser/flight-detail.html +++ b/adserver/templates/adserver/advertiser/flight-detail.html @@ -51,13 +51,17 @@

{% block heading %}{% trans 'Flight' %}: {{ flight.name }}{% endblock headin
{% else %} {% url 'advertisement_create' advertiser.slug flight.slug as create_ad_url %} -

{% blocktrans %}There are no live ads in this flight yet but you can create one.{% endblocktrans %}

+ {% url 'advertisement_bulk_create' advertiser.slug flight.slug as bulk_create_ad_url %} +

{% blocktrans %}There are no live ads in this flight yet but you can create one or create them in bulk.{% endblocktrans %}

{% endif %} diff --git a/adserver/tests/data/bulk_ad_upload.csv b/adserver/tests/data/bulk_ad_upload.csv new file mode 100644 index 00000000..b937807c --- /dev/null +++ b/adserver/tests/data/bulk_ad_upload.csv @@ -0,0 +1,3 @@ +Name,Live,Link URL,Image URL,Headline,Content,Call to Action +Ad1,true,http://example.com/ad1,https://ethicalads.blob.core.windows.net/media/images/2022/11/ethicalads-housead.png,Ad headline,"Ad content",Ad CTA +Ad2,true,http://example.com/ad2,https://ethicalads.blob.core.windows.net/media/images/2022/11/ethicalads-housead.png,Ad headline2,"Ad content2",Ad CTA2 diff --git a/adserver/tests/data/bulk_ad_upload_invalid.csv b/adserver/tests/data/bulk_ad_upload_invalid.csv new file mode 100644 index 00000000..c6b7ade5 --- /dev/null +++ b/adserver/tests/data/bulk_ad_upload_invalid.csv @@ -0,0 +1,2 @@ +Name,Live,Link URL,Image URL,Headline,Content,Call to Action +Invalid Ad1,true,http://example.com/ad1,https://ethicalads.blob.core.windows.net/media/images/2022/11/ethicalads-housead.png,Ad headline,"Ad content that is so long that it will raise an error since it's too long and there's a maximum",Ad CTA diff --git a/adserver/tests/test_advertiser_dashboard.py b/adserver/tests/test_advertiser_dashboard.py index 9a273cc1..ef23b6dc 100644 --- a/adserver/tests/test_advertiser_dashboard.py +++ b/adserver/tests/test_advertiser_dashboard.py @@ -1,5 +1,6 @@ import datetime +import bs4 from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType @@ -11,6 +12,7 @@ from django.urls import reverse from django_dynamic_fixture import get from django_slack.utils import get_backend +from django.conf import settings from ..constants import PAID_CAMPAIGN from ..constants import PUBLISHER_HOUSE_CAMPAIGN @@ -752,6 +754,62 @@ def test_ad_create_view(self): Advertisement.objects.filter(flight=self.flight, name="New Name").exists() ) + def test_ad_bulk_create_view(self): + url = reverse( + "advertisement_bulk_create", + kwargs={ + "advertiser_slug": self.advertiser.slug, + "flight_slug": self.flight.slug, + }, + ) + + # Anonymous - no access + response = self.client.get(url) + self.assertEqual(response.status_code, 302) + self.assertTrue(response["location"].startswith("/accounts/login/")) + + self.client.force_login(self.user) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Bulk create ads") + + with open(settings.BASE_DIR + "/adserver/tests/data/bulk_ad_upload_invalid.csv") as fd: + resp = self.client.post(url, data={ + "advertisements": fd, + }) + + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, "Total text for 'Invalid Ad1' must be 100 or less") + + with open(settings.BASE_DIR + "/adserver/tests/data/bulk_ad_upload.csv") as fd: + resp = self.client.post(url, data={ + "advertisements": fd, + }) + + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, "Preview and save your ads") + + soup = bs4.BeautifulSoup(resp.content) + elem = soup.find("input", attrs={"name": "signed_advertisements"}) + self.assertIsNotNone(elem) + + signed_ads = elem.attrs["value"] + + resp = self.client.post(url, follow=True, data={ + "signed_advertisements": signed_ads, + }) + + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, "Successfully uploaded") + + signed_ads = "invalid" + resp = self.client.post(url, follow=True, data={ + "signed_advertisements": signed_ads, + }) + + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, "Upload expired or invalid") + def test_ad_copy_view(self): url = reverse( "advertisement_copy", diff --git a/adserver/tests/test_forms.py b/adserver/tests/test_forms.py index ceb7dcc2..efc83be4 100644 --- a/adserver/tests/test_forms.py +++ b/adserver/tests/test_forms.py @@ -303,7 +303,6 @@ def test_ad_create_form(self): ad = form.save() self.assertEqual(ad.flight, self.flight) self.assertEqual(ad.name, self.ad_data["name"]) - self.assertEqual(ad.slug, "test-campaign-another-test") # Save an ad with the same name and make sure the slug is made unique form = AdvertisementForm(data=self.ad_data, flight=self.flight) @@ -311,7 +310,6 @@ def test_ad_create_form(self): ad = form.save() self.assertEqual(ad.flight, self.flight) self.assertEqual(ad.name, self.ad_data["name"]) - self.assertTrue(ad.slug.startswith("test-campaign-another-test-")) # Another with no need to rewrite the slug self.ad_data["name"] = "Test Campaign Third Test" @@ -319,7 +317,7 @@ def test_ad_create_form(self): form = AdvertisementForm(data=self.ad_data, flight=self.flight) self.assertTrue(form.is_valid(), form.errors) ad = form.save() - self.assertEqual(ad.slug, "test-campaign-third-test") + self.assertEqual(ad.name, self.ad_data["name"]) self.assertEqual(ad.content, self.ad_data["content"]) def test_ad_multiple_ad_types(self): diff --git a/adserver/urls.py b/adserver/urls.py index cd49ebac..a8facd3f 100644 --- a/adserver/urls.py +++ b/adserver/urls.py @@ -7,6 +7,7 @@ from .views import AccountOverviewView from .views import AccountSupportView from .views import AdClickProxyView +from .views import AdvertisementBulkCreateView from .views import AdvertisementCopyView from .views import AdvertisementCreateView from .views import AdvertisementDetailView @@ -248,6 +249,11 @@ AdvertisementCreateView.as_view(), name="advertisement_create", ), + path( + r"advertiser//flights//advertisements/bulk-create/", + AdvertisementBulkCreateView.as_view(), + name="advertisement_bulk_create", + ), path( r"advertiser//flights//advertisements//", AdvertisementDetailView.as_view(), diff --git a/adserver/views.py b/adserver/views.py index 1de2b4e5..36e48a21 100644 --- a/adserver/views.py +++ b/adserver/views.py @@ -5,6 +5,7 @@ import logging import string import urllib +from collections import namedtuple from datetime import datetime from datetime import timedelta @@ -18,7 +19,10 @@ from django.contrib.auth.mixins import UserPassesTestMixin from django.contrib.sites.shortcuts import get_current_site from django.core import mail +from django.core import signing from django.core.exceptions import ValidationError +from django.core.files.images import ImageFile +from django.core.files.storage import default_storage from django.db import models from django.http import Http404 from django.http import HttpResponse @@ -61,6 +65,7 @@ from .forms import AccountForm from .forms import AdvertisementCopyForm from .forms import AdvertisementForm +from .forms import BulkAdvertisementUploadCSVForm from .forms import FlightAutoRenewForm from .forms import FlightCreateForm from .forms import FlightForm @@ -766,6 +771,152 @@ def get_success_url(self): ) +class AdvertisementBulkCreateView( + AdvertiserAccessMixin, + UserPassesTestMixin, + FormView, +): + """ + Bulk create view for advertisements. + + Data is validated on upload and then stored in a signed field while being previewed. + When the user submits the previewed data, the signed data is stored in the database. + """ + + form_class = BulkAdvertisementUploadCSVForm + model = Advertisement + template_name = "adserver/advertiser/advertisement-bulk-create.html" + MAXIMUM_SIGNED_TIME_LENGTH = 24 * 60 * 60 + + def dispatch(self, request, *args, **kwargs): + self.advertiser = get_object_or_404( + Advertiser, slug=self.kwargs["advertiser_slug"] + ) + self.flight = get_object_or_404( + Flight, + slug=self.kwargs["flight_slug"], + campaign__advertiser=self.advertiser, + ) + return super().dispatch(request, *args, **kwargs) + + def post(self, *args, **kwargs): + if "advertisements" in self.request.FILES: + # This is step 1 of uploading the file + form = self.get_form() + if form.is_valid(): + # CSV uploaded successfully - show preview + preview_ad_type = self.flight.campaign.allowed_ad_types( + exclude_deprecated=True + ).first() + + signer = self.get_signer() + ads = form.get_ads() + ad_objects = [] + + # Used as a hack to store the image URL since we haven't created the image file field yet + Image = namedtuple("Image", ["url"]) + + for ad in ads: + ad_objects.append( + Advertisement( + name=ad["name"], + image=Image(ad["image_url"]), + live=ad["live"], + link=ad["link"], + headline=ad["headline"], + content=ad["content"], + cta=ad["cta"], + ) + ) + + context = self.get_context_data() + context["preview_ad_type"] = preview_ad_type + context["preview_ads"] = ad_objects + context["signed_advertisements"] = signer.sign_object(ads) + return self.render_to_response(context) + else: + return self.form_invalid(form) + elif "signed_advertisements" in self.request.POST: + # Process previewed ads and save them + signer = self.get_signer() + + try: + ads = signer.unsign_object( + self.request.POST["signed_advertisements"], + max_age=self.MAXIMUM_SIGNED_TIME_LENGTH, + ) + except signing.BadSignature: + messages.error( + self.request, + _("Upload expired or invalid. Please upload ads again."), + ) + return redirect(self.get_error_url()) + + # Save the ads which have already been validated + for ad in ads: + ad_object = Advertisement( + flight=self.flight, + name=ad["name"], + slug=Advertisement.generate_slug(ad["name"]), + image=ImageFile( + default_storage.open(ad["image_path"]), name=ad["image_name"] + ), + live=ad["live"], + link=ad["link"], + headline=ad["headline"], + content=ad["content"], + cta=ad["cta"], + ) + ad_object.save() + ad_object.ad_types.set( + self.flight.campaign.allowed_ad_types(exclude_deprecated=True) + ) + + messages.success( + self.request, + _("Successfully uploaded %(num_ads)s ads") % {"num_ads": len(ads)}, + ) + return redirect(self.get_success_url()) + else: + return redirect(self.get_error_url()) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context.update( + { + "advertiser": self.advertiser, + "flight": self.flight, + } + ) + return context + + def get_form_kwargs(self): + kwargs = super().get_form_kwargs() + kwargs["flight"] = self.flight + return kwargs + + def get_signer(self): + return signing.TimestampSigner() + + def get_success_url(self): + return reverse( + "flight_detail", + kwargs={ + "advertiser_slug": self.advertiser.slug, + "flight_slug": self.flight.slug, + }, + ) + + def get_error_url(self): + return reverse( + "advertisement_bulk_create", + kwargs={ + "advertiser_slug": self.advertiser.slug, + "flight_slug": self.flight.slug, + }, + ) + + class AdvertisementCopyView(AdvertiserAccessMixin, UserPassesTestMixin, FormView): """Create a copy of an existing ad.""" diff --git a/assets/files/advertisement-bulk-create-template.csv b/assets/files/advertisement-bulk-create-template.csv new file mode 100644 index 00000000..8208a15e --- /dev/null +++ b/assets/files/advertisement-bulk-create-template.csv @@ -0,0 +1,2 @@ +Name,Live,Link URL,Image URL,Headline,Content,Call to Action +Your Ad Name (required),"Whether to make your ad immediately live (required, true or false)",Your ad landing page URL (required),"Your ad image URL (required, the size must be 240*180px)",Ad headline (optional),"Ad content(required, the length of the headline, content and CTA combined must be 100 characters or less)",Ad CTA (optional) diff --git a/config/settings/base.py b/config/settings/base.py index a54df89f..885521af 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -272,6 +272,7 @@ STATICFILES_DIRS = [ os.path.join(BASE_DIR, "assets", "dist"), os.path.join(BASE_DIR, "assets", "img"), + os.path.join(BASE_DIR, "assets", "files"), ]