From 82cd91d923be346552e7db950f489f198e87d57d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:46:12 -0600 Subject: [PATCH 01/23] add portfolios view --- src/registrar/admin.py | 4 ++- src/registrar/models/user.py | 3 +++ .../django/admin/user_change_form.html | 26 +++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6b42cf96b..640037847 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -962,7 +962,9 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): domain_ids = user_domain_roles.values_list("domain_id", flat=True) domains = Domain.objects.filter(id__in=domain_ids).exclude(state=Domain.State.DELETED) - extra_context = {"domain_requests": domain_requests, "domains": domains} + portfolio_ids = obj.get_portfolios().values_list("portfolio", flat=True) + portfolios = models.Portfolio.objects.filter(id__in=portfolio_ids) + extra_context = {"domain_requests": domain_requests, "domains": domains, "portfolios": portfolios} return super().change_view(request, object_id, form_url, extra_context) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index a7ea1e14a..48bde5281 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -244,6 +244,9 @@ class User(AbstractUser): if permission: return permission.portfolio return None + + def get_portfolios(self): + return self.portfolio_permissions.all() @classmethod def needs_identity_verification(cls, email, uuid): diff --git a/src/registrar/templates/django/admin/user_change_form.html b/src/registrar/templates/django/admin/user_change_form.html index 005d67aec..c78fae6cb 100644 --- a/src/registrar/templates/django/admin/user_change_form.html +++ b/src/registrar/templates/django/admin/user_change_form.html @@ -1,7 +1,33 @@ {% extends 'django/admin/email_clipboard_change_form.html' %} {% load i18n static %} +{% block field_sets %} + {% for fieldset in adminform %} + {% include "django/admin/includes/email_clipboard_fieldset.html" %} + {% endfor %} +{% endblock %} + {% block after_related_objects %} + {% if portfolios %} +
+

Portfolio information

+
+
+

Portfolios

+ +
+
+
+ {% endif %} +

Associated requests and domains

