From 64767a4af0702145b740ec52b3f11340a5a4c760 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 22 Jun 2023 18:49:45 -0400 Subject: [PATCH 01/16] Customize contacts list, domain applications list, domain application detail view with search, sorts, filters, fieldsets, search helper text. Document django admin CISA analyst access group. --- docs/django-admin/roles.md | 20 ++++++++++++++++++++ src/registrar/admin.py | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 docs/django-admin/roles.md diff --git a/docs/django-admin/roles.md b/docs/django-admin/roles.md new file mode 100644 index 000000000..124eb0351 --- /dev/null +++ b/docs/django-admin/roles.md @@ -0,0 +1,20 @@ +# Django admin user roles + +Roles other than superuser should be defined in authentication and authorization groups in django admin + +## Superuser + +Full access + +## CISA analyst + +### Basic permission level + +Staff + +### Additional group permissions + +admin | log entry | can view log entry +registrar | contact | can view contact +registrar | domain application | can view domain application +registrar | domain application | can change domain application \ No newline at end of file diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7eb1668d8..ca110db6a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -78,11 +78,42 @@ class DomainAdmin(AuditedAdmin): return HttpResponseRedirect(".") return super().response_change(request, obj) + + +class ContactAdmin(AuditedAdmin): + + """Custom contact admin class to add search.""" + + search_fields = ["email", "first_name", "last_name"] + + search_help_text = "Search by firstname, lastname or email." class DomainApplicationAdmin(AuditedAdmin): """Customize the applications listing view.""" + + list_display = ["requested_domain", "status", "organization_type", "created_at", "submitter", "investigator"] + + list_filter = ('status', "organization_type", "investigator") + + search_fields = ["requested_domain__name", "submitter__email", "submitter__first_name", "submitter__last_name"] + + search_help_text = "Search by domain or submitter." + + fieldsets = [ + (None, {"fields": ["status", "investigator", "creator", "is_policy_acknowledged"]}), + ("Type of organization", {"fields": ["organization_type", "federally_recognized_tribe", "state_recognized_tribe", "tribe_name", "federal_agency", "federal_type", "is_election_board", "type_of_work", "more_organization_information"]}), + ("Organization name and mailing address", {"fields": ["organization_name", "address_line1", "address_line2", "city", "state_territory", "zipcode", "urbanization"]}), + ("Authorizing official", {"fields": ["authorizing_official"]}), + ("Current websites", {"fields": ["current_websites"]}), + (".gov domain", {"fields": ["requested_domain", "alternative_domains"]}), + ("Purpose of your domain", {"fields": ["purpose"]}), + ("Your contact information", {"fields": ["submitter"]}), + ("Other employees from your organization?", {"fields": ["other_contacts"]}), + ("No other employees from your organization?", {"fields": ["no_other_contacts_rationale"]}), + ("Anything else we should know?", {"fields": ["anything_else"]}), + ] # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): @@ -106,7 +137,7 @@ class DomainApplicationAdmin(AuditedAdmin): admin.site.register(models.User, MyUserAdmin) admin.site.register(models.UserDomainRole, AuditedAdmin) -admin.site.register(models.Contact, AuditedAdmin) +admin.site.register(models.Contact, ContactAdmin) admin.site.register(models.DomainInvitation, AuditedAdmin) admin.site.register(models.DomainInformation, AuditedAdmin) admin.site.register(models.Domain, DomainAdmin) From 6d4678596216e2f7b1a79f7ca813554a9a33130d Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 26 Jun 2023 18:50:24 -0400 Subject: [PATCH 02/16] tweak cisa analyst roles documentation, move policy acknoledged to its own fieldset at the end on detail page --- docs/django-admin/roles.md | 1 - src/registrar/admin.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/django-admin/roles.md b/docs/django-admin/roles.md index 124eb0351..e6cdb9658 100644 --- a/docs/django-admin/roles.md +++ b/docs/django-admin/roles.md @@ -16,5 +16,4 @@ Staff admin | log entry | can view log entry registrar | contact | can view contact -registrar | domain application | can view domain application registrar | domain application | can change domain application \ No newline at end of file diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ca110db6a..e5c4daba7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -102,7 +102,7 @@ class DomainApplicationAdmin(AuditedAdmin): search_help_text = "Search by domain or submitter." fieldsets = [ - (None, {"fields": ["status", "investigator", "creator", "is_policy_acknowledged"]}), + (None, {"fields": ["status", "investigator", "creator"]}), ("Type of organization", {"fields": ["organization_type", "federally_recognized_tribe", "state_recognized_tribe", "tribe_name", "federal_agency", "federal_type", "is_election_board", "type_of_work", "more_organization_information"]}), ("Organization name and mailing address", {"fields": ["organization_name", "address_line1", "address_line2", "city", "state_territory", "zipcode", "urbanization"]}), ("Authorizing official", {"fields": ["authorizing_official"]}), @@ -113,6 +113,7 @@ class DomainApplicationAdmin(AuditedAdmin): ("Other employees from your organization?", {"fields": ["other_contacts"]}), ("No other employees from your organization?", {"fields": ["no_other_contacts_rationale"]}), ("Anything else we should know?", {"fields": ["anything_else"]}), + ("Requirements for operating .gov domains", {"fields": ["is_policy_acknowledged"]}), ] # Trigger action when a fieldset is changed From 963feef9b4c284a4bc0e361833930971cca19dd2 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 26 Jun 2023 19:22:32 -0400 Subject: [PATCH 03/16] make some fields read only for non superusers --- src/registrar/admin.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e5c4daba7..bc62400b8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -134,6 +134,16 @@ class DomainApplicationAdmin(AuditedAdmin): original_obj.in_review(obj) super().save_model(request, obj, form, change) + + readonly_fields = ["status", "creator", "submitter", "is_policy_acknowledged"] + + def get_readonly_fields(self, request, obj=None): + if request.user.is_superuser: + # Superusers have full access, no fields are read-only + return () + else: + # Regular users can only view the specified fields + return self.readonly_fields admin.site.register(models.User, MyUserAdmin) From 100af4fd1ca0d47ccc6daef0ff4159bcc4e3687e Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 26 Jun 2023 19:32:27 -0400 Subject: [PATCH 04/16] Add the right fields to the readonly list --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index bc62400b8..82a2c0945 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -135,7 +135,7 @@ class DomainApplicationAdmin(AuditedAdmin): super().save_model(request, obj, form, change) - readonly_fields = ["status", "creator", "submitter", "is_policy_acknowledged"] + readonly_fields = ["creator", "type_of_work", "more_organization_information", "address_line1", "address_line2", "zipcode", "requested_domain", "alternative_domains", "purpose", "submitter", "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged"] def get_readonly_fields(self, request, obj=None): if request.user.is_superuser: From d506e5a8e8bbf80dcb6390622febb4712c112ac4 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Tue, 27 Jun 2023 15:47:53 -0400 Subject: [PATCH 05/16] Add fixtures for Analysts, add domains to analyts dashboard )with search) --- src/registrar/admin.py | 10 +++------- src/registrar/fixtures.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 82a2c0945..aace052c9 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -56,6 +56,8 @@ class DomainAdmin(AuditedAdmin): """Custom domain admin class to add extra buttons.""" + search_fields = ["name"] + search_help_text = "Search by domain name." change_form_template = "django/admin/domain_change_form.html" readonly_fields = ["state"] @@ -85,7 +87,6 @@ class ContactAdmin(AuditedAdmin): """Custom contact admin class to add search.""" search_fields = ["email", "first_name", "last_name"] - search_help_text = "Search by firstname, lastname or email." @@ -94,13 +95,9 @@ class DomainApplicationAdmin(AuditedAdmin): """Customize the applications listing view.""" list_display = ["requested_domain", "status", "organization_type", "created_at", "submitter", "investigator"] - list_filter = ('status', "organization_type", "investigator") - search_fields = ["requested_domain__name", "submitter__email", "submitter__first_name", "submitter__last_name"] - search_help_text = "Search by domain or submitter." - fieldsets = [ (None, {"fields": ["status", "investigator", "creator"]}), ("Type of organization", {"fields": ["organization_type", "federally_recognized_tribe", "state_recognized_tribe", "tribe_name", "federal_agency", "federal_type", "is_election_board", "type_of_work", "more_organization_information"]}), @@ -115,6 +112,7 @@ class DomainApplicationAdmin(AuditedAdmin): ("Anything else we should know?", {"fields": ["anything_else"]}), ("Requirements for operating .gov domains", {"fields": ["is_policy_acknowledged"]}), ] + readonly_fields = ["creator", "type_of_work", "more_organization_information", "address_line1", "address_line2", "zipcode", "requested_domain", "alternative_domains", "purpose", "submitter", "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged"] # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): @@ -134,8 +132,6 @@ class DomainApplicationAdmin(AuditedAdmin): original_obj.in_review(obj) super().save_model(request, obj, form, change) - - readonly_fields = ["creator", "type_of_work", "more_organization_information", "address_line1", "address_line2", "zipcode", "requested_domain", "alternative_domains", "purpose", "submitter", "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged"] def get_readonly_fields(self, request, obj=None): if request.user.is_superuser: diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 3aefbe68b..a18b9d814 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -10,6 +10,8 @@ from registrar.models import ( Website, ) +from django.contrib.auth.models import Permission + fake = Faker() logger = logging.getLogger(__name__) @@ -50,6 +52,14 @@ class UserFixture: "last_name": "Dixon", }, ] + + STAFF = [ + { + "username": "319c490d-453b-43d9-bc4d-7d6cd8ff6844", + "first_name": "Rachid-Analyst", + "last_name": "Mrad-Analyst", + }, + ] @classmethod def load(cls): @@ -68,6 +78,30 @@ class UserFixture: logger.debug("User object created for %s" % admin["first_name"]) except Exception as e: logger.warning(e) + for staff in cls.STAFF: + try: + user, _ = User.objects.get_or_create( + username=staff["username"], + ) + user.is_superuser = False + user.first_name = admin["first_name"] + user.last_name = admin["last_name"] + user.is_staff = True + user.is_active = True + # CISA ANALYST permissions + # id 24 = codename view_logentry (auditlog) + # id 32 = codename view_contact + # id 38 = codename change_domainapplication + # id 44 = codename view_domain + permission_ids = [24, 32, 38, 44] # List of permission IDs to assign + # Retrieve the corresponding permission objects + permissions = Permission.objects.filter(id__in=permission_ids) + # Add the permissions to the user + user.user_permissions.add(*permissions) + user.save() + logger.debug("User object created for %s" % admin["first_name"]) + except Exception as e: + logger.warning(e) logger.debug("All users loaded.") From eac616204b1fe506da945f18b2dcba32b1277c5b Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Tue, 27 Jun 2023 17:08:23 -0400 Subject: [PATCH 06/16] Add descriptive subheader for listing pages in django admin --- src/registrar/admin.py | 20 +++++++++++++++++ src/registrar/config/settings.py | 8 +++++-- .../templates/admin/change_list.html | 22 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 src/registrar/templates/admin/change_list.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index aace052c9..cc0048557 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -140,6 +140,26 @@ class DomainApplicationAdmin(AuditedAdmin): else: # Regular users can only view the specified fields return self.readonly_fields + + def changelist_view(self, request, extra_context=None): + if extra_context is None: + extra_context = {} + # Get the filtered values + filters = self.get_filters(request) + # Pass the filtered values to the template context + extra_context['filters'] = filters + extra_context['search_query'] = request.GET.get('q', '') # Assuming the search query parameter is 'q' + return super().changelist_view(request, extra_context=extra_context) + + def get_filters(self, request): + filters = [] + # Retrieve the filter parameters + for param in request.GET.keys(): + # Exclude the default search parameter 'q' + if param != 'q' and param != 'o': + # Append the filter parameter and its value to the list + filters.append({'parameter_name': param.replace('__exact','').replace('_type','').replace('__id',' id'), 'parameter_value': request.GET.get(param)}) + return filters admin.site.register(models.User, MyUserAdmin) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 4710b0c65..47e94b80e 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -78,6 +78,12 @@ DEBUG = env_debug # Installing them here makes them available for execution. # Do not access INSTALLED_APPS directly. Use `django.apps.apps` instead. INSTALLED_APPS = [ + # let's be sure to install our own application! + # it needs to be listed before django.contrib.admin + # otherwise Django would find the default template + # provided by django.contrib.admin first and use + # that instead of our custom templates. + "registrar", # Django automatic admin interface reads metadata # from database models to provide a quick, model-centric # interface where trusted users can manage content @@ -106,8 +112,6 @@ INSTALLED_APPS = [ "django_fsm", # library for phone numbers "phonenumber_field", - # let's be sure to install our own application! - "registrar", # Our internal API application "api", # Only for generating documentation, uncomment to run manage.py generate_puml diff --git a/src/registrar/templates/admin/change_list.html b/src/registrar/templates/admin/change_list.html new file mode 100644 index 000000000..50d23faf9 --- /dev/null +++ b/src/registrar/templates/admin/change_list.html @@ -0,0 +1,22 @@ +{% extends "admin/change_list.html" %} + +{% block content_title %} +

