Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CONSVC-1729] feat: introduce approval for partners #39

Merged
merged 2 commits into from
Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions consvc_shepherd/admin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json

from django.contrib import admin, messages
from django.core.exceptions import ValidationError
from django.utils import timezone

from consvc_shepherd.models import SettingsSnapshot
Expand Down Expand Up @@ -28,10 +29,13 @@ class ModelAdmin(admin.ModelAdmin):
actions = [publish_snapshot]

def save_model(self, request, obj, form, change) -> None:
json_settings = obj.settings_type.to_dict()
obj.json_settings = json_settings
obj.created_by = request.user
super(ModelAdmin, self).save_model(request, obj, form, change)
if obj.settings_type.is_active:
json_settings = obj.settings_type.to_dict()
obj.json_settings = json_settings
obj.created_by = request.user
super(ModelAdmin, self).save_model(request, obj, form, change)
else:
raise ValidationError("Partner Selected is not active")

def get_readonly_fields(self, request, obj=None):
if obj:
Expand Down
12 changes: 12 additions & 0 deletions consvc_shepherd/models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from django.contrib.auth import get_user_model
from django.core.exceptions import ValidationError
from django.db import models

from contile.models import Partner
Expand All @@ -23,3 +24,14 @@ class SettingsSnapshot(models.Model):

def __str__(self):
return f"{self.name}: {self.created_on}"

def clean(self):
if (
not self.settings_type.is_active
or self.settings_type.last_approved_by is None
):
raise ValidationError("Partner Selected is not approved")

def save(self, *args, **kwargs):
self.full_clean()
return super(SettingsSnapshot, self).save(*args, **kwargs)
11 changes: 8 additions & 3 deletions consvc_shepherd/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@
from consvc_shepherd.tests.factories import UserFactory


class MyAdminTest(TestCase):
class SettingsSnapshotAdminTest(TestCase):
def setUp(self):
request_factory = RequestFactory()
self.request = request_factory.get("/admin")
self.request.user = UserFactory()

site = AdminSite()
self.admin = ModelAdmin(SettingsSnapshot, site)
self.partner = Partner.objects.create(name="Partner1")
self.partner = Partner.objects.create(
name="Partner1", is_active=True, last_approved_by=self.request.user
)

self.mock_storage = mock.patch(
"django.core.files.storage.default_storage." "open"
Expand Down Expand Up @@ -51,7 +53,10 @@ def test_save_model_generates_json(self):

expected_json = self.partner.to_dict()
self.admin.save_model(
self.request, SettingsSnapshot(settings_type=self.partner), None, {}
self.request,
SettingsSnapshot(name="dev", settings_type=self.partner),
None,
{},
)

self.assertEqual(SettingsSnapshot.objects.all().count(), 1)
Expand Down
31 changes: 30 additions & 1 deletion contile/admin.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from django import forms
from django.contrib import admin
from django.contrib import admin, messages
from django.contrib.postgres.forms import SimpleArrayField

from contile.models import Advertiser, AdvertiserUrl, Partner
Expand All @@ -12,6 +12,24 @@ class Meta:
fields = "__all__"


@admin.action(description="Approve Partner Settings")
def approve_partner_settings(modeladmin, request, queryset):
if len(queryset) > 1:
messages.error(request, "Only 1 partner can be approved at a time")
else:
partner = queryset[0]
if partner.last_updated_by != request.user:
partner.is_active = True
partner.last_approved_by = request.user
partner.save()
messages.info(request, f"Partner: {partner.name} has been approved")
else:
messages.error(
request,
"This change can't be approved by the same editor, please get another reviewer for approval",
)


class PartnerForm(forms.ModelForm):
click_hosts = SimpleArrayField(
forms.CharField(),
Expand All @@ -23,6 +41,9 @@ class PartnerForm(forms.ModelForm):
label="Impression Hosts (list by separated commas)",
required=False,
)
is_active = forms.BooleanField()
last_updated_by = forms.CharField()
last_approved_by = forms.CharField()


class AdUrlInline(admin.TabularInline):
Expand All @@ -39,5 +60,13 @@ class AdvertiserListAdmin(admin.ModelAdmin):

@admin.register(Partner)
class PartnerListAdmin(admin.ModelAdmin):
readonly_fields = ["is_active", "last_updated_by", "last_approved_by"]
model = Partner
form = PartnerForm
actions = [approve_partner_settings]

def save_model(self, request, obj, form, change):
obj.is_active = False
obj.last_updated_by = request.user
obj.last_approved_by = None
super(PartnerListAdmin, self).save_model(request, obj, form, change)
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Generated by Django 4.0.4 on 2022-04-27 06:46

from django.conf import settings
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
("contile", "0001_initial"),
]