From c70e098f5401e3860de73232aec883e0e827bf22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:46:31 -0600 Subject: [PATCH 02/23] Update user_change_form.html --- src/registrar/templates/django/admin/user_change_form.html | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/registrar/templates/django/admin/user_change_form.html b/src/registrar/templates/django/admin/user_change_form.html index c78fae6cb..3a7ea5f92 100644 --- a/src/registrar/templates/django/admin/user_change_form.html +++ b/src/registrar/templates/django/admin/user_change_form.html @@ -1,12 +1,6 @@ {% extends 'django/admin/email_clipboard_change_form.html' %} {% load i18n static %} -{% block field_sets %} - {% for fieldset in adminform %} - {% include "django/admin/includes/email_clipboard_fieldset.html" %} - {% endfor %} -{% endblock %} - {% block after_related_objects %} {% if portfolios %}
From 1cb14f8d6a0ef03fbae3d4ea3b01244dba1f3489 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:48:26 -0600 Subject: [PATCH 03/23] Update user_change_form.html --- src/registrar/templates/django/admin/user_change_form.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/user_change_form.html b/src/registrar/templates/django/admin/user_change_form.html index 3a7ea5f92..b545bed23 100644 --- a/src/registrar/templates/django/admin/user_change_form.html +++ b/src/registrar/templates/django/admin/user_change_form.html @@ -11,7 +11,7 @@
    {% for portfolio in portfolios %}
  • - + {{ portfolio }}
  • From c57405a918c673adc672b62236f67f27f13f3e22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 15:25:06 -0600 Subject: [PATCH 04/23] Update test_admin.py --- src/registrar/tests/test_admin.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a435c6a69..2e9122c38 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2,6 +2,7 @@ from datetime import datetime from django.utils import timezone from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from api.tests.common import less_console_noise_decorator from django.urls import reverse from registrar.admin import ( @@ -41,6 +42,7 @@ from registrar.models import ( TransitionDomain, Portfolio, Suborganization, + UserPortfolioPermission, ) from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial @@ -1223,6 +1225,25 @@ class TestMyUserAdmin(MockDbForSharedTests): self.assertNotContains(response, "Portfolio roles:") self.assertNotContains(response, "Portfolio additional permissions:") + + @less_console_noise_decorator + def test_user_can_see_related_portfolios(self): + """Tests if a user can see the portfolios they are associated with on the user page""" + portfolio, _ = Portfolio.objects.get_or_create(organization_name="test", creator=self.user) + permission, _ = UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + self.user.refresh_from_db() + self.client.force_login(self.user) + response = self.client.get( + "/admin/registrar/user/{}/change/".format(self.user.id), + follow=True, + ) + expected_href = reverse("admin:registrar_portfolio_change", args=[portfolio.pk]) + self.assertContains(response, expected_href) + self.assertContains(response, str(portfolio)) + permission.delete() + portfolio.delete() class AuditedAdminTest(TestCase): From 7a77cb8444343ae897f580dd65fc487bbebeb63d Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 12:37:01 -0500 Subject: [PATCH 05/23] initial stab at a new json formatter --- src/registrar/config/settings.py | 43 ++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 73aecad7a..016dda4d9 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -23,6 +23,8 @@ from cfenv import AppEnv # type: ignore from pathlib import Path from typing import Final from botocore.config import Config +import json +from django.utils.log import ServerFormatter # # # ### # Setup code goes here # @@ -192,7 +194,7 @@ MIDDLEWARE = [ "registrar.registrar_middleware.CheckPortfolioMiddleware", ] -# application object used by Django’s built-in servers (e.g. `runserver`) +# application object used by Django's built-in servers (e.g. `runserver`) WSGI_APPLICATION = "registrar.config.wsgi.application" # endregion @@ -410,7 +412,7 @@ LANGUAGE_COOKIE_SECURE = True # and to interpret datetimes entered in forms TIME_ZONE = "UTC" -# enable Django’s translation system +# enable Django's translation system USE_I18N = True # enable localized formatting of numbers and dates @@ -445,6 +447,30 @@ PHONENUMBER_DEFAULT_REGION = "US" # logger.error("Can't do this important task. Something is very wrong.") # logger.critical("Going to crash now.") +class JsonFormatter(logging.Formatter): + def __init__(self): + super().__init__(datefmt="%d/%b/%Y %H:%M:%S") + + def format(self, record): + log_record = { + "timestamp": self.formatTime(record, self.datefmt), + "level": record.levelname, + "name": record.name, + "lineno": record.lineno, + "message": record.getMessage(), + } + return json.dumps(log_record) + +class JsonServerFormatter(ServerFormatter): + def format(self, record): + formatted_record = super().format(record) + log_entry = { + "server_time": record.server_time, + "level": record.levelname, + "message": formatted_record + } + return json.dumps(log_entry) + LOGGING = { "version": 1, # Don't import Django's existing loggers @@ -459,10 +485,11 @@ LOGGING = { "simple": { "format": "%(levelname)s %(message)s", }, - "django.server": { - "()": "django.utils.log.ServerFormatter", - "format": "[{server_time}] {message}", - "style": "{", + "json.server": { + "()": JsonServerFormatter, + }, + "json": { + "()": JsonFormatter, }, }, # define where log messages will be sent; @@ -471,12 +498,12 @@ LOGGING = { "console": { "level": env_log_level, "class": "logging.StreamHandler", - "formatter": "verbose", + "formatter": "json", }, "django.server": { "level": "INFO", "class": "logging.StreamHandler", - "formatter": "django.server", + "formatter": "json.server", }, # No file logger is configured, # because containerized apps From e2fde1f96808f0c00899e483905e6da4a7ee4dbe Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 12:59:18 -0500 Subject: [PATCH 06/23] fix imports --- src/registrar/config/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 016dda4d9..0b68a1574 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -24,6 +24,7 @@ from pathlib import Path from typing import Final from botocore.config import Config import json +import logging from django.utils.log import ServerFormatter # # # ### From afd941e27ffa7db39cf179255fb25edd5ec7b6c0 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:26:54 -0500 Subject: [PATCH 07/23] temp: add error message to test logs --- src/registrar/models/domain_request.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index b80e063cd..9cee17b18 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,4 +1,5 @@ from __future__ import annotations +import traceback from typing import Union import logging from django.apps import apps @@ -644,6 +645,8 @@ class DomainRequest(TimeStampedModel): self.sync_organization_type() self.sync_yes_no_form_fields() + logger.error(traceback.print_stack()) + if self._cached_status != self.status: self.last_status_update = timezone.now().date() From a61e8a16707466afee92af9ac35211f17fc162e2 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:42:31 -0500 Subject: [PATCH 08/23] more temp changes --- src/registrar/models/domain_request.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 9cee17b18..f6e215a27 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -644,8 +644,10 @@ class DomainRequest(TimeStampedModel): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() - - logger.error(traceback.print_stack()) + try: + raise ValueError("TEST TEST TEST") + except Exception as e: + logger.error(e) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From d9898e3af2643985e091f3d72904643749e09458 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:52:13 -0500 Subject: [PATCH 09/23] moar testing!!! --- src/registrar/models/domain_request.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f6e215a27..95a20d4cd 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -645,9 +645,9 @@ class DomainRequest(TimeStampedModel): self.sync_organization_type() self.sync_yes_no_form_fields() try: - raise ValueError("TEST TEST TEST") - except Exception as e: - logger.error(e) + raise Exception("TEST TEST TEST") + except Exception: + logger.error(traceback.format_exc) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From bdec43e2f349ac52dbac79ef0abeaf5ebe7ac5d5 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:57:58 -0500 Subject: [PATCH 10/23] forgot parentheses :( --- src/registrar/models/domain_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 95a20d4cd..af8d22b5f 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -647,7 +647,7 @@ class DomainRequest(TimeStampedModel): try: raise Exception("TEST TEST TEST") except Exception: - logger.error(traceback.format_exc) + logger.error(traceback.format_exc()) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From 9d4ab22dc4bed49b0e3b226ff9e69e45ff18c8b7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 5 Sep 2024 08:53:37 -0600 Subject: [PATCH 11/23] fix unit test --- src/registrar/models/user.py | 2 +- src/registrar/tests/test_admin.py | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 48bde5281..0af6c357b 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -244,7 +244,7 @@ class User(AbstractUser): if permission: return permission.portfolio return None - + def get_portfolios(self): return self.portfolio_permissions.all() diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2e9122c38..3e4b8fb45 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -3,6 +3,7 @@ from django.utils import timezone from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices +from django_webtest import WebTest # type: ignore from api.tests.common import less_console_noise_decorator from django.urls import reverse from registrar.admin import ( @@ -972,7 +973,7 @@ class TestListHeaderAdmin(TestCase): ) -class TestMyUserAdmin(MockDbForSharedTests): +class TestMyUserAdmin(MockDbForSharedTests, WebTest): """Tests for the MyUserAdmin class as super or staff user Notes: @@ -992,6 +993,7 @@ class TestMyUserAdmin(MockDbForSharedTests): def setUp(self): super().setUp() + self.app.set_user(self.superuser.username) self.client = Client(HTTP_HOST="localhost:8080") def tearDown(self): @@ -1225,20 +1227,15 @@ class TestMyUserAdmin(MockDbForSharedTests): self.assertNotContains(response, "Portfolio roles:") self.assertNotContains(response, "Portfolio additional permissions:") - + @less_console_noise_decorator def test_user_can_see_related_portfolios(self): """Tests if a user can see the portfolios they are associated with on the user page""" - portfolio, _ = Portfolio.objects.get_or_create(organization_name="test", creator=self.user) + portfolio, _ = Portfolio.objects.get_or_create(organization_name="test", creator=self.superuser) permission, _ = UserPortfolioPermission.objects.get_or_create( - user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - self.user.refresh_from_db() - self.client.force_login(self.user) - response = self.client.get( - "/admin/registrar/user/{}/change/".format(self.user.id), - follow=True, + user=self.superuser, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) + response = self.app.get(reverse("admin:registrar_user_change", args=[self.superuser.pk])) expected_href = reverse("admin:registrar_portfolio_change", args=[portfolio.pk]) self.assertContains(response, expected_href) self.assertContains(response, str(portfolio)) From 3955d5648fe52f0a2939b4cc4930e63bd206b21f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:05:13 -0600 Subject: [PATCH 12/23] Update user.py --- src/registrar/models/user.py | 73 ++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 0af6c357b..4e789ff0c 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -224,14 +224,87 @@ class User(AbstractUser): ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) def has_domain_requests_portfolio_permission(self, portfolio): + # BEGIN + # Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_requests_flag = flag_is_active(request, "organization_requests") + if not has_organization_requests_flag: + return False + # END return self._has_portfolio_permission( portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) + def has_view_members_portfolio_permission(self, portfolio): + # BEGIN + # Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_members_flag = flag_is_active(request, "organization_members") + if not has_organization_members_flag: + return False + # END + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MEMBERS) + + def has_edit_members_portfolio_permission(self, portfolio): + # BEGIN + # Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_members_flag = flag_is_active(request, "organization_members") + if not has_organization_members_flag: + return False + # END + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_MEMBERS) + def has_view_all_domains_permission(self, portfolio): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + def has_edit_requests(self, portfolio): + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) + + def portfolio_role_summary(self, portfolio): + """Returns a list of roles based on the user's permissions.""" + roles = [] + + # Define the conditions and their corresponding roles + conditions_roles = [ + (self.has_edit_suborganization(portfolio), ["Admin"]), + ( + self.has_view_all_domains_permission(portfolio) + and self.has_domain_requests_portfolio_permission(portfolio) + and self.has_edit_requests(portfolio), + ["View-only admin", "Domain requestor"], + ), + ( + self.has_view_all_domains_permission(portfolio) + and self.has_domain_requests_portfolio_permission(portfolio), + ["View-only admin"], + ), + ( + self.has_base_portfolio_permission(portfolio) + and self.has_edit_requests(portfolio) + and self.has_domains_portfolio_permission(portfolio), + ["Domain requestor", "Domain manager"], + ), + (self.has_base_portfolio_permission(portfolio) and self.has_edit_requests(portfolio), ["Domain requestor"]), + ( + self.has_base_portfolio_permission(portfolio) and self.has_domains_portfolio_permission(portfolio), + ["Domain manager"], + ), + (self.has_base_portfolio_permission(portfolio), ["Member"]), + ] + + # Evaluate conditions and add roles + for condition, role_list in conditions_roles: + if condition: + roles.extend(role_list) + break + + return roles + # Field specific permission checks def has_view_suborganization(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) From c5c3043eaaaebe0129faae4cab6a559636ca6657 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:05:37 -0600 Subject: [PATCH 13/23] Update user.py --- src/registrar/models/user.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 4e789ff0c..45ffbadb7 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -6,7 +6,7 @@ from django.db.models import Q from django.http import HttpRequest from registrar.models import DomainInformation, UserDomainRole -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices from .domain_invitation import DomainInvitation from .portfolio_invitation import PortfolioInvitation @@ -64,32 +64,6 @@ class User(AbstractUser): # after they login. FIXTURE_USER = "fixture_user", "Created by fixtures" - PORTFOLIO_ROLE_PERMISSIONS = { - UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.EDIT_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.EDIT_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - ], - } - # #### Constants for choice fields #### RESTRICTED = "restricted" STATUS_CHOICES = ((RESTRICTED, RESTRICTED),) From 9e7fa80bd0c786b9c69ead25bd84e6115bff9999 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:07:01 -0600 Subject: [PATCH 14/23] Update user.py --- src/registrar/models/user.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 45ffbadb7..901ab62af 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -236,6 +236,19 @@ class User(AbstractUser): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + # Field specific permission checks + def has_view_suborganization(self, portfolio): + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) + + def has_edit_suborganization(self, portfolio): + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) + + def get_first_portfolio(self): + permission = self.portfolio_permissions.first() + if permission: + return permission.portfolio + return None + def has_edit_requests(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) @@ -279,19 +292,6 @@ class User(AbstractUser): return roles - # Field specific permission checks - def has_view_suborganization(self, portfolio): - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) - - def has_edit_suborganization(self, portfolio): - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) - - def get_first_portfolio(self): - permission = self.portfolio_permissions.first() - if permission: - return permission.portfolio - return None - def get_portfolios(self): return self.portfolio_permissions.all() From 99cfa0ee971b10daf618e8fc293f68d8b03aeffc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:38:56 -0600 Subject: [PATCH 15/23] linting --- src/registrar/tests/test_admin.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 5bdf3560f..83114b3b3 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2,7 +2,6 @@ from datetime import datetime from django.utils import timezone from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from django_webtest import WebTest # type: ignore from api.tests.common import less_console_noise_decorator from django.urls import reverse @@ -44,13 +43,11 @@ from registrar.models import ( Portfolio, Suborganization, UserPortfolioPermission, + UserDomainRole, + SeniorOfficial, + PortfolioInvitation, + VerifiedByStaff, ) -from registrar.models.portfolio_invitation import PortfolioInvitation -from registrar.models.senior_official import SeniorOfficial -from registrar.models.user_domain_role import UserDomainRole -from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( MockDbForSharedTests, AuditedAdminMockData, @@ -63,10 +60,11 @@ from .common import ( multiple_unalphabetical_domain_objects, GenericTestHelper, ) +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model from unittest.mock import ANY, patch, Mock -from django_webtest import WebTest # type: ignore + import logging From eb3cb1e3d821d7e181c7b32e8784c3715101a633 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:34:32 -0500 Subject: [PATCH 16/23] Don't use json locally, plus review changes --- src/registrar/config/settings.py | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 0b68a1574..886f2df50 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -448,21 +448,8 @@ PHONENUMBER_DEFAULT_REGION = "US" # logger.error("Can't do this important task. Something is very wrong.") # logger.critical("Going to crash now.") -class JsonFormatter(logging.Formatter): - def __init__(self): - super().__init__(datefmt="%d/%b/%Y %H:%M:%S") - - def format(self, record): - log_record = { - "timestamp": self.formatTime(record, self.datefmt), - "level": record.levelname, - "name": record.name, - "lineno": record.lineno, - "message": record.getMessage(), - } - return json.dumps(log_record) - class JsonServerFormatter(ServerFormatter): + """Formats logs into JSON for easier and more accurate processing.""" def format(self, record): formatted_record = super().format(record) log_entry = { @@ -489,9 +476,6 @@ LOGGING = { "json.server": { "()": JsonServerFormatter, }, - "json": { - "()": JsonFormatter, - }, }, # define where log messages will be sent; # each logger can have one or more handlers @@ -499,7 +483,7 @@ LOGGING = { "console": { "level": env_log_level, "class": "logging.StreamHandler", - "formatter": "json", + "formatter": "verbose", }, "django.server": { "level": "INFO", From ed4eeb469f4f7f8c3d8e9b324cb10ab147a9b673 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:42:26 -0500 Subject: [PATCH 17/23] linter fixes --- src/registrar/config/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 886f2df50..75881ab3f 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -24,7 +24,6 @@ from pathlib import Path from typing import Final from botocore.config import Config import json -import logging from django.utils.log import ServerFormatter # # # ### @@ -448,6 +447,7 @@ PHONENUMBER_DEFAULT_REGION = "US" # logger.error("Can't do this important task. Something is very wrong.") # logger.critical("Going to crash now.") + class JsonServerFormatter(ServerFormatter): """Formats logs into JSON for easier and more accurate processing.""" def format(self, record): @@ -459,6 +459,7 @@ class JsonServerFormatter(ServerFormatter): } return json.dumps(log_entry) + LOGGING = { "version": 1, # Don't import Django's existing loggers From 0488b452735bf2de99d9581e9fc1dbceccff17cf Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:49:27 -0500 Subject: [PATCH 18/23] remove test log --- src/registrar/models/domain_request.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index af8d22b5f..b4a988165 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -644,10 +644,6 @@ class DomainRequest(TimeStampedModel): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() - try: - raise Exception("TEST TEST TEST") - except Exception: - logger.error(traceback.format_exc()) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From 635d8480f17a4380abfb2bd3ebc125c90bbe0565 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:50:10 -0500 Subject: [PATCH 19/23] remove unused import --- src/registrar/models/domain_request.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index b4a988165..b80e063cd 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,5 +1,4 @@ from __future__ import annotations -import traceback from typing import Union import logging from django.apps import apps From 180a0240e52d75e359854b2da5d7d3d64dccdd49 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 13:09:48 -0500 Subject: [PATCH 20/23] conditionally set log format --- src/registrar/config/settings.py | 39 ++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 75881ab3f..40d4f6933 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -24,6 +24,7 @@ from pathlib import Path from typing import Final from botocore.config import Config import json +import logging from django.utils.log import ServerFormatter # # # ### @@ -59,7 +60,7 @@ env_db_url = env.dj_db_url("DATABASE_URL") env_debug = env.bool("DJANGO_DEBUG", default=False) env_is_production = env.bool("IS_PRODUCTION", default=False) env_log_level = env.str("DJANGO_LOG_LEVEL", "DEBUG") -env_base_url = env.str("DJANGO_BASE_URL") +env_base_url: str = env.str("DJANGO_BASE_URL") env_getgov_public_site_url = env.str("GETGOV_PUBLIC_SITE_URL", "") env_oidc_active_provider = env.str("OIDC_ACTIVE_PROVIDER", "identity sandbox") @@ -448,8 +449,24 @@ PHONENUMBER_DEFAULT_REGION = "US" # logger.critical("Going to crash now.") +class JsonFormatter(logging.Formatter): + """Formats logs into JSON for better parsing""" + def __init__(self): + super().__init__(datefmt="%d/%b/%Y %H:%M:%S") + + def format(self, record): + log_record = { + "timestamp": self.formatTime(record, self.datefmt), + "level": record.levelname, + "name": record.name, + "lineno": record.lineno, + "message": record.getMessage(), + } + return json.dumps(log_record) + + class JsonServerFormatter(ServerFormatter): - """Formats logs into JSON for easier and more accurate processing.""" + """Formats server logs into JSON for better parsing""" def format(self, record): formatted_record = super().format(record) log_entry = { @@ -459,6 +476,12 @@ class JsonServerFormatter(ServerFormatter): } return json.dumps(log_entry) +# default to json formatted logs +server_formatter, console_formatter = 'json.server', 'json' + +# don't use json format locally, it makes logs hard to read in console +if "localhost" in env_base_url: + server_formatter, console_formatter = 'django.server', 'verbose' LOGGING = { "version": 1, @@ -474,9 +497,17 @@ LOGGING = { "simple": { "format": "%(levelname)s %(message)s", }, + "django.server": { + "()": "django.utils.log.ServerFormatter", + "format": "[{server_time}] {message}", + "style": "{", + }, "json.server": { "()": JsonServerFormatter, }, + "json": { + "()": JsonFormatter, + }, }, # define where log messages will be sent; # each logger can have one or more handlers @@ -484,12 +515,12 @@ LOGGING = { "console": { "level": env_log_level, "class": "logging.StreamHandler", - "formatter": "verbose", + "formatter": console_formatter, }, "django.server": { "level": "INFO", "class": "logging.StreamHandler", - "formatter": "json.server", + "formatter": server_formatter, }, # No file logger is configured, # because containerized apps From d33f9ae8e2c7a966a7d61230387136eb85352f37 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 13:34:19 -0500 Subject: [PATCH 21/23] linter errors --- src/registrar/config/settings.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 40d4f6933..b2a65fe23 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -451,6 +451,7 @@ PHONENUMBER_DEFAULT_REGION = "US" class JsonFormatter(logging.Formatter): """Formats logs into JSON for better parsing""" + def __init__(self): super().__init__(datefmt="%d/%b/%Y %H:%M:%S") @@ -467,21 +468,19 @@ class JsonFormatter(logging.Formatter): class JsonServerFormatter(ServerFormatter): """Formats server logs into JSON for better parsing""" + def format(self, record): formatted_record = super().format(record) - log_entry = { - "server_time": record.server_time, - "level": record.levelname, - "message": formatted_record - } + log_entry = {"server_time": record.server_time, "level": record.levelname, "message": formatted_record} return json.dumps(log_entry) + # default to json formatted logs -server_formatter, console_formatter = 'json.server', 'json' +server_formatter, console_formatter = "json.server", "json" # don't use json format locally, it makes logs hard to read in console if "localhost" in env_base_url: - server_formatter, console_formatter = 'django.server', 'verbose' + server_formatter, console_formatter = "django.server", "verbose" LOGGING = { "version": 1, From 519ca82ee304cd4486f38a1982c645cb45001bb7 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 10:21:30 -0500 Subject: [PATCH 22/23] Remove type checking that was causing linter issues --- src/epplibwrapper/cert.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/epplibwrapper/cert.py b/src/epplibwrapper/cert.py index 15ff16c06..9abcbee35 100644 --- a/src/epplibwrapper/cert.py +++ b/src/epplibwrapper/cert.py @@ -1,7 +1,7 @@ import os import tempfile -from django.conf import settings +from django.conf import settings # type: ignore class Cert: @@ -12,7 +12,7 @@ class Cert: variable but Python's ssl library requires a file. """ - def __init__(self, data=settings.SECRET_REGISTRY_CERT) -> None: + def __init__(self, data = settings.SECRET_REGISTRY_CERT) -> None: # type: ignore self.filename = self._write(data) def __del__(self): @@ -31,4 +31,4 @@ class Key(Cert): """Location of private key as written to disk.""" def __init__(self) -> None: - super().__init__(data=settings.SECRET_REGISTRY_KEY) + super().__init__(data=settings.SECRET_REGISTRY_KEY) # type: ignore From 529178cd84b272182587620a5b6d8047c4beeeb1 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 12:55:12 -0500 Subject: [PATCH 23/23] linter error fixes --- src/epplibwrapper/cert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/cert.py b/src/epplibwrapper/cert.py index 9abcbee35..589736a04 100644 --- a/src/epplibwrapper/cert.py +++ b/src/epplibwrapper/cert.py @@ -12,7 +12,7 @@ class Cert: variable but Python's ssl library requires a file. """ - def __init__(self, data = settings.SECRET_REGISTRY_CERT) -> None: # type: ignore + def __init__(self, data=settings.SECRET_REGISTRY_CERT) -> None: # type: ignore self.filename = self._write(data) def __del__(self):