From e4deaa4be9395153f3735c5c93e5fb90d6ae3810 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Wed, 23 Oct 2024 15:57:15 -0700 Subject: [PATCH 01/17] Downgrade deps --- requirements/analyzer.in | 3 +++ requirements/analyzer.txt | 47 ++++++--------------------------------- 2 files changed, 10 insertions(+), 40 deletions(-) diff --git a/requirements/analyzer.in b/requirements/analyzer.in index d849a58a..0447140e 100644 --- a/requirements/analyzer.in +++ b/requirements/analyzer.in @@ -35,3 +35,6 @@ sentence-transformers # Numpy v2 has some breaking changes (for now) numpy<2 + +# https://github.com/FunAudioLLM/CosyVoice/issues/516#issuecomment-2423324107 +huggingface_hub<0.25 diff --git a/requirements/analyzer.txt b/requirements/analyzer.txt index 78e65cda..953b000d 100644 --- a/requirements/analyzer.txt +++ b/requirements/analyzer.txt @@ -53,17 +53,17 @@ filelock==3.16.1 # huggingface-hub # torch # transformers - # triton floret==0.10.5 # via textacy -fsspec==2024.9.0 +fsspec==2024.10.0 # via # huggingface-hub # torch htmldate==1.9.1 # via trafilatura -huggingface-hub==0.25.2 +huggingface-hub==0.24.7 # via + # -r analyzer.in # sentence-transformers # transformers idna==3.10 @@ -98,7 +98,7 @@ lxml-html-clean==0.3.1 # via lxml marisa-trie==1.2.1 # via language-data -markupsafe==3.0.1 +markupsafe==3.0.2 # via jinja2 mpmath==1.3.0 # via sympy @@ -129,37 +129,6 @@ numpy==1.26.4 # thinc # torchvision # transformers -nvidia-cublas-cu12==12.1.3.1 - # via - # nvidia-cudnn-cu12 - # nvidia-cusolver-cu12 - # torch -nvidia-cuda-cupti-cu12==12.1.105 - # via torch -nvidia-cuda-nvrtc-cu12==12.1.105 - # via torch -nvidia-cuda-runtime-cu12==12.1.105 - # via torch -nvidia-cudnn-cu12==9.1.0.70 - # via torch -nvidia-cufft-cu12==11.0.2.54 - # via torch -nvidia-curand-cu12==10.3.2.106 - # via torch -nvidia-cusolver-cu12==11.4.5.107 - # via torch -nvidia-cusparse-cu12==12.1.0.106 - # via - # nvidia-cusolver-cu12 - # torch -nvidia-nccl-cu12==2.20.5 - # via torch -nvidia-nvjitlink-cu12==12.6.77 - # via - # nvidia-cusolver-cu12 - # nvidia-cusparse-cu12 -nvidia-nvtx-cu12==12.1.105 - # via torch packaging==24.1 # via # huggingface-hub @@ -254,7 +223,7 @@ srsly==2.4.8 # spacy-transformers # thinc # weasel -sympy==1.13.3 +sympy==1.13.1 # via torch textacy==0.13.0 # via -r analyzer.in @@ -268,12 +237,12 @@ tokenizers==0.13.3 # via transformers toolz==1.0.0 # via cytoolz -torch==2.4.1 +torch==2.5.0 # via # sentence-transformers # spacy-transformers # torchvision -torchvision==0.19.1 +torchvision==0.20.0 # via sentence-transformers tqdm==4.66.5 # via @@ -289,8 +258,6 @@ transformers==4.26.1 # via # sentence-transformers # spacy-transformers -triton==3.0.0 - # via torch typer==0.7.0 # via # pathy From d5ccac856c81e1e18d8121e9ff17de3948ff5200 Mon Sep 17 00:00:00 2001 From: ericholscher <25510+ericholscher@users.noreply.github.com> Date: Sun, 27 Oct 2024 00:04:06 +0000 Subject: [PATCH 02/17] Dependencies: all packages updated via pip-tools --- requirements/analyzer.txt | 35 +++++++++++++++++++++++++++++++++++ requirements/base.txt | 9 +++++---- requirements/development.txt | 19 ++++++++++--------- requirements/production.txt | 17 +++++++++-------- requirements/testing.txt | 2 +- 5 files changed, 60 insertions(+), 22 deletions(-) diff --git a/requirements/analyzer.txt b/requirements/analyzer.txt index 953b000d..b5ef4e18 100644 --- a/requirements/analyzer.txt +++ b/requirements/analyzer.txt @@ -53,6 +53,7 @@ filelock==3.16.1 # huggingface-hub # torch # transformers + # triton floret==0.10.5 # via textacy fsspec==2024.10.0 @@ -129,6 +130,38 @@ numpy==1.26.4 # thinc # torchvision # transformers +nvidia-cublas-cu12==12.4.5.8 + # via + # nvidia-cudnn-cu12 + # nvidia-cusolver-cu12 + # torch +nvidia-cuda-cupti-cu12==12.4.127 + # via torch +nvidia-cuda-nvrtc-cu12==12.4.127 + # via torch +nvidia-cuda-runtime-cu12==12.4.127 + # via torch +nvidia-cudnn-cu12==9.1.0.70 + # via torch +nvidia-cufft-cu12==11.2.1.3 + # via torch +nvidia-curand-cu12==10.3.5.147 + # via torch +nvidia-cusolver-cu12==11.6.1.9 + # via torch +nvidia-cusparse-cu12==12.3.1.170 + # via + # nvidia-cusolver-cu12 + # torch +nvidia-nccl-cu12==2.21.5 + # via torch +nvidia-nvjitlink-cu12==12.4.127 + # via + # nvidia-cusolver-cu12 + # nvidia-cusparse-cu12 + # torch +nvidia-nvtx-cu12==12.4.127 + # via torch packaging==24.1 # via # huggingface-hub @@ -258,6 +291,8 @@ transformers==4.26.1 # via # sentence-transformers # spacy-transformers +triton==3.1.0 + # via torch typer==0.7.0 # via # pathy diff --git a/requirements/base.txt b/requirements/base.txt index 724a92f8..c2916fac 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -15,6 +15,7 @@ amqp==5.2.0 asgiref==3.8.1 # via # django + # django-allauth # django-cors-headers # django-countries async-timeout==4.0.3 @@ -62,7 +63,7 @@ django==5.0.9 # django-slack # djangorestframework # jsonfield -django-allauth==65.0.2 +django-allauth==65.1.0 # via -r base.in django-cors-headers==4.5.0 # via -r base.in @@ -88,7 +89,7 @@ djangorestframework==3.15.2 # via -r base.in djangorestframework-jsonp==1.0.2 # via -r base.in -frozenlist==1.4.1 +frozenlist==1.5.0 # via # aiohttp # aiosignal @@ -122,7 +123,7 @@ python-dateutil==2.9.0.post0 # via celery pytz==2024.2 # via -r base.in -redis==5.1.1 +redis==5.2.0 # via celery requests==2.32.3 # via @@ -163,7 +164,7 @@ webencodings==0.5.1 # via bleach whitenoise==6.7.0 # via -r base.in -yarl==1.15.2 +yarl==1.16.0 # via aiohttp # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/development.txt b/requirements/development.txt index 4fa9ae60..6be6edba 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -21,6 +21,7 @@ anyio==4.6.2.post1 asgiref==3.8.1 # via # django + # django-allauth # django-cors-headers # django-countries asttokens==2.4.1 @@ -67,7 +68,7 @@ click-repl==0.3.0 # via celery colorama==0.4.6 # via sphinx-autobuild -coverage==7.6.3 +coverage==7.6.4 # via # -r development.in # django-coverage-plugin @@ -95,7 +96,7 @@ django==5.0.9 # django-slack # djangorestframework # jsonfield -django-allauth==65.0.2 +django-allauth==65.1.0 # via -r base.in django-cors-headers==4.5.0 # via -r base.in @@ -145,7 +146,7 @@ filelock==3.16.1 # via # tox # virtualenv -frozenlist==1.4.1 +frozenlist==1.5.0 # via # aiohttp # aiosignal @@ -168,7 +169,7 @@ ip2proxy==3.4.2 # via -r base.in ipdb==0.13.13 # via -r development.in -ipython==8.28.0 +ipython==8.29.0 # via # -r development.in # ipdb @@ -180,7 +181,7 @@ jsonfield==3.1.0 # via -r base.in kombu==5.4.2 # via celery -markupsafe==3.0.1 +markupsafe==3.0.2 # via jinja2 matplotlib-inline==0.1.7 # via ipython @@ -245,7 +246,7 @@ pyyaml==6.0.2 # via # pre-commit # responses -redis==5.1.1 +redis==5.2.0 # via celery requests==2.32.3 # via @@ -314,7 +315,7 @@ stripe==4.2.0 # via # -r base.in # dj-stripe -tokenize-rt==6.0.0 +tokenize-rt==6.1.0 # via django-upgrade tomli==2.0.2 # via @@ -357,7 +358,7 @@ vine==5.1.0 # amqp # celery # kombu -virtualenv==20.26.6 +virtualenv==20.27.0 # via # pre-commit # tox @@ -371,7 +372,7 @@ websockets==13.1 # via sphinx-autobuild whitenoise==6.7.0 # via -r base.in -yarl==1.15.2 +yarl==1.16.0 # via aiohttp # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/production.txt b/requirements/production.txt index ad653073..dc2dab0a 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -15,6 +15,7 @@ amqp==5.2.0 asgiref==3.8.1 # via # django + # django-allauth # django-cors-headers # django-countries async-timeout==4.0.3 @@ -57,7 +58,7 @@ click-repl==0.3.0 # via celery crispy-bootstrap4==2024.10 # via -r base.in -cryptography==43.0.1 +cryptography==43.0.3 # via azure-storage-blob dj-stripe==2.8.4 # via -r base.in @@ -77,7 +78,7 @@ django==5.0.9 # django-storages # djangorestframework # jsonfield -django-allauth==65.0.2 +django-allauth==65.1.0 # via -r base.in django-anymail==12.0 # via -r production.in @@ -109,7 +110,7 @@ djangorestframework==3.15.2 # via -r base.in djangorestframework-jsonp==1.0.2 # via -r base.in -frozenlist==1.4.1 +frozenlist==1.5.0 # via # aiohttp # aiosignal @@ -135,7 +136,7 @@ multidict==6.1.0 # via # aiohttp # yarl -newrelic==10.1.0 +newrelic==10.2.0 # via -r production.in packaging==24.1 # via gunicorn @@ -145,7 +146,7 @@ prompt-toolkit==3.0.48 # via click-repl propcache==0.2.0 # via yarl -psycopg2-binary==2.9.9 +psycopg2-binary==2.9.10 # via -r production.in pycparser==2.22 # via cffi @@ -155,7 +156,7 @@ python-dateutil==2.9.0.post0 # via celery pytz==2024.2 # via -r base.in -redis==5.1.1 +redis==5.2.0 # via # celery # django-redis @@ -166,7 +167,7 @@ requests==2.32.3 # django-slack # geoip2 # stripe -sentry-sdk==2.16.0 +sentry-sdk==2.17.0 # via -r production.in six==1.16.0 # via @@ -208,7 +209,7 @@ webencodings==0.5.1 # via bleach whitenoise==6.7.0 # via -r base.in -yarl==1.15.2 +yarl==1.16.0 # via aiohttp # The following packages are considered to be unsafe in a requirements file: diff --git a/requirements/testing.txt b/requirements/testing.txt index eb878781..3c202318 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -28,5 +28,5 @@ tomli==2.0.2 # via tox tox==3.28.0 # via -r testing.in -virtualenv==20.26.6 +virtualenv==20.27.0 # via tox From 6256adda6863ff7afd3a85c7c7885592c0fe2848 Mon Sep 17 00:00:00 2001 From: Eric Holscher <25510+ericholscher@users.noreply.github.com> Date: Mon, 28 Oct 2024 08:43:32 -0700 Subject: [PATCH 03/17] Fix custom.css --- docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index 4105c1e5..e7760635 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -135,7 +135,7 @@ # html_sidebars = {} html_css_files = [ - "_static/css/custom.css", + "css/custom.css", ] html_js_files = [] From ad150aae15d5e554db3d53ebd6511cc50edd8fc6 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 28 Oct 2024 09:30:28 -0700 Subject: [PATCH 04/17] Remove explicit docs ad placement --- docs/_templates/ethicalads.html | 21 --------------------- docs/_templates/layout.html | 8 -------- 2 files changed, 29 deletions(-) delete mode 100644 docs/_templates/ethicalads.html delete mode 100644 docs/_templates/layout.html diff --git a/docs/_templates/ethicalads.html b/docs/_templates/ethicalads.html deleted file mode 100644 index 394c36dd..00000000 --- a/docs/_templates/ethicalads.html +++ /dev/null @@ -1,21 +0,0 @@ -
-
-
- - - diff --git a/docs/_templates/layout.html b/docs/_templates/layout.html deleted file mode 100644 index 30ef35bc..00000000 --- a/docs/_templates/layout.html +++ /dev/null @@ -1,8 +0,0 @@ -{% extends "!layout.html" %} - - -{% block document %} - {{ super() }} - - {% include "ethicalads.html" %} -{% endblock document %} From f35e258b1244ed51e8bfc02cd1bc6f2a2d9cabe0 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Thu, 31 Oct 2024 16:41:23 -0700 Subject: [PATCH 05/17] Bulk ad upload --- adserver/forms.py | 159 ++++++++++++++++++ .../advertisement-bulk-create-template.csv | 2 + .../advertiser/advertisement-bulk-create.html | 57 +++++++ .../adserver/advertiser/flight-detail.html | 11 +- adserver/tests/data/bulk_ad_upload.csv | 3 + adserver/tests/test_advertiser_dashboard.py | 50 ++++++ adserver/urls.py | 14 ++ adserver/views.py | 156 +++++++++++++++++ 8 files changed, 449 insertions(+), 3 deletions(-) create mode 100644 adserver/templates/adserver/advertiser/advertisement-bulk-create-template.csv create mode 100644 adserver/templates/adserver/advertiser/advertisement-bulk-create.html create mode 100644 adserver/tests/data/bulk_ad_upload.csv diff --git a/adserver/forms.py b/adserver/forms.py index 829cc3c5..c43ceb38 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 @@ -1150,6 +1157,158 @@ 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", + ] + MAXIMUM_TEXT_LENGTH = 100 + IMAGE_WIDTH = 240 + IMAGE_HEIGHT = 180 + + 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.""" + 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.") + % {"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() + + text_length = len(f"{headline}{content}{cta}") + if text_length > self.MAXIMUM_TEXT_LENGTH: + raise forms.ValidationError( + "Total text for '%(ad)s' must be %(max_chars)s or less (it is %(text_len)s)" + % { + "ad": name, + "max_chars": self.MAXIMUM_TEXT_LENGTH, + "text_len": text_length, + } + ) + + for url in (image_url, link_url): + try: + url_validator(url) + except ValidationError: + raise forms.ValidationError( + _("'%(url)s' is an invalid URL.") % {"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'.") % {"image": image_url} + ) + + image = BytesIO(image_resp.raw.read()) + width, height = get_image_dimensions(image) + if width is None or height is None: + forms.ValidationError( + _("Image for %(name)s isn't a valid image"), + params={ + "name": name, + }, + ) + if width != self.IMAGE_WIDTH or height != self.IMAGE_HEIGHT: + 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": self.IMAGE_WIDTH, + "required_height": self.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/templates/adserver/advertiser/advertisement-bulk-create-template.csv b/adserver/templates/adserver/advertiser/advertisement-bulk-create-template.csv new file mode 100644 index 00000000..8208a15e --- /dev/null +++ b/adserver/templates/adserver/advertiser/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/adserver/templates/adserver/advertiser/advertisement-bulk-create.html b/adserver/templates/adserver/advertiser/advertisement-bulk-create.html new file mode 100644 index 00000000..41f231d1 --- /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 }} +
  • {% trans 'Flights' %}
    • +
  • {{ flight.name }}
    • +
  • {% trans 'Bulk create ads' %}
    • +{% endblock breadcrumbs %} + + +{% block content_container %} + +

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

    + +
    + +
    + + {% if not preview_ads %} + {% url 'advertiser_bulk_create_template' 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
    -
    {% trans 'Live ads' %}
    +
    {% trans 'Live ads' %}
    -
    {% 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/test_advertiser_dashboard.py b/adserver/tests/test_advertiser_dashboard.py index 9a273cc1..200ac9ff 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,54 @@ 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.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/urls.py b/adserver/urls.py index cd49ebac..04caa9f8 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 @@ -143,6 +144,14 @@ name="publisher_uplift_report_export", ), # Advertiser management and reporting + path( + r"advertiser/advertisement-bulk-create-template.csv", + TemplateView.as_view( + template_name="adserver/advertiser/advertisement-bulk-create-template.csv", + content_type="text/csv", + ), + name="advertiser_bulk_create_template", + ), path( r"advertiser//", AdvertiserMainView.as_view(), @@ -248,6 +257,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..9f6dddd1 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 @@ -32,6 +36,7 @@ from django.urls import reverse_lazy from django.utils import timezone from django.utils.crypto import get_random_string +from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ from django.views import View from django.views.generic import CreateView @@ -61,6 +66,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 +772,156 @@ 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=self.get_ad_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_ad_slug(self, name): + slug = slugify(f"{name}") + + while Advertisement.objects.filter(slug=slug).exists(): + random_chars = get_random_string(8) + slug = slugify(f"{name}-{random_chars}") + + return slug + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + context.update( + { + "advertiser": self.advertiser, + "flight": self.flight, + } + ) + return context + + 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.""" From 036827d35fa041578b7ee67784fa90c6d1274e7e Mon Sep 17 00:00:00 2001 From: David Fischer Date: Fri, 1 Nov 2024 13:34:15 -0700 Subject: [PATCH 06/17] Cleanup some issues with ad types and add tests --- adserver/forms.py | 96 ++++++++++++------- .../tests/data/bulk_ad_upload_invalid.csv | 2 + adserver/tests/test_advertiser_dashboard.py | 8 ++ adserver/views.py | 5 + 4 files changed, 79 insertions(+), 32 deletions(-) create mode 100644 adserver/tests/data/bulk_ad_upload_invalid.csv diff --git a/adserver/forms.py b/adserver/forms.py index c43ceb38..ab7b174d 100644 --- a/adserver/forms.py +++ b/adserver/forms.py @@ -1173,9 +1173,6 @@ class BulkAdvertisementUploadCSVForm(forms.Form): "Content", "Call to Action", ] - MAXIMUM_TEXT_LENGTH = 100 - IMAGE_WIDTH = 240 - IMAGE_HEIGHT = 180 advertisements = forms.FileField( label=_("Advertisements"), help_text=_("Upload a CSV using our ad template") @@ -1183,6 +1180,11 @@ class BulkAdvertisementUploadCSVForm(forms.Form): 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" @@ -1215,8 +1217,8 @@ def clean_advertisements(self): for fieldname in self.REQUIRED_FIELD_NAMES: if fieldname not in reader.fieldnames: raise forms.ValidationError( - _("Missing required field %(fieldname)s.") - % {"fieldname": fieldname} + _("Missing required field %(fieldname)s."), + params={"fieldname": fieldname}, ) ads = [] @@ -1229,23 +1231,12 @@ def clean_advertisements(self): content = row["Content"].strip() cta = row["Call to Action"].strip() - text_length = len(f"{headline}{content}{cta}") - if text_length > self.MAXIMUM_TEXT_LENGTH: - raise forms.ValidationError( - "Total text for '%(ad)s' must be %(max_chars)s or less (it is %(text_len)s)" - % { - "ad": name, - "max_chars": self.MAXIMUM_TEXT_LENGTH, - "text_len": text_length, - } - ) - for url in (image_url, link_url): try: url_validator(url) except ValidationError: raise forms.ValidationError( - _("'%(url)s' is an invalid URL.") % {"url": url} + _("'%(url)s' is an invalid URL."), params={"url": url} ) image_resp = None @@ -1256,32 +1247,73 @@ def clean_advertisements(self): if not image_resp or not image_resp.ok: raise forms.ValidationError( - _("Could not retrieve image '%(image)s'.") % {"image": image_url} + _("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: - forms.ValidationError( + raise forms.ValidationError( _("Image for %(name)s isn't a valid image"), params={ "name": name, }, ) - if width != self.IMAGE_WIDTH or height != self.IMAGE_HEIGHT: - 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": self.IMAGE_WIDTH, - "required_height": self.IMAGE_HEIGHT, - "width": width, - "height": height, - }, + + text_length = len(f"{headline}{content}{cta}") + + for ad_type in self.flight.campaign.allowed_ad_types( + exclude_deprecated=True + ): + log.info( + "Ad-type=%s, required-width=%s, required-height=%s", + ad_type, + ad_type.image_width, + ad_type.image_height, ) + if ad_type.max_text_length and text_length > ad_type.max_text_length: + 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": text_length, + }, + ) + + if all( + ( + ad_type.has_image, + 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 + ), + ) + ): + 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) 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 200ac9ff..ef23b6dc 100644 --- a/adserver/tests/test_advertiser_dashboard.py +++ b/adserver/tests/test_advertiser_dashboard.py @@ -773,6 +773,14 @@ def test_ad_bulk_create_view(self): 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, diff --git a/adserver/views.py b/adserver/views.py index 9f6dddd1..916ff988 100644 --- a/adserver/views.py +++ b/adserver/views.py @@ -900,6 +900,11 @@ def get_context_data(self, **kwargs): ) 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() From 76ebdf747d17947606170c83dfe7b67fcd24d1af Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 4 Nov 2024 12:41:58 -0800 Subject: [PATCH 07/17] Fix a celerybeat task configuration --- config/settings/base.py | 8 ++++---- config/settings/production.py | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/config/settings/base.py b/config/settings/base.py index 1e6c318d..a54df89f 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -40,9 +40,9 @@ import ethicalads_ext # noqa - ext = True + ADSERVER_EXT = True except ImportError: - ext = False + ADSERVER_EXT = False # Quick-start development settings - unsuitable for production # See https://docs.djangoproject.com/en/4.2/howto/deployment/checklist/ @@ -533,9 +533,9 @@ ) if ADSERVER_ANALYZER_BACKEND: INSTALLED_APPS.append("adserver.analyzer") -if ADSERVER_ANALYZER_BACKEND and ext: +if ADSERVER_ANALYZER_BACKEND and ADSERVER_EXT: INSTALLED_APPS.append("ethicalads_ext.embedding") -if ext: +if ADSERVER_EXT: INSTALLED_APPS.append("ethicalads_ext.support") # Whether Do Not Track is enabled for the ad server diff --git a/config/settings/production.py b/config/settings/production.py index 2b3e0379..e874761b 100644 --- a/config/settings/production.py +++ b/config/settings/production.py @@ -195,8 +195,9 @@ "task": "adserver.analyzer.tasks.daily_analyze_urls", "schedule": crontab(hour="4", minute="0"), } +if "ethicalads_ext.embedding" in INSTALLED_APPS: CELERY_BEAT_SCHEDULE["every-day-analyze-advertiser-urls"] = { - "task": "adserver.analyzer.tasks.daily_analyze_advertiser_urls", + "task": "ethicalads_ext.embedding.tasks.daily_analyze_advertiser_urls", "schedule": crontab(hour="4", minute="30"), } From 478f4e3c4952166c8cf92e64d1f52a4d6737f51a Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 4 Nov 2024 15:22:56 -0800 Subject: [PATCH 08/17] Serve CSV template through staticfiles --- .../adserver/advertiser/advertisement-bulk-create.html | 2 +- adserver/urls.py | 8 -------- .../files}/advertisement-bulk-create-template.csv | 0 config/settings/base.py | 1 + 4 files changed, 2 insertions(+), 9 deletions(-) rename {adserver/templates/adserver/advertiser => assets/files}/advertisement-bulk-create-template.csv (100%) diff --git a/adserver/templates/adserver/advertiser/advertisement-bulk-create.html b/adserver/templates/adserver/advertiser/advertisement-bulk-create.html index 41f231d1..414428b3 100644 --- a/adserver/templates/adserver/advertiser/advertisement-bulk-create.html +++ b/adserver/templates/adserver/advertiser/advertisement-bulk-create.html @@ -25,7 +25,7 @@

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

    {% if not preview_ads %} - {% url 'advertiser_bulk_create_template' as bulk_create_template_url %} + {% 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.' %}

    diff --git a/adserver/urls.py b/adserver/urls.py index 04caa9f8..a8facd3f 100644 --- a/adserver/urls.py +++ b/adserver/urls.py @@ -144,14 +144,6 @@ name="publisher_uplift_report_export", ), # Advertiser management and reporting - path( - r"advertiser/advertisement-bulk-create-template.csv", - TemplateView.as_view( - template_name="adserver/advertiser/advertisement-bulk-create-template.csv", - content_type="text/csv", - ), - name="advertiser_bulk_create_template", - ), path( r"advertiser//", AdvertiserMainView.as_view(), diff --git a/adserver/templates/adserver/advertiser/advertisement-bulk-create-template.csv b/assets/files/advertisement-bulk-create-template.csv similarity index 100% rename from adserver/templates/adserver/advertiser/advertisement-bulk-create-template.csv rename to assets/files/advertisement-bulk-create-template.csv diff --git a/config/settings/base.py b/config/settings/base.py index 1e6c318d..9acba064 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"), ] From 81a5cd23159fd987266210b7210b99cb808fedde Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 4 Nov 2024 16:12:26 -0800 Subject: [PATCH 09/17] Centralize code on checking image width/height, text len --- adserver/forms.py | 46 ++++++---------------------------------------- adserver/models.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/adserver/forms.py b/adserver/forms.py index ab7b174d..24507af5 100644 --- a/adserver/forms.py +++ b/adserver/forms.py @@ -943,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( @@ -982,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( @@ -1262,18 +1248,12 @@ def clean_advertisements(self): }, ) - text_length = len(f"{headline}{content}{cta}") + ad_text = f"{headline}{content}{cta}" for ad_type in self.flight.campaign.allowed_ad_types( exclude_deprecated=True ): - log.info( - "Ad-type=%s, required-width=%s, required-height=%s", - ad_type, - ad_type.image_width, - ad_type.image_height, - ) - if ad_type.max_text_length and text_length > ad_type.max_text_length: + 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)" @@ -1281,25 +1261,11 @@ def clean_advertisements(self): params={ "ad": name, "max_chars": ad_type.max_text_length, - "text_len": text_length, + "text_len": len(ad_text), }, ) - if all( - ( - ad_type.has_image, - 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): raise forms.ValidationError( _( "Images must be %(required_width)s * %(required_height)s " diff --git a/adserver/models.py b/adserver/models.py index 4bbddc18..a8f5c791 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 @@ -1511,6 +1512,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): """ From bb50e018ba9fbe2c261fef603a2fb35e54d2347f Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 4 Nov 2024 16:46:35 -0800 Subject: [PATCH 10/17] Centralize slug generation --- adserver/forms.py | 14 +------------- adserver/models.py | 28 +++++++++++++--------------- adserver/tests/test_forms.py | 4 +--- adserver/views.py | 12 +----------- 4 files changed, 16 insertions(+), 42 deletions(-) diff --git a/adserver/forms.py b/adserver/forms.py index 24507af5..4f1b6a8a 100644 --- a/adserver/forms.py +++ b/adserver/forms.py @@ -1094,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) diff --git a/adserver/models.py b/adserver/models.py index a8f5c791..714fb145 100644 --- a/adserver/models.py +++ b/adserver/models.py @@ -26,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 @@ -1658,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() @@ -1704,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/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/views.py b/adserver/views.py index 916ff988..36e48a21 100644 --- a/adserver/views.py +++ b/adserver/views.py @@ -36,7 +36,6 @@ from django.urls import reverse_lazy from django.utils import timezone from django.utils.crypto import get_random_string -from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ from django.views import View from django.views.generic import CreateView @@ -858,7 +857,7 @@ def post(self, *args, **kwargs): ad_object = Advertisement( flight=self.flight, name=ad["name"], - slug=self.get_ad_slug(ad["name"]), + slug=Advertisement.generate_slug(ad["name"]), image=ImageFile( default_storage.open(ad["image_path"]), name=ad["image_name"] ), @@ -881,15 +880,6 @@ def post(self, *args, **kwargs): else: return redirect(self.get_error_url()) - def get_ad_slug(self, name): - slug = slugify(f"{name}") - - while Advertisement.objects.filter(slug=slug).exists(): - random_chars = get_random_string(8) - slug = slugify(f"{name}-{random_chars}") - - return slug - def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context.update( From 51f3f2cc9a543469094bcf0590d2d9e5064a7b12 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 13 Nov 2024 14:44:25 -0800 Subject: [PATCH 11/17] Add a visual cue for renewing flights on listview --- adserver/templates/adserver/advertiser/flight-list.html | 2 ++ adserver/templates/adserver/advertiser/overview.html | 3 +++ 2 files changed, 5 insertions(+) diff --git a/adserver/templates/adserver/advertiser/flight-list.html b/adserver/templates/adserver/advertiser/flight-list.html index acb441eb..c6f6e750 100644 --- a/adserver/templates/adserver/advertiser/flight-list.html +++ b/adserver/templates/adserver/advertiser/flight-list.html @@ -56,6 +56,8 @@

    {% block heading %}{% trans "Manage advertising flights" %}{% endblock headi {{ flight.name }} {% if not flight.live %} + {% elif flight.auto_renew %} + {% endif %} {{ flight.start_date }} diff --git a/adserver/templates/adserver/advertiser/overview.html b/adserver/templates/adserver/advertiser/overview.html index fe3e88a0..76a1780b 100644 --- a/adserver/templates/adserver/advertiser/overview.html +++ b/adserver/templates/adserver/advertiser/overview.html @@ -123,6 +123,9 @@

    {% blocktrans %}{{ state }} flights{% endblocktrans %} {{ flight.name }} + {% if flight.auto_renew %} + + {% endif %} {{ flight.end_date }} {{ flight.ctr|floatformat:3 }}% From 90a4a762e230db6f87b13add65ae11995e8ce5c7 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Thu, 14 Nov 2024 16:15:29 -0800 Subject: [PATCH 12/17] Add roles for advertisers and publishers - Add 3 permission levels for advertisers and publishers - The migration will default everybody to the highest permission - Role permissions are checked in the UI and in the backend - Admin users can change publisher settings, and invite users. - Managers can edit things - Reporters can't change anything. --- adserver/auth/admin.py | 19 +++- .../0009_user_advertiser_publisher_roles.py | 70 ++++++++++++++ adserver/auth/models.py | 56 ++++++++++- adserver/forms.py | 10 ++ adserver/mixins.py | 59 ++++++++++-- .../advertiser/advertisement-detail.html | 6 ++ .../adserver/advertiser/flight-detail.html | 16 +++- .../adserver/advertiser/flight-list.html | 6 ++ .../templates/adserver/advertiser/users.html | 32 +++++-- adserver/templates/adserver/base.html | 4 + adserver/templates/adserver/dashboard.html | 1 + .../publisher/fallback-ads-detail.html | 6 ++ .../adserver/publisher/fallback-ads-list.html | 14 ++- .../templates/adserver/publisher/users.html | 32 +++++-- adserver/templatetags/ad_extras.py | 52 ++++++++++ adserver/tests/test_advertiser_dashboard.py | 84 +++++++++++++++- adserver/tests/test_publisher_dashboard.py | 96 ++++++++++++++++--- adserver/views.py | 74 ++++++++++---- 18 files changed, 574 insertions(+), 63 deletions(-) create mode 100644 adserver/auth/migrations/0009_user_advertiser_publisher_roles.py diff --git a/adserver/auth/admin.py b/adserver/auth/admin.py index 928742e0..e184013c 100644 --- a/adserver/auth/admin.py +++ b/adserver/auth/admin.py @@ -6,6 +6,22 @@ from simple_history.admin import SimpleHistoryAdmin from .models import User +from .models import UserAdvertiserMember +from .models import UserPublisherMember + + +class UserAdvertiserInline(admin.TabularInline): + """For inlining the user-advertiser relationship.""" + + model = UserAdvertiserMember + raw_id_fields = ("advertiser",) + + +class UserPublisherInline(admin.TabularInline): + """For inlining the user-publisher relationship.""" + + model = UserPublisherMember + raw_id_fields = ("publisher",) @admin.register(User) @@ -19,8 +35,6 @@ class UserAdmin(SimpleHistoryAdmin): _("Ad server details"), { "fields": ( - "advertisers", - "publishers", "flight_notifications", "notify_on_completed_flights", # DEPRECATED ) @@ -43,6 +57,7 @@ class UserAdmin(SimpleHistoryAdmin): {"fields": ("last_login", "updated_date", "created_date")}, ), ) + inlines = (UserAdvertiserInline, UserPublisherInline) list_display = ( "email", "name", diff --git a/adserver/auth/migrations/0009_user_advertiser_publisher_roles.py b/adserver/auth/migrations/0009_user_advertiser_publisher_roles.py new file mode 100644 index 00000000..5f84afe6 --- /dev/null +++ b/adserver/auth/migrations/0009_user_advertiser_publisher_roles.py @@ -0,0 +1,70 @@ +""" +This migration was autocreated but has been customized. + +See: https://docs.djangoproject.com/en/5.0/howto/writing-migrations/#changing-a-manytomanyfield-to-use-a-through-model +""" + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('adserver', '0098_rotation_aggregation'), + ('adserver_auth', '0008_data_flight_notifications'), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + state_operations=[ + migrations.CreateModel( + name='UserAdvertiserMember', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + # We add this in a separate operation below + # ('role', models.CharField(choices=[('Admin', 'Admin'), ('Manager', 'Manager'), ('Reporter', 'Reporter')], default='Admin', max_length=100)), + ('advertiser', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='adserver.advertiser')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'db_table': 'adserver_auth_user_advertisers', + }, + ), + migrations.AlterField( + model_name='user', + name='advertisers', + field=models.ManyToManyField(blank=True, through='adserver_auth.UserAdvertiserMember', to='adserver.advertiser'), + ), + migrations.CreateModel( + name='UserPublisherMember', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + # We add this in a separate operation below + # ('role', models.CharField(choices=[('Admin', 'Admin'), ('Manager', 'Manager'), ('Reporter', 'Reporter')], default='Admin', max_length=100)), + ('publisher', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='adserver.publisher')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'db_table': 'adserver_auth_user_publishers', + }, + ), + migrations.AlterField( + model_name='user', + name='publishers', + field=models.ManyToManyField(blank=True, through='adserver_auth.UserPublisherMember', to='adserver.publisher'), + ), + ], + ), + migrations.AddField( + model_name="useradvertisermember", + name="role", + field=models.CharField(choices=[('Admin', 'Admin'), ('Manager', 'Manager'), ('Reporter', 'Reporter')], default='Admin', max_length=100), + ), + migrations.AddField( + model_name="userpublishermember", + name="role", + field=models.CharField(choices=[('Admin', 'Admin'), ('Manager', 'Manager'), ('Reporter', 'Reporter')], default='Admin', max_length=100), + ), + ] diff --git a/adserver/auth/models.py b/adserver/auth/models.py index 9d2d7889..cea8c166 100644 --- a/adserver/auth/models.py +++ b/adserver/auth/models.py @@ -79,8 +79,12 @@ class User(AbstractBaseUser, PermissionsMixin): created_date = models.DateTimeField(_("create date"), auto_now_add=True) # A user may have access to zero or more advertisers or publishers - advertisers = models.ManyToManyField(Advertiser, blank=True) - publishers = models.ManyToManyField(Publisher, blank=True) + advertisers = models.ManyToManyField( + Advertiser, blank=True, through="UserAdvertiserMember" + ) + publishers = models.ManyToManyField( + Publisher, blank=True, through="UserPublisherMember" + ) # Notifications flight_notifications = models.BooleanField( @@ -146,3 +150,51 @@ def invite_user(self): [self.email], ) return True + + +class UserAdvertiserMember(models.Model): + """User-Advertiser 'through' model.""" + + ROLE_ADMIN = "Admin" + ROLE_MANAGER = "Manager" + ROLE_REPORTER = "Reporter" + ROLES = (ROLE_ADMIN, ROLE_MANAGER, ROLE_REPORTER) + + user = models.ForeignKey(User, on_delete=models.CASCADE) + advertiser = models.ForeignKey(Advertiser, on_delete=models.CASCADE) + role = models.CharField( + max_length=100, + choices=( + (ROLE_ADMIN, _(ROLE_ADMIN)), + (ROLE_MANAGER, _(ROLE_MANAGER)), + (ROLE_REPORTER, _(ROLE_REPORTER)), + ), + default=ROLE_ADMIN, + ) + + class Meta: + db_table = "adserver_auth_user_advertisers" + + +class UserPublisherMember(models.Model): + """User-Publisher 'through' model.""" + + ROLE_ADMIN = "Admin" + ROLE_MANAGER = "Manager" + ROLE_REPORTER = "Reporter" + ROLES = (ROLE_ADMIN, ROLE_MANAGER, ROLE_REPORTER) + + user = models.ForeignKey(User, on_delete=models.CASCADE) + publisher = models.ForeignKey(Publisher, on_delete=models.CASCADE) + role = models.CharField( + max_length=100, + choices=( + (ROLE_ADMIN, _(ROLE_ADMIN)), + (ROLE_MANAGER, _(ROLE_MANAGER)), + (ROLE_REPORTER, _(ROLE_REPORTER)), + ), + default=ROLE_ADMIN, + ) + + class Meta: + db_table = "adserver_auth_user_publishers" diff --git a/adserver/forms.py b/adserver/forms.py index 829cc3c5..6e3ef925 100644 --- a/adserver/forms.py +++ b/adserver/forms.py @@ -29,6 +29,7 @@ from django.utils.translation import gettext from django.utils.translation import gettext_lazy as _ +from .auth.models import UserAdvertiserMember from .models import Advertisement from .models import Campaign from .models import Flight @@ -1322,6 +1323,14 @@ class InviteUserForm(forms.ModelForm): without a duplicate being created. """ + role = forms.ChoiceField( + required=True, + # This form works for both publishers and advertisers + # Currently, the roles are the same for both + # If that ever changes, this will need an update + choices=((r, r) for r in UserAdvertiserMember.ROLES), + ) + def __init__(self, *args, **kwargs): """Add the form helper and customize the look of the form.""" super().__init__(*args, **kwargs) @@ -1331,6 +1340,7 @@ def __init__(self, *args, **kwargs): "", Field("name"), Field("email", placeholder="user@yourdomain.com"), + Field("role"), css_class="my-3", ), Submit("submit", "Send invite"), diff --git a/adserver/mixins.py b/adserver/mixins.py index 9a38baf0..87116da5 100644 --- a/adserver/mixins.py +++ b/adserver/mixins.py @@ -9,10 +9,11 @@ from django.core.paginator import Paginator from django.db import connection from django.db import models -from django.shortcuts import get_object_or_404 from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ +from .auth.models import UserAdvertiserMember +from .auth.models import UserPublisherMember from .constants import ALL_CAMPAIGN_TYPES from .constants import CAMPAIGN_TYPES from .models import Advertiser @@ -34,40 +35,80 @@ class AdvertiserAccessMixin: """Mixin for checking advertiser access that works with the ``UserPassesTestMixin``.""" advertiser_slug_parameter = "advertiser_slug" + allowed_roles = ( + UserAdvertiserMember.ROLE_ADMIN, + UserAdvertiserMember.ROLE_MANAGER, + UserAdvertiserMember.ROLE_REPORTER, + ) def test_func(self): """The user must have access on the advertiser or be staff.""" if self.request.user.is_anonymous: return False - advertiser = get_object_or_404( - Advertiser, slug=self.kwargs[self.advertiser_slug_parameter] - ) return ( self.request.user.is_staff - or advertiser in self.request.user.advertisers.all() + or self.request.user.useradvertisermember_set.filter( + advertiser__slug=self.kwargs[self.advertiser_slug_parameter], + role__in=self.allowed_roles, + ).exists() ) +class AdvertiserAdminAccessMixin(AdvertiserAccessMixin): + """Mixin for checking advertiser ADMIN role that works with the ``UserPassesTestMixin``.""" + + allowed_roles = (UserAdvertiserMember.ROLE_ADMIN,) + + +class AdvertiserManagerAccessMixin(AdvertiserAccessMixin): + """Mixin for checking advertiser Manager or Admin role that works with the ``UserPassesTestMixin``.""" + + allowed_roles = ( + UserAdvertiserMember.ROLE_ADMIN, + UserAdvertiserMember.ROLE_MANAGER, + ) + + class PublisherAccessMixin: """Mixin for checking publisher access that works with the ``UserPassesTestMixin``.""" publisher_slug_parameter = "publisher_slug" + allowed_roles = ( + UserPublisherMember.ROLE_ADMIN, + UserPublisherMember.ROLE_MANAGER, + UserPublisherMember.ROLE_REPORTER, + ) def test_func(self): """The user must have access on the publisher or be staff.""" if self.request.user.is_anonymous: return False - publisher = get_object_or_404( - Publisher, slug=self.kwargs[self.publisher_slug_parameter] - ) return ( self.request.user.is_staff - or publisher in self.request.user.publishers.all() + or self.request.user.userpublishermember_set.filter( + publisher__slug=self.kwargs[self.publisher_slug_parameter], + role__in=self.allowed_roles, + ).exists() ) +class PublisherAdminAccessMixin(PublisherAccessMixin): + """Mixin for checking publisher ADMIN role that works with the ``UserPassesTestMixin``.""" + + allowed_roles = (UserPublisherMember.ROLE_ADMIN,) + + +class PublisherManagerAccessMixin(PublisherAccessMixin): + """Mixin for checking publisher Manager or Admin role that works with the ``UserPassesTestMixin``.""" + + allowed_roles = ( + UserPublisherMember.ROLE_ADMIN, + UserPublisherMember.ROLE_MANAGER, + ) + + class AdvertisementValidateLinkMixin: """ Mixin for validating the landing page returns a 200. diff --git a/adserver/templates/adserver/advertiser/advertisement-detail.html b/adserver/templates/adserver/advertiser/advertisement-detail.html index ba2a2c7f..8cb50ab0 100644 --- a/adserver/templates/adserver/advertiser/advertisement-detail.html +++ b/adserver/templates/adserver/advertiser/advertisement-detail.html @@ -2,6 +2,10 @@ {% load i18n %} {% load static %} {% load humanize %} +{% load ad_extras %} + + +{% advertiser_role request.user advertiser as user_advertiser_role %} {% block title %}{% trans 'Advertisement: ' %}{{ advertisement.name }}{% endblock %} @@ -61,9 +65,11 @@

    {% block heading %}{% trans 'Advertisement: ' %}{{ advertisement.name }}{% e

    + {% if request.user.is_staff or user_advertiser_role == "Admin" or user_advertiser_role == "Manager" %}
    {% trans 'Edit advertisement' %}
    + {% endif %}
    {% endblock content_container %} diff --git a/adserver/templates/adserver/advertiser/flight-detail.html b/adserver/templates/adserver/advertiser/flight-detail.html index 2cfdab0d..7364cb8d 100644 --- a/adserver/templates/adserver/advertiser/flight-detail.html +++ b/adserver/templates/adserver/advertiser/flight-detail.html @@ -2,6 +2,10 @@ {% load i18n %} {% load static %} {% load humanize %} +{% load ad_extras %} + + +{% advertiser_role request.user advertiser as user_advertiser_role %} {% block title %}{{ flight.name }}{% endblock %} @@ -30,10 +34,12 @@

    {% block heading %}{% trans 'Flight' %}: {{ flight.name }}{% endblock headin {% trans 'See full report' %} + {% if request.user.is_staff or user_advertiser_role == "Admin" or user_advertiser_role == "Manager" %} {% if flight.auto_renew %}{% trans 'Renewing' %}{% else %}{% trans 'Automatically renew' %}{% endif %} + {% endif %} {% if "adserver.change_flight" in perms %} @@ -54,6 +60,7 @@

    {% block heading %}{% trans 'Flight' %}: {{ flight.name }}{% endblock headin

    {% trans 'Live ads' %}
     + {% endif %} {% if live_ads %} @@ -73,7 +81,13 @@

    {% trans 'Live ads' %}
    {% 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 %}

    +

    + {% if user_advertiser_role == "Reporter" %} + {% blocktrans %}There are no live ads in this flight yet.{% endblocktrans %} + {% else %} + {% blocktrans %}There are no live ads in this flight yet but you can create one.{% endblocktrans %} + {% endif %} +

    {% endif %} diff --git a/adserver/templates/adserver/advertiser/flight-list.html b/adserver/templates/adserver/advertiser/flight-list.html index acb441eb..939ce156 100644 --- a/adserver/templates/adserver/advertiser/flight-list.html +++ b/adserver/templates/adserver/advertiser/flight-list.html @@ -2,6 +2,10 @@ {% load i18n %} {% load static %} {% load humanize %} +{% load ad_extras %} + + +{% advertiser_role request.user advertiser as user_advertiser_role %} {% block title %}{% trans 'Manage Advertising' %}{% endblock %} @@ -25,10 +29,12 @@

    {% block heading %}{% trans "Manage advertising flights" %}{% endblock headi {% trans 'Create flight' %} {% endif %} + {% if request.user.is_staff or user_advertiser_role == "Admin" or user_advertiser_role == "Manager" %} {% trans 'Request a new flight' %} + {% endif %} diff --git a/adserver/templates/adserver/advertiser/users.html b/adserver/templates/adserver/advertiser/users.html index 40c236a1..d6504f01 100644 --- a/adserver/templates/adserver/advertiser/users.html +++ b/adserver/templates/adserver/advertiser/users.html @@ -1,6 +1,10 @@ {% extends "adserver/advertiser/overview.html" %} {% load crispy_forms_tags %} {% load i18n %} +{% load ad_extras %} + + +{% advertiser_role request.user advertiser as user_advertiser_role %} {% block title %}{% trans 'Authorized Users' %} - {{ advertiser }}{% endblock %} @@ -15,33 +19,47 @@ {% block content_container %}

    {% blocktrans %}Authorized users for {{ advertiser }}{% endblocktrans %}

    -

    {% trans 'These are users who have access to manage ads and view reports.' %}

    +

    {% trans 'These are users who have access to manage ads and view reports. The role levels are:' %}

    +
    +
    {% trans 'Admin' %}
    +
    {% trans 'Can invite new users to collaborate as well as all permissions below.' %}
    +
    {% trans 'Manager' %}
    +
    {% trans 'Can manage advertisements and request new flights as well as all permissions below.' %}
    +
    {% trans 'Reporter' %}
    +
    {% trans 'Can only view reports but not change any ads or flights.' %}
    +
    - {% if users %} + {% if members %}
    + - {% for user in users %} + {% for member in members %} - - + + + diff --git a/adserver/templates/adserver/base.html b/adserver/templates/adserver/base.html index e7991522..68d7057e 100644 --- a/adserver/templates/adserver/base.html +++ b/adserver/templates/adserver/base.html @@ -1,5 +1,6 @@ {% extends 'base.html' %} {% load i18n %} +{% load ad_extras %} {% block body_classes %}adserver-dashboard{% endblock body_classes %} @@ -143,6 +144,7 @@
    {{ advertiser }}
    {% elif publisher %} + {% publisher_role request.user publisher as user_publisher_role %}
    {{ publisher }}
    {% trans 'Name' %} {% trans 'Email' %}{% trans 'Role' %} {% trans 'Options' %}
    {{ user.name }}{{ user.email }}{{ member.user.name }}{{ member.user.email }}{{ member.role }} - {% if request.user.id != user.id %} - {% trans 'remove' %} + {% if request.user.is_staff or user_advertiser_role == "Admin" %} + {% if request.user.id != member.user.id %} + {% trans 'remove' %} + {% endif %} {% endif %}
    {% else %} -

    {% blocktrans %}There are no fallback ads yet but you can create one.{% endblocktrans %}

    +

    + {% if user_publisher_role == "Reporter" %} + {% blocktrans %}There are no fallback ads yet.{% endblocktrans %} + {% else %} + {% blocktrans %}There are no fallback ads yet but you can create one.{% endblocktrans %} + {% endif %} +

    {% endif %}
    diff --git a/adserver/templates/adserver/publisher/users.html b/adserver/templates/adserver/publisher/users.html index 289dca04..885dd4dd 100644 --- a/adserver/templates/adserver/publisher/users.html +++ b/adserver/templates/adserver/publisher/users.html @@ -1,6 +1,10 @@ {% extends "adserver/publisher/overview.html" %} {% load crispy_forms_tags %} {% load i18n %} +{% load ad_extras %} + + +{% publisher_role request.user publisher as user_publisher_role %} {% block title %}{% trans 'Authorized Users' %} - {{ publisher }}{% endblock %} @@ -15,33 +19,47 @@ {% block content_container %}

    {% blocktrans %}Authorized users for {{ publisher }}{% endblocktrans %}

    -

    {% trans 'These are users who have access to manage ads and view reports.' %}

    +

    {% trans 'These are users who have access to manage ads and view reports. The role levels are:' %}

    +
    +
    {% trans 'Admin' %}
    +
    {% trans 'Can invite new users to collaborate, update settings, as well as all permissions below.' %}
    +
    {% trans 'Manager' %}
    +
    {% trans 'Can manage fallback ads as well as all permissions below.' %}
    +
    {% trans 'Reporter' %}
    +
    {% trans 'Can only view reports but not change anything.' %}
    +
    - {% if users %} + {% if members %}
    + - {% for user in users %} + {% for member in members %} - - + + + diff --git a/adserver/templatetags/ad_extras.py b/adserver/templatetags/ad_extras.py index 3e48edff..f2fa7690 100644 --- a/adserver/templatetags/ad_extras.py +++ b/adserver/templatetags/ad_extras.py @@ -14,3 +14,55 @@ def advertisement_preview(ad, ad_type=None): ad_type = ad.ad_types.first() return mark_safe(ad.render_ad(ad_type, preview=True)) + + +@register.simple_tag +def advertiser_role(user, advertiser): + """ + Returns the users role in this advertiser or None if the user has no permissions. + + Staff status is not taken into account. Caches the result on the user so future calls + don't involve a DB lookup. + """ + if not hasattr(user, "_advertiser_roles"): + user._advertiser_roles = {} + + if advertiser.pk in user._advertiser_roles: + return user._advertiser_roles[advertiser.pk] + + membership = user.useradvertisermember_set.filter( + advertiser=advertiser, + ).first() + + role = None + if membership: + role = membership.role + + user._advertiser_roles[advertiser.pk] = role + return role + + +@register.simple_tag +def publisher_role(user, publisher): + """ + Returns the users role in this publisher or None if the user has no permissions. + + Staff status is not taken into account. Caches the result on the user so future calls + don't involve a DB lookup. + """ + if not hasattr(user, "_publisher_roles"): + user._publisher_roles = {} + + if publisher.pk in user._publisher_roles: + return user._publisher_roles[publisher.pk] + + membership = user.userpublishermember_set.filter( + publisher=publisher, + ).first() + + role = None + if membership: + role = membership.role + + user._publisher_roles[publisher.pk] = role + return role diff --git a/adserver/tests/test_advertiser_dashboard.py b/adserver/tests/test_advertiser_dashboard.py index 9a273cc1..c12b9dd0 100644 --- a/adserver/tests/test_advertiser_dashboard.py +++ b/adserver/tests/test_advertiser_dashboard.py @@ -25,6 +25,7 @@ from ..tasks import daily_update_advertisers from ..utils import get_ad_day from .common import ONE_PIXEL_PNG_BYTES +from ..auth.models import UserAdvertiserMember User = get_user_model() @@ -442,6 +443,18 @@ def test_flight_autorenew_view(self): # Regular user - access to this advertiser self.client.force_login(self.user) + + # Make it a reporter who can't access + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = "Reporter" + member.save() + + resp = self.client.get(url) + self.assertEqual(resp.status_code, 403) + + member.role = "Manager" + member.save() + resp = self.client.get(url) self.assertEqual(resp.status_code, 200) self.assertContains(resp, "Flight auto-renewal") @@ -575,6 +588,19 @@ def test_flight_request_view(self): self.assertTrue(resp["location"].startswith("/accounts/login/")) # Regular user - access to this advertiser + self.client.force_login(self.user) + + # Make it a reporter who can't access + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = "Reporter" + member.save() + + resp = self.client.get(url) + self.assertEqual(resp.status_code, 403) + + member.role = "Manager" + member.save() + self.client.force_login(self.user) resp = self.client.get(url) self.assertEqual(resp.status_code, 200) @@ -697,6 +723,18 @@ def test_ad_update_view(self): self.assertTrue(response["location"].startswith("/accounts/login/")) self.client.force_login(self.user) + + # Make it a reporter who can't access + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = "Reporter" + member.save() + + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + + member.role = "Manager" + member.save() + response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, self.ad1.name) @@ -732,6 +770,18 @@ def test_ad_create_view(self): self.assertTrue(response["location"].startswith("/accounts/login/")) self.client.force_login(self.user) + + # Make it a reporter who can't access + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = "Reporter" + member.save() + + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + + member.role = "Manager" + member.save() + response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, "Create advertisement") @@ -767,6 +817,18 @@ def test_ad_copy_view(self): self.assertTrue(response["location"].startswith("/accounts/login/")) self.client.force_login(self.user) + + # Make it a reporter who can't access + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = "Reporter" + member.save() + + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + + member.role = "Manager" + member.save() + response = self.client.get(url) self.assertContains(response, "Re-use your previous ads") self.assertContains(response, self.ad1.name) @@ -828,11 +890,17 @@ def test_authorized_users(self): self.assertEqual(response.status_code, 302) self.assertTrue(response["location"].startswith("/accounts/login/")) + # Make it a manager who can't invite users + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = "Manager" + member.save() + self.client.force_login(self.user) response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, self.user.name) self.assertContains(response, self.user.email) + self.assertNotContains(response, "Invite user") self.user.advertisers.remove(self.advertiser) @@ -885,7 +953,17 @@ def test_authorized_users_invite(self): self.assertEqual(response.status_code, 302) self.assertTrue(response["location"].startswith("/accounts/login/")) + # Make it a manager who can't invite users + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = "Manager" + member.save() + self.client.force_login(self.user) + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + + member.role = "Admin" + member.save() response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -893,7 +971,7 @@ def test_authorized_users_invite(self): response = self.client.post( url, - data={"name": "Another User", "email": "another@example.com"}, + data={"name": "Another User", "email": "another@example.com", "role": "Manager"}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -914,7 +992,7 @@ def test_authorized_users_invite_existing(self): response = self.client.post( url, - data={"name": name, "email": email}, + data={"name": name, "email": email, "role": "Manager"}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -924,7 +1002,7 @@ def test_authorized_users_invite_existing(self): # Invite the same user again to check that the user isn't created again response = self.client.post( url, - data={"name": "Yet Another User", "email": email}, + data={"name": "Yet Another User", "email": email, "role": "Manager"}, follow=True, ) self.assertEqual(response.status_code, 200) diff --git a/adserver/tests/test_publisher_dashboard.py b/adserver/tests/test_publisher_dashboard.py index 741bd4da..51dc74c3 100644 --- a/adserver/tests/test_publisher_dashboard.py +++ b/adserver/tests/test_publisher_dashboard.py @@ -19,6 +19,7 @@ from ..models import Publisher from ..models import PublisherPayout from ..tasks import daily_update_publishers +from ..auth.models import UserPublisherMember class TestPublisherDashboardViews(TestCase): @@ -152,7 +153,18 @@ def test_publisher_settings(self): self.assertEqual(resp.status_code, 302) self.assertTrue(resp["location"].startswith("/accounts/login/")) - self.client.force_login(self.staff_user) + member = UserPublisherMember.objects.create( + user=self.user, + publisher=self.publisher1, + role="Manager", + ) + self.client.force_login(self.user) + + resp = self.client.get(url) + self.assertEqual(resp.status_code, 403) + + member.role = "Admin" + member.save() resp = self.client.get(url) self.assertEqual(resp.status_code, 200) @@ -391,15 +403,20 @@ def test_authorized_users(self): self.assertEqual(response.status_code, 302) self.assertTrue(response["location"].startswith("/accounts/login/")) - self.user.publishers.add(self.publisher1) + member = UserPublisherMember.objects.create( + user=self.user, + publisher=self.publisher1, + role="Manager", + ) self.client.force_login(self.user) response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, self.user.name) self.assertContains(response, self.user.email) + self.assertNotContains(response, "Invite user") - self.user.publishers.remove(self.publisher1) + member.delete() self.client.force_login(self.staff_user) response = self.client.get(url) @@ -453,16 +470,27 @@ def test_authorized_users_invite(self): self.assertEqual(response.status_code, 302) self.assertTrue(response["location"].startswith("/accounts/login/")) - self.user.publishers.add(self.publisher1) + member = UserPublisherMember.objects.create( + user=self.user, + publisher=self.publisher1, + role="Manager", + ) + self.client.force_login(self.user) + response = self.client.get(url) + self.assertEqual(response.status_code, 403) + + member.role = "Admin" + member.save() + response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, "Invite user to") response = self.client.post( url, - data={"name": "Another User", "email": "another@example.com"}, + data={"name": "Another User", "email": "another@example.com", "role": "Reporter"}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -486,7 +514,7 @@ def test_authorized_users_invite_existing(self): response = self.client.post( url, - data={"name": name, "email": email}, + data={"name": name, "email": email, "role": "Reporter"}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -496,7 +524,7 @@ def test_authorized_users_invite_existing(self): # Invite the same user again to check that the user isn't created again response = self.client.post( url, - data={"name": "Yet Another User", "email": email}, + data={"name": "Yet Another User", "email": email, "role": "Reporter"}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -572,11 +600,23 @@ def test_fallback_ads_list(self): resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - self.user.publishers.add(self.publisher) + member = UserPublisherMember.objects.create( + user=self.user, + publisher=self.publisher, + role="Manager", + ) resp = self.client.get(url) self.assertEqual(resp.status_code, 200) self.assertContains(resp, "Test Ad 1") + self.assertContains(resp, "Create fallback ad") + + member.role = "Reporter" + member.save() + + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertNotContains(resp, "Create fallback ad") def test_fallback_ads_detail(self): url = reverse( @@ -594,11 +634,23 @@ def test_fallback_ads_detail(self): resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - self.user.publishers.add(self.publisher) + member = UserPublisherMember.objects.create( + user=self.user, + publisher=self.publisher, + role="Manager", + ) resp = self.client.get(url) self.assertEqual(resp.status_code, 200) self.assertContains(resp, "Test Ad 1") + self.assertContains(resp, "Edit fallback ad") + + member.role = "Reporter" + member.save() + + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertNotContains(resp, "Edit fallback ad") def test_fallback_ads_update(self): url = reverse( @@ -616,7 +668,18 @@ def test_fallback_ads_update(self): resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - self.user.publishers.add(self.publisher) + member = UserPublisherMember.objects.create( + user=self.user, + publisher=self.publisher, + role="Reporter", + ) + + # Reporters can't edit + resp = self.client.get(url) + self.assertEqual(resp.status_code, 403) + + member.role = "Manager" + member.save() resp = self.client.get(url) self.assertEqual(resp.status_code, 200) @@ -653,7 +716,18 @@ def test_fallback_ads_create(self): resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - self.user.publishers.add(self.publisher) + member = UserPublisherMember.objects.create( + user=self.user, + publisher=self.publisher, + role="Reporter", + ) + + # Reporter can't create + resp = self.client.get(url) + self.assertEqual(resp.status_code, 403) + + member.role = "Manager" + member.save() resp = self.client.get(url) self.assertEqual(resp.status_code, 200) diff --git a/adserver/views.py b/adserver/views.py index 1de2b4e5..b415fe9a 100644 --- a/adserver/views.py +++ b/adserver/views.py @@ -49,6 +49,8 @@ from rest_framework.authtoken.models import Token from user_agents import parse as parse_user_agent +from .auth.models import UserAdvertiserMember +from .auth.models import UserPublisherMember from .constants import ALL_CAMPAIGN_TYPES from .constants import CAMPAIGN_TYPES from .constants import CLICKS @@ -71,10 +73,14 @@ from .forms import SupportForm from .mixins import AdvertisementValidateLinkMixin from .mixins import AdvertiserAccessMixin +from .mixins import AdvertiserAdminAccessMixin +from .mixins import AdvertiserManagerAccessMixin from .mixins import AllReportMixin from .mixins import GeoReportMixin from .mixins import KeywordReportMixin from .mixins import PublisherAccessMixin +from .mixins import PublisherAdminAccessMixin +from .mixins import PublisherManagerAccessMixin from .mixins import ReportQuerysetMixin from .models import AdImpression from .models import AdType @@ -405,7 +411,9 @@ def get_success_url(self): ) -class FlightSetAutoRenewView(AdvertiserAccessMixin, UserPassesTestMixin, UpdateView): +class FlightSetAutoRenewView( + AdvertiserManagerAccessMixin, UserPassesTestMixin, UpdateView +): """Allow advertisers to set a flight to auto-renew or not.""" form_class = FlightAutoRenewForm @@ -515,7 +523,7 @@ def get_success_url(self): ) -class FlightRequestView(AdvertiserAccessMixin, UserPassesTestMixin, CreateView): +class FlightRequestView(AdvertiserManagerAccessMixin, UserPassesTestMixin, CreateView): """Create a new flight for an advertiser.""" form_class = FlightRequestForm @@ -653,7 +661,7 @@ def get_object(self, queryset=None): class AdvertisementUpdateView( - AdvertiserAccessMixin, + AdvertiserManagerAccessMixin, UserPassesTestMixin, AdvertisementValidateLinkMixin, UpdateView, @@ -707,7 +715,7 @@ def get_success_url(self): class AdvertisementCreateView( - AdvertiserAccessMixin, + AdvertiserManagerAccessMixin, UserPassesTestMixin, AdvertisementValidateLinkMixin, CreateView, @@ -766,7 +774,9 @@ def get_success_url(self): ) -class AdvertisementCopyView(AdvertiserAccessMixin, UserPassesTestMixin, FormView): +class AdvertisementCopyView( + AdvertiserManagerAccessMixin, UserPassesTestMixin, FormView +): """Create a copy of an existing ad.""" form_class = AdvertisementCopyForm @@ -1544,8 +1554,8 @@ class AdvertiserAuthorizedUsersView( ): """Authorized users for an advertiser.""" - context_object_name = "users" - model = get_user_model() + context_object_name = "members" + model = UserAdvertiserMember template_name = "adserver/advertiser/users.html" def get_context_data(self, **kwargs): @@ -1557,11 +1567,15 @@ def get_queryset(self): self.advertiser = get_object_or_404( Advertiser, slug=self.kwargs.get("advertiser_slug", "") ) - return self.advertiser.user_set.all() + return ( + UserAdvertiserMember.objects.filter(advertiser=self.advertiser) + .select_related() + .all() + ) class AdvertiserAuthorizedUsersInviteView( - AdvertiserAccessMixin, UserPassesTestMixin, CreateView + AdvertiserAdminAccessMixin, UserPassesTestMixin, CreateView ): """Invite additional authorized users for an advertiser.""" @@ -1577,7 +1591,15 @@ def dispatch(self, request, *args, **kwargs): def form_valid(self, form): result = super().form_valid(form) - self.object.advertisers.add(self.advertiser) + + # Add m2m and role + role = form.cleaned_data["role"] + UserAdvertiserMember.objects.create( + advertiser=self.advertiser, + user=self.object, + role=role, + ) + messages.success( self.request, _("Successfully invited %(user)s to %(advertiser)s") @@ -1597,7 +1619,7 @@ def get_success_url(self): class AdvertiserAuthorizedUsersRemoveView( - AdvertiserAccessMixin, UserPassesTestMixin, TemplateView + AdvertiserAdminAccessMixin, UserPassesTestMixin, TemplateView ): """ Remove authorized users for an advertiser. @@ -1999,7 +2021,7 @@ def get_object(self, queryset=None): class PublisherFallbackAdsUpdateView( - FallbackAdsMixin, PublisherAccessMixin, UserPassesTestMixin, UpdateView + FallbackAdsMixin, PublisherManagerAccessMixin, UserPassesTestMixin, UpdateView ): """Update a fallback ad.""" @@ -2042,7 +2064,7 @@ def get_success_url(self): class PublisherFallbackAdsCreateView( - FallbackAdsMixin, PublisherAccessMixin, UserPassesTestMixin, CreateView + FallbackAdsMixin, PublisherManagerAccessMixin, UserPassesTestMixin, CreateView ): """Create a fallback ad.""" @@ -2088,7 +2110,7 @@ def get_success_url(self): ) -class PublisherSettingsView(PublisherAccessMixin, UserPassesTestMixin, UpdateView): +class PublisherSettingsView(PublisherAdminAccessMixin, UserPassesTestMixin, UpdateView): """Settings configuration for a publisher.""" form_class = PublisherSettingsForm @@ -2119,7 +2141,7 @@ def get_success_url(self): class PublisherStripeOauthConnectView( - PublisherAccessMixin, UserPassesTestMixin, RedirectView + PublisherAdminAccessMixin, UserPassesTestMixin, RedirectView ): """Redirect the user to the correct Stripe connect URL for the publisher.""" @@ -2408,7 +2430,7 @@ def get_context_data(self, **kwargs): # pylint: disable=too-many-locals class PublisherAuthorizedUsersView(PublisherAccessMixin, UserPassesTestMixin, ListView): """Authorized users for a publisher.""" - context_object_name = "users" + context_object_name = "members" model = get_user_model() template_name = "adserver/publisher/users.html" @@ -2421,11 +2443,15 @@ def get_queryset(self): self.publisher = get_object_or_404( Publisher, slug=self.kwargs.get("publisher_slug", "") ) - return self.publisher.user_set.all() + return ( + UserPublisherMember.objects.filter(publisher=self.publisher) + .select_related() + .all() + ) class PublisherAuthorizedUsersInviteView( - PublisherAccessMixin, UserPassesTestMixin, CreateView + PublisherAdminAccessMixin, UserPassesTestMixin, CreateView ): """Invite additional authorized users for a publisher.""" @@ -2441,7 +2467,15 @@ def dispatch(self, request, *args, **kwargs): def form_valid(self, form): result = super().form_valid(form) - self.object.publishers.add(self.publisher) + + # Add m2m and role + role = form.cleaned_data["role"] + UserPublisherMember.objects.create( + publisher=self.publisher, + user=self.object, + role=role, + ) + messages.success( self.request, _("Successfully invited %(user)s to %(publisher)s") @@ -2461,7 +2495,7 @@ def get_success_url(self): class PublisherAuthorizedUsersRemoveView( - PublisherAccessMixin, UserPassesTestMixin, TemplateView + PublisherAdminAccessMixin, UserPassesTestMixin, TemplateView ): """ Remove authorized users for a publisher. From dc82d3c9b600426a309e41f8fc559043aabaa56f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 Nov 2024 21:10:43 +0000 Subject: [PATCH 13/17] Bump aiohttp from 3.10.10 to 3.10.11 in /requirements Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.10.10 to 3.10.11. - [Release notes](https://github.com/aio-libs/aiohttp/releases) - [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst) - [Commits](https://github.com/aio-libs/aiohttp/compare/v3.10.10...v3.10.11) --- updated-dependencies: - dependency-name: aiohttp dependency-type: indirect ... Signed-off-by: dependabot[bot] --- requirements/base.txt | 2 +- requirements/development.txt | 2 +- requirements/production.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index c2916fac..ddab8e6d 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -6,7 +6,7 @@ # aiohappyeyeballs==2.4.3 # via aiohttp -aiohttp==3.10.10 +aiohttp==3.10.11 # via geoip2 aiosignal==1.3.1 # via aiohttp diff --git a/requirements/development.txt b/requirements/development.txt index 6be6edba..76021611 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -6,7 +6,7 @@ # aiohappyeyeballs==2.4.3 # via aiohttp -aiohttp==3.10.10 +aiohttp==3.10.11 # via geoip2 aiosignal==1.3.1 # via aiohttp diff --git a/requirements/production.txt b/requirements/production.txt index dc2dab0a..f9200c10 100644 --- a/requirements/production.txt +++ b/requirements/production.txt @@ -6,7 +6,7 @@ # aiohappyeyeballs==2.4.3 # via aiohttp -aiohttp==3.10.10 +aiohttp==3.10.11 # via geoip2 aiosignal==1.3.1 # via aiohttp From d15884ea6bbfb506da26f2fa14bf17696bfa92cc Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 18 Nov 2024 15:43:13 -0800 Subject: [PATCH 14/17] Use role constants in tests --- adserver/tests/test_advertiser_dashboard.py | 32 ++++++++++----------- adserver/tests/test_publisher_dashboard.py | 32 ++++++++++----------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/adserver/tests/test_advertiser_dashboard.py b/adserver/tests/test_advertiser_dashboard.py index c12b9dd0..55de382e 100644 --- a/adserver/tests/test_advertiser_dashboard.py +++ b/adserver/tests/test_advertiser_dashboard.py @@ -446,13 +446,13 @@ def test_flight_autorenew_view(self): # Make it a reporter who can't access member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) - member.role = "Reporter" + member.role = UserAdvertiserMember.ROLE_REPORTER member.save() resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - member.role = "Manager" + member.role = UserAdvertiserMember.ROLE_MANAGER member.save() resp = self.client.get(url) @@ -592,13 +592,13 @@ def test_flight_request_view(self): # Make it a reporter who can't access member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) - member.role = "Reporter" + member.role = UserAdvertiserMember.ROLE_REPORTER member.save() resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - member.role = "Manager" + member.role = UserAdvertiserMember.ROLE_MANAGER member.save() self.client.force_login(self.user) @@ -726,13 +726,13 @@ def test_ad_update_view(self): # Make it a reporter who can't access member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) - member.role = "Reporter" + member.role = UserAdvertiserMember.ROLE_REPORTER member.save() response = self.client.get(url) self.assertEqual(response.status_code, 403) - member.role = "Manager" + member.role = UserAdvertiserMember.ROLE_MANAGER member.save() response = self.client.get(url) @@ -773,13 +773,13 @@ def test_ad_create_view(self): # Make it a reporter who can't access member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) - member.role = "Reporter" + member.role = UserAdvertiserMember.ROLE_REPORTER member.save() response = self.client.get(url) self.assertEqual(response.status_code, 403) - member.role = "Manager" + member.role = UserAdvertiserMember.ROLE_MANAGER member.save() response = self.client.get(url) @@ -820,13 +820,13 @@ def test_ad_copy_view(self): # Make it a reporter who can't access member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) - member.role = "Reporter" + member.role = UserAdvertiserMember.ROLE_REPORTER member.save() response = self.client.get(url) self.assertEqual(response.status_code, 403) - member.role = "Manager" + member.role = UserAdvertiserMember.ROLE_MANAGER member.save() response = self.client.get(url) @@ -892,7 +892,7 @@ def test_authorized_users(self): # Make it a manager who can't invite users member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) - member.role = "Manager" + member.role = UserAdvertiserMember.ROLE_MANAGER member.save() self.client.force_login(self.user) @@ -955,14 +955,14 @@ def test_authorized_users_invite(self): # Make it a manager who can't invite users member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) - member.role = "Manager" + member.role = UserAdvertiserMember.ROLE_MANAGER member.save() self.client.force_login(self.user) response = self.client.get(url) self.assertEqual(response.status_code, 403) - member.role = "Admin" + member.role = UserAdvertiserMember.ROLE_ADMIN member.save() response = self.client.get(url) @@ -971,7 +971,7 @@ def test_authorized_users_invite(self): response = self.client.post( url, - data={"name": "Another User", "email": "another@example.com", "role": "Manager"}, + data={"name": "Another User", "email": "another@example.com", "role": UserAdvertiserMember.ROLE_MANAGER}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -992,7 +992,7 @@ def test_authorized_users_invite_existing(self): response = self.client.post( url, - data={"name": name, "email": email, "role": "Manager"}, + data={"name": name, "email": email, "role": UserAdvertiserMember.ROLE_MANAGER}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -1002,7 +1002,7 @@ def test_authorized_users_invite_existing(self): # Invite the same user again to check that the user isn't created again response = self.client.post( url, - data={"name": "Yet Another User", "email": email, "role": "Manager"}, + data={"name": "Yet Another User", "email": email, "role": UserAdvertiserMember.ROLE_MANAGER}, follow=True, ) self.assertEqual(response.status_code, 200) diff --git a/adserver/tests/test_publisher_dashboard.py b/adserver/tests/test_publisher_dashboard.py index 51dc74c3..a9cad891 100644 --- a/adserver/tests/test_publisher_dashboard.py +++ b/adserver/tests/test_publisher_dashboard.py @@ -156,14 +156,14 @@ def test_publisher_settings(self): member = UserPublisherMember.objects.create( user=self.user, publisher=self.publisher1, - role="Manager", + role=UserPublisherMember.ROLE_MANAGER, ) self.client.force_login(self.user) resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - member.role = "Admin" + member.role = UserPublisherMember.ROLE_ADMIN member.save() resp = self.client.get(url) @@ -406,7 +406,7 @@ def test_authorized_users(self): member = UserPublisherMember.objects.create( user=self.user, publisher=self.publisher1, - role="Manager", + role=UserPublisherMember.ROLE_MANAGER, ) self.client.force_login(self.user) @@ -473,7 +473,7 @@ def test_authorized_users_invite(self): member = UserPublisherMember.objects.create( user=self.user, publisher=self.publisher1, - role="Manager", + role=UserPublisherMember.ROLE_MANAGER, ) self.client.force_login(self.user) @@ -481,7 +481,7 @@ def test_authorized_users_invite(self): response = self.client.get(url) self.assertEqual(response.status_code, 403) - member.role = "Admin" + member.role = UserPublisherMember.ROLE_ADMIN member.save() response = self.client.get(url) @@ -490,7 +490,7 @@ def test_authorized_users_invite(self): response = self.client.post( url, - data={"name": "Another User", "email": "another@example.com", "role": "Reporter"}, + data={"name": "Another User", "email": "another@example.com", "role": UserPublisherMember.ROLE_REPORTER}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -514,7 +514,7 @@ def test_authorized_users_invite_existing(self): response = self.client.post( url, - data={"name": name, "email": email, "role": "Reporter"}, + data={"name": name, "email": email, "role": UserPublisherMember.ROLE_REPORTER}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -524,7 +524,7 @@ def test_authorized_users_invite_existing(self): # Invite the same user again to check that the user isn't created again response = self.client.post( url, - data={"name": "Yet Another User", "email": email, "role": "Reporter"}, + data={"name": "Yet Another User", "email": email, "role": UserPublisherMember.ROLE_REPORTER}, follow=True, ) self.assertEqual(response.status_code, 200) @@ -603,7 +603,7 @@ def test_fallback_ads_list(self): member = UserPublisherMember.objects.create( user=self.user, publisher=self.publisher, - role="Manager", + role=UserPublisherMember.ROLE_MANAGER, ) resp = self.client.get(url) @@ -611,7 +611,7 @@ def test_fallback_ads_list(self): self.assertContains(resp, "Test Ad 1") self.assertContains(resp, "Create fallback ad") - member.role = "Reporter" + member.role = UserPublisherMember.ROLE_REPORTER member.save() resp = self.client.get(url) @@ -637,7 +637,7 @@ def test_fallback_ads_detail(self): member = UserPublisherMember.objects.create( user=self.user, publisher=self.publisher, - role="Manager", + role=UserPublisherMember.ROLE_MANAGER, ) resp = self.client.get(url) @@ -645,7 +645,7 @@ def test_fallback_ads_detail(self): self.assertContains(resp, "Test Ad 1") self.assertContains(resp, "Edit fallback ad") - member.role = "Reporter" + member.role = UserPublisherMember.ROLE_REPORTER member.save() resp = self.client.get(url) @@ -671,14 +671,14 @@ def test_fallback_ads_update(self): member = UserPublisherMember.objects.create( user=self.user, publisher=self.publisher, - role="Reporter", + role=UserPublisherMember.ROLE_REPORTER, ) # Reporters can't edit resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - member.role = "Manager" + member.role = UserPublisherMember.ROLE_MANAGER member.save() resp = self.client.get(url) @@ -719,14 +719,14 @@ def test_fallback_ads_create(self): member = UserPublisherMember.objects.create( user=self.user, publisher=self.publisher, - role="Reporter", + role=UserPublisherMember.ROLE_REPORTER, ) # Reporter can't create resp = self.client.get(url) self.assertEqual(resp.status_code, 403) - member.role = "Manager" + member.role = UserPublisherMember.ROLE_MANAGER member.save() resp = self.client.get(url) From c9c45f169f00379aaa4e104c05bff488449f9910 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Mon, 18 Nov 2024 16:55:48 -0800 Subject: [PATCH 15/17] Feedback updates --- adserver/auth/models.py | 82 +++++++++++++++++++ adserver/mixins.py | 34 ++++---- .../advertiser/advertisement-detail.html | 7 +- .../adserver/advertiser/flight-detail.html | 15 ++-- .../adserver/advertiser/flight-list.html | 7 +- .../templates/adserver/advertiser/users.html | 11 +-- adserver/templates/adserver/base.html | 4 +- .../publisher/fallback-ads-detail.html | 7 +- .../adserver/publisher/fallback-ads-list.html | 12 ++- .../templates/adserver/publisher/users.html | 9 +- adserver/templatetags/ad_extras.py | 59 ++++++------- adserver/tests/test_advertiser_dashboard.py | 27 ++++++ 12 files changed, 185 insertions(+), 89 deletions(-) diff --git a/adserver/auth/models.py b/adserver/auth/models.py index cea8c166..0fe9ec92 100644 --- a/adserver/auth/models.py +++ b/adserver/auth/models.py @@ -119,6 +119,54 @@ def get_full_name(self): def get_short_name(self): return self.get_full_name() + def get_advertiser_role(self, advertiser): + """ + Returns the users role in this advertiser or None if the user has no permissions. + + Staff status is not taken into account. Caches the result on the user so future calls + don't involve a DB lookup. + """ + if not hasattr(self, "_advertiser_roles"): + self._advertiser_roles = {} + + if advertiser.pk in self._advertiser_roles: + return self._advertiser_roles[advertiser.pk] + + membership = self.useradvertisermember_set.filter( + advertiser=advertiser, + ).first() + + role = None + if membership: + role = membership.role + + self._advertiser_roles[advertiser.pk] = role + return role + + def get_publisher_role(self, publisher): + """ + Returns the users role in this publisher or None if the user has no permissions. + + Staff status is not taken into account. Caches the result on the user so future calls + don't involve a DB lookup. + """ + if not hasattr(self, "_publisher_roles"): + self._publisher_roles = {} + + if publisher.pk in self._publisher_roles: + return self._publisher_roles[publisher.pk] + + membership = self.userpublishermember_set.filter( + publisher=publisher, + ).first() + + role = None + if membership: + role = membership.role + + self._publisher_roles[publisher.pk] = role + return role + def get_password_reset_url(self): temp_key = default_token_generator.make_token(self) path = reverse( @@ -135,6 +183,36 @@ def get_password_reset_url(self): scheme=scheme, domain=domain, path=path ) + def has_advertiser_permission(self, advertiser): + role = self.get_advertiser_role(advertiser) + return role is not None + + def has_advertiser_manager_permission(self, advertiser): + role = self.get_advertiser_role(advertiser) + return role in ( + UserAdvertiserMember.ROLE_ADMIN, + UserAdvertiserMember.ROLE_MANAGER, + ) + + def has_advertiser_admin_permission(self, advertiser): + role = self.get_advertiser_role(advertiser) + return role == UserAdvertiserMember.ROLE_ADMIN + + def has_publisher_permission(self, publisher): + role = self.get_publisher_role(publisher) + return role is not None + + def has_publisher_manager_permission(self, publisher): + role = self.get_publisher_role(publisher) + return role in ( + UserPublisherMember.ROLE_ADMIN, + UserPublisherMember.ROLE_MANAGER, + ) + + def has_publisher_admin_permission(self, publisher): + role = self.get_publisher_role(publisher) + return role == UserPublisherMember.ROLE_ADMIN + def invite_user(self): site = get_current_site(request=None) @@ -173,6 +251,8 @@ class UserAdvertiserMember(models.Model): ) class Meta: + # This was migrated from a regular many-to-many + # To do that, we needed to start with the same table db_table = "adserver_auth_user_advertisers" @@ -197,4 +277,6 @@ class UserPublisherMember(models.Model): ) class Meta: + # This was migrated from a regular many-to-many + # To do that, we needed to start with the same table db_table = "adserver_auth_user_publishers" diff --git a/adserver/mixins.py b/adserver/mixins.py index 87116da5..202352a9 100644 --- a/adserver/mixins.py +++ b/adserver/mixins.py @@ -41,18 +41,21 @@ class AdvertiserAccessMixin: UserAdvertiserMember.ROLE_REPORTER, ) + def check_advertiser_role(self, advertiser): + return self.request.user.get_advertiser_role(advertiser) in self.allowed_roles + def test_func(self): """The user must have access on the advertiser or be staff.""" if self.request.user.is_anonymous: return False - return ( - self.request.user.is_staff - or self.request.user.useradvertisermember_set.filter( - advertiser__slug=self.kwargs[self.advertiser_slug_parameter], - role__in=self.allowed_roles, - ).exists() - ) + advertiser = Advertiser.objects.filter( + slug=self.kwargs[self.advertiser_slug_parameter] + ).first() + if not advertiser: + return False + + return self.request.user.is_staff or self.check_advertiser_role(advertiser) class AdvertiserAdminAccessMixin(AdvertiserAccessMixin): @@ -80,18 +83,21 @@ class PublisherAccessMixin: UserPublisherMember.ROLE_REPORTER, ) + def check_publisher_role(self, publisher): + return self.request.user.get_publisher_role(publisher) in self.allowed_roles + def test_func(self): """The user must have access on the publisher or be staff.""" if self.request.user.is_anonymous: return False - return ( - self.request.user.is_staff - or self.request.user.userpublishermember_set.filter( - publisher__slug=self.kwargs[self.publisher_slug_parameter], - role__in=self.allowed_roles, - ).exists() - ) + publisher = Publisher.objects.filter( + slug=self.kwargs[self.publisher_slug_parameter] + ).first() + if not publisher: + return False + + return self.request.user.is_staff or self.check_publisher_role(publisher) class PublisherAdminAccessMixin(PublisherAccessMixin): diff --git a/adserver/templates/adserver/advertiser/advertisement-detail.html b/adserver/templates/adserver/advertiser/advertisement-detail.html index 8cb50ab0..6685be0d 100644 --- a/adserver/templates/adserver/advertiser/advertisement-detail.html +++ b/adserver/templates/adserver/advertiser/advertisement-detail.html @@ -5,9 +5,6 @@ {% load ad_extras %} -{% advertiser_role request.user advertiser as user_advertiser_role %} - - {% block title %}{% trans 'Advertisement: ' %}{{ advertisement.name }}{% endblock %} @@ -21,6 +18,8 @@ {% block content_container %} +{% advertiser_manager_role request.user advertiser as has_advertiser_edit_permission %} +

    {% block heading %}{% trans 'Advertisement: ' %}{{ advertisement.name }}{% endblock heading %}

    {% if not advertisement.live or not advertisement.flight.live %} @@ -65,7 +64,7 @@

    {% block heading %}{% trans 'Advertisement: ' %}{{ advertisement.name }}{% e
    - {% if request.user.is_staff or user_advertiser_role == "Admin" or user_advertiser_role == "Manager" %} + {% if has_advertiser_edit_permission %}
    {% trans 'Edit advertisement' %}
    diff --git a/adserver/templates/adserver/advertiser/flight-detail.html b/adserver/templates/adserver/advertiser/flight-detail.html index 7364cb8d..f1467aa0 100644 --- a/adserver/templates/adserver/advertiser/flight-detail.html +++ b/adserver/templates/adserver/advertiser/flight-detail.html @@ -5,9 +5,6 @@ {% load ad_extras %} -{% advertiser_role request.user advertiser as user_advertiser_role %} - - {% block title %}{{ flight.name }}{% endblock %} @@ -20,6 +17,8 @@ {% block content_container %} +{% advertiser_manager_role request.user advertiser as has_advertiser_edit_permission %} +

    {% block heading %}{% trans 'Flight' %}: {{ flight.name }}{% endblock heading %}

    @@ -34,7 +33,7 @@

    {% block heading %}{% trans 'Flight' %}: {{ flight.name }}{% endblock headin {% trans 'See full report' %} - {% if request.user.is_staff or user_advertiser_role == "Admin" or user_advertiser_role == "Manager" %} + {% if has_advertiser_edit_permission %} {% if flight.auto_renew %}{% trans 'Renewing' %}{% else %}{% trans 'Automatically renew' %}{% endif %} @@ -60,7 +59,7 @@

    {% block heading %}{% trans 'Flight' %}: {{ flight.name }}{% endblock headin

    {% trans 'Live ads' %}

     {% endfor %} diff --git a/adserver/templates/adserver/base.html b/adserver/templates/adserver/base.html index 68d7057e..f303b8f4 100644 --- a/adserver/templates/adserver/base.html +++ b/adserver/templates/adserver/base.html @@ -144,7 +144,7 @@
    {{ advertiser }}
    {% elif publisher %} - {% publisher_role request.user publisher as user_publisher_role %} + {% publisher_admin_role request.user publisher as has_publisher_admin_permission %}
    {{ publisher }}
    {% trans 'Name' %} {% trans 'Email' %}{% trans 'Role' %} {% trans 'Options' %}
    {{ user.name }}{{ user.email }}{{ member.user.name }}{{ member.user.email }}{{ member.role }} - {% if request.user.id != user.id %} - {% trans 'remove' %} + {% if request.user.is_staff or user_publisher_role == "Admin" %} + {% if request.user.id != member.user.id %} + {% trans 'remove' %} + {% endif %} {% endif %}
    {{ member.role }} - {% if request.user.is_staff or user_advertiser_role == "Admin" %} - {% if request.user.id != member.user.id %} + {% if has_advertiser_admin_permission and request.user.id != member.user.id %} {% trans 'remove' %} {% endif %} - {% endif %}
    {{ member.user.email }} {{ member.role }} - {% if request.user.is_staff or user_publisher_role == "Admin" %} + {% if has_publisher_admin_permission %} {% if request.user.id != member.user.id %} {% trans 'remove' %} {% endif %} diff --git a/adserver/templatetags/ad_extras.py b/adserver/templatetags/ad_extras.py index f2fa7690..b75055ae 100644 --- a/adserver/templatetags/ad_extras.py +++ b/adserver/templatetags/ad_extras.py @@ -1,9 +1,12 @@ """Custom template tags for advertisements.""" +import logging + from django import template from django.utils.safestring import mark_safe +log = logging.getLogger(__name__) # noqa register = template.Library() @@ -17,52 +20,40 @@ def advertisement_preview(ad, ad_type=None): @register.simple_tag -def advertiser_role(user, advertiser): +def advertiser_manager_role(user, advertiser): """ - Returns the users role in this advertiser or None if the user has no permissions. + Returns True if the user has manager or higher role on this advertiser and False otherwise. - Staff status is not taken into account. Caches the result on the user so future calls - don't involve a DB lookup. + Return True for staff. """ - if not hasattr(user, "_advertiser_roles"): - user._advertiser_roles = {} - - if advertiser.pk in user._advertiser_roles: - return user._advertiser_roles[advertiser.pk] + return user.is_staff or user.has_advertiser_manager_permission(advertiser) - membership = user.useradvertisermember_set.filter( - advertiser=advertiser, - ).first() - role = None - if membership: - role = membership.role +@register.simple_tag +def advertiser_admin_role(user, advertiser): + """ + Returns True if the user has admin role on this advertiser and False otherwise. - user._advertiser_roles[advertiser.pk] = role - return role + Return True for staff. + """ + return user.is_staff or user.has_advertiser_admin_permission(advertiser) @register.simple_tag -def publisher_role(user, publisher): +def publisher_manager_role(user, publisher): """ - Returns the users role in this publisher or None if the user has no permissions. + Returns True if the user has manager or higher role on this publisher and False otherwise. - Staff status is not taken into account. Caches the result on the user so future calls - don't involve a DB lookup. + Return True for staff. """ - if not hasattr(user, "_publisher_roles"): - user._publisher_roles = {} + return user.is_staff or user.has_publisher_manager_permission(publisher) - if publisher.pk in user._publisher_roles: - return user._publisher_roles[publisher.pk] - membership = user.userpublishermember_set.filter( - publisher=publisher, - ).first() - - role = None - if membership: - role = membership.role +@register.simple_tag +def publisher_admin_role(user, publisher): + """ + Returns True if the user has admin role on this publisher and False otherwise. - user._publisher_roles[publisher.pk] = role - return role + Return True for staff. + """ + return user.is_staff or user.has_publisher_admin_permission(publisher) diff --git a/adserver/tests/test_advertiser_dashboard.py b/adserver/tests/test_advertiser_dashboard.py index 55de382e..ecaf6e4d 100644 --- a/adserver/tests/test_advertiser_dashboard.py +++ b/adserver/tests/test_advertiser_dashboard.py @@ -204,6 +204,15 @@ def test_flight_list_view(self): self.assertEqual(response.status_code, 200) self.assertContains(response, self.flight.name) + # Make it a reporter who can't request a new flight + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = UserAdvertiserMember.ROLE_REPORTER + member.save() + + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, "Request a new flight") + def test_flight_detail_view(self): url = reverse( "flight_detail", @@ -234,6 +243,15 @@ def test_flight_detail_view(self): self.assertEqual(response.status_code, 200) self.assertContains(response, self.ad1.name) + # Make it a reporter who can't edit + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = UserAdvertiserMember.ROLE_REPORTER + member.save() + + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, "Create advertisement") + def test_flight_detail_metadata(self): url = reverse( "flight_detail", @@ -707,6 +725,15 @@ def test_ad_detail_view(self): self.assertEqual(response.status_code, 200) self.assertContains(response, self.ad1.name) + # Make it a reporter who can't access + member = UserAdvertiserMember.objects.get(user=self.user, advertiser=self.advertiser) + member.role = UserAdvertiserMember.ROLE_REPORTER + member.save() + + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, "Edit advertisement") + def test_ad_update_view(self): url = reverse( "advertisement_update", From 5972624e3cad18952a8d9fc596ad8ac3dbec6aa0 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 20 Nov 2024 15:42:20 -0800 Subject: [PATCH 16/17] Release v5.10.0 --- CHANGELOG.rst | 21 +++++++++++++++++++++ package.json | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a5643abd..1c128ac5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,27 @@ CHANGELOG .. This is included by docs/developer/changelog.rst +Version v5.10.0 +-------------- + +This release added a few advertiser features including +role based user accounts (for publishers too), +some visual cues, and bulk ad creation. +Other changes were mostly minor fixes, dependencies, and documentation. + +:Date: November 20, 2024 + + * @dependabot[bot]: Bump aiohttp from 3.10.10 to 3.10.11 in /requirements (#943) + * @davidfischer: Add roles for advertisers and publishers (#941) + * @davidfischer: Add a visual cue for renewing flights on listview (#940) + * @davidfischer: Fix a celerybeat task configuration (#938) + * @mithucste30: Unknown task (#937) + * @davidfischer: Bulk ad upload (#935) + * @davidfischer: Remove explicit docs ad placement (#934) + * @ericholscher: Fix custom.css (#933) + * @davidfischer: Analyzer versions hotfix (#931) + + Version v5.9.0 -------------- diff --git a/package.json b/package.json index 15c1806a..bcfc9a8a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ethical-ad-server", - "version": "5.9.0", + "version": "5.10.0", "description": "", "main": "index.js", "engines": { From b3ce1d0f1a3ea0f6922f2630af67e078dd8a4ff6 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Wed, 20 Nov 2024 15:49:12 -0800 Subject: [PATCH 17/17] Fix docs heading --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1c128ac5..92d57efc 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,7 +7,7 @@ CHANGELOG Version v5.10.0 --------------- +--------------- This release added a few advertiser features including role based user accounts (for publishers too),