From 97139076aeeb1ba333a0bd73c40e478df7f5026f Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 18 Jun 2024 17:56:25 -0600 Subject: [PATCH 1/6] Added portfolio to admin frontend for domain_request and domain_information --- src/registrar/admin.py | 3 ++- src/registrar/models/portfolio.py | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8a691c7fa..ed43661dc 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1233,7 +1233,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): search_help_text = "Search by domain." fieldsets = [ - (None, {"fields": ["creator", "submitter", "domain_request", "notes"]}), + (None, {"fields": ["portfolio","creator", "submitter", "domain_request", "notes"]}), (".gov domain", {"fields": ["domain"]}), ("Contacts", {"fields": ["authorizing_official", "other_contacts", "no_other_contacts_rationale"]}), ("Background info", {"fields": ["anything_else"]}), @@ -1482,6 +1482,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): None, { "fields": [ + "portfolio", "status", "rejection_reason", "action_needed_reason", diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index a05422960..9fefacc31 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -97,3 +97,6 @@ class Portfolio(TimeStampedModel): verbose_name="security contact e-mail", max_length=320, ) + + def __str__(self) -> str: + return f"{self.organization_name}" \ No newline at end of file From df719f2108971caedd1f9af59e285814b22ff67f Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 19 Jun 2024 22:55:20 -0600 Subject: [PATCH 2/6] Added logic to hide certain fields from non-superusers in Admin --- src/registrar/admin.py | 81 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ed43661dc..51fad1c58 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1233,7 +1233,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): search_help_text = "Search by domain." fieldsets = [ - (None, {"fields": ["portfolio","creator", "submitter", "domain_request", "notes"]}), + (None, {"fields": ["portfolio", "creator", "submitter", "domain_request", "notes"]}), (".gov domain", {"fields": ["domain"]}), ("Contacts", {"fields": ["authorizing_official", "other_contacts", "no_other_contacts_rationale"]}), ("Background info", {"fields": ["anything_else"]}), @@ -1319,6 +1319,33 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): change_form_template = "django/admin/domain_information_change_form.html" + + superuser_only_fields = [ + "portfolio", + ] + + # DEVELOPER's NOTE: + # Normally, to exclude a field from an Admin form, we could simply utilize + # Django's "exclude" feature. However, it causes a "missing key" error if we + # go that route for this particular form. The error gets thrown by our + # custom fieldset.html code and is due to the fact that "exclude" removes + # fields from base_fields but not fieldsets. Rather than reworking our + # custom frontend, it seems more straightforward (and easier to read) to simply + # modify the fieldsets list so that it excludes any fields we want to remove + # based on permissions (eg. superuser_only_fields) or other conditions. + def get_fieldsets(self, request, obj=None): + fieldsets = super().get_fieldsets(request, obj) + + # Create a modified version of fieldsets without the 'isbn' field + if not request.user.has_perm("registrar.full_access_permission"): + modified_fieldsets = [] + for name, data in fieldsets: + fields = data.get('fields', []) + fields = tuple(field for field in fields if field not in self.superuser_only_fields) + modified_fieldsets.append((name, {'fields': fields})) + return modified_fieldsets + return fieldsets + def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. We have 1 conditions that determine which fields are read-only: @@ -1593,6 +1620,53 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] filter_horizontal = ("current_websites", "alternative_domains", "other_contacts") + superuser_only_fields = [ + "portfolio", + ] + + # DEVELOPER's NOTE: + # Normally, to exclude a field from an Admin form, we could simply utilize + # Django's "exclude" feature. However, it causes a "missing key" error if we + # go that route for this particular form. The error gets thrown by our + # custom fieldset.html code and is due to the fact that "exclude" removes + # fields from base_fields but not fieldsets. Rather than reworking our + # custom frontend, it seems more straightforward (and easier to read) to simply + # modify the fieldsets list so that it excludes any fields we want to remove + # based on permissions (eg. superuser_only_fields) or other conditions. + def get_fieldsets(self, request, obj=None): + fieldsets = super().get_fieldsets(request, obj) + + # Create a modified version of fieldsets without the 'isbn' field + if not request.user.has_perm("registrar.full_access_permission"): + modified_fieldsets = [] + for name, data in fieldsets: + fields = data.get('fields', []) + fields = tuple(field for field in fields if field not in self.superuser_only_fields) + modified_fieldsets.append((name, {'fields': fields})) + return modified_fieldsets + return fieldsets + + # Fields only superusers can view + # exclude = ['address_line1', ] + # widgets = {'portfolio': forms.HiddenInput()} + + + # def get_form(self, request, obj, **kwargs): + # if request.user.has_perm("registrar.full_access_permission"): + # self.exclude = self.superuser_only_fields + # # self.fieldsets[1][1]['fields'][0].append('portfolio') + # # self.fieldsets[1][1]['fields'].pop('status') + # form = super(DomainRequestAdmin, self).get_form(request, obj, **kwargs) + # return form + + + # if not request.user.has_perm("registrar.full_access_permission"): + + # for fieldset in self.fieldsets: + # for field in fieldset[0]["fields"]: + # if field== + + # Table ordering # NOTE: This impacts the select2 dropdowns (combobox) # Currentl, there's only one for requests on DomainInfo @@ -1851,6 +1925,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().change_view(request, object_id, form_url, extra_context) def process_log_entry(self, log_entry): + """Process a log entry and return filtered entry dictionary if applicable.""" changes = log_entry.changes status_changed = "status" in changes @@ -1906,7 +1981,8 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return entry return None - + + class TransitionDomainAdmin(ListHeaderAdmin): """Custom transition domain admin class.""" @@ -2562,6 +2638,7 @@ class PortfolioAdmin(ListHeaderAdmin): # "requestor", # ] + def save_model(self, request, obj, form, change): if obj.creator is not None: From 7c2c9a1f273c7639f6ed8a1ac56c84f04d96ebb2 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 19 Jun 2024 23:00:01 -0600 Subject: [PATCH 3/6] linted --- src/registrar/admin.py | 49 ++++++++----------------------- src/registrar/models/portfolio.py | 2 +- 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 51fad1c58..65f287adc 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1319,33 +1319,32 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): change_form_template = "django/admin/domain_information_change_form.html" - superuser_only_fields = [ "portfolio", ] # DEVELOPER's NOTE: - # Normally, to exclude a field from an Admin form, we could simply utilize + # Normally, to exclude a field from an Admin form, we could simply utilize # Django's "exclude" feature. However, it causes a "missing key" error if we # go that route for this particular form. The error gets thrown by our # custom fieldset.html code and is due to the fact that "exclude" removes - # fields from base_fields but not fieldsets. Rather than reworking our + # fields from base_fields but not fieldsets. Rather than reworking our # custom frontend, it seems more straightforward (and easier to read) to simply # modify the fieldsets list so that it excludes any fields we want to remove # based on permissions (eg. superuser_only_fields) or other conditions. def get_fieldsets(self, request, obj=None): fieldsets = super().get_fieldsets(request, obj) - + # Create a modified version of fieldsets without the 'isbn' field if not request.user.has_perm("registrar.full_access_permission"): modified_fieldsets = [] for name, data in fieldsets: - fields = data.get('fields', []) + fields = data.get("fields", []) fields = tuple(field for field in fields if field not in self.superuser_only_fields) - modified_fieldsets.append((name, {'fields': fields})) + modified_fieldsets.append((name, {"fields": fields})) return modified_fieldsets return fieldsets - + def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. We have 1 conditions that determine which fields are read-only: @@ -1625,48 +1624,27 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] # DEVELOPER's NOTE: - # Normally, to exclude a field from an Admin form, we could simply utilize + # Normally, to exclude a field from an Admin form, we could simply utilize # Django's "exclude" feature. However, it causes a "missing key" error if we # go that route for this particular form. The error gets thrown by our # custom fieldset.html code and is due to the fact that "exclude" removes - # fields from base_fields but not fieldsets. Rather than reworking our + # fields from base_fields but not fieldsets. Rather than reworking our # custom frontend, it seems more straightforward (and easier to read) to simply # modify the fieldsets list so that it excludes any fields we want to remove # based on permissions (eg. superuser_only_fields) or other conditions. def get_fieldsets(self, request, obj=None): fieldsets = super().get_fieldsets(request, obj) - + # Create a modified version of fieldsets without the 'isbn' field if not request.user.has_perm("registrar.full_access_permission"): modified_fieldsets = [] for name, data in fieldsets: - fields = data.get('fields', []) + fields = data.get("fields", []) fields = tuple(field for field in fields if field not in self.superuser_only_fields) - modified_fieldsets.append((name, {'fields': fields})) + modified_fieldsets.append((name, {"fields": fields})) return modified_fieldsets return fieldsets - # Fields only superusers can view - # exclude = ['address_line1', ] - # widgets = {'portfolio': forms.HiddenInput()} - - - # def get_form(self, request, obj, **kwargs): - # if request.user.has_perm("registrar.full_access_permission"): - # self.exclude = self.superuser_only_fields - # # self.fieldsets[1][1]['fields'][0].append('portfolio') - # # self.fieldsets[1][1]['fields'].pop('status') - # form = super(DomainRequestAdmin, self).get_form(request, obj, **kwargs) - # return form - - - # if not request.user.has_perm("registrar.full_access_permission"): - - # for fieldset in self.fieldsets: - # for field in fieldset[0]["fields"]: - # if field== - - # Table ordering # NOTE: This impacts the select2 dropdowns (combobox) # Currentl, there's only one for requests on DomainInfo @@ -1925,7 +1903,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().change_view(request, object_id, form_url, extra_context) def process_log_entry(self, log_entry): - """Process a log entry and return filtered entry dictionary if applicable.""" changes = log_entry.changes status_changed = "status" in changes @@ -1981,8 +1958,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return entry return None - - + class TransitionDomainAdmin(ListHeaderAdmin): """Custom transition domain admin class.""" @@ -2638,7 +2614,6 @@ class PortfolioAdmin(ListHeaderAdmin): # "requestor", # ] - def save_model(self, request, obj, form, change): if obj.creator is not None: diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 9fefacc31..0ea036bb7 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -99,4 +99,4 @@ class Portfolio(TimeStampedModel): ) def __str__(self) -> str: - return f"{self.organization_name}" \ No newline at end of file + return f"{self.organization_name}" From 7a5de9c3ddbdae889f915cb86a06bd56a88f29c2 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 20 Jun 2024 13:49:32 -0600 Subject: [PATCH 4/6] (updated comments) --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 65f287adc..fa132724a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1335,7 +1335,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): def get_fieldsets(self, request, obj=None): fieldsets = super().get_fieldsets(request, obj) - # Create a modified version of fieldsets without the 'isbn' field + # Create a modified version of fieldsets to exclude certain fields if not request.user.has_perm("registrar.full_access_permission"): modified_fieldsets = [] for name, data in fieldsets: @@ -1635,7 +1635,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): def get_fieldsets(self, request, obj=None): fieldsets = super().get_fieldsets(request, obj) - # Create a modified version of fieldsets without the 'isbn' field + # Create a modified version of fieldsets to exclude certain fields if not request.user.has_perm("registrar.full_access_permission"): modified_fieldsets = [] for name, data in fieldsets: From 343fe4e2dbcdfbd9ab272f314746df2d78e2c555 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 20 Jun 2024 14:34:20 -0600 Subject: [PATCH 5/6] Fixed logic for DomainInformationInline form --- src/registrar/admin.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fa132724a..8d53545cf 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1333,14 +1333,14 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): # modify the fieldsets list so that it excludes any fields we want to remove # based on permissions (eg. superuser_only_fields) or other conditions. def get_fieldsets(self, request, obj=None): - fieldsets = super().get_fieldsets(request, obj) + fieldsets = self.fieldsets # Create a modified version of fieldsets to exclude certain fields if not request.user.has_perm("registrar.full_access_permission"): modified_fieldsets = [] for name, data in fieldsets: fields = data.get("fields", []) - fields = tuple(field for field in fields if field not in self.superuser_only_fields) + fields = tuple(field for field in fields if field not in DomainInformationAdmin.superuser_only_fields) modified_fieldsets.append((name, {"fields": fields})) return modified_fieldsets return fieldsets @@ -1990,13 +1990,7 @@ class DomainInformationInline(admin.StackedInline): template = "django/admin/includes/domain_info_inline_stacked.html" model = models.DomainInformation - fieldsets = copy.deepcopy(DomainInformationAdmin.fieldsets) - # remove .gov domain from fieldset - for index, (title, f) in enumerate(fieldsets): - if title == ".gov domain": - del fieldsets[index] - break - + fieldsets = DomainInformationAdmin.fieldsets readonly_fields = DomainInformationAdmin.readonly_fields analyst_readonly_fields = DomainInformationAdmin.analyst_readonly_fields @@ -2042,6 +2036,23 @@ class DomainInformationInline(admin.StackedInline): def get_readonly_fields(self, request, obj=None): return DomainInformationAdmin.get_readonly_fields(self, request, obj=None) + + # Re-route the get_fieldsets method to utilize DomainInformationAdmin.get_fieldsets + # since that has all the logic for excluding certain fields according to user permissions. + # Then modify the remaining fields to further trim out any we don't want for this inline + # form + def get_fieldsets(self, request, obj=None): + # Grab fieldsets from DomainInformationAdmin so that it handles all logic + # for permission-based field visibility. + modified_fieldsets = DomainInformationAdmin.get_fieldsets(self, request, obj=None) + + # remove .gov domain from fieldset + for index, (title, f) in enumerate(modified_fieldsets): + if title == ".gov domain": + del modified_fieldsets[index] + break + + return modified_fieldsets class DomainResource(FsmModelResource): From 62ab9bb7cfc99abb276522b63e44825dc72c03b7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 20 Jun 2024 14:49:36 -0600 Subject: [PATCH 6/6] linted --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8d53545cf..f59258e18 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2036,7 +2036,7 @@ class DomainInformationInline(admin.StackedInline): def get_readonly_fields(self, request, obj=None): return DomainInformationAdmin.get_readonly_fields(self, request, obj=None) - + # Re-route the get_fieldsets method to utilize DomainInformationAdmin.get_fieldsets # since that has all the logic for excluding certain fields according to user permissions. # Then modify the remaining fields to further trim out any we don't want for this inline @@ -2045,7 +2045,7 @@ class DomainInformationInline(admin.StackedInline): # Grab fieldsets from DomainInformationAdmin so that it handles all logic # for permission-based field visibility. modified_fieldsets = DomainInformationAdmin.get_fieldsets(self, request, obj=None) - + # remove .gov domain from fieldset for index, (title, f) in enumerate(modified_fieldsets): if title == ".gov domain":