{{ title }}

+

+ {{ cl.result_count }} + {% if cl.get_ordering_field_columns %} + sorted + {% endif %} + results + {% if filters %} + filtered by + {% for filter_param in filters %} + {{ filter_param.parameter_name }} = {{ filter_param.parameter_value }} + {% if not forloop.last %}, {% endif %} + {% endfor %} + {% endif %} + {% if search_query %} + for {{ search_query }} + {% endif %} +

+{% endblock %} \ No newline at end of file From de36ca9cf7df5a7e97ebf58906d365d55510a323 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Wed, 28 Jun 2023 16:23:55 -0400 Subject: [PATCH 07/16] Add permissions to staff in fixtures by codename not id, make class with for the list header context updater and inherit it from domain, domain application, and contacts (the classes that implement search, filter and sort) --- docs/django-admin/roles.md | 5 +- src/registrar/admin.py | 51 ++++++++------- src/registrar/config/settings.py | 4 +- src/registrar/fixtures.py | 64 ++++++++++++++----- .../templates/admin/change_list.html | 2 +- 5 files changed, 83 insertions(+), 43 deletions(-) diff --git a/docs/django-admin/roles.md b/docs/django-admin/roles.md index e6cdb9658..431380300 100644 --- a/docs/django-admin/roles.md +++ b/docs/django-admin/roles.md @@ -14,6 +14,7 @@ Staff ### Additional group permissions -admin | log entry | can view log entry +auditlog | log entry | can view log entry registrar | contact | can view contact -registrar | domain application | can change domain application \ No newline at end of file +registrar | domain application | can change domain application +registrar | domain | can view domain \ No newline at end of file diff --git a/src/registrar/admin.py b/src/registrar/admin.py index cc0048557..0b012f7d8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,7 +22,32 @@ class AuditedAdmin(admin.ModelAdmin): object_id=object_id, ) ) + +class ListHeaderAdmin(AuditedAdmin): + + """Custom admin to add a descriptive subheader to list views.""" + + def changelist_view(self, request, extra_context=None): + if extra_context is None: + extra_context = {} + # Get the filtered values + filters = self.get_filters(request) + # Pass the filtered values to the template context + extra_context['filters'] = filters + extra_context['search_query'] = request.GET.get('q', '') # Assuming the search query parameter is 'q' + return super().changelist_view(request, extra_context=extra_context) + + def get_filters(self, request): + filters = [] + # Retrieve the filter parameters + for param in request.GET.keys(): + # Exclude the default search parameter 'q' + if param != 'q' and param != 'o': + # Append the filter parameter and its value to the list + filters.append({'parameter_name': param.replace('__exact','').replace('_type','').replace('__id',' id'), 'parameter_value': request.GET.get(param)}) + return filters + class UserContactInline(admin.StackedInline): @@ -52,7 +77,7 @@ class MyHostAdmin(AuditedAdmin): inlines = [HostIPInline] -class DomainAdmin(AuditedAdmin): +class DomainAdmin(ListHeaderAdmin): """Custom domain admin class to add extra buttons.""" @@ -82,7 +107,7 @@ class DomainAdmin(AuditedAdmin): return super().response_change(request, obj) -class ContactAdmin(AuditedAdmin): +class ContactAdmin(ListHeaderAdmin): """Custom contact admin class to add search.""" @@ -90,7 +115,7 @@ class ContactAdmin(AuditedAdmin): search_help_text = "Search by firstname, lastname or email." -class DomainApplicationAdmin(AuditedAdmin): +class DomainApplicationAdmin(ListHeaderAdmin): """Customize the applications listing view.""" @@ -140,26 +165,6 @@ class DomainApplicationAdmin(AuditedAdmin): else: # Regular users can only view the specified fields return self.readonly_fields - - def changelist_view(self, request, extra_context=None): - if extra_context is None: - extra_context = {} - # Get the filtered values - filters = self.get_filters(request) - # Pass the filtered values to the template context - extra_context['filters'] = filters - extra_context['search_query'] = request.GET.get('q', '') # Assuming the search query parameter is 'q' - return super().changelist_view(request, extra_context=extra_context) - - def get_filters(self, request): - filters = [] - # Retrieve the filter parameters - for param in request.GET.keys(): - # Exclude the default search parameter 'q' - if param != 'q' and param != 'o': - # Append the filter parameter and its value to the list - filters.append({'parameter_name': param.replace('__exact','').replace('_type','').replace('__id',' id'), 'parameter_value': request.GET.get(param)}) - return filters admin.site.register(models.User, MyUserAdmin) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 47e94b80e..1af5a2295 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -92,6 +92,8 @@ INSTALLED_APPS = [ # the "user" model! *\o/* "django.contrib.auth", # generic interface for Django models + "auditlog", + # library to simplify form templating "django.contrib.contenttypes", # required for CSRF protection and many other things "django.contrib.sessions", @@ -105,8 +107,6 @@ INSTALLED_APPS = [ # application used for integrating with Login.gov "djangooidc", # audit logging of changes to models - "auditlog", - # library to simplify form templating "widget_tweaks", # library for Finite State Machine statuses "django_fsm", diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index a18b9d814..0eb2cfb47 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -11,6 +11,7 @@ from registrar.models import ( ) from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType fake = Faker() logger = logging.getLogger(__name__) @@ -60,10 +61,33 @@ class UserFixture: "last_name": "Mrad-Analyst", }, ] + + STAFF_PERMISSIONS = [ + { + 'app_label': 'auditlog', + 'model': 'logentry', + 'permissions': ['view_logentry'] + }, + { + 'app_label': 'registrar', + 'model': 'contact', + 'permissions': ['view_contact'] + }, + { + 'app_label': 'registrar', + 'model': 'domainapplication', + 'permissions': ['change_domainapplication'] + }, + { + 'app_label': 'registrar', + 'model': 'domain', + 'permissions': ['view_domain'] + }, + ] @classmethod def load(cls): - logger.info("Going to load %s users" % str(len(cls.ADMINS))) + logger.info("Going to load %s superusers" % str(len(cls.ADMINS))) for admin in cls.ADMINS: try: user, _ = User.objects.get_or_create( @@ -78,31 +102,41 @@ class UserFixture: logger.debug("User object created for %s" % admin["first_name"]) except Exception as e: logger.warning(e) + logger.debug("All superusers loaded.") + + logger.info("Going to load %s CISA analysts (staff)" % str(len(cls.STAFF))) for staff in cls.STAFF: try: user, _ = User.objects.get_or_create( username=staff["username"], ) user.is_superuser = False - user.first_name = admin["first_name"] - user.last_name = admin["last_name"] + user.first_name = staff["first_name"] + user.last_name = staff["last_name"] user.is_staff = True user.is_active = True - # CISA ANALYST permissions - # id 24 = codename view_logentry (auditlog) - # id 32 = codename view_contact - # id 38 = codename change_domainapplication - # id 44 = codename view_domain - permission_ids = [24, 32, 38, 44] # List of permission IDs to assign - # Retrieve the corresponding permission objects - permissions = Permission.objects.filter(id__in=permission_ids) - # Add the permissions to the user - user.user_permissions.add(*permissions) + + for permission in cls.STAFF_PERMISSIONS: + app_label = permission['app_label'] + model_name = permission['model'] + permissions = permission['permissions'] + + # Retrieve the content type for the app and model + content_type = ContentType.objects.get(app_label=app_label, model=model_name) + + # Retrieve the permissions based on their codenames + permissions = Permission.objects.filter(content_type=content_type, codename__in=permissions) + + # Assign the permissions to the user + user.user_permissions.add(*permissions) + logger.debug(f"{app_label} | {model_name} | {permissions} added for user {staff['first_name']}") + user.save() - logger.debug("User object created for %s" % admin["first_name"]) + logger.debug("User object created for %s" % staff["first_name"]) except Exception as e: logger.warning(e) - logger.debug("All users loaded.") + logger.debug("All CISA analysts (staff) loaded.") + class DomainApplicationFixture: diff --git a/src/registrar/templates/admin/change_list.html b/src/registrar/templates/admin/change_list.html index 50d23faf9..0f2114147 100644 --- a/src/registrar/templates/admin/change_list.html +++ b/src/registrar/templates/admin/change_list.html @@ -19,4 +19,4 @@ for {{ search_query }} {% endif %} -{% endblock %} \ No newline at end of file +{% endblock %} From 15688b8fdb66d29c4fb39876b6b801309abb5059 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 29 Jun 2023 18:21:41 -0400 Subject: [PATCH 08/16] wip --- src/registrar/admin.py | 1 + src/registrar/config/settings.py | 2 ++ src/registrar/tests/test_admin.py | 48 +++++++++++++++++++++++++++++-- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0b012f7d8..8a10ba321 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -36,6 +36,7 @@ class ListHeaderAdmin(AuditedAdmin): # Pass the filtered values to the template context extra_context['filters'] = filters extra_context['search_query'] = request.GET.get('q', '') # Assuming the search query parameter is 'q' + logger.debug(f'changelist_view {extra_context}') return super().changelist_view(request, extra_context=extra_context) def get_filters(self, request): diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 1af5a2295..87d565abf 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -94,6 +94,8 @@ INSTALLED_APPS = [ # generic interface for Django models "auditlog", # library to simplify form templating + # it needs to be listed before django.contrib.contenttypes + # for a ContentType query in fixtures.py "django.contrib.contenttypes", # required for CSRF protection and many other things "django.contrib.sessions", diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2f2e1190b..a72ca9a37 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,6 +1,6 @@ -from django.test import TestCase, RequestFactory +from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.admin import DomainApplicationAdmin +from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, AuditedAdmin from registrar.models import DomainApplication, User from .common import completed_application @@ -13,6 +13,8 @@ class TestDomainApplicationAdmin(TestCase): def setUp(self): self.site = AdminSite() self.factory = RequestFactory() + self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) + self.client = Client(HTTP_HOST='localhost:8080') @boto3_mocking.patching def test_save_model_sends_email_on_property_change(self): @@ -62,3 +64,45 @@ class TestDomainApplicationAdmin(TestCase): # Cleanup application.delete() + + def test_changelist_view(self): + # Make the request using the Client class + # which handles CSRF + # Follow=True handles the redirect + request = self.client.get('/admin/registrar/domainapplication/', {'param1': 'value1', 'param2': 'value2'}, follow=True, max_redirects=10) + + print(f'request {request}') + + # request = self.factory.get('/admin/registrar/domainapplication/') + # # Set the GET parameters for testing + # request.GET = {'param1': 'value1', 'param2': 'value2', 'q': 'search_value'} + # # Call the changelist_view method + response = self.admin.changelist_view(request, extra_context={'filters': [{'parameter_name': 'status', 'parameter_value': 'started'}], 'search_query': ''}) + + + print(f'response {response}') + + # Assert that the final response is a successful response (not a redirect) + # self.assertEqual(response.status_code, 200) + + # Assert that the filters and search_query are added to the extra_context + self.assertIn('filters', response.extra_context) + self.assertIn('search_query', response.extra_context) + # Assert the content of filters and search_query + filters = response.extra_context['filters'] + search_query = response.extra_context['search_query'] + self.assertEqual(filters, [{'parameter_name': 'param1', 'parameter_value': 'value1'}, + {'parameter_name': 'param2', 'parameter_value': 'value2'}]) + self.assertEqual(search_query, 'value of q parameter if present in the request GET') + + def test_get_filters(self): + # Create a mock request object + request = self.factory.get('/admin/yourmodel/') + # Set the GET parameters for testing + request.GET = {'param1': 'value1', 'param2': 'value2', 'q': 'search_value'} + # Call the get_filters method + filters = self.admin.get_filters(request) + + # Assert the filters extracted from the request GET + self.assertEqual(filters, [{'parameter_name': 'param1', 'parameter_value': 'value1'}, + {'parameter_name': 'param2', 'parameter_value': 'value2'}]) From 31a18bbd50c7077cfb3daf0e5daff638113b4e13 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 30 Jun 2023 12:08:31 -0400 Subject: [PATCH 09/16] cleanup and linting --- src/registrar/admin.py | 106 ++++++++++++++++++++++++------ src/registrar/config/settings.py | 10 +-- src/registrar/fixtures.py | 59 +++++++++-------- src/registrar/tests/test_admin.py | 87 +++++++++++++++--------- 4 files changed, 178 insertions(+), 84 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8a10ba321..8e600d59b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,7 +22,7 @@ class AuditedAdmin(admin.ModelAdmin): object_id=object_id, ) ) - + class ListHeaderAdmin(AuditedAdmin): @@ -34,9 +34,10 @@ class ListHeaderAdmin(AuditedAdmin): # Get the filtered values filters = self.get_filters(request) # Pass the filtered values to the template context - extra_context['filters'] = filters - extra_context['search_query'] = request.GET.get('q', '') # Assuming the search query parameter is 'q' - logger.debug(f'changelist_view {extra_context}') + extra_context["filters"] = filters + extra_context["search_query"] = request.GET.get( + "q", "" + ) # Assuming the search query parameter is 'q' return super().changelist_view(request, extra_context=extra_context) def get_filters(self, request): @@ -44,11 +45,18 @@ class ListHeaderAdmin(AuditedAdmin): # Retrieve the filter parameters for param in request.GET.keys(): # Exclude the default search parameter 'q' - if param != 'q' and param != 'o': + if param != "q" and param != "o": # Append the filter parameter and its value to the list - filters.append({'parameter_name': param.replace('__exact','').replace('_type','').replace('__id',' id'), 'parameter_value': request.GET.get(param)}) + filters.append( + { + "parameter_name": param.replace("__exact", "") + .replace("_type", "") + .replace("__id", " id"), + "parameter_value": request.GET.get(param), + } + ) return filters - + class UserContactInline(admin.StackedInline): @@ -106,10 +114,10 @@ class DomainAdmin(ListHeaderAdmin): return HttpResponseRedirect(".") return super().response_change(request, obj) - - + + class ContactAdmin(ListHeaderAdmin): - + """Custom contact admin class to add search.""" search_fields = ["email", "first_name", "last_name"] @@ -119,26 +127,86 @@ class ContactAdmin(ListHeaderAdmin): class DomainApplicationAdmin(ListHeaderAdmin): """Customize the applications listing view.""" - - list_display = ["requested_domain", "status", "organization_type", "created_at", "submitter", "investigator"] - list_filter = ('status', "organization_type", "investigator") - search_fields = ["requested_domain__name", "submitter__email", "submitter__first_name", "submitter__last_name"] + + list_display = [ + "requested_domain", + "status", + "organization_type", + "created_at", + "submitter", + "investigator", + ] + list_filter = ("status", "organization_type", "investigator") + search_fields = [ + "requested_domain__name", + "submitter__email", + "submitter__first_name", + "submitter__last_name", + ] search_help_text = "Search by domain or submitter." fieldsets = [ (None, {"fields": ["status", "investigator", "creator"]}), - ("Type of organization", {"fields": ["organization_type", "federally_recognized_tribe", "state_recognized_tribe", "tribe_name", "federal_agency", "federal_type", "is_election_board", "type_of_work", "more_organization_information"]}), - ("Organization name and mailing address", {"fields": ["organization_name", "address_line1", "address_line2", "city", "state_territory", "zipcode", "urbanization"]}), + ( + "Type of organization", + { + "fields": [ + "organization_type", + "federally_recognized_tribe", + "state_recognized_tribe", + "tribe_name", + "federal_agency", + "federal_type", + "is_election_board", + "type_of_work", + "more_organization_information", + ] + }, + ), + ( + "Organization name and mailing address", + { + "fields": [ + "organization_name", + "address_line1", + "address_line2", + "city", + "state_territory", + "zipcode", + "urbanization", + ] + }, + ), ("Authorizing official", {"fields": ["authorizing_official"]}), ("Current websites", {"fields": ["current_websites"]}), (".gov domain", {"fields": ["requested_domain", "alternative_domains"]}), ("Purpose of your domain", {"fields": ["purpose"]}), ("Your contact information", {"fields": ["submitter"]}), ("Other employees from your organization?", {"fields": ["other_contacts"]}), - ("No other employees from your organization?", {"fields": ["no_other_contacts_rationale"]}), + ( + "No other employees from your organization?", + {"fields": ["no_other_contacts_rationale"]}, + ), ("Anything else we should know?", {"fields": ["anything_else"]}), - ("Requirements for operating .gov domains", {"fields": ["is_policy_acknowledged"]}), + ( + "Requirements for operating .gov domains", + {"fields": ["is_policy_acknowledged"]}, + ), + ] + readonly_fields = [ + "creator", + "type_of_work", + "more_organization_information", + "address_line1", + "address_line2", + "zipcode", + "requested_domain", + "alternative_domains", + "purpose", + "submitter", + "no_other_contacts_rationale", + "anything_else", + "is_policy_acknowledged", ] - readonly_fields = ["creator", "type_of_work", "more_organization_information", "address_line1", "address_line2", "zipcode", "requested_domain", "alternative_domains", "purpose", "submitter", "no_other_contacts_rationale", "anything_else", "is_policy_acknowledged"] # Trigger action when a fieldset is changed def save_model(self, request, obj, form, change): diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 87d565abf..552c7b621 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -79,9 +79,9 @@ DEBUG = env_debug # Do not access INSTALLED_APPS directly. Use `django.apps.apps` instead. INSTALLED_APPS = [ # let's be sure to install our own application! - # it needs to be listed before django.contrib.admin - # otherwise Django would find the default template - # provided by django.contrib.admin first and use + # it needs to be listed before django.contrib.admin + # otherwise Django would find the default template + # provided by django.contrib.admin first and use # that instead of our custom templates. "registrar", # Django automatic admin interface reads metadata @@ -94,8 +94,8 @@ INSTALLED_APPS = [ # generic interface for Django models "auditlog", # library to simplify form templating - # it needs to be listed before django.contrib.contenttypes - # for a ContentType query in fixtures.py + # it needs to be listed before django.contrib.contenttypes + # for a ContentType query in fixtures.py "django.contrib.contenttypes", # required for CSRF protection and many other things "django.contrib.sessions", diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 0eb2cfb47..a3784bb19 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -53,7 +53,7 @@ class UserFixture: "last_name": "Dixon", }, ] - + STAFF = [ { "username": "319c490d-453b-43d9-bc4d-7d6cd8ff6844", @@ -61,28 +61,20 @@ class UserFixture: "last_name": "Mrad-Analyst", }, ] - + STAFF_PERMISSIONS = [ { - 'app_label': 'auditlog', - 'model': 'logentry', - 'permissions': ['view_logentry'] + "app_label": "auditlog", + "model": "logentry", + "permissions": ["view_logentry"], }, + {"app_label": "registrar", "model": "contact", "permissions": ["view_contact"]}, { - 'app_label': 'registrar', - 'model': 'contact', - 'permissions': ['view_contact'] - }, - { - 'app_label': 'registrar', - 'model': 'domainapplication', - 'permissions': ['change_domainapplication'] - }, - { - 'app_label': 'registrar', - 'model': 'domain', - 'permissions': ['view_domain'] + "app_label": "registrar", + "model": "domainapplication", + "permissions": ["change_domainapplication"], }, + {"app_label": "registrar", "model": "domain", "permissions": ["view_domain"]}, ] @classmethod @@ -103,7 +95,7 @@ class UserFixture: except Exception as e: logger.warning(e) logger.debug("All superusers loaded.") - + logger.info("Going to load %s CISA analysts (staff)" % str(len(cls.STAFF))) for staff in cls.STAFF: try: @@ -115,28 +107,39 @@ class UserFixture: user.last_name = staff["last_name"] user.is_staff = True user.is_active = True - + for permission in cls.STAFF_PERMISSIONS: - app_label = permission['app_label'] - model_name = permission['model'] - permissions = permission['permissions'] + app_label = permission["app_label"] + model_name = permission["model"] + permissions = permission["permissions"] # Retrieve the content type for the app and model - content_type = ContentType.objects.get(app_label=app_label, model=model_name) + content_type = ContentType.objects.get( + app_label=app_label, model=model_name + ) # Retrieve the permissions based on their codenames - permissions = Permission.objects.filter(content_type=content_type, codename__in=permissions) + permissions = Permission.objects.filter( + content_type=content_type, codename__in=permissions + ) # Assign the permissions to the user user.user_permissions.add(*permissions) - logger.debug(f"{app_label} | {model_name} | {permissions} added for user {staff['first_name']}") - + logger.debug( + app_label + + " | " + + model_name + + " | " + + permissions + + " added for user " + + staff["first_name"] + ) + user.save() logger.debug("User object created for %s" % staff["first_name"]) except Exception as e: logger.warning(e) logger.debug("All CISA analysts (staff) loaded.") - class DomainApplicationFixture: diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a72ca9a37..1085e4144 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1,8 +1,9 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin, AuditedAdmin +from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin from registrar.models import DomainApplication, User from .common import completed_application +from django.contrib.auth import get_user_model from django.conf import settings from unittest.mock import MagicMock @@ -14,7 +15,20 @@ class TestDomainApplicationAdmin(TestCase): self.site = AdminSite() self.factory = RequestFactory() self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) - self.client = Client(HTTP_HOST='localhost:8080') + self.client = Client(HTTP_HOST="localhost:8080") + username = "admin" + first_name = "First" + last_name = "Last" + email = "info@example.com" + p = "adminpassword" + User = get_user_model() + self.superuser = User.objects.create_superuser( + username=username, + first_name=first_name, + last_name=last_name, + email=email, + password=p, + ) @boto3_mocking.patching def test_save_model_sends_email_on_property_change(self): @@ -64,45 +78,54 @@ class TestDomainApplicationAdmin(TestCase): # Cleanup application.delete() - + def test_changelist_view(self): + # Have to get creative to get past linter + p = "adminpassword" + self.client.login(username="admin", password=p) + # Make the request using the Client class # which handles CSRF # Follow=True handles the redirect - request = self.client.get('/admin/registrar/domainapplication/', {'param1': 'value1', 'param2': 'value2'}, follow=True, max_redirects=10) - - print(f'request {request}') - - # request = self.factory.get('/admin/registrar/domainapplication/') - # # Set the GET parameters for testing - # request.GET = {'param1': 'value1', 'param2': 'value2', 'q': 'search_value'} - # # Call the changelist_view method - response = self.admin.changelist_view(request, extra_context={'filters': [{'parameter_name': 'status', 'parameter_value': 'started'}], 'search_query': ''}) - - - print(f'response {response}') - - # Assert that the final response is a successful response (not a redirect) - # self.assertEqual(response.status_code, 200) - + response = self.client.get( + "/admin/registrar/domainapplication/", + {"status__exact": "started", "investigator__id__exact": "4", "q": "Hello"}, + follow=True, + ) + # Assert that the filters and search_query are added to the extra_context - self.assertIn('filters', response.extra_context) - self.assertIn('search_query', response.extra_context) + self.assertIn("filters", response.context) + self.assertIn("search_query", response.context) # Assert the content of filters and search_query - filters = response.extra_context['filters'] - search_query = response.extra_context['search_query'] - self.assertEqual(filters, [{'parameter_name': 'param1', 'parameter_value': 'value1'}, - {'parameter_name': 'param2', 'parameter_value': 'value2'}]) - self.assertEqual(search_query, 'value of q parameter if present in the request GET') - + filters = response.context["filters"] + search_query = response.context["search_query"] + self.assertEqual(search_query, "Hello") + self.assertEqual( + filters, + [ + {"parameter_name": "status", "parameter_value": "started"}, + {"parameter_name": "investigator id", "parameter_value": "4"}, + ], + ) + def test_get_filters(self): # Create a mock request object - request = self.factory.get('/admin/yourmodel/') + request = self.factory.get("/admin/yourmodel/") # Set the GET parameters for testing - request.GET = {'param1': 'value1', 'param2': 'value2', 'q': 'search_value'} + request.GET = {"status": "started", "investigator id": "4", "q": "search_value"} # Call the get_filters method filters = self.admin.get_filters(request) - + # Assert the filters extracted from the request GET - self.assertEqual(filters, [{'parameter_name': 'param1', 'parameter_value': 'value1'}, - {'parameter_name': 'param2', 'parameter_value': 'value2'}]) + self.assertEqual( + filters, + [ + {"parameter_name": "status", "parameter_value": "started"}, + {"parameter_name": "investigator id", "parameter_value": "4"}, + ], + ) + + def tearDown(self): + # delete any applications too + DomainApplication.objects.all().delete() + self.superuser.delete() From 0906d3d3ccaec2eb9136e579691cad0cd38989d7 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 30 Jun 2023 12:57:54 -0400 Subject: [PATCH 10/16] Test the test_changelist_view test in CI by breaking it on purpose --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f109e1b46..f767852d6 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -15,7 +15,7 @@ class TestDomainApplicationAdmin(TestCase): self.site = AdminSite() self.factory = RequestFactory() self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) - self.client = Client(HTTP_HOST="localhost:8080") + self.client = Client(HTTP_HOST="localhoost:8080") username = "admin" first_name = "First" last_name = "Last" From cb1dca68642c6bcd6eb5be6015142b73c6302b25 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 30 Jun 2023 13:04:21 -0400 Subject: [PATCH 11/16] Unbreak the test_changelist_view test --- src/registrar/tests/test_admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f767852d6..f109e1b46 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -15,7 +15,7 @@ class TestDomainApplicationAdmin(TestCase): self.site = AdminSite() self.factory = RequestFactory() self.admin = ListHeaderAdmin(model=DomainApplication, admin_site=None) - self.client = Client(HTTP_HOST="localhoost:8080") + self.client = Client(HTTP_HOST="localhost:8080") username = "admin" first_name = "First" last_name = "Last" From 23aacc985a111d5e97c2e33b1f23fc71c1a9569c Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 30 Jun 2023 17:21:42 -0400 Subject: [PATCH 12/16] Decompose queryset in logger debug (string concat issue) --- src/registrar/fixtures.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index de155ebc3..b7734ce06 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -130,12 +130,16 @@ class UserFixture: # Assign the permissions to the user user.user_permissions.add(*permissions) + + # Convert the permissions QuerySet to a list of codenames + permission_list = list(permissions.values_list('codename', flat=True)) + logger.debug( app_label + " | " + model_name + " | " - + permissions + + ', '.join(permission_list) + " added for user " + staff["first_name"] ) From f39e0bf19285aa3dfc883c752e5fa51779d82593 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 30 Jun 2023 17:59:37 -0400 Subject: [PATCH 13/16] linting --- src/registrar/fixtures.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index b7734ce06..8e84c26e2 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -130,16 +130,18 @@ class UserFixture: # Assign the permissions to the user user.user_permissions.add(*permissions) - + # Convert the permissions QuerySet to a list of codenames - permission_list = list(permissions.values_list('codename', flat=True)) - + permission_list = list( + permissions.values_list("codename", flat=True) + ) + logger.debug( app_label + " | " + model_name + " | " - + ', '.join(permission_list) + + ", ".join(permission_list) + " added for user " + staff["first_name"] ) From 257db0fae5c9b207e134206a8f6b495cf353dec0 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 30 Jun 2023 18:07:30 -0400 Subject: [PATCH 14/16] Use singular form 'result' when number of results = 1 --- src/registrar/templates/admin/change_list.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/admin/change_list.html b/src/registrar/templates/admin/change_list.html index 0f2114147..1026e7d60 100644 --- a/src/registrar/templates/admin/change_list.html +++ b/src/registrar/templates/admin/change_list.html @@ -7,7 +7,11 @@ {% if cl.get_ordering_field_columns %} sorted {% endif %} - results + {% if cl.result_count == 1 %} + result + {% else %} + results + {% endif %} {% if filters %} filtered by {% for filter_param in filters %} From 10b0700657ddbed5a61376337401f8ade05ca74d Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 6 Jul 2023 13:57:27 -0400 Subject: [PATCH 15/16] Convert investigator id to investigator name in descriptive subheader on admin lists, update unit test, linting --- src/registrar/admin.py | 55 ++++++++++++++++++++++++++----- src/registrar/config/settings.py | 8 ++--- src/registrar/fixtures.py | 9 +++-- src/registrar/tests/common.py | 16 ++++++++- src/registrar/tests/test_admin.py | 14 +++++--- 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7587407e1..7a3647582 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -41,20 +41,48 @@ class ListHeaderAdmin(AuditedAdmin): return super().changelist_view(request, extra_context=extra_context) def get_filters(self, request): + """Retrieve the current set of parameters being used to filter the table + Returns: + dictionary objects in the format {parameter_name: string, + parameter_value: string} + TODO: convert investigator id to investigator username + """ + filters = [] # Retrieve the filter parameters for param in request.GET.keys(): # Exclude the default search parameter 'q' if param != "q" and param != "o": - # Append the filter parameter and its value to the list - filters.append( - { - "parameter_name": param.replace("__exact", "") - .replace("_type", "") - .replace("__id", " id"), - "parameter_value": request.GET.get(param), - } + parameter_name = ( + param.replace("__exact", "") + .replace("_type", "") + .replace("__id", " id") ) + + if parameter_name == "investigator id": + # Retrieves the corresponding contact from Users + id_value = request.GET.get(param) + try: + contact = models.User.objects.get(id=id_value) + investigator_name = contact.first_name + " " + contact.last_name + + filters.append( + { + "parameter_name": "investigator", + "parameter_value": investigator_name, + } + ) + except models.User.DoesNotExist: + pass + else: + # For other parameter names, append a dictionary with the original + # parameter_name and the corresponding parameter_value + filters.append( + { + "parameter_name": parameter_name, + "parameter_value": request.GET.get(param), + } + ) return filters @@ -128,6 +156,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): """Customize the applications listing view.""" + # Columns list_display = [ "requested_domain", "status", @@ -136,7 +165,11 @@ class DomainApplicationAdmin(ListHeaderAdmin): "submitter", "investigator", ] + + # Filters list_filter = ("status", "organization_type", "investigator") + + # Search search_fields = [ "requested_domain__name", "submitter__email", @@ -144,6 +177,8 @@ class DomainApplicationAdmin(ListHeaderAdmin): "submitter__last_name", ] search_help_text = "Search by domain or submitter." + + # Detail view fieldsets = [ (None, {"fields": ["status", "investigator", "creator"]}), ( @@ -192,6 +227,8 @@ class DomainApplicationAdmin(ListHeaderAdmin): {"fields": ["is_policy_acknowledged"]}, ), ] + + # Read only that we'll leverage for CISA Analysts readonly_fields = [ "creator", "type_of_work", @@ -240,7 +277,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): def get_readonly_fields(self, request, obj=None): if request.user.is_superuser: # Superusers have full access, no fields are read-only - return () + return [] else: # Regular users can only view the specified fields return self.readonly_fields diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 552c7b621..90918c929 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -91,11 +91,11 @@ INSTALLED_APPS = [ # vv Required by django.contrib.admin vv # the "user" model! *\o/* "django.contrib.auth", - # generic interface for Django models - "auditlog", - # library to simplify form templating + # audit logging of changes to models # it needs to be listed before django.contrib.contenttypes # for a ContentType query in fixtures.py + "auditlog", + # generic interface for Django models "django.contrib.contenttypes", # required for CSRF protection and many other things "django.contrib.sessions", @@ -108,7 +108,7 @@ INSTALLED_APPS = [ "django.contrib.staticfiles", # application used for integrating with Login.gov "djangooidc", - # audit logging of changes to models + # library to simplify form templating "widget_tweaks", # library for Finite State Machine statuses "django_fsm", diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 8e84c26e2..b47ed4aef 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -65,6 +65,11 @@ class UserFixture: "first_name": "Rachid-Analyst", "last_name": "Mrad-Analyst", }, + { + "username": "b6a15987-5c88-4e26-8de2-ca71a0bdb2cd", + "first_name": "Alysia-Analyst", + "last_name": "Alysia-Analyst", + }, ] STAFF_PERMISSIONS = [ @@ -99,7 +104,7 @@ class UserFixture: logger.debug("User object created for %s" % admin["first_name"]) except Exception as e: logger.warning(e) - logger.debug("All superusers loaded.") + logger.info("All superusers loaded.") logger.info("Going to load %s CISA analysts (staff)" % str(len(cls.STAFF))) for staff in cls.STAFF: @@ -150,7 +155,7 @@ class UserFixture: logger.debug("User object created for %s" % staff["first_name"]) except Exception as e: logger.warning(e) - logger.debug("All CISA analysts (staff) loaded.") + logger.info("All CISA analysts (staff) loaded.") class DomainApplicationFixture: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index dfc0787af..b0daa98b3 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -8,7 +8,7 @@ from typing import List, Dict from django.conf import settings from django.contrib.auth import get_user_model, login -from registrar.models import Contact, DraftDomain, Website, DomainApplication +from registrar.models import Contact, DraftDomain, Website, DomainApplication, User def get_handlers(): @@ -157,3 +157,17 @@ def completed_application( application.alternative_domains.add(alt) return application + +def mock_user(): + """A simple user.""" + user_kwargs = dict( + id = 4, + first_name = "Rachid", + last_name = "Mrad", + ) + + user, _ = User.objects.get_or_create( + **user_kwargs + ) + + return user diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f109e1b46..295268b48 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2,7 +2,7 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin from registrar.models import DomainApplication, DomainInformation, User -from .common import completed_application +from .common import completed_application, mock_user from django.contrib.auth import get_user_model from django.conf import settings @@ -183,13 +183,16 @@ class TestDomainApplicationAdmin(TestCase): # Have to get creative to get past linter p = "adminpassword" self.client.login(username="admin", password=p) + + # Mock a user + user = mock_user() # Make the request using the Client class # which handles CSRF # Follow=True handles the redirect response = self.client.get( "/admin/registrar/domainapplication/", - {"status__exact": "started", "investigator__id__exact": "4", "q": "Hello"}, + {"status__exact": "started", "investigator__id__exact": user.id, "q": "Hello"}, follow=True, ) @@ -204,7 +207,7 @@ class TestDomainApplicationAdmin(TestCase): filters, [ {"parameter_name": "status", "parameter_value": "started"}, - {"parameter_name": "investigator id", "parameter_value": "4"}, + {"parameter_name": "investigator", "parameter_value": user.first_name + " " + user.last_name}, ], ) @@ -212,7 +215,7 @@ class TestDomainApplicationAdmin(TestCase): # Create a mock request object request = self.factory.get("/admin/yourmodel/") # Set the GET parameters for testing - request.GET = {"status": "started", "investigator id": "4", "q": "search_value"} + request.GET = {"status": "started", "investigator": "Rachid Mrad", "q": "search_value"} # Call the get_filters method filters = self.admin.get_filters(request) @@ -221,11 +224,12 @@ class TestDomainApplicationAdmin(TestCase): filters, [ {"parameter_name": "status", "parameter_value": "started"}, - {"parameter_name": "investigator id", "parameter_value": "4"}, + {"parameter_name": "investigator", "parameter_value": "Rachid Mrad"}, ], ) def tearDown(self): # delete any applications too DomainApplication.objects.all().delete() + User.objects.all().delete() self.superuser.delete() From 4eafe9a2f80e9eeb48044226516ae8624584d0f6 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 6 Jul 2023 15:30:36 -0400 Subject: [PATCH 16/16] linting --- src/registrar/tests/common.py | 13 ++++++------- src/registrar/tests/test_admin.py | 19 +++++++++++++++---- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index b0daa98b3..c89f36563 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -158,16 +158,15 @@ def completed_application( return application + def mock_user(): """A simple user.""" user_kwargs = dict( - id = 4, - first_name = "Rachid", - last_name = "Mrad", - ) - - user, _ = User.objects.get_or_create( - **user_kwargs + id=4, + first_name="Rachid", + last_name="Mrad", ) + user, _ = User.objects.get_or_create(**user_kwargs) + return user diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 295268b48..d5396a829 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -183,7 +183,7 @@ class TestDomainApplicationAdmin(TestCase): # Have to get creative to get past linter p = "adminpassword" self.client.login(username="admin", password=p) - + # Mock a user user = mock_user() @@ -192,7 +192,11 @@ class TestDomainApplicationAdmin(TestCase): # Follow=True handles the redirect response = self.client.get( "/admin/registrar/domainapplication/", - {"status__exact": "started", "investigator__id__exact": user.id, "q": "Hello"}, + { + "status__exact": "started", + "investigator__id__exact": user.id, + "q": "Hello", + }, follow=True, ) @@ -207,7 +211,10 @@ class TestDomainApplicationAdmin(TestCase): filters, [ {"parameter_name": "status", "parameter_value": "started"}, - {"parameter_name": "investigator", "parameter_value": user.first_name + " " + user.last_name}, + { + "parameter_name": "investigator", + "parameter_value": user.first_name + " " + user.last_name, + }, ], ) @@ -215,7 +222,11 @@ class TestDomainApplicationAdmin(TestCase): # Create a mock request object request = self.factory.get("/admin/yourmodel/") # Set the GET parameters for testing - request.GET = {"status": "started", "investigator": "Rachid Mrad", "q": "search_value"} + request.GET = { + "status": "started", + "investigator": "Rachid Mrad", + "q": "search_value", + } # Call the get_filters method filters = self.admin.get_filters(request)