From dc6fdce40692099ec5cda31dbc1546021ff9a5d1 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Mon, 13 Jan 2025 16:23:55 +0100 Subject: [PATCH 1/4] update with django admin view for plans --- codecov_auth/admin.py | 83 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/codecov_auth/admin.py b/codecov_auth/admin.py index bb717ac8d6..985bc675d4 100644 --- a/codecov_auth/admin.py +++ b/codecov_auth/admin.py @@ -8,7 +8,7 @@ from django.contrib.admin.models import LogEntry from django.db.models import OuterRef, Subquery from django.db.models.fields import BLANK_CHOICE_DASH -from django.forms import CheckboxInput, Select +from django.forms import CheckboxInput, Select, Textarea from django.http import HttpRequest from django.shortcuts import redirect, render from django.utils import timezone @@ -18,6 +18,8 @@ AccountsUsers, InvoiceBilling, StripeBilling, + Plans, + Tiers, ) from shared.plan.constants import USER_PLAN_REPRESENTATIONS from shared.plan.service import PlanService @@ -708,3 +710,82 @@ class AccountsUsersAdmin(AdminMixin, admin.ModelAdmin): ] fields = readonly_fields + ["account", "user"] + +class PlansInline(admin.TabularInline): + model = Plans + extra = 1 + verbose_name_plural = "Plans (click save to commit changes)" + verbose_name = "Plan" + readonly_fields = ["name"] + fields = [ + "name", + "marketing_name", + "base_unit_price", + "billing_rate", + "max_seats", + "monthly_uploads_limit", + "paid_plan", + "is_active", + ] + formfield_overrides = { + Plans._meta.get_field("benefits"): {"widget": Textarea(attrs={"rows": 3})}, + } + + +@admin.register(Tiers) +class TiersAdmin(admin.ModelAdmin): + list_display = ( + "tier_name", + "bundle_analysis", + "test_analytics", + "flaky_test_detection", + "project_coverage", + "private_repo_support", + ) + list_editable = ( + "bundle_analysis", + "test_analytics", + "flaky_test_detection", + "project_coverage", + "private_repo_support", + ) + search_fields = ("tier_name__iregex",) + inlines = [PlansInline] + fields = [ + "tier_name", + "bundle_analysis", + "test_analytics", + "flaky_test_detection", + "project_coverage", + "private_repo_support", + ] + + +@admin.register(Plans) +class PlansAdmin(admin.ModelAdmin): + list_display = ("name", "marketing_name", "base_unit_price", "is_active", "paid_plan") + list_filter = ("is_active", "paid_plan", "billing_rate") + search_fields = ("name__iregex", "marketing_name__iregex") + fields = [ + "tier", + "name", + "marketing_name", + "base_unit_price", + "benefits", + "billing_rate", + "is_active", + "max_seats", + "monthly_uploads_limit", + "paid_plan", + ] + formfield_overrides = { + Plans._meta.get_field("benefits"): {"widget": Textarea(attrs={"rows": 3})}, + } + autocomplete_fields = ["tier"] # a dropdown for selecting related Tiers + + def save_model(self, request, obj, form, change): + if obj.base_unit_price < 0: + raise forms.ValidationError("Base unit price cannot be negative.") + if obj.max_seats is not None and obj.max_seats < 0: + raise forms.ValidationError("Max seats cannot be negative.") + super().save_model(request, obj, form, change) \ No newline at end of file From e2a2bdd7c47ec2eccbc9041546e995105e6b017b Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Wed, 15 Jan 2025 13:29:19 +0100 Subject: [PATCH 2/4] final touch, make sure validation works + add tests --- codecov_auth/admin.py | 60 +++++++---- codecov_auth/tests/test_admin.py | 165 ++++++++++++++++++++++++++++++- requirements.in | 2 +- requirements.txt | 14 +-- 4 files changed, 214 insertions(+), 27 deletions(-) diff --git a/codecov_auth/admin.py b/codecov_auth/admin.py index 985bc675d4..db1f91eb13 100644 --- a/codecov_auth/admin.py +++ b/codecov_auth/admin.py @@ -17,9 +17,9 @@ Account, AccountsUsers, InvoiceBilling, + Plan, StripeBilling, - Plans, - Tiers, + Tier, ) from shared.plan.constants import USER_PLAN_REPRESENTATIONS from shared.plan.service import PlanService @@ -711,12 +711,12 @@ class AccountsUsersAdmin(AdminMixin, admin.ModelAdmin): fields = readonly_fields + ["account", "user"] + class PlansInline(admin.TabularInline): - model = Plans + model = Plan extra = 1 verbose_name_plural = "Plans (click save to commit changes)" verbose_name = "Plan" - readonly_fields = ["name"] fields = [ "name", "marketing_name", @@ -728,12 +728,12 @@ class PlansInline(admin.TabularInline): "is_active", ] formfield_overrides = { - Plans._meta.get_field("benefits"): {"widget": Textarea(attrs={"rows": 3})}, + Plan._meta.get_field("benefits"): {"widget": Textarea(attrs={"rows": 3})}, } -@admin.register(Tiers) -class TiersAdmin(admin.ModelAdmin): +@admin.register(Tier) +class TierAdmin(admin.ModelAdmin): list_display = ( "tier_name", "bundle_analysis", @@ -761,9 +761,40 @@ class TiersAdmin(admin.ModelAdmin): ] -@admin.register(Plans) -class PlansAdmin(admin.ModelAdmin): - list_display = ("name", "marketing_name", "base_unit_price", "is_active", "paid_plan") +class PlanAdminForm(forms.ModelForm): + class Meta: + model = Plan + fields = "__all__" + + def clean_base_unit_price(self): + base_unit_price = self.cleaned_data.get("base_unit_price") + if base_unit_price is not None and base_unit_price < 0: + raise forms.ValidationError("Base unit price cannot be negative.") + return base_unit_price + + def clean_max_seats(self): + max_seats = self.cleaned_data.get("max_seats") + if max_seats is not None and max_seats < 0: + raise forms.ValidationError("Max seats cannot be negative.") + return max_seats + + def clean_monthly_uploads_limit(self): + monthly_uploads_limit = self.cleaned_data.get("monthly_uploads_limit") + if monthly_uploads_limit is not None and monthly_uploads_limit < 0: + raise forms.ValidationError("Monthly uploads limit cannot be negative.") + return monthly_uploads_limit + + +@admin.register(Plan) +class PlanAdmin(admin.ModelAdmin): + form = PlanAdminForm + list_display = ( + "name", + "marketing_name", + "base_unit_price", + "is_active", + "paid_plan", + ) list_filter = ("is_active", "paid_plan", "billing_rate") search_fields = ("name__iregex", "marketing_name__iregex") fields = [ @@ -779,13 +810,6 @@ class PlansAdmin(admin.ModelAdmin): "paid_plan", ] formfield_overrides = { - Plans._meta.get_field("benefits"): {"widget": Textarea(attrs={"rows": 3})}, + Plan._meta.get_field("benefits"): {"widget": Textarea(attrs={"rows": 3})}, } autocomplete_fields = ["tier"] # a dropdown for selecting related Tiers - - def save_model(self, request, obj, form, change): - if obj.base_unit_price < 0: - raise forms.ValidationError("Base unit price cannot be negative.") - if obj.max_seats is not None and obj.max_seats < 0: - raise forms.ValidationError("Max seats cannot be negative.") - super().save_model(request, obj, form, change) \ No newline at end of file diff --git a/codecov_auth/tests/test_admin.py b/codecov_auth/tests/test_admin.py index 55d24d2a9c..bd9d0ca4e6 100644 --- a/codecov_auth/tests/test_admin.py +++ b/codecov_auth/tests/test_admin.py @@ -18,9 +18,11 @@ InvoiceBillingFactory, OrganizationLevelTokenFactory, OwnerFactory, + PlanFactory, SentryUserFactory, SessionFactory, StripeBillingFactory, + TierFactory, UserFactory, ) from shared.django_apps.core.tests.factories import PullFactory, RepositoryFactory @@ -39,7 +41,14 @@ UserAdmin, find_and_remove_stale_users, ) -from codecov_auth.models import OrganizationLevelToken, Owner, SentryUser, User +from codecov_auth.models import ( + OrganizationLevelToken, + Owner, + Plan, + SentryUser, + Tier, + User, +) from core.models import Pull @@ -881,3 +890,157 @@ def test_account_widget(self): self.assertFalse(form.base_fields["account"].widget.can_add_related) self.assertFalse(form.base_fields["account"].widget.can_change_related) self.assertFalse(form.base_fields["account"].widget.can_delete_related) + + +class PlanAdminTest(TestCase): + def setUp(self): + self.staff_user = UserFactory(is_staff=True) + self.client.force_login(user=self.staff_user) + admin_site = AdminSite() + admin_site.register(Plan) + + def test_plan_admin_modal_display(self): + plan = PlanFactory() + response = self.client.get( + reverse("admin:codecov_auth_plan_change", args=[plan.pk]) + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, plan.name) + + def test_plan_modal_tiers_display(self): + tier = TierFactory() + plan = PlanFactory(tier=tier) + response = self.client.get( + reverse("admin:codecov_auth_plan_change", args=[plan.pk]) + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, tier.tier_name) + + def test_add_plans_modal_action(self): + plan = PlanFactory(base_unit_price=10, max_seats=5) + tier = TierFactory() + data = { + "action": "add_plans", + ACTION_CHECKBOX_NAME: [plan.pk], + "tier_id": tier.pk, + } + response = self.client.post( + reverse("admin:codecov_auth_plan_changelist"), data=data + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/admin/codecov_auth/plan/") + + def test_plan_change_form(self): + plan = PlanFactory() + response = self.client.get( + reverse("admin:codecov_auth_plan_change", args=[plan.pk]) + ) + self.assertEqual(response.status_code, 200) + for field in [ + "tier", + "name", + "marketing_name", + "base_unit_price", + "benefits", + "billing_rate", + "is_active", + "max_seats", + "monthly_uploads_limit", + "paid_plan", + ]: + self.assertContains(response, f"id_{field}") + + def test_plan_change_form_validation(self): + plan = PlanFactory(base_unit_price=-10) + + response = self.client.post( + reverse("admin:codecov_auth_plan_change", args=[plan.pk]), + { + "tier": plan.tier_id, + "name": plan.name, + "marketing_name": plan.marketing_name, + "base_unit_price": -10, + "benefits": plan.benefits, + "is_active": plan.is_active, + "paid_plan": plan.paid_plan, + }, + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Base unit price cannot be negative.") + + response = self.client.post( + reverse("admin:codecov_auth_plan_change", args=[plan.pk]), + { + "tier": plan.tier_id, + "name": plan.name, + "marketing_name": plan.marketing_name, + "base_unit_price": plan.base_unit_price, + "benefits": plan.benefits, + "is_active": plan.is_active, + "max_seats": -5, + "paid_plan": plan.paid_plan, + }, + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Max seats cannot be negative.") + + response = self.client.post( + reverse("admin:codecov_auth_plan_change", args=[plan.pk]), + { + "tier": plan.tier_id, + "name": plan.name, + "marketing_name": plan.marketing_name, + "benefits": plan.benefits, + "is_active": plan.is_active, + "monthly_uploads_limit": -5, + "paid_plan": plan.paid_plan, + }, + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Monthly uploads limit cannot be negative.") + + +class TierAdminTest(TestCase): + def setUp(self): + self.staff_user = UserFactory(is_staff=True) + self.client.force_login(user=self.staff_user) + admin_site = AdminSite() + admin_site.register(Tier) + + def test_tier_modal_plans_display(self): + tier = TierFactory() + response = self.client.get( + reverse("admin:codecov_auth_tier_change", args=[tier.pk]) + ) + self.assertEqual(response.status_code, 200) + self.assertContains(response, tier.tier_name) + + def test_add_plans_modal_action(self): + tier = TierFactory() + plan = PlanFactory() + data = { + "action": "add_plans", + ACTION_CHECKBOX_NAME: [plan.pk], + "tier_id": tier.pk, + } + response = self.client.post( + reverse("admin:codecov_auth_tier_changelist"), data=data + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/admin/codecov_auth/tier/") + + def test_tier_change_form(self): + tier = TierFactory() + response = self.client.get( + reverse("admin:codecov_auth_tier_change", args=[tier.pk]) + ) + self.assertEqual(response.status_code, 200) + for field in [ + "tier_name", + "bundle_analysis", + "test_analytics", + "flaky_test_detection", + "project_coverage", + "private_repo_support", + ]: + self.assertContains(response, f"id_{field}") diff --git a/requirements.in b/requirements.in index 339ad96b77..d3ee91814a 100644 --- a/requirements.in +++ b/requirements.in @@ -26,7 +26,7 @@ freezegun google-cloud-pubsub gunicorn>=22.0.0 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem -https://github.com/codecov/shared/archive/c3288a017d49d72a85d6f42a2056d3063cfd8dda.tar.gz#egg=shared +https://github.com/codecov/shared/archive/d5d7c208f9716ac0f48543f84b635aa28d15f0f2.tar.gz#egg=shared https://github.com/photocrowd/django-cursor-pagination/archive/f560902696b0c8509e4d95c10ba0d62700181d84.tar.gz idna>=3.7 minio diff --git a/requirements.txt b/requirements.txt index 0d83703bf4..bde680e09f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -202,8 +202,6 @@ googleapis-common-protos[grpc]==1.59.1 # grpcio-status graphql-core==3.2.3 # via ariadne -greenlet==3.1.1 - # via sqlalchemy grpc-google-iam-v1==0.12.6 # via google-cloud-pubsub grpcio==1.62.1 @@ -337,9 +335,11 @@ pyasn1-modules==0.2.8 # via google-auth pycparser==2.20 # via cffi -pydantic==2.8.2 - # via -r requirements.in -pydantic-core==2.20.1 +pydantic==2.10.5 + # via + # -r requirements.in + # shared +pydantic-core==2.27.2 # via pydantic pyjwt==2.8.0 # via @@ -416,7 +416,7 @@ sentry-sdk[celery]==2.13.0 # shared setproctitle==1.1.10 # via -r requirements.in -shared @ https://github.com/codecov/shared/archive/c3288a017d49d72a85d6f42a2056d3063cfd8dda.tar.gz +shared @ https://github.com/codecov/shared/archive/d5d7c208f9716ac0f48543f84b635aa28d15f0f2.tar.gz # via -r requirements.in simplejson==3.17.2 # via -r requirements.in @@ -451,7 +451,7 @@ text-unidecode==1.3 # via faker toml==0.10.2 # via pre-commit -typing-extensions==4.6.2 +typing-extensions==4.12.2 # via # aiodataloader # ariadne From af8eaf9ebb5075c06b3fedabb518813d36bf2930 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Thu, 16 Jan 2025 11:54:34 +0100 Subject: [PATCH 3/4] update with suggestions --- codecov_auth/admin.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/codecov_auth/admin.py b/codecov_auth/admin.py index db1f91eb13..26c2be5f47 100644 --- a/codecov_auth/admin.py +++ b/codecov_auth/admin.py @@ -766,19 +766,19 @@ class Meta: model = Plan fields = "__all__" - def clean_base_unit_price(self): + def clean_base_unit_price(self) -> int | None: base_unit_price = self.cleaned_data.get("base_unit_price") if base_unit_price is not None and base_unit_price < 0: raise forms.ValidationError("Base unit price cannot be negative.") return base_unit_price - def clean_max_seats(self): + def clean_max_seats(self) -> int | None: max_seats = self.cleaned_data.get("max_seats") if max_seats is not None and max_seats < 0: raise forms.ValidationError("Max seats cannot be negative.") return max_seats - def clean_monthly_uploads_limit(self): + def clean_monthly_uploads_limit(self) -> int | None: monthly_uploads_limit = self.cleaned_data.get("monthly_uploads_limit") if monthly_uploads_limit is not None and monthly_uploads_limit < 0: raise forms.ValidationError("Monthly uploads limit cannot be negative.") @@ -794,8 +794,11 @@ class PlanAdmin(admin.ModelAdmin): "base_unit_price", "is_active", "paid_plan", + "max_seats", + "monthly_uploads_limit", + "billing_rate", ) - list_filter = ("is_active", "paid_plan", "billing_rate") + list_filter = ("is_active", "paid_plan", "billing_rate", "tier") search_fields = ("name__iregex", "marketing_name__iregex") fields = [ "tier", From 632a3d3ce5c5de546655876c628d61dfbae888e8 Mon Sep 17 00:00:00 2001 From: RulaKhaled Date: Thu, 16 Jan 2025 11:58:53 +0100 Subject: [PATCH 4/4] linter yelling --- utils/test_utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/utils/test_utils.py b/utils/test_utils.py index 6deeb8251a..6cac27e04e 100644 --- a/utils/test_utils.py +++ b/utils/test_utils.py @@ -45,10 +45,10 @@ def app(self) -> str: migrate_to = None def setUp(self) -> None: - assert ( - self.migrate_from and self.migrate_to - ), "TestCase '{}' must define migrate_from and migrate_to properties".format( - type(self).__name__ + assert self.migrate_from and self.migrate_to, ( + "TestCase '{}' must define migrate_from and migrate_to properties".format( + type(self).__name__ + ) ) self.migrate_from = [(self.app, self.migrate_from)] self.migrate_to = [(self.app, self.migrate_to)]