From 302fc9e563d29c20a02a29d2a78ef83df7b994a4 Mon Sep 17 00:00:00 2001 From: Ivajlo Karabojkov Date: Sat, 29 May 2021 16:55:08 +0300 Subject: [PATCH 1/2] Move settings check to dashboard Use fewer database hits for better performance. Note: As part of the settings interface the following middleware functions were moved to the Dashboard: tcms.core.middleware.CheckSettingsMiddleware tcms.core.middleware.CheckUnappliedMigrationsMiddleware --- .github/workflows/testing.yml | 8 ++-- tcms/core/middleware.py | 57 ---------------------------- tcms/core/tests/test_middleware.py | 51 ------------------------- tcms/core/tests/test_views.py | 60 ++++++++++++++++++++++++++++-- tcms/core/views.py | 52 +++++++++++++++++++++++++- tcms/settings/common.py | 2 - 6 files changed, 111 insertions(+), 119 deletions(-) delete mode 100644 tcms/core/tests/test_middleware.py diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index b631216789..dd129970eb 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -35,8 +35,8 @@ jobs: coverage report -m bash <(curl -s https://codecov.io/bash) - test_check_unapplied_migrations_middleware: - name: middleware for unapplied migrations + check_for_unapplied_migrations: + name: check for unapplied migrations runs-on: ubuntu-latest strategy: matrix: @@ -59,8 +59,8 @@ jobs: - name: Run test run: | export LANG=en-us - export TEST_CHECK_UNAPPLIED_MIGRATIONS_MIDDLEWARE=1 - coverage run --source='.' ./manage.py test -v2 --noinput --settings=tcms.settings.test tcms.core.tests.test_middleware.TestCheckUnappliedMigrationsMiddleware + export TEST_DASHBOARD_CHECK_UNAPPLIED_MIGRATIONS=1 + coverage run --source='.' ./manage.py test -v2 --noinput --settings=tcms.settings.test tcms.core.tests.test_views.TestDashboardCheckMigrations - name: Send coverage to codecov.io run: | diff --git a/tcms/core/middleware.py b/tcms/core/middleware.py index 9ebfad2da4..593d9c6279 100644 --- a/tcms/core/middleware.py +++ b/tcms/core/middleware.py @@ -1,16 +1,11 @@ # pylint: disable=no-self-use, too-few-public-methods from django.conf import settings -from django.contrib import messages from django.contrib.sites.models import Site -from django.db import DEFAULT_DB_ALIAS, connections -from django.db.migrations.executor import MigrationExecutor from django.db.utils import OperationalError, ProgrammingError from django.http import HttpResponseRedirect from django.urls import reverse from django.utils.deprecation import MiddlewareMixin -from django.utils.safestring import mark_safe -from django.utils.translation import gettext_lazy as _ class CheckDBStructureExistsMiddleware(MiddlewareMixin): @@ -23,55 +18,3 @@ def process_request(self, request): # Redirect to Setup view return HttpResponseRedirect(reverse("init-db")) return None - - -class CheckSettingsMiddleware(MiddlewareMixin): - def process_request(self, request): - doc_url = "https://kiwitcms.readthedocs.io/en/latest/admin.html#configure-kiwi-s-base-url" - if request.path == "/init-db/": - return - site = Site.objects.get(pk=settings.SITE_ID) - - if site.domain == "127.0.0.1:8000": - messages.add_message( - request, - messages.ERROR, - mark_safe( # nosec:B308:B703 - _( - "Base URL is not configured! " - 'See documentation and ' - 'change it' - ) - % { - "doc_url": doc_url, - "admin_url": reverse("admin:sites_site_change", args=[site.pk]), - } - ), - ) - - -class CheckUnappliedMigrationsMiddleware(MiddlewareMixin): - def process_request(self, request): - if request.path == "/init-db/": - return - doc_url = ( - "https://kiwitcms.readthedocs.io/en/latest/" - "installing_docker.html#initial-configuration-of-running-container" - ) - executor = MigrationExecutor(connections[DEFAULT_DB_ALIAS]) - plan = executor.migration_plan(executor.loader.graph.leaf_nodes()) - if plan: - messages.add_message( - request, - messages.ERROR, - mark_safe( # nosec:B308:B703 - _( - "You have %(unapplied_migration_count)s unapplied migration(s). " - 'See documentation' - ) - % { - "unapplied_migration_count": len(plan), - "doc_url": doc_url, - } - ), - ) diff --git a/tcms/core/tests/test_middleware.py b/tcms/core/tests/test_middleware.py deleted file mode 100644 index 0ad49ce53d..0000000000 --- a/tcms/core/tests/test_middleware.py +++ /dev/null @@ -1,51 +0,0 @@ -import os -import unittest - -from django import test -from django.conf import settings -from django.contrib.sites.models import Site -from django.core.management import call_command -from django.urls import reverse -from django.utils.translation import gettext_lazy as _ - - -@unittest.skipUnless( - os.getenv("TEST_CHECK_UNAPPLIED_MIGRATIONS_MIDDLEWARE"), - "CheckUnappliedMigrationsMiddleware testing not enabled", -) -class TestCheckUnappliedMigrationsMiddleware(test.TransactionTestCase): - def test_unapplied_migrations(self): - call_command("migrate", "bugs", "zero", verbosity=2, interactive=False) - unapplied_migration_message = _( - "unapplied migration(s). See " - 'documentation' - ) - response = self.client.get("/", follow=True) - self.assertContains(response, unapplied_migration_message) - - -class TestCheckSettingsMiddleware(test.TestCase): - @classmethod - def setUpClass(cls): - super().setUpClass() - doc_url = "https://kiwitcms.readthedocs.io/en/latest/admin.html#configure-kiwi-s-base-url" - cls.base_url_error_message = _( - "Base URL is not configured! " - 'See documentation and ' - 'change it' - ) % { - "doc_url": doc_url, - "admin_url": reverse("admin:sites_site_change", args=[settings.SITE_ID]), - } - - def test_base_url_not_configured(self): - response = self.client.get("/", follow=True) - self.assertContains(response, self.base_url_error_message) - - def test_base_url_configured(self): - site = Site.objects.create(domain="example.com", name="example") - with test.override_settings(SITE_ID=site.pk): - response = self.client.get("/", follow=True) - self.assertNotContains(response, self.base_url_error_message) diff --git a/tcms/core/tests/test_views.py b/tcms/core/tests/test_views.py index 90ed3cf3cc..43983a18b1 100644 --- a/tcms/core/tests/test_views.py +++ b/tcms/core/tests/test_views.py @@ -1,23 +1,42 @@ # -*- coding: utf-8 -*- # pylint: disable=too-many-ancestors +import os +import unittest from http import HTTPStatus from django import test +from django.conf import settings +from django.contrib.sites.models import Site +from django.core.management import call_command from django.urls import include, path, reverse from django.utils.translation import gettext_lazy as _ from tcms import urls -from tcms.tests import BaseCaseRun -from tcms.tests.factories import TestExecutionFactory, TestPlanFactory, TestRunFactory +from tcms.tests import LoggedInTestCase +from tcms.tests.factories import ( + TestExecutionFactory, + TestPlanFactory, + TestRunFactory, + UserFactory, +) -class TestDashboard(BaseCaseRun): +class TestDashboard(LoggedInTestCase): @classmethod def setUpTestData(cls): super().setUpTestData() # used to reproduce Sentry #KIWI-TCMS-38 where rendering fails # with that particular value cls.chinese_tp = TestPlanFactory(name="缺货反馈测试需求", author=cls.tester) + doc_url = "https://kiwitcms.readthedocs.io/en/latest/admin.html#configure-kiwi-s-base-url" + cls.base_url_error_message = _( + "Base URL is not configured! " + 'See documentation and ' + 'change it' + ) % { + "doc_url": doc_url, + "admin_url": reverse("admin:sites_site_change", args=[settings.SITE_ID]), + } def test_when_not_logged_in_redirects_to_login(self): self.client.logout() @@ -53,6 +72,41 @@ def test_dashboard_shows_testruns_for_execution_assignee(self): response = self.client.get(reverse("core-views-index")) self.assertContains(response, execution.run.summary) + def test_check_base_url_not_configured(self): + response = self.client.get("/", follow=True) + self.assertContains(response, self.base_url_error_message) + + def test_check_base_url_configured(self): + site = Site.objects.create(domain="example.com", name="example") + with test.override_settings(SITE_ID=site.pk): + response = self.client.get("/", follow=True) + self.assertNotContains(response, self.base_url_error_message) + + +@unittest.skipUnless( + os.getenv("TEST_DASHBOARD_CHECK_UNAPPLIED_MIGRATIONS"), + "Check for missing migrations testing is not enabled", +) +class TestDashboardCheckMigrations(test.TransactionTestCase): + unapplied_migration_message = _( + "unapplied migration(s). See " + 'documentation' + ) + + def test_check_unapplied_migrations(self): + call_command("migrate", "bugs", "zero", verbosity=2, interactive=False) + tester = UserFactory() + tester.set_password("password") + tester.save() + self.client.login( # nosec:B106:hardcoded_password_funcarg + username=tester.username, + password="password", + ) + response = self.client.get("/", follow=True) + self.assertContains(response, self.unapplied_migration_message) + def exception_view(request): raise Exception diff --git a/tcms/core/views.py b/tcms/core/views.py index a2052bfeb6..0c519a2e4b 100644 --- a/tcms/core/views.py +++ b/tcms/core/views.py @@ -5,14 +5,19 @@ from django import http from django.conf import settings +from django.contrib import messages from django.contrib.auth.decorators import login_required +from django.contrib.sites.models import Site +from django.db import DEFAULT_DB_ALIAS, connections +from django.db.migrations.executor import MigrationExecutor from django.db.models import Count, Q from django.http import HttpResponseRedirect, StreamingHttpResponse from django.template import loader from django.urls import reverse from django.utils import timezone, translation from django.utils.decorators import method_decorator -from django.utils.translation import trans_real +from django.utils.safestring import mark_safe +from django.utils.translation import trans_real, gettext_lazy as _ from django.views import i18n from django.views.decorators.csrf import requires_csrf_token from django.views.generic.base import TemplateView, View @@ -29,7 +34,50 @@ class DashboardView(TemplateView): template_name = "dashboard.html" def get_context_data(self, **kwargs): - """List all recent TestPlans and TestRuns""" + # Check if domain is configured + site = Site.objects.get(pk=settings.SITE_ID) + doc_url = "https://kiwitcms.readthedocs.io/en/latest/admin.html#configure-kiwi-s-base-url" + if site.domain == "127.0.0.1:8000": + messages.add_message( + self.request, + messages.ERROR, + mark_safe( # nosec:B308:B703 + _( + "Base URL is not configured! " + 'See documentation and ' + 'change it' + ) + % { + "doc_url": doc_url, + "admin_url": reverse("admin:sites_site_change", args=[site.pk]), + } + ), + ) + + # Check for missing migrations + doc_url = ( + "https://kiwitcms.readthedocs.io/en/latest/" + "installing_docker.html#initial-configuration-of-running-container" + ) + executor = MigrationExecutor(connections[DEFAULT_DB_ALIAS]) + plan = executor.migration_plan(executor.loader.graph.leaf_nodes()) + if plan: + messages.add_message( + self.request, + messages.ERROR, + mark_safe( # nosec:B308:B703 + _( + "You have %(unapplied_migration_count)s unapplied migration(s). " + 'See documentation' + ) + % { + "unapplied_migration_count": len(plan), + "doc_url": doc_url, + } + ), + ) + + # List all recent TestPlans and TestRuns test_plans = ( TestPlan.objects.filter(author=self.request.user) .order_by("-pk") diff --git a/tcms/settings/common.py b/tcms/settings/common.py index 0d56480c0b..55df5d7e79 100644 --- a/tcms/settings/common.py +++ b/tcms/settings/common.py @@ -136,8 +136,6 @@ "django.contrib.auth.middleware.AuthenticationMiddleware", "django.contrib.messages.middleware.MessageMiddleware", "simple_history.middleware.HistoryRequestMiddleware", - "tcms.core.middleware.CheckSettingsMiddleware", - "tcms.core.middleware.CheckUnappliedMigrationsMiddleware", ] From f1532d5c8242ef4e43cca616b0d0f3cf3aa3e4f2 Mon Sep 17 00:00:00 2001 From: Ivajlo Karabojkov Date: Wed, 2 Jun 2021 23:27:10 +0300 Subject: [PATCH 2/2] Minor code sorting change --- tcms/core/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tcms/core/views.py b/tcms/core/views.py index 0c519a2e4b..1c643657c0 100644 --- a/tcms/core/views.py +++ b/tcms/core/views.py @@ -17,7 +17,8 @@ from django.utils import timezone, translation from django.utils.decorators import method_decorator from django.utils.safestring import mark_safe -from django.utils.translation import trans_real, gettext_lazy as _ +from django.utils.translation import gettext_lazy as _ +from django.utils.translation import trans_real from django.views import i18n from django.views.decorators.csrf import requires_csrf_token from django.views.generic.base import TemplateView, View