operations = [
migrations.AddField(
model_name="partner",
name="is_active",
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name="partner",
name="last_approved_by",
field=models.ForeignKey(
blank=True,
null=True,
on_delete=models.deletion.CASCADE,
related_name="approved_by",
to=settings.AUTH_USER_MODEL,
),
),
migrations.AddField(
model_name="partner",
name="last_updated_by",
field=models.ForeignKey(
blank=True,
null=True,
on_delete=models.deletion.CASCADE,
related_name="updated_by",
to=settings.AUTH_USER_MODEL,
),
),
]
17 changes: 16 additions & 1 deletion contile/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.contrib.auth import get_user_model
from django.contrib.postgres.fields import ArrayField
from django.core.exceptions import ValidationError
from django.db import models
Expand All @@ -23,6 +24,21 @@ class Partner(models.Model):
default=list,
blank=True,
)
is_active = models.BooleanField(default=False)
last_updated_by = models.ForeignKey(
get_user_model(),
related_name="updated_by",
on_delete=models.CASCADE,
blank=True,
null=True,
)
last_approved_by = models.ForeignKey(
get_user_model(),
related_name="approved_by",
on_delete=models.CASCADE,
blank=True,
null=True,
)

def to_dict(self):
partner_dict = {
Expand All @@ -37,7 +53,6 @@ def to_dict(self):
return partner_dict

def clean(self):

for c_host in self.click_hosts:
is_valid_host(c_host)
for i_host in self.impression_hosts:
Expand Down
55 changes: 55 additions & 0 deletions contile/tests/test_admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import mock
from django.contrib.admin.sites import AdminSite
from django.test import RequestFactory, TestCase

from consvc_shepherd.tests.factories import UserFactory
from contile.admin import PartnerListAdmin, approve_partner_settings
from contile.models import Partner


class PartnerAdminTest(TestCase):
def setUp(self):
request_factory = RequestFactory()
self.request = request_factory.get("/admin")
self.request.user = UserFactory()

site = AdminSite()
self.admin = PartnerListAdmin(Partner, site)
self.partner = Partner.objects.create(
name="Partner1", last_updated_by=self.request.user
)

def test_approve_partner_settings(self):
request = mock.Mock()
request.user = UserFactory()
self.assertFalse(self.partner.is_active)
self.assertIsNone(self.partner.last_approved_by)
approve_partner_settings(None, request, [self.partner])
self.assertTrue(self.partner.is_active)
self.assertIsNotNone(self.partner.last_approved_by)

def test_partner_admin_save_model(self):
request = mock.Mock()
request.user = UserFactory()

self.partner.is_active = True
self.partner.is_updated_by = UserFactory()
self.partner.is_approved_by = UserFactory()
self.partner.save()
self.admin.save_model(request, self.partner, None, {})
self.assertFalse(self.partner.is_active)
self.assertIsNone(self.partner.last_approved_by)
self.assertEqual(self.partner.last_updated_by, request.user)

def test_approval_fails_when_updater_and_approver_are_the_same(self):
request = mock.Mock()
request.user = UserFactory()

self.partner.is_active = False
self.partner.last_updated_by = request.user
self.partner.last_approved_by = None
self.partner.save()
approve_partner_settings(None, request, [self.partner])
self.assertFalse(self.partner.is_active)
self.assertIsNone(self.partner.last_approved_by)
self.assertEqual(self.partner.last_updated_by, request.user)