From c13e63ec63977e3c2404271d145f4a9643b583ec Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 22 Apr 2024 09:49:35 -0400 Subject: [PATCH 01/76] base import export code in admin --- src/Pipfile | 1 + src/registrar/admin.py | 52 +++++++++++++++++++++++++++++--- src/registrar/config/settings.py | 2 ++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/Pipfile b/src/Pipfile index 9366423f1..0c0e84500 100644 --- a/src/Pipfile +++ b/src/Pipfile @@ -32,6 +32,7 @@ fred-epplib = {git = "https://github.com/cisagov/epplib.git", ref = "master"} pyzipper="*" tblib = "*" django-admin-multiple-choice-list-filter = "*" +django-import-export = "*" [dev-packages] django-debug-toolbar = "*" diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 31f031456..7fbf4dff4 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -28,12 +28,20 @@ from django.utils.safestring import mark_safe from django.utils.html import escape from django.contrib.auth.forms import UserChangeForm, UsernameField from django_admin_multiple_choice_list_filter.list_filters import MultipleChoiceListFilter +from import_export import resources +from import_export.admin import ImportExportModelAdmin from django.utils.translation import gettext_lazy as _ logger = logging.getLogger(__name__) +class UserResource(resources.ModelResource): + + class Meta: + model = models.User + + class MyUserAdminForm(UserChangeForm): """This form utilizes the custom widget for its class's ManyToMany UIs. @@ -468,9 +476,11 @@ class UserContactInline(admin.StackedInline): model = models.Contact -class MyUserAdmin(BaseUserAdmin): +class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): """Custom user admin class to use our inlines.""" + resource_classes = [UserResource] + form = MyUserAdminForm class Meta: @@ -848,9 +858,17 @@ class WebsiteAdmin(ListHeaderAdmin): return response -class UserDomainRoleAdmin(ListHeaderAdmin): +class UserDomainRoleResource(resources.ModelResource): + + class Meta: + model = models.UserDomainRole + + +class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" + resource_classes = [UserDomainRoleResource] + class Meta: """Contains meta information about this class""" @@ -931,9 +949,17 @@ class DomainInvitationAdmin(ListHeaderAdmin): change_form_template = "django/admin/email_clipboard_change_form.html" -class DomainInformationAdmin(ListHeaderAdmin): +class DomainInformationResource(resources.ModelResource): + + class Meta: + model = models.DomainInformation + + +class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Customize domain information admin class.""" + resource_classes = [DomainInformationResource] + form = DomainInformationAdminForm # Columns @@ -1060,9 +1086,17 @@ class DomainInformationAdmin(ListHeaderAdmin): return readonly_fields # Read-only fields for analysts -class DomainRequestAdmin(ListHeaderAdmin): +class DomainRequestResource(resources.ModelResource): + + class Meta: + model = models.DomainRequest + + +class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom domain requests admin class.""" + resource_classes = [DomainRequestResource] + form = DomainRequestAdminForm change_form_template = "django/admin/domain_request_change_form.html" @@ -1590,9 +1624,17 @@ class DomainInformationInline(admin.StackedInline): return DomainInformationAdmin.get_readonly_fields(self, request, obj=None) -class DomainAdmin(ListHeaderAdmin): +class DomainResource(resources.ModelResource): + + class Meta: + model = models.Domain + + +class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom domain admin class to add extra buttons.""" + resource_classes = [DomainResource] + class ElectionOfficeFilter(admin.SimpleListFilter): """Define a custom filter for is_election_board""" diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 54b65e83e..62e676839 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -148,6 +148,8 @@ INSTALLED_APPS = [ "corsheaders", # library for multiple choice filters in django admin "django_admin_multiple_choice_list_filter", + # library for export and import of data + 'import_export', ] # Middleware are routines for processing web requests. From 2514caa3d838afa0ed785730fc5409f1d7ac2ec6 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 22 Apr 2024 10:35:40 -0400 Subject: [PATCH 02/76] merged freezing of django-fsm --- src/Pipfile.lock | 133 ++++++++++++++++++++++++++++++++++++++++++- src/requirements.txt | 10 ++++ 2 files changed, 141 insertions(+), 2 deletions(-) diff --git a/src/Pipfile.lock b/src/Pipfile.lock index 5940f455e..e485a5202 100644 --- a/src/Pipfile.lock +++ b/src/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "ce10883aef7e1ce10421d99b3ac35ebf419857a3fe468f0e2d93785f4323eaa8" + "sha256": "99cf9a4f3912639c02105889046a9eede7a29822fab6f9a04ca25f95e29513c0" }, "pipfile-spec": 6, "requires": {}, @@ -272,6 +272,14 @@ "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'", "version": "==0.7.1" }, + "diff-match-patch": { + "hashes": [ + "sha256:953019cdb9c9d2c9e47b5b12bcff3cf4746fc4598eb406076fa1fc27e6a1f15c", + "sha256:dce43505fb7b1b317de7195579388df0746d90db07015ed47a85e5e44930ef93" + ], + "markers": "python_version >= '3.7'", + "version": "==20230430" + }, "dj-database-url": { "hashes": [ "sha256:04bc34b248d4c21aaa13e4ab419ae6575ef5f10f3df735ce7da97722caa356e0", @@ -352,6 +360,15 @@ "index": "pypi", "version": "==2.8.1" }, + "django-import-export": { + "hashes": [ + "sha256:2eac09e8cec8670f36e24314760448011ad23c51e8fb930d55f50d0c3c926da0", + "sha256:4deabc557801d368093608c86fd0f4831bc9540e2ea41ca2f023e2efb3eb6f48" + ], + "index": "pypi", + "markers": "python_version >= '3.8'", + "version": "==3.3.8" + }, "django-login-required-middleware": { "hashes": [ "sha256:847ae9a69fd7a07618ed53192b3c06946af70a0caf6d0f4eb40a8f37593cd970" @@ -390,6 +407,14 @@ "markers": "python_version >= '3.8'", "version": "==11.0.0" }, + "et-xmlfile": { + "hashes": [ + "sha256:8eb9e2bc2f8c97e37a2dc85a09ecdcdec9d8a396530a6d5a33b30b9a92da0c5c", + "sha256:a2ba85d1d6a74ef63837eed693bcb89c3f752169b0e3e7ae5b16ca5e1b3deada" + ], + "markers": "python_version >= '3.6'", + "version": "==1.1.0" + }, "faker": { "hashes": [ "sha256:34b947581c2bced340c39b35f89dbfac4f356932cfff8fe893bde854903f0e6e", @@ -725,6 +750,12 @@ "markers": "python_version >= '3.8'", "version": "==1.3.3" }, + "markuppy": { + "hashes": [ + "sha256:1adee2c0a542af378fe84548ff6f6b0168f3cb7f426b46961038a2bcfaad0d5f" + ], + "version": "==1.14" + }, "markupsafe": { "hashes": [ "sha256:00e046b6dd71aa03a41079792f8473dc494d564611a8f89bbbd7cb93295ebdcf", @@ -799,6 +830,13 @@ "markers": "python_version >= '3.8'", "version": "==3.21.1" }, + "odfpy": { + "hashes": [ + "sha256:db766a6e59c5103212f3cc92ec8dd50a0f3a02790233ed0b52148b70d3c438ec", + "sha256:fc3b8d1bc098eba4a0fda865a76d9d1e577c4ceec771426bcb169a82c5e9dfe0" + ], + "version": "==1.4.1" + }, "oic": { "hashes": [ "sha256:385a1f64bb59519df1e23840530921bf416740240f505ea6d161e331d3d39fad", @@ -808,6 +846,13 @@ "markers": "python_version ~= '3.7'", "version": "==1.6.1" }, + "openpyxl": { + "hashes": [ + "sha256:a6f5977418eff3b2d5500d54d9db50c8277a368436f4e4f8ddb1be3422870184", + "sha256:f91456ead12ab3c6c2e9491cf33ba6d08357d802192379bb482f1033ade496f5" + ], + "version": "==3.1.2" + }, "orderedmultidict": { "hashes": [ "sha256:04070bbb5e87291cc9bfa51df413677faf2141c73c61d2a5f7b26bea3cd882ad", @@ -1080,6 +1125,62 @@ "markers": "python_version >= '3.8'", "version": "==1.0.1" }, + "pyyaml": { + "hashes": [ + "sha256:04ac92ad1925b2cff1db0cfebffb6ffc43457495c9b3c39d3fcae417d7125dc5", + "sha256:062582fca9fabdd2c8b54a3ef1c978d786e0f6b3a1510e0ac93ef59e0ddae2bc", + "sha256:0d3304d8c0adc42be59c5f8a4d9e3d7379e6955ad754aa9d6ab7a398b59dd1df", + "sha256:1635fd110e8d85d55237ab316b5b011de701ea0f29d07611174a1b42f1444741", + "sha256:184c5108a2aca3c5b3d3bf9395d50893a7ab82a38004c8f61c258d4428e80206", + "sha256:18aeb1bf9a78867dc38b259769503436b7c72f7a1f1f4c93ff9a17de54319b27", + "sha256:1d4c7e777c441b20e32f52bd377e0c409713e8bb1386e1099c2415f26e479595", + "sha256:1e2722cc9fbb45d9b87631ac70924c11d3a401b2d7f410cc0e3bbf249f2dca62", + "sha256:1fe35611261b29bd1de0070f0b2f47cb6ff71fa6595c077e42bd0c419fa27b98", + "sha256:28c119d996beec18c05208a8bd78cbe4007878c6dd15091efb73a30e90539696", + "sha256:326c013efe8048858a6d312ddd31d56e468118ad4cdeda36c719bf5bb6192290", + "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9", + "sha256:42f8152b8dbc4fe7d96729ec2b99c7097d656dc1213a3229ca5383f973a5ed6d", + "sha256:49a183be227561de579b4a36efbb21b3eab9651dd81b1858589f796549873dd6", + "sha256:4fb147e7a67ef577a588a0e2c17b6db51dda102c71de36f8549b6816a96e1867", + "sha256:50550eb667afee136e9a77d6dc71ae76a44df8b3e51e41b77f6de2932bfe0f47", + "sha256:510c9deebc5c0225e8c96813043e62b680ba2f9c50a08d3724c7f28a747d1486", + "sha256:5773183b6446b2c99bb77e77595dd486303b4faab2b086e7b17bc6bef28865f6", + "sha256:596106435fa6ad000c2991a98fa58eeb8656ef2325d7e158344fb33864ed87e3", + "sha256:6965a7bc3cf88e5a1c3bd2e0b5c22f8d677dc88a455344035f03399034eb3007", + "sha256:69b023b2b4daa7548bcfbd4aa3da05b3a74b772db9e23b982788168117739938", + "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0", + "sha256:704219a11b772aea0d8ecd7058d0082713c3562b4e271b849ad7dc4a5c90c13c", + "sha256:7e07cbde391ba96ab58e532ff4803f79c4129397514e1413a7dc761ccd755735", + "sha256:81e0b275a9ecc9c0c0c07b4b90ba548307583c125f54d5b6946cfee6360c733d", + "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28", + "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4", + "sha256:9046c58c4395dff28dd494285c82ba00b546adfc7ef001486fbf0324bc174fba", + "sha256:9eb6caa9a297fc2c2fb8862bc5370d0303ddba53ba97e71f08023b6cd73d16a8", + "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef", + "sha256:a0cd17c15d3bb3fa06978b4e8958dcdc6e0174ccea823003a106c7d4d7899ac5", + "sha256:afd7e57eddb1a54f0f1a974bc4391af8bcce0b444685d936840f125cf046d5bd", + "sha256:b1275ad35a5d18c62a7220633c913e1b42d44b46ee12554e5fd39c70a243d6a3", + "sha256:b786eecbdf8499b9ca1d697215862083bd6d2a99965554781d0d8d1ad31e13a0", + "sha256:ba336e390cd8e4d1739f42dfe9bb83a3cc2e80f567d8805e11b46f4a943f5515", + "sha256:baa90d3f661d43131ca170712d903e6295d1f7a0f595074f151c0aed377c9b9c", + "sha256:bc1bf2925a1ecd43da378f4db9e4f799775d6367bdb94671027b73b393a7c42c", + "sha256:bd4af7373a854424dabd882decdc5579653d7868b8fb26dc7d0e99f823aa5924", + "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34", + "sha256:bfdf460b1736c775f2ba9f6a92bca30bc2095067b8a9d77876d1fad6cc3b4a43", + "sha256:c8098ddcc2a85b61647b2590f825f3db38891662cfc2fc776415143f599bb859", + "sha256:d2b04aac4d386b172d5b9692e2d2da8de7bfb6c387fa4f801fbf6fb2e6ba4673", + "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54", + "sha256:d858aa552c999bc8a8d57426ed01e40bef403cd8ccdd0fc5f6f04a00414cac2a", + "sha256:e7d73685e87afe9f3b36c799222440d6cf362062f78be1013661b00c5c6f678b", + "sha256:f003ed9ad21d6a4713f0a9b5a7a0a79e08dd0f221aff4525a2be4c346ee60aab", + "sha256:f22ac1c3cac4dbc50079e965eba2c1058622631e526bd9afd45fedd49ba781fa", + "sha256:faca3bdcf85b2fc05d06ff3fbc1f83e1391b3e724afa3feba7d13eeab355484c", + "sha256:fca0e3a251908a499833aa292323f32437106001d436eca0e6e7833256674585", + "sha256:fd1592b3fdf65fff2ad0004b5e363300ef59ced41c2e6b3a99d4089fa8c5435d", + "sha256:fd66fc5d0da6d9815ba2cebeb4205f95818ff4b79c3ebe268e75d961704af52f" + ], + "version": "==6.0.1" + }, "pyzipper": { "hashes": [ "sha256:0adca90a00c36a93fbe49bfa8c5add452bfe4ef85a1b8e3638739dd1c7b26bfc", @@ -1130,6 +1231,21 @@ "markers": "python_version >= '3.8'", "version": "==0.5.0" }, + "tablib": { + "extras": [ + "html", + "ods", + "xls", + "xlsx", + "yaml" + ], + "hashes": [ + "sha256:9821caa9eca6062ff7299fa645e737aecff982e6b2b42046928a6413c8dabfd9", + "sha256:f6661dfc45e1d4f51fa8a6239f9c8349380859a5bfaa73280645f046d6c96e33" + ], + "markers": "python_version >= '3.8'", + "version": "==3.5.0" + }, "tblib": { "hashes": [ "sha256:80a6c77e59b55e83911e1e607c649836a69c103963c5f28a46cbeef44acf8129", @@ -1165,6 +1281,20 @@ "markers": "python_version >= '3.8'", "version": "==6.6.0" }, + "xlrd": { + "hashes": [ + "sha256:6a33ee89877bd9abc1158129f6e94be74e2679636b8a205b43b85206c3f0bbdd", + "sha256:f72f148f54442c6b056bf931dbc34f986fd0c3b0b6b5a58d013c9aef274d0c88" + ], + "version": "==2.0.1" + }, + "xlwt": { + "hashes": [ + "sha256:a082260524678ba48a297d922cc385f58278b8aa68741596a87de01a9c628b2e", + "sha256:c59912717a9b28f1a3c2a98fd60741014b06b043936dcecbc113eaaada156c88" + ], + "version": "==1.3.0" + }, "zope.event": { "hashes": [ "sha256:2832e95014f4db26c47a13fdaef84cef2f4df37e66b59d8f1f4a8f319a632c26", @@ -1589,7 +1719,6 @@ "sha256:fd1592b3fdf65fff2ad0004b5e363300ef59ced41c2e6b3a99d4089fa8c5435d", "sha256:fd66fc5d0da6d9815ba2cebeb4205f95818ff4b79c3ebe268e75d961704af52f" ], - "markers": "python_version >= '3.6'", "version": "==6.0.1" }, "rich": { diff --git a/src/requirements.txt b/src/requirements.txt index 3c005f162..067a174e9 100644 --- a/src/requirements.txt +++ b/src/requirements.txt @@ -10,6 +10,7 @@ cffi==1.16.0; platform_python_implementation != 'PyPy' charset-normalizer==3.3.2; python_full_version >= '3.7.0' cryptography==42.0.5; python_version >= '3.7' defusedxml==0.7.1; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4' +diff-match-patch==20230430; python_version >= '3.7' dj-database-url==2.1.0 dj-email-url==1.0.6 django==4.2.10; python_version >= '3.8' @@ -20,10 +21,12 @@ django-cache-url==3.4.5 django-cors-headers==4.3.1; python_version >= '3.8' django-csp==3.8 django-fsm==2.8.1 +django-import-export==3.3.8; python_version >= '3.8' django-login-required-middleware==0.9.0 django-phonenumber-field[phonenumberslite]==7.3.0; python_version >= '3.8' django-widget-tweaks==1.5.0; python_version >= '3.8' environs[django]==11.0.0; python_version >= '3.8' +et-xmlfile==1.1.0; python_version >= '3.6' faker==24.11.0; python_version >= '3.8' fred-epplib@ git+https://github.com/cisagov/epplib.git@d56d183f1664f34c40ca9716a3a9a345f0ef561c furl==2.1.3 @@ -35,9 +38,12 @@ idna==3.7; python_version >= '3.5' jmespath==1.0.1; python_version >= '3.7' lxml==5.2.1; python_version >= '3.6' mako==1.3.3; python_version >= '3.8' +markuppy==1.14 markupsafe==2.1.5; python_version >= '3.7' marshmallow==3.21.1; python_version >= '3.8' +odfpy==1.4.1 oic==1.6.1; python_version ~= '3.7' +openpyxl==3.1.2 orderedmultidict==1.0.1 packaging==24.0; python_version >= '3.7' phonenumberslite==8.13.35 @@ -50,15 +56,19 @@ pydantic-settings==2.2.1; python_version >= '3.8' pyjwkest==1.4.2 python-dateutil==2.9.0.post0; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3' python-dotenv==1.0.1; python_version >= '3.8' +pyyaml==6.0.1 pyzipper==0.3.6; python_version >= '3.4' requests==2.31.0; python_version >= '3.7' s3transfer==0.10.1; python_version >= '3.8' setuptools==69.5.1; python_version >= '3.8' six==1.16.0; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3' sqlparse==0.5.0; python_version >= '3.8' +tablib[html,ods,xls,xlsx,yaml]==3.5.0; python_version >= '3.8' tblib==3.0.0; python_version >= '3.8' typing-extensions==4.11.0; python_version >= '3.8' urllib3==2.2.1; python_version >= '3.8' whitenoise==6.6.0; python_version >= '3.8' +xlrd==2.0.1 +xlwt==1.3.0 zope.event==5.0; python_version >= '3.7' zope.interface==6.3; python_version >= '3.7' From 18658d027b2c885d92a8965a008501cf76dbb497 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 2 May 2024 08:29:38 -0400 Subject: [PATCH 03/76] wip, hardcoded with lots of debugging, needs to be generalized --- src/registrar/admin.py | 313 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 311 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 40cecc367..3219be625 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,17 +1,21 @@ from datetime import date import logging import copy +import traceback +import warnings from django import forms from django.db.models import Value, CharField, Q from django.db.models.functions import Concat, Coalesce +from django.db.transaction import TransactionManagementError from django.http import HttpResponseRedirect from django.shortcuts import redirect -from django_fsm import get_available_FIELD_transitions +from django_fsm import get_available_FIELD_transitions, FSMField from django.contrib import admin, messages from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.auth.models import Group from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError from django.urls import reverse from dateutil.relativedelta import relativedelta # type: ignore from epplibwrapper.errors import ErrorCode, RegistryError @@ -28,7 +32,8 @@ from django.utils.safestring import mark_safe from django.utils.html import escape from django.contrib.auth.forms import UserChangeForm, UsernameField from django_admin_multiple_choice_list_filter.list_filters import MultipleChoiceListFilter -from import_export import resources +from import_export import resources, widgets +from import_export.results import RowResult from import_export.admin import ImportExportModelAdmin from django.utils.translation import gettext_lazy as _ @@ -40,6 +45,42 @@ class UserResource(resources.ModelResource): class Meta: model = models.User + import_id_fields = ('id', 'username',) + + def import_data( + self, + dataset, + dry_run=False, + raise_errors=False, + use_transactions=None, + collect_failed_rows=False, + rollback_on_validation_errors=False, + **kwargs + ): + logger.info("in import_data") + logger.info(dataset) + return super().import_data(dataset,dry_run,raise_errors,use_transactions,collect_failed_rows,rollback_on_validation_errors,**kwargs) + + + def before_import(self, dataset, using_transactions, dry_run, **kwargs): + logger.info("in before_import") + + def import_row( + self, + row, + instance_loader, + using_transactions=True, + dry_run=False, + raise_errors=None, + **kwargs + ): + logger.info("in import_row") + logger.info(row) + return super().import_row(row,instance_loader,using_transactions,dry_run,raise_errors,**kwargs) + + def after_import_row(self, row, row_result, **kwargs): + logger.info(row_result.original) + logger.info(row_result.instance) class MyUserAdminForm(UserChangeForm): @@ -1630,8 +1671,276 @@ class DomainInformationInline(admin.StackedInline): class DomainResource(resources.ModelResource): + WIDGETS_MAP = { + "ManyToManyField": "get_m2m_widget", + "OneToOneField": "get_fk_widget", + "ForeignKey": "get_fk_widget", + "CharField": widgets.CharWidget, + "DecimalField": widgets.DecimalWidget, + "DateTimeField": widgets.DateTimeWidget, + "DateField": widgets.DateWidget, + "TimeField": widgets.TimeWidget, + "DurationField": widgets.DurationWidget, + "FloatField": widgets.FloatWidget, + "IntegerField": widgets.IntegerWidget, + "PositiveIntegerField": widgets.IntegerWidget, + "BigIntegerField": widgets.IntegerWidget, + "PositiveSmallIntegerField": widgets.IntegerWidget, + "SmallIntegerField": widgets.IntegerWidget, + "SmallAutoField": widgets.IntegerWidget, + "AutoField": widgets.IntegerWidget, + "BigAutoField": widgets.IntegerWidget, + "NullBooleanField": widgets.BooleanWidget, + "BooleanField": widgets.BooleanWidget, + "JSONField": widgets.JSONWidget, + } + class Meta: model = models.Domain + # exclude = ('state') + + + def import_row( + self, + row, + instance_loader, + using_transactions=True, + dry_run=False, + raise_errors=None, + **kwargs + ): + """ + Imports data from ``tablib.Dataset``. Refer to :doc:`import_workflow` + for a more complete description of the whole import process. + + :param row: A ``dict`` of the row to import + + :param instance_loader: The instance loader to be used to load the row + + :param using_transactions: If ``using_transactions`` is set, a transaction + is being used to wrap the import + + :param dry_run: If ``dry_run`` is set, or error occurs, transaction + will be rolled back. + """ + if raise_errors is not None: + warnings.warn( + "raise_errors argument is deprecated and " + "will be removed in a future release.", + category=DeprecationWarning, + ) + + logger.info("in import_row") + skip_diff = self._meta.skip_diff + row_result = self.get_row_result_class()() + if self._meta.store_row_values: + row_result.row_values = row + original = None + try: + self.before_import_row(row, **kwargs) + logger.info("after before_import_row") + instance, new = self.get_or_init_instance(instance_loader, row) + logger.info("after get_or_init_instance") + logger.info(type(instance)) + for f in sorted(instance._meta.fields): + if callable(getattr(f, "get_internal_type", None)): + internal_type = f.get_internal_type() + logger.info(f"{f.name} {internal_type} {type(f)}") + logger.info(f"type == fsmfield: {isinstance(f, FSMField)}") + for attr_name, attr_value in vars(instance).items(): + logger.info(f"{attr_name}: {attr_value}") + if isinstance(attr_value, FSMField): + logger.info(f"FSMField: {attr_name}: {attr_value}") + self.after_import_instance(instance, new, **kwargs) + if new: + row_result.import_type = RowResult.IMPORT_TYPE_NEW + else: + row_result.import_type = RowResult.IMPORT_TYPE_UPDATE + row_result.new_record = new + if not skip_diff: + original = copy.deepcopy(instance) + diff = self.get_diff_class()(self, original, new) + if self.for_delete(row, instance): + if new: + row_result.import_type = RowResult.IMPORT_TYPE_SKIP + if not skip_diff: + diff.compare_with(self, None, dry_run) + else: + row_result.import_type = RowResult.IMPORT_TYPE_DELETE + row_result.add_instance_info(instance) + if self._meta.store_instance: + row_result.instance = instance + self.delete_instance(instance, using_transactions, dry_run) + if not skip_diff: + diff.compare_with(self, None, dry_run) + else: + import_validation_errors = {} + try: + logger.info("about to import_obj") + self.import_obj(instance, row, dry_run, **kwargs) + logger.info("got past import_obj") + except ValidationError as e: + # Validation errors from import_obj() are passed on to + # validate_instance(), where they can be combined with model + # instance validation errors if necessary + import_validation_errors = e.update_error_dict( + import_validation_errors + ) + + if self.skip_row(instance, original, row, import_validation_errors): + row_result.import_type = RowResult.IMPORT_TYPE_SKIP + else: + self.validate_instance(instance, import_validation_errors) + self.save_instance(instance, new, using_transactions, dry_run) + self.save_m2m(instance, row, using_transactions, dry_run) + row_result.add_instance_info(instance) + if self._meta.store_instance: + row_result.instance = instance + if not skip_diff: + diff.compare_with(self, instance, dry_run) + if not new: + row_result.original = original + + if not skip_diff and not self._meta.skip_html_diff: + row_result.diff = diff.as_html() + self.after_import_row(row, row_result, **kwargs) + + except ValidationError as e: + row_result.import_type = RowResult.IMPORT_TYPE_INVALID + row_result.validation_error = e + except Exception as e: + row_result.import_type = RowResult.IMPORT_TYPE_ERROR + # There is no point logging a transaction error for each row + # when only the original error is likely to be relevant + if not isinstance(e, TransactionManagementError): + logger.debug(e, exc_info=e) + tb_info = traceback.format_exc() + row_result.errors.append(self.get_error_result_class()(e, tb_info, row)) + + return row_result + + # def get_fsm_fields_from_model(self): + # logger.info("in get_fsm_fields_from_model") + # fsm_fields = [] + # for f in sorted(self._meta.model._meta.fields): + # # if callable(getattr(f, "get_internal_type", None)): + # # internal_type = f.get_internal_type() + # # logger.info(f"{f.name} {internal_type} {type(f)}") + # # logger.info(f"type == fsmfield: {isinstance(f, FSMField)}") + # if isinstance(f, FSMField): + # fsm_fields.append(f.name) + # return fsm_fields + + # def get_fsm_fields_from_row(self, fsm_fields, row=None): + # fields_and_values = [] + # for f in fsm_fields: + # if f in row: + # fields_and_values.append((f, row[f])) + # return fields_and_values + + # def init_instance(self, row=None): + # logger.info("initializing instance") + # logger.info(f"row: {row}") + # #get fields which are fsm fields + # fsm_fields = self.get_fsm_fields_from_model() + # #then get field values from row and return an array of tuples + # fields_and_values = self.get_fsm_fields_from_row(fsm_fields, row) + # logger.info(fields_and_values) + # #then set those tuples to kwargs + # kwargs = dict(fields_and_values) + # #then pass kwargs to model() + # #return self._meta.model(state='ready') + # return self._meta.model(**kwargs) + # #return super().init_instance(row) + + def init_instance(self, row=None): + logger.info("Initializing instance") + logger.info(f"Row: {row}") + + # Get fields which are fsm fields + fsm_fields = [] + + for f in sorted(self._meta.model._meta.fields): + if isinstance(f, FSMField): + if row and f.name in row: + fsm_fields.append((f.name, row[f.name])) + + logger.info(f"Fsm fields: {fsm_fields}") + + # Convert fields_and_values to kwargs + kwargs = dict(fsm_fields) + + # Initialize model instance with kwargs + return self._meta.model(**kwargs) + + def get_instance(self, instance_loader, row): + """ + If all 'import_id_fields' are present in the dataset, calls + the :doc:`InstanceLoader `. Otherwise, + returns `None`. + """ + logger.info("get_instance is called") + import_id_fields = [self.fields[f] for f in self.get_import_id_fields()] + for field in import_id_fields: + if field.column_name not in row: + return + return instance_loader.get_instance(row) + + def import_data( + self, + dataset, + dry_run=False, + raise_errors=False, + use_transactions=None, + collect_failed_rows=False, + rollback_on_validation_errors=False, + **kwargs + ): + logger.info("in import_data") + logger.info(dataset) + return super().import_data(dataset,dry_run,raise_errors,use_transactions,collect_failed_rows,rollback_on_validation_errors,**kwargs) + + + def before_import(self, dataset, using_transactions, dry_run, **kwargs): + logger.info("in before_import") + + # def import_row( + # self, + # row, + # instance_loader, + # using_transactions=True, + # dry_run=False, + # raise_errors=None, + # **kwargs + # ): + # logger.info("in import_row") + # logger.info(row) + # return super().import_row(row,instance_loader,using_transactions,dry_run,raise_errors,**kwargs) + + def after_import_row(self, row, row_result, **kwargs): + logger.info(row_result.original) + logger.info(row_result.instance) + + def import_field(self, field, obj, data, is_m2m=False, **kwargs): + logger.info(f"import_field: ${field}") + logger.info(field.attribute) + logger.info(field.widget) + logger.info(type(obj)) + logger.info(obj._meta.fields) + is_fsm = False + for f in obj._meta.fields: + logger.info(f.name) + logger.info(type(f)) + if field.attribute == f.name and isinstance(f, FSMField): + is_fsm = True + # if field.attribute in sorted(obj._meta.fields): + # logger.info("in fields") + # if not isinstance(obj._meta.fields[field.attribute], FSMField): + if not is_fsm: + logger.info("not fsm field") + #if (field.attribute != 'state'): + super().import_field(field, obj, data, is_m2m, **kwargs) + logger.info("finished importing") class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): From 059585d3e195b8bb120333fa5136353fc687ddee Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 2 May 2024 09:28:33 -0600 Subject: [PATCH 04/76] add try/catch and update model --- .../0090_alter_publiccontact_registry_id.py | 24 +++++++++++++ src/registrar/models/domain.py | 36 ++++++++++--------- src/registrar/models/public_contact.py | 1 + 3 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 src/registrar/migrations/0090_alter_publiccontact_registry_id.py diff --git a/src/registrar/migrations/0090_alter_publiccontact_registry_id.py b/src/registrar/migrations/0090_alter_publiccontact_registry_id.py new file mode 100644 index 000000000..c8bdc1574 --- /dev/null +++ b/src/registrar/migrations/0090_alter_publiccontact_registry_id.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.10 on 2024-05-02 15:27 + +from django.db import migrations, models +import registrar.models.public_contact + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0089_user_verification_type"), + ] + + operations = [ + migrations.AlterField( + model_name="publiccontact", + name="registry_id", + field=models.CharField( + default=registrar.models.public_contact.get_id, + help_text="Auto generated ID to track this contact in the registry", + max_length=16, + unique=True, + ), + ), + ] diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 7f53bb234..c49050d4a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -7,7 +7,7 @@ from typing import Optional from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore -from django.db import models +from django.db import IntegrityError, models from django.utils import timezone from typing import Any from registrar.models.host import Host @@ -758,7 +758,7 @@ class Domain(TimeStampedModel, DomainHelper): so follow on additions will update the current registrant""" logger.info("making registrant contact") - self._set_singleton_contact(contact=contact, expectedType=contact.ContactTypeChoices.REGISTRANT) + self.try_set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.REGISTRANT) @Cache def administrative_contact(self) -> PublicContact | None: @@ -769,10 +769,7 @@ class Domain(TimeStampedModel, DomainHelper): @administrative_contact.setter # type: ignore def administrative_contact(self, contact: PublicContact): logger.info("making admin contact") - if contact.contact_type != contact.ContactTypeChoices.ADMINISTRATIVE: - raise ValueError("Cannot set a registrant contact with a different contact type") - self._make_contact_in_registry(contact=contact) - self._update_domain_with_contact(contact, rem=False) + self.try_set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.ADMINISTRATIVE) def _update_epp_contact(self, contact: PublicContact): """Sends UpdateContact to update the actual contact object, @@ -832,6 +829,16 @@ class Domain(TimeStampedModel, DomainHelper): logger.error("Error changing to new registrant error code is %s, error is %s" % (e.code, e)) # TODO-error handling better here? + def try_set_singleton_contact(self, contact: PublicContact, expected_type: str): # noqa + """Runs the _set_singleton_contact function, while catching an IntegrityError + from the DB and handling it appropriately""" + try: + self._set_singleton_contact(contact=contact, expectedType=expected_type) + except IntegrityError as err: + # If this error occurs, it indicates that our DB tried to create + # a duplicate PublicContact + logger.error(f"A contact with this registry ID already exists. Error: {err}") + def _set_singleton_contact(self, contact: PublicContact, expectedType: str): # noqa """Sets the contacts by adding them to the registry as new contacts, updates the contact if it is already in epp, @@ -850,12 +857,13 @@ class Domain(TimeStampedModel, DomainHelper): # get publicContact objects that have the matching # domain and type but a different id # like in highlander we there can only be one - hasOtherContact = ( + duplicate_contacts = ( PublicContact.objects.exclude(registry_id=contact.registry_id) .filter(domain=self, contact_type=contact.contact_type) - .exists() ) + + # if no record exists with this contact type # make contact in registry, duplicate and errors handled there errorCode = self._make_contact_in_registry(contact) @@ -871,14 +879,10 @@ class Domain(TimeStampedModel, DomainHelper): logger.info("_set_singleton_contact()-> contact has been added to the registry") # if has conflicting contacts in our db remove them - if hasOtherContact: + if duplicate_contacts.exists(): logger.info("_set_singleton_contact()-> updating domain, removing old contact") - existing_contact = ( - PublicContact.objects.exclude(registry_id=contact.registry_id) - .filter(domain=self, contact_type=contact.contact_type) - .get() - ) + existing_contact = duplicate_contacts.get() if isRegistrant: # send update domain only for registant contacts @@ -925,7 +929,7 @@ class Domain(TimeStampedModel, DomainHelper): from domain information (not domain request) and should have the security email from DomainRequest""" logger.info("making security contact in registry") - self._set_singleton_contact(contact, expectedType=contact.ContactTypeChoices.SECURITY) + self.try_set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.SECURITY) @Cache def technical_contact(self) -> PublicContact | None: @@ -936,7 +940,7 @@ class Domain(TimeStampedModel, DomainHelper): @technical_contact.setter # type: ignore def technical_contact(self, contact: PublicContact): logger.info("making technical contact") - self._set_singleton_contact(contact, expectedType=contact.ContactTypeChoices.TECHNICAL) + self.try_set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.TECHNICAL) def is_active(self) -> bool: """Currently just returns if the state is created, diff --git a/src/registrar/models/public_contact.py b/src/registrar/models/public_contact.py index 00d065404..5b3679995 100644 --- a/src/registrar/models/public_contact.py +++ b/src/registrar/models/public_contact.py @@ -48,6 +48,7 @@ class PublicContact(TimeStampedModel): help_text="For which type of WHOIS contact", ) registry_id = models.CharField( + unique=True, max_length=16, default=get_id, null=False, From 7ba71f4f2c5abff8092adfbde7498b8b15d07f0a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 2 May 2024 09:46:07 -0600 Subject: [PATCH 05/76] Delete duplicate on get_or_create --- src/registrar/models/domain.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c49050d4a..2dbe72560 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1966,10 +1966,11 @@ class Domain(TimeStampedModel, DomainHelper): domain=self, ) - # Raise an error if we find duplicates. - # This should not occur + # If we find duplicates, log it and delete the newest one. if db_contact.count() > 1: - raise Exception(f"Multiple contacts found for {public_contact.contact_type}") + logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") + newest_duplicate = db_contact.order_by('-created_at').first() + newest_duplicate.delete() # Save to DB if it doesn't exist already. if db_contact.count() == 0: @@ -1981,16 +1982,14 @@ class Domain(TimeStampedModel, DomainHelper): existing_contact = db_contact.get() - # Does the item we're grabbing match - # what we have in our DB? + # Does the item we're grabbing match what we have in our DB? if existing_contact.email != public_contact.email or existing_contact.registry_id != public_contact.registry_id: existing_contact.delete() public_contact.save() logger.warning("Requested PublicContact is out of sync " "with DB.") return public_contact - # If it already exists, we can - # assume that the DB instance was updated - # during set, so we should just use that. + + # If it already exists, we can assume that the DB instance was updated during set, so we should just use that. return existing_contact def _registrant_to_public_contact(self, registry_id: str): From 011a974a1e39c3365e53fbeb68312d2e472a97bb Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 2 May 2024 13:17:04 -0400 Subject: [PATCH 06/76] abstracted into FsmModelResource --- src/registrar/admin.py | 316 ++++++----------------------------------- 1 file changed, 46 insertions(+), 270 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 3219be625..1be5131e1 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -41,6 +41,50 @@ from django.utils.translation import gettext_lazy as _ logger = logging.getLogger(__name__) +class FsmModelResource(resources.ModelResource): + + def init_instance(self, row=None): + logger.info("Initializing instance") + logger.info(f"Row: {row}") + + # Get fields which are fsm fields + fsm_fields = [] + + for f in sorted(self._meta.model._meta.fields): + if isinstance(f, FSMField): + if row and f.name in row: + fsm_fields.append((f.name, row[f.name])) + + logger.info(f"Fsm fields: {fsm_fields}") + + # Convert fields_and_values to kwargs + kwargs = dict(fsm_fields) + + # Initialize model instance with kwargs + return self._meta.model(**kwargs) + + def import_field(self, field, obj, data, is_m2m=False, **kwargs): + # logger.info(f"import_field: ${field}") + # logger.info(field.attribute) + # logger.info(field.widget) + # logger.info(type(obj)) + # logger.info(obj._meta.fields) + is_fsm = False + for f in obj._meta.fields: + # logger.info(f.name) + # logger.info(type(f)) + if field.attribute == f.name and isinstance(f, FSMField): + is_fsm = True + # if field.attribute in sorted(obj._meta.fields): + # logger.info("in fields") + # if not isinstance(obj._meta.fields[field.attribute], FSMField): + if not is_fsm: + # logger.info("not fsm field") + #if (field.attribute != 'state'): + super().import_field(field, obj, data, is_m2m, **kwargs) + # logger.info("finished importing") + + class UserResource(resources.ModelResource): class Meta: @@ -1129,7 +1173,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): return readonly_fields # Read-only fields for analysts -class DomainRequestResource(resources.ModelResource): +class DomainRequestResource(FsmModelResource): class Meta: model = models.DomainRequest @@ -1669,278 +1713,10 @@ class DomainInformationInline(admin.StackedInline): return DomainInformationAdmin.get_readonly_fields(self, request, obj=None) -class DomainResource(resources.ModelResource): - - WIDGETS_MAP = { - "ManyToManyField": "get_m2m_widget", - "OneToOneField": "get_fk_widget", - "ForeignKey": "get_fk_widget", - "CharField": widgets.CharWidget, - "DecimalField": widgets.DecimalWidget, - "DateTimeField": widgets.DateTimeWidget, - "DateField": widgets.DateWidget, - "TimeField": widgets.TimeWidget, - "DurationField": widgets.DurationWidget, - "FloatField": widgets.FloatWidget, - "IntegerField": widgets.IntegerWidget, - "PositiveIntegerField": widgets.IntegerWidget, - "BigIntegerField": widgets.IntegerWidget, - "PositiveSmallIntegerField": widgets.IntegerWidget, - "SmallIntegerField": widgets.IntegerWidget, - "SmallAutoField": widgets.IntegerWidget, - "AutoField": widgets.IntegerWidget, - "BigAutoField": widgets.IntegerWidget, - "NullBooleanField": widgets.BooleanWidget, - "BooleanField": widgets.BooleanWidget, - "JSONField": widgets.JSONWidget, - } +class DomainResource(FsmModelResource): class Meta: model = models.Domain - # exclude = ('state') - - - def import_row( - self, - row, - instance_loader, - using_transactions=True, - dry_run=False, - raise_errors=None, - **kwargs - ): - """ - Imports data from ``tablib.Dataset``. Refer to :doc:`import_workflow` - for a more complete description of the whole import process. - - :param row: A ``dict`` of the row to import - - :param instance_loader: The instance loader to be used to load the row - - :param using_transactions: If ``using_transactions`` is set, a transaction - is being used to wrap the import - - :param dry_run: If ``dry_run`` is set, or error occurs, transaction - will be rolled back. - """ - if raise_errors is not None: - warnings.warn( - "raise_errors argument is deprecated and " - "will be removed in a future release.", - category=DeprecationWarning, - ) - - logger.info("in import_row") - skip_diff = self._meta.skip_diff - row_result = self.get_row_result_class()() - if self._meta.store_row_values: - row_result.row_values = row - original = None - try: - self.before_import_row(row, **kwargs) - logger.info("after before_import_row") - instance, new = self.get_or_init_instance(instance_loader, row) - logger.info("after get_or_init_instance") - logger.info(type(instance)) - for f in sorted(instance._meta.fields): - if callable(getattr(f, "get_internal_type", None)): - internal_type = f.get_internal_type() - logger.info(f"{f.name} {internal_type} {type(f)}") - logger.info(f"type == fsmfield: {isinstance(f, FSMField)}") - for attr_name, attr_value in vars(instance).items(): - logger.info(f"{attr_name}: {attr_value}") - if isinstance(attr_value, FSMField): - logger.info(f"FSMField: {attr_name}: {attr_value}") - self.after_import_instance(instance, new, **kwargs) - if new: - row_result.import_type = RowResult.IMPORT_TYPE_NEW - else: - row_result.import_type = RowResult.IMPORT_TYPE_UPDATE - row_result.new_record = new - if not skip_diff: - original = copy.deepcopy(instance) - diff = self.get_diff_class()(self, original, new) - if self.for_delete(row, instance): - if new: - row_result.import_type = RowResult.IMPORT_TYPE_SKIP - if not skip_diff: - diff.compare_with(self, None, dry_run) - else: - row_result.import_type = RowResult.IMPORT_TYPE_DELETE - row_result.add_instance_info(instance) - if self._meta.store_instance: - row_result.instance = instance - self.delete_instance(instance, using_transactions, dry_run) - if not skip_diff: - diff.compare_with(self, None, dry_run) - else: - import_validation_errors = {} - try: - logger.info("about to import_obj") - self.import_obj(instance, row, dry_run, **kwargs) - logger.info("got past import_obj") - except ValidationError as e: - # Validation errors from import_obj() are passed on to - # validate_instance(), where they can be combined with model - # instance validation errors if necessary - import_validation_errors = e.update_error_dict( - import_validation_errors - ) - - if self.skip_row(instance, original, row, import_validation_errors): - row_result.import_type = RowResult.IMPORT_TYPE_SKIP - else: - self.validate_instance(instance, import_validation_errors) - self.save_instance(instance, new, using_transactions, dry_run) - self.save_m2m(instance, row, using_transactions, dry_run) - row_result.add_instance_info(instance) - if self._meta.store_instance: - row_result.instance = instance - if not skip_diff: - diff.compare_with(self, instance, dry_run) - if not new: - row_result.original = original - - if not skip_diff and not self._meta.skip_html_diff: - row_result.diff = diff.as_html() - self.after_import_row(row, row_result, **kwargs) - - except ValidationError as e: - row_result.import_type = RowResult.IMPORT_TYPE_INVALID - row_result.validation_error = e - except Exception as e: - row_result.import_type = RowResult.IMPORT_TYPE_ERROR - # There is no point logging a transaction error for each row - # when only the original error is likely to be relevant - if not isinstance(e, TransactionManagementError): - logger.debug(e, exc_info=e) - tb_info = traceback.format_exc() - row_result.errors.append(self.get_error_result_class()(e, tb_info, row)) - - return row_result - - # def get_fsm_fields_from_model(self): - # logger.info("in get_fsm_fields_from_model") - # fsm_fields = [] - # for f in sorted(self._meta.model._meta.fields): - # # if callable(getattr(f, "get_internal_type", None)): - # # internal_type = f.get_internal_type() - # # logger.info(f"{f.name} {internal_type} {type(f)}") - # # logger.info(f"type == fsmfield: {isinstance(f, FSMField)}") - # if isinstance(f, FSMField): - # fsm_fields.append(f.name) - # return fsm_fields - - # def get_fsm_fields_from_row(self, fsm_fields, row=None): - # fields_and_values = [] - # for f in fsm_fields: - # if f in row: - # fields_and_values.append((f, row[f])) - # return fields_and_values - - # def init_instance(self, row=None): - # logger.info("initializing instance") - # logger.info(f"row: {row}") - # #get fields which are fsm fields - # fsm_fields = self.get_fsm_fields_from_model() - # #then get field values from row and return an array of tuples - # fields_and_values = self.get_fsm_fields_from_row(fsm_fields, row) - # logger.info(fields_and_values) - # #then set those tuples to kwargs - # kwargs = dict(fields_and_values) - # #then pass kwargs to model() - # #return self._meta.model(state='ready') - # return self._meta.model(**kwargs) - # #return super().init_instance(row) - - def init_instance(self, row=None): - logger.info("Initializing instance") - logger.info(f"Row: {row}") - - # Get fields which are fsm fields - fsm_fields = [] - - for f in sorted(self._meta.model._meta.fields): - if isinstance(f, FSMField): - if row and f.name in row: - fsm_fields.append((f.name, row[f.name])) - - logger.info(f"Fsm fields: {fsm_fields}") - - # Convert fields_and_values to kwargs - kwargs = dict(fsm_fields) - - # Initialize model instance with kwargs - return self._meta.model(**kwargs) - - def get_instance(self, instance_loader, row): - """ - If all 'import_id_fields' are present in the dataset, calls - the :doc:`InstanceLoader `. Otherwise, - returns `None`. - """ - logger.info("get_instance is called") - import_id_fields = [self.fields[f] for f in self.get_import_id_fields()] - for field in import_id_fields: - if field.column_name not in row: - return - return instance_loader.get_instance(row) - - def import_data( - self, - dataset, - dry_run=False, - raise_errors=False, - use_transactions=None, - collect_failed_rows=False, - rollback_on_validation_errors=False, - **kwargs - ): - logger.info("in import_data") - logger.info(dataset) - return super().import_data(dataset,dry_run,raise_errors,use_transactions,collect_failed_rows,rollback_on_validation_errors,**kwargs) - - - def before_import(self, dataset, using_transactions, dry_run, **kwargs): - logger.info("in before_import") - - # def import_row( - # self, - # row, - # instance_loader, - # using_transactions=True, - # dry_run=False, - # raise_errors=None, - # **kwargs - # ): - # logger.info("in import_row") - # logger.info(row) - # return super().import_row(row,instance_loader,using_transactions,dry_run,raise_errors,**kwargs) - - def after_import_row(self, row, row_result, **kwargs): - logger.info(row_result.original) - logger.info(row_result.instance) - - def import_field(self, field, obj, data, is_m2m=False, **kwargs): - logger.info(f"import_field: ${field}") - logger.info(field.attribute) - logger.info(field.widget) - logger.info(type(obj)) - logger.info(obj._meta.fields) - is_fsm = False - for f in obj._meta.fields: - logger.info(f.name) - logger.info(type(f)) - if field.attribute == f.name and isinstance(f, FSMField): - is_fsm = True - # if field.attribute in sorted(obj._meta.fields): - # logger.info("in fields") - # if not isinstance(obj._meta.fields[field.attribute], FSMField): - if not is_fsm: - logger.info("not fsm field") - #if (field.attribute != 'state'): - super().import_field(field, obj, data, is_m2m, **kwargs) - logger.info("finished importing") class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): From a9509ebdcef177c162c45a011b5c3f7d7faaec1d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 2 May 2024 12:10:01 -0600 Subject: [PATCH 07/76] Update domain.py --- src/registrar/models/domain.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 2dbe72560..18a7cab91 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1969,6 +1969,7 @@ class Domain(TimeStampedModel, DomainHelper): # If we find duplicates, log it and delete the newest one. if db_contact.count() > 1: logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") + # Q: Should we be deleting the newest or the oldest? Does it even matter? newest_duplicate = db_contact.order_by('-created_at').first() newest_duplicate.delete() From 41089aadfdd83c7bf58863a1902e22cf85aa3e16 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 2 May 2024 13:00:02 -0600 Subject: [PATCH 08/76] Cleanup --- src/registrar/models/domain.py | 65 ++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 18a7cab91..e8b52eb78 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -830,15 +830,68 @@ class Domain(TimeStampedModel, DomainHelper): # TODO-error handling better here? def try_set_singleton_contact(self, contact: PublicContact, expected_type: str): # noqa - """Runs the _set_singleton_contact function, while catching an IntegrityError - from the DB and handling it appropriately""" + """ + Attempts to set a singleton contact for a domain, + handling potential IntegrityErrors due to duplicate entries. + + This function wraps the `_set_singleton_contact` method, + which is intended to assign a unique contact of a specified type to a domain. + + If a duplicate is detected, all duplicate entries are deleted before retrying the operation. + + Args: + contact (PublicContact): The contact instance to set as a singleton. + expected_type (str): The expected type of the contact (e.g., 'technical', 'administrative'). + + Raises: + IntegrityError: Re-raises the IntegrityError if an unexpected number of duplicates are found, + or on the rerun of _set_singleton_contact. + """ try: self._set_singleton_contact(contact=contact, expectedType=expected_type) except IntegrityError as err: - # If this error occurs, it indicates that our DB tried to create - # a duplicate PublicContact + # If this error occurs, it indicates that our DB tried to create a duplicate PublicContact logger.error(f"A contact with this registry ID already exists. Error: {err}") + # Delete the duplicates and try again. + duplicates = PublicContact.objects.filter( + registry_id=contact.registry_id, + contact_type=contact.contact_type, + domain=self, + ) + + # If we find duplicates, log it and delete the newest one. + if duplicates.count() <= 1: + # Something weird happened - we got an integrity error with one or fewer records + # which should not be possible. + logger.error( + "try_set_singleton_contact() -> " + f"An IntegrityError was raised but {duplicates.count()} entries exist" + ) + # Raise the error. This is a case in which it is appropriate to do so. + raise err + else: + logger.warning("try_set_singleton_contact() -> Duplicate contacts found. Deleting duplicate.") + self._delete_duplicates(duplicates) + + # Try to set the contact again. If we get an error at this point, then + # this indicates a very bad data issue in the DB (duplicate registry_ids on other domains). + # This requires patching the DB and epp with a script due to the epp <---> DB link. + # Trying to fix that here will just result in corrupt EPP records. + self._set_singleton_contact(contact=contact, expectedType=expected_type) + + def _delete_duplicates(self, duplicates): + """Given a list of duplicates, delete all but the oldest one.""" + + # Q: Should we be deleting the newest or the oldest? Does it even matter? + oldest_duplicate = duplicates.order_by('created_at').first() + + # Exclude the oldest entry + duplicates_to_delete = duplicates.exclude(id=oldest_duplicate.id) + + # Delete all duplicates + duplicates_to_delete.delete() + def _set_singleton_contact(self, contact: PublicContact, expectedType: str): # noqa """Sets the contacts by adding them to the registry as new contacts, updates the contact if it is already in epp, @@ -1969,9 +2022,7 @@ class Domain(TimeStampedModel, DomainHelper): # If we find duplicates, log it and delete the newest one. if db_contact.count() > 1: logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") - # Q: Should we be deleting the newest or the oldest? Does it even matter? - newest_duplicate = db_contact.order_by('-created_at').first() - newest_duplicate.delete() + self._delete_duplicates(db_contact) # Save to DB if it doesn't exist already. if db_contact.count() == 0: From a2540128e78cee6a0aae7e4fcffa11b6b9be6d76 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 2 May 2024 15:17:49 -0600 Subject: [PATCH 09/76] Cleanup --- src/registrar/models/domain.py | 59 +++------------------------------- 1 file changed, 4 insertions(+), 55 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index e8b52eb78..4f64b3089 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -758,7 +758,7 @@ class Domain(TimeStampedModel, DomainHelper): so follow on additions will update the current registrant""" logger.info("making registrant contact") - self.try_set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.REGISTRANT) + self._set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.REGISTRANT) @Cache def administrative_contact(self) -> PublicContact | None: @@ -769,7 +769,7 @@ class Domain(TimeStampedModel, DomainHelper): @administrative_contact.setter # type: ignore def administrative_contact(self, contact: PublicContact): logger.info("making admin contact") - self.try_set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.ADMINISTRATIVE) + self._set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.ADMINISTRATIVE) def _update_epp_contact(self, contact: PublicContact): """Sends UpdateContact to update the actual contact object, @@ -829,57 +829,6 @@ class Domain(TimeStampedModel, DomainHelper): logger.error("Error changing to new registrant error code is %s, error is %s" % (e.code, e)) # TODO-error handling better here? - def try_set_singleton_contact(self, contact: PublicContact, expected_type: str): # noqa - """ - Attempts to set a singleton contact for a domain, - handling potential IntegrityErrors due to duplicate entries. - - This function wraps the `_set_singleton_contact` method, - which is intended to assign a unique contact of a specified type to a domain. - - If a duplicate is detected, all duplicate entries are deleted before retrying the operation. - - Args: - contact (PublicContact): The contact instance to set as a singleton. - expected_type (str): The expected type of the contact (e.g., 'technical', 'administrative'). - - Raises: - IntegrityError: Re-raises the IntegrityError if an unexpected number of duplicates are found, - or on the rerun of _set_singleton_contact. - """ - try: - self._set_singleton_contact(contact=contact, expectedType=expected_type) - except IntegrityError as err: - # If this error occurs, it indicates that our DB tried to create a duplicate PublicContact - logger.error(f"A contact with this registry ID already exists. Error: {err}") - - # Delete the duplicates and try again. - duplicates = PublicContact.objects.filter( - registry_id=contact.registry_id, - contact_type=contact.contact_type, - domain=self, - ) - - # If we find duplicates, log it and delete the newest one. - if duplicates.count() <= 1: - # Something weird happened - we got an integrity error with one or fewer records - # which should not be possible. - logger.error( - "try_set_singleton_contact() -> " - f"An IntegrityError was raised but {duplicates.count()} entries exist" - ) - # Raise the error. This is a case in which it is appropriate to do so. - raise err - else: - logger.warning("try_set_singleton_contact() -> Duplicate contacts found. Deleting duplicate.") - self._delete_duplicates(duplicates) - - # Try to set the contact again. If we get an error at this point, then - # this indicates a very bad data issue in the DB (duplicate registry_ids on other domains). - # This requires patching the DB and epp with a script due to the epp <---> DB link. - # Trying to fix that here will just result in corrupt EPP records. - self._set_singleton_contact(contact=contact, expectedType=expected_type) - def _delete_duplicates(self, duplicates): """Given a list of duplicates, delete all but the oldest one.""" @@ -982,7 +931,7 @@ class Domain(TimeStampedModel, DomainHelper): from domain information (not domain request) and should have the security email from DomainRequest""" logger.info("making security contact in registry") - self.try_set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.SECURITY) + self._set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.SECURITY) @Cache def technical_contact(self) -> PublicContact | None: @@ -993,7 +942,7 @@ class Domain(TimeStampedModel, DomainHelper): @technical_contact.setter # type: ignore def technical_contact(self, contact: PublicContact): logger.info("making technical contact") - self.try_set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.TECHNICAL) + self._set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.TECHNICAL) def is_active(self) -> bool: """Currently just returns if the state is created, From 9058e0dc97955d1da59f4893c66395922350962d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 2 May 2024 15:24:30 -0600 Subject: [PATCH 10/76] Update domain.py --- src/registrar/models/domain.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 4f64b3089..a8004eb07 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -758,7 +758,7 @@ class Domain(TimeStampedModel, DomainHelper): so follow on additions will update the current registrant""" logger.info("making registrant contact") - self._set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.REGISTRANT) + self._set_singleton_contact(contact=contact, expectedType=contact.ContactTypeChoices.REGISTRANT) @Cache def administrative_contact(self) -> PublicContact | None: @@ -769,7 +769,7 @@ class Domain(TimeStampedModel, DomainHelper): @administrative_contact.setter # type: ignore def administrative_contact(self, contact: PublicContact): logger.info("making admin contact") - self._set_singleton_contact(contact=contact, expected_type=contact.ContactTypeChoices.ADMINISTRATIVE) + self._set_singleton_contact(contact=contact, expectedType=contact.ContactTypeChoices.ADMINISTRATIVE) def _update_epp_contact(self, contact: PublicContact): """Sends UpdateContact to update the actual contact object, @@ -931,7 +931,7 @@ class Domain(TimeStampedModel, DomainHelper): from domain information (not domain request) and should have the security email from DomainRequest""" logger.info("making security contact in registry") - self._set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.SECURITY) + self._set_singleton_contact(contact, expectedType=contact.ContactTypeChoices.SECURITY) @Cache def technical_contact(self) -> PublicContact | None: @@ -942,7 +942,7 @@ class Domain(TimeStampedModel, DomainHelper): @technical_contact.setter # type: ignore def technical_contact(self, contact: PublicContact): logger.info("making technical contact") - self._set_singleton_contact(contact, expected_type=contact.ContactTypeChoices.TECHNICAL) + self._set_singleton_contact(contact, expectedType=contact.ContactTypeChoices.TECHNICAL) def is_active(self) -> bool: """Currently just returns if the state is created, From 5c78618ac0437fa50bf6ff80f96ee3a082bf34b8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 2 May 2024 15:29:45 -0600 Subject: [PATCH 11/76] Cleanup --- src/registrar/models/domain.py | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index a8004eb07..6d6b628ad 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -829,18 +829,6 @@ class Domain(TimeStampedModel, DomainHelper): logger.error("Error changing to new registrant error code is %s, error is %s" % (e.code, e)) # TODO-error handling better here? - def _delete_duplicates(self, duplicates): - """Given a list of duplicates, delete all but the oldest one.""" - - # Q: Should we be deleting the newest or the oldest? Does it even matter? - oldest_duplicate = duplicates.order_by('created_at').first() - - # Exclude the oldest entry - duplicates_to_delete = duplicates.exclude(id=oldest_duplicate.id) - - # Delete all duplicates - duplicates_to_delete.delete() - def _set_singleton_contact(self, contact: PublicContact, expectedType: str): # noqa """Sets the contacts by adding them to the registry as new contacts, updates the contact if it is already in epp, @@ -859,13 +847,10 @@ class Domain(TimeStampedModel, DomainHelper): # get publicContact objects that have the matching # domain and type but a different id # like in highlander we there can only be one - duplicate_contacts = ( - PublicContact.objects.exclude(registry_id=contact.registry_id) - .filter(domain=self, contact_type=contact.contact_type) + duplicate_contacts = PublicContact.objects.exclude(registry_id=contact.registry_id).filter( + domain=self, contact_type=contact.contact_type ) - - # if no record exists with this contact type # make contact in registry, duplicate and errors handled there errorCode = self._make_contact_in_registry(contact) @@ -1971,7 +1956,15 @@ class Domain(TimeStampedModel, DomainHelper): # If we find duplicates, log it and delete the newest one. if db_contact.count() > 1: logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") - self._delete_duplicates(db_contact) + + # Q: Should we be deleting the newest or the oldest? Does it even matter? + oldest_duplicate = db_contact.order_by("created_at").first() + + # Exclude the oldest entry + duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) + + # Delete all duplicates + duplicates_to_delete.delete() # Save to DB if it doesn't exist already. if db_contact.count() == 0: From 780504c9f26c96672c920d449d62073bf7225cce Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 3 May 2024 07:14:06 -0400 Subject: [PATCH 12/76] update to allow DomainRequest to import properly --- src/registrar/admin.py | 24 +-------- .../models/utility/generic_helper.py | 53 ++++++++++--------- 2 files changed, 29 insertions(+), 48 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1be5131e1..7231c634e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,13 +1,10 @@ from datetime import date import logging import copy -import traceback -import warnings from django import forms from django.db.models import Value, CharField, Q from django.db.models.functions import Concat, Coalesce -from django.db.transaction import TransactionManagementError from django.http import HttpResponseRedirect from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField @@ -15,7 +12,6 @@ from django.contrib import admin, messages from django.contrib.auth.admin import UserAdmin as BaseUserAdmin from django.contrib.auth.models import Group from django.contrib.contenttypes.models import ContentType -from django.core.exceptions import ValidationError from django.urls import reverse from dateutil.relativedelta import relativedelta # type: ignore from epplibwrapper.errors import ErrorCode, RegistryError @@ -32,8 +28,7 @@ from django.utils.safestring import mark_safe from django.utils.html import escape from django.contrib.auth.forms import UserChangeForm, UsernameField from django_admin_multiple_choice_list_filter.list_filters import MultipleChoiceListFilter -from import_export import resources, widgets -from import_export.results import RowResult +from import_export import resources from import_export.admin import ImportExportModelAdmin from django.utils.translation import gettext_lazy as _ @@ -44,8 +39,6 @@ logger = logging.getLogger(__name__) class FsmModelResource(resources.ModelResource): def init_instance(self, row=None): - logger.info("Initializing instance") - logger.info(f"Row: {row}") # Get fields which are fsm fields fsm_fields = [] @@ -55,8 +48,6 @@ class FsmModelResource(resources.ModelResource): if row and f.name in row: fsm_fields.append((f.name, row[f.name])) - logger.info(f"Fsm fields: {fsm_fields}") - # Convert fields_and_values to kwargs kwargs = dict(fsm_fields) @@ -64,25 +55,12 @@ class FsmModelResource(resources.ModelResource): return self._meta.model(**kwargs) def import_field(self, field, obj, data, is_m2m=False, **kwargs): - # logger.info(f"import_field: ${field}") - # logger.info(field.attribute) - # logger.info(field.widget) - # logger.info(type(obj)) - # logger.info(obj._meta.fields) is_fsm = False for f in obj._meta.fields: - # logger.info(f.name) - # logger.info(type(f)) if field.attribute == f.name and isinstance(f, FSMField): is_fsm = True - # if field.attribute in sorted(obj._meta.fields): - # logger.info("in fields") - # if not isinstance(obj._meta.fields[field.attribute], FSMField): if not is_fsm: - # logger.info("not fsm field") - #if (field.attribute != 'state'): super().import_field(field, obj, data, is_m2m, **kwargs) - # logger.info("finished importing") class UserResource(resources.ModelResource): diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 892298967..12ec85e57 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -102,34 +102,37 @@ class CreateOrUpdateOrganizationTypeHelper: def _handle_existing_instance(self, force_update_when_no_are_changes_found=False): # == Init variables == # - # Instance is already in the database, fetch its current state - current_instance = self.sender.objects.get(id=self.instance.id) + try: + # Instance is already in the database, fetch its current state + current_instance = self.sender.objects.get(id=self.instance.id) - # Check the new and old values - generic_org_type_changed = self.instance.generic_org_type != current_instance.generic_org_type - is_election_board_changed = self.instance.is_election_board != current_instance.is_election_board - organization_type_changed = self.instance.organization_type != current_instance.organization_type + # Check the new and old values + generic_org_type_changed = self.instance.generic_org_type != current_instance.generic_org_type + is_election_board_changed = self.instance.is_election_board != current_instance.is_election_board + organization_type_changed = self.instance.organization_type != current_instance.organization_type - # == Check for invalid conditions before proceeding == # - if organization_type_changed and (generic_org_type_changed or is_election_board_changed): - # Since organization type is linked with generic_org_type and election board, - # we have to update one or the other, not both. - # This will not happen in normal flow as it is not possible otherwise. - raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") - elif not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): - # No changes found - if force_update_when_no_are_changes_found: - # If we want to force an update anyway, we can treat this record like - # its a new one in that we check for "None" values rather than changes. - self._handle_new_instance() - else: - # == Update the linked values == # - # Find out which field needs updating - organization_type_needs_update = generic_org_type_changed or is_election_board_changed - generic_org_type_needs_update = organization_type_changed + # == Check for invalid conditions before proceeding == # + if organization_type_changed and (generic_org_type_changed or is_election_board_changed): + # Since organization type is linked with generic_org_type and election board, + # we have to update one or the other, not both. + # This will not happen in normal flow as it is not possible otherwise. + raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") + elif not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): + # No changes found + if force_update_when_no_are_changes_found: + # If we want to force an update anyway, we can treat this record like + # its a new one in that we check for "None" values rather than changes. + self._handle_new_instance() + else: + # == Update the linked values == # + # Find out which field needs updating + organization_type_needs_update = generic_org_type_changed or is_election_board_changed + generic_org_type_needs_update = organization_type_changed - # Update the field - self._update_fields(organization_type_needs_update, generic_org_type_needs_update) + # Update the field + self._update_fields(organization_type_needs_update, generic_org_type_needs_update) + except: + pass def _update_fields(self, organization_type_needs_update, generic_org_type_needs_update): """ From b5fb28e1568105a0c96b63a2af76de3e19a0eeb2 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 3 May 2024 09:29:20 -0400 Subject: [PATCH 13/76] added Contacts --- src/registrar/admin.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 744babb95..da630c04c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -785,9 +785,17 @@ class MyHostAdmin(AuditedAdmin): inlines = [HostIPInline] -class ContactAdmin(ListHeaderAdmin): +class ContactResource(resources.ModelResource): + + class Meta: + model = models.Contact + + +class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom contact admin class to add search.""" + resource_classes = [ContactResource] + search_fields = ["email", "first_name", "last_name"] search_help_text = "Search by first name, last name or email." list_display = [ From 92746e606180a4815388a904fd1039d339cafeda Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 3 May 2024 11:16:18 -0600 Subject: [PATCH 14/76] Add some logging, use unique_together rather than unique --- .../0090_alter_publiccontact_registry_id.py | 24 ------------------- ...090_alter_publiccontact_unique_together.py | 17 +++++++++++++ src/registrar/models/domain.py | 13 ++++++++-- src/registrar/models/public_contact.py | 10 +++++++- 4 files changed, 37 insertions(+), 27 deletions(-) delete mode 100644 src/registrar/migrations/0090_alter_publiccontact_registry_id.py create mode 100644 src/registrar/migrations/0090_alter_publiccontact_unique_together.py diff --git a/src/registrar/migrations/0090_alter_publiccontact_registry_id.py b/src/registrar/migrations/0090_alter_publiccontact_registry_id.py deleted file mode 100644 index c8bdc1574..000000000 --- a/src/registrar/migrations/0090_alter_publiccontact_registry_id.py +++ /dev/null @@ -1,24 +0,0 @@ -# Generated by Django 4.2.10 on 2024-05-02 15:27 - -from django.db import migrations, models -import registrar.models.public_contact - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0089_user_verification_type"), - ] - - operations = [ - migrations.AlterField( - model_name="publiccontact", - name="registry_id", - field=models.CharField( - default=registrar.models.public_contact.get_id, - help_text="Auto generated ID to track this contact in the registry", - max_length=16, - unique=True, - ), - ), - ] diff --git a/src/registrar/migrations/0090_alter_publiccontact_unique_together.py b/src/registrar/migrations/0090_alter_publiccontact_unique_together.py new file mode 100644 index 000000000..a476bfa04 --- /dev/null +++ b/src/registrar/migrations/0090_alter_publiccontact_unique_together.py @@ -0,0 +1,17 @@ +# Generated by Django 4.2.10 on 2024-05-03 17:06 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0089_user_verification_type"), + ] + + operations = [ + migrations.AlterUniqueTogether( + name="publiccontact", + unique_together={("contact_type", "registry_id", "domain")}, + ), + ] diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 6d6b628ad..0719dd075 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1207,24 +1207,28 @@ class Domain(TimeStampedModel, DomainHelper): def get_default_security_contact(self): """Gets the default security contact.""" + logger.info("get_default_security_contact() -> Adding default security contact") contact = PublicContact.get_default_security() contact.domain = self return contact def get_default_administrative_contact(self): """Gets the default administrative contact.""" + logger.info("get_default_security_contact() -> Adding administrative security contact") contact = PublicContact.get_default_administrative() contact.domain = self return contact def get_default_technical_contact(self): """Gets the default technical contact.""" + logger.info("get_default_security_contact() -> Adding technical security contact") contact = PublicContact.get_default_technical() contact.domain = self return contact def get_default_registrant_contact(self): """Gets the default registrant contact.""" + logger.info("get_default_security_contact() -> Adding default registrant contact") contact = PublicContact.get_default_registrant() contact.domain = self return contact @@ -1238,6 +1242,7 @@ class Domain(TimeStampedModel, DomainHelper): Returns: PublicContact | None """ + logger.info(f"get_contact_in_keys() -> Grabbing a {contact_type} contact from cache") # Registrant doesn't exist as an array, and is of # a special data type, so we need to handle that. if contact_type == PublicContact.ContactTypeChoices.REGISTRANT: @@ -1300,6 +1305,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.error(e.code) raise e if e.code == ErrorCode.OBJECT_DOES_NOT_EXIST and self.state == Domain.State.UNKNOWN: + logger.info("_get_or_create_domain() -> Switching to dns_needed from unknown") # avoid infinite loop already_tried_to_create = True self.dns_needed_from_unknown() @@ -1310,6 +1316,7 @@ class Domain(TimeStampedModel, DomainHelper): raise e def addRegistrant(self): + """Adds a default registrant contact""" registrant = PublicContact.get_default_registrant() registrant.domain = self registrant.save() # calls the registrant_contact.setter @@ -1337,6 +1344,8 @@ class Domain(TimeStampedModel, DomainHelper): self.addAllDefaults() def addAllDefaults(self): + """Adds default security, technical, and administrative contacts""" + logger.info("addAllDefaults() -> Adding default security, technical, and administrative contacts") security_contact = self.get_default_security_contact() security_contact.save() @@ -1351,7 +1360,7 @@ class Domain(TimeStampedModel, DomainHelper): """place a clienthold on a domain (no longer should resolve) ignoreEPP (boolean) - set to true to by-pass EPP (used for transition domains) """ - # TODO - ensure all requirements for client hold are made here + # (check prohibited statuses) logger.info("clientHold()-> inside clientHold") @@ -1359,7 +1368,6 @@ class Domain(TimeStampedModel, DomainHelper): # include this ignoreEPP flag if not ignoreEPP: self._place_client_hold() - # TODO -on the client hold ticket any additional error handling here @transition(field="state", source=[State.READY, State.ON_HOLD], target=State.READY) def revert_client_hold(self, ignoreEPP=False): @@ -1561,6 +1569,7 @@ class Domain(TimeStampedModel, DomainHelper): def _get_or_create_contact(self, contact: PublicContact): """Try to fetch info about a contact. Create it if it does not exist.""" + logger.info(f"_get_or_create_contact() -> Fetching contact info") try: return self._request_contact_info(contact) except RegistryError as e: diff --git a/src/registrar/models/public_contact.py b/src/registrar/models/public_contact.py index 5b3679995..b5dde3dd9 100644 --- a/src/registrar/models/public_contact.py +++ b/src/registrar/models/public_contact.py @@ -19,6 +19,15 @@ def get_id(): class PublicContact(TimeStampedModel): """Contact information intended to be published in WHOIS.""" + class Meta: + """Contains meta info about this class""" + # Creates a composite primary key with these fields. + # We can share the same registry id, but only if the contact type is + # different or if the domain is different. + # For instance - we don't desire to have two admin contacts with the same id + # on the same domain. + unique_together = [("contact_type", "registry_id", "domain")] + class ContactTypeChoices(models.TextChoices): """These are the types of contacts accepted by the registry.""" @@ -48,7 +57,6 @@ class PublicContact(TimeStampedModel): help_text="For which type of WHOIS contact", ) registry_id = models.CharField( - unique=True, max_length=16, default=get_id, null=False, From 8f606312101e02aac237491a6b5266df22087072 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 3 May 2024 12:12:17 -0600 Subject: [PATCH 15/76] Cleanup --- src/registrar/models/domain.py | 4 ++-- src/registrar/models/public_contact.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 0719dd075..a8953200d 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -7,7 +7,7 @@ from typing import Optional from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore -from django.db import IntegrityError, models +from django.db import models from django.utils import timezone from typing import Any from registrar.models.host import Host @@ -1967,7 +1967,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") # Q: Should we be deleting the newest or the oldest? Does it even matter? - oldest_duplicate = db_contact.order_by("created_at").first() + oldest_duplicate = db_contact.order_by("created_at") # Exclude the oldest entry duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) diff --git a/src/registrar/models/public_contact.py b/src/registrar/models/public_contact.py index b5dde3dd9..71ed07de5 100644 --- a/src/registrar/models/public_contact.py +++ b/src/registrar/models/public_contact.py @@ -21,9 +21,10 @@ class PublicContact(TimeStampedModel): class Meta: """Contains meta info about this class""" + # Creates a composite primary key with these fields. # We can share the same registry id, but only if the contact type is - # different or if the domain is different. + # different or if the domain is different. # For instance - we don't desire to have two admin contacts with the same id # on the same domain. unique_together = [("contact_type", "registry_id", "domain")] From ec26f03b44d30b7b2019f16c5f0d0ad83ff65638 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 3 May 2024 12:21:30 -0600 Subject: [PATCH 16/76] Update domain.py --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index a8953200d..2bcc50292 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1967,7 +1967,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") # Q: Should we be deleting the newest or the oldest? Does it even matter? - oldest_duplicate = db_contact.order_by("created_at") + oldest_duplicate = db_contact.order_by("created_at").first() # Exclude the oldest entry duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) From e32525563ec4dcc3ae71c095f2cca9b1f216780f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 3 May 2024 16:46:18 -0400 Subject: [PATCH 17/76] commented and reformatted --- src/registrar/admin.py | 59 +++++++++++--------------------- src/registrar/config/settings.py | 2 +- 2 files changed, 21 insertions(+), 40 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index da630c04c..89f8391b3 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -37,26 +37,43 @@ logger = logging.getLogger(__name__) class FsmModelResource(resources.ModelResource): + """ModelResource is extended to support importing of tables which + have FSMFields. ModelResource is extended with the following changes + to existing behavior: + When new objects are to be imported, FSMFields are initialized before + the object is initialized. This is because FSMFields do not allow + direct modification. + When objects, which are to be imported, are updated, the FSMFields + are skipped.""" def init_instance(self, row=None): + """Overrides the init_instance method of ModelResource. Returns + an instance of the model, with the FSMFields already initialized + from data in the row.""" # Get fields which are fsm fields fsm_fields = [] - for f in sorted(self._meta.model._meta.fields): + for f in self._meta.model._meta.fields: if isinstance(f, FSMField): if row and f.name in row: fsm_fields.append((f.name, row[f.name])) - # Convert fields_and_values to kwargs + # Convert fsm_fields fields_and_values to kwargs kwargs = dict(fsm_fields) # Initialize model instance with kwargs return self._meta.model(**kwargs) def import_field(self, field, obj, data, is_m2m=False, **kwargs): + """Overrides the import_field method of ModelResource. If the + field being imported is an FSMField, it is not imported.""" + is_fsm = False + + # check each field in the object for f in obj._meta.fields: + # if the field is an instance of FSMField if field.attribute == f.name and isinstance(f, FSMField): is_fsm = True if not is_fsm: @@ -67,42 +84,6 @@ class UserResource(resources.ModelResource): class Meta: model = models.User - import_id_fields = ('id', 'username',) - - def import_data( - self, - dataset, - dry_run=False, - raise_errors=False, - use_transactions=None, - collect_failed_rows=False, - rollback_on_validation_errors=False, - **kwargs - ): - logger.info("in import_data") - logger.info(dataset) - return super().import_data(dataset,dry_run,raise_errors,use_transactions,collect_failed_rows,rollback_on_validation_errors,**kwargs) - - - def before_import(self, dataset, using_transactions, dry_run, **kwargs): - logger.info("in before_import") - - def import_row( - self, - row, - instance_loader, - using_transactions=True, - dry_run=False, - raise_errors=None, - **kwargs - ): - logger.info("in import_row") - logger.info(row) - return super().import_row(row,instance_loader,using_transactions,dry_run,raise_errors,**kwargs) - - def after_import_row(self, row, row_result, **kwargs): - logger.info(row_result.original) - logger.info(row_result.instance) class MyUserAdminForm(UserChangeForm): @@ -1761,7 +1742,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom domain admin class to add extra buttons.""" resource_classes = [DomainResource] - + class ElectionOfficeFilter(admin.SimpleListFilter): """Define a custom filter for is_election_board""" diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 062e2492e..a18adabde 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -149,7 +149,7 @@ INSTALLED_APPS = [ # library for multiple choice filters in django admin "django_admin_multiple_choice_list_filter", # library for export and import of data - 'import_export', + "import_export", ] # Middleware are routines for processing web requests. From 80aca78194a6a63785c97580e0a5522f4df7f582 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 3 May 2024 17:16:16 -0400 Subject: [PATCH 18/76] only show import on non_production --- .../admin/import_export/change_list_import_item.html | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 src/registrar/templates/admin/import_export/change_list_import_item.html diff --git a/src/registrar/templates/admin/import_export/change_list_import_item.html b/src/registrar/templates/admin/import_export/change_list_import_item.html new file mode 100644 index 000000000..b640331cb --- /dev/null +++ b/src/registrar/templates/admin/import_export/change_list_import_item.html @@ -0,0 +1,8 @@ +{% load i18n %} +{% load admin_urls %} + +{% if has_import_permission %} + {% if not IS_PRODUCTION %} +
  • {% trans "Import" %}
    • + {% endif %} +{% endif %} \ No newline at end of file From c5748c6bc1bbd896ce63c7b440c398d48aadcbf0 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 3 May 2024 18:04:02 -0400 Subject: [PATCH 19/76] added documentation --- docs/operations/import_export.md | 57 ++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 docs/operations/import_export.md diff --git a/docs/operations/import_export.md b/docs/operations/import_export.md new file mode 100644 index 000000000..3e57e5152 --- /dev/null +++ b/docs/operations/import_export.md @@ -0,0 +1,57 @@ +# Export / Import Tables + +A means is provided to export and import individual tables from +one environment to another. This allows for replication of +production data in a development environment. Import and export +are provided through the django admin interface, through a modified +library, django-import-export. Each supported model has an Import +and an Export button on the list view. + +### Export + +When exporting models from the source environment, make sure that +no filters are selected. This will ensure that all rows of the model +are exported. Due to database dependencies, the following models +need to be exported: + +* User +* Contact +* Domain +* DomainRequest +* DomainInformation +* DomainUserRole + +### Import + +When importing into the target environment, if the target environment +is different than the source environment, it must be prepared for the +import. This involves clearing out rows in the appropriate tables so +that there are no database conflicts on import. + +#### Preparing Target Environment + +Delete all rows from tables in the following order through django admin: + +* DomainInformation +* DomainRequest +* Domain +* User (all but the current user) +* Contact + +It may not be necessary, but advisable to also remove rows from these tables: + +* Websites +* DraftDomain +* Host + +#### Importing into Target Environment + +Once target environment is prepared, files can be imported in the following +order: + +* User +* Contact +* Domain +* DomainRequest +* DomainInformation +* UserDomainRole \ No newline at end of file From 9bd0d9e6b7c33c4d795494fbe21a9041396d010c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 May 2024 08:09:51 -0600 Subject: [PATCH 20/76] Update domain.py --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 2bcc50292..a63abf364 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1569,7 +1569,7 @@ class Domain(TimeStampedModel, DomainHelper): def _get_or_create_contact(self, contact: PublicContact): """Try to fetch info about a contact. Create it if it does not exist.""" - logger.info(f"_get_or_create_contact() -> Fetching contact info") + logger.info("_get_or_create_contact() -> Fetching contact info") try: return self._request_contact_info(contact) except RegistryError as e: From 06c605a62fa5b24d4f66d0b0d1c6a5f5fb723356 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 May 2024 08:15:46 -0600 Subject: [PATCH 21/76] Overzealous linter --- src/registrar/models/domain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index a63abf364..92c38b8d7 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1969,8 +1969,8 @@ class Domain(TimeStampedModel, DomainHelper): # Q: Should we be deleting the newest or the oldest? Does it even matter? oldest_duplicate = db_contact.order_by("created_at").first() - # Exclude the oldest entry - duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) + # Exclude the oldest + duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) # noqa # Delete all duplicates duplicates_to_delete.delete() From c3db67bf1d50dbdbd91cae77a9d2c8b80602f698 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 May 2024 08:24:00 -0600 Subject: [PATCH 22/76] Add check for linter --- src/registrar/models/domain.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 92c38b8d7..359df1c27 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1969,8 +1969,13 @@ class Domain(TimeStampedModel, DomainHelper): # Q: Should we be deleting the newest or the oldest? Does it even matter? oldest_duplicate = db_contact.order_by("created_at").first() - # Exclude the oldest - duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) # noqa + # The linter wants this check on the id, though in practice + # this should be otherwise impossible. + if hasattr(oldest_duplicate, "id"): + # Exclude the oldest + duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) + else: + duplicates_to_delete = db_contact # Delete all duplicates duplicates_to_delete.delete() From 86717945dec79312b4c8cf1db87c4a99b6e0b3ae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 May 2024 08:31:46 -0600 Subject: [PATCH 23/76] Update domain.py --- src/registrar/models/domain.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 359df1c27..f2640e8ec 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1962,16 +1962,16 @@ class Domain(TimeStampedModel, DomainHelper): domain=self, ) - # If we find duplicates, log it and delete the newest one. + # If we find duplicates, log it and delete the newest ones. if db_contact.count() > 1: logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") # Q: Should we be deleting the newest or the oldest? Does it even matter? oldest_duplicate = db_contact.order_by("created_at").first() - # The linter wants this check on the id, though in practice + # The linter wants this check on the id / oldest_duplicate, though in practice # this should be otherwise impossible. - if hasattr(oldest_duplicate, "id"): + if oldest_duplicate is not None and hasattr(oldest_duplicate, "id"): # Exclude the oldest duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) else: @@ -1980,6 +1980,13 @@ class Domain(TimeStampedModel, DomainHelper): # Delete all duplicates duplicates_to_delete.delete() + # Do a second filter to grab the latest content + db_contact = PublicContact.objects.filter( + registry_id=public_contact.registry_id, + contact_type=public_contact.contact_type, + domain=self, + ) + # Save to DB if it doesn't exist already. if db_contact.count() == 0: # Doesn't run custom save logic, just saves to DB From 54c65a6d85ea49c9e9da101f32fc25aaf65f652d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 6 May 2024 12:16:09 -0400 Subject: [PATCH 24/76] fixed failing tests, formatted for linter --- src/registrar/models/utility/generic_helper.py | 2 +- src/registrar/tests/test_admin.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index d1d890da4..7d3586770 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -131,7 +131,7 @@ class CreateOrUpdateOrganizationTypeHelper: # Update the field self._update_fields(organization_type_needs_update, generic_org_type_needs_update) - except: + except self.sender.DoesNotExist: pass def _update_fields(self, organization_type_needs_update, generic_org_type_needs_update): diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index cf00994be..97e48279d 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -508,10 +508,9 @@ class TestDomainAdmin(MockEppLib, WebTest): domain_request.approve() response = self.client.get("/admin/registrar/domain/") - # There are 4 template references to Federal (4) plus four references in the table # for our actual domain_request - self.assertContains(response, "Federal", count=36) + self.assertContains(response, "Federal", count=48) # This may be a bit more robust self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist @@ -1267,7 +1266,7 @@ class TestDomainRequestAdmin(MockEppLib): response = self.client.get("/admin/registrar/domainrequest/?generic_org_type__exact=federal") # There are 2 template references to Federal (4) and two in the results data # of the request - self.assertContains(response, "Federal", count=34) + self.assertContains(response, "Federal", count=46) # This may be a bit more robust self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist From af3877f2e351d2df4b25d8a98b116a1b17582d71 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 07:35:02 -0400 Subject: [PATCH 25/76] unit tests completed --- src/registrar/tests/test_admin.py | 48 ++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 97e48279d..d8f4975e8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -21,6 +21,7 @@ from registrar.admin import ( MyHostAdmin, UserDomainRoleAdmin, VerifiedByStaffAdmin, + FsmModelResource, ) from registrar.models import ( Domain, @@ -52,7 +53,7 @@ from .common import ( ) from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model -from unittest.mock import ANY, call, patch +from unittest.mock import ANY, call, patch, Mock from unittest import skip from django.conf import settings @@ -62,6 +63,42 @@ import logging logger = logging.getLogger(__name__) +class TestFsmModelResource(TestCase): + def setUp(self): + self.resource = FsmModelResource() + + def test_init_instance(self): + """Test initializing an instance of a class with a FSM field""" + + # Mock a row with FSMField data + row_data = {'state': 'ready'} + + self.resource._meta.model = Domain + + instance = self.resource.init_instance(row=row_data) + + # Assert that the instance is initialized correctly + self.assertIsInstance(instance, Domain) + self.assertEqual(instance.state, 'ready') + + def test_import_field(self): + """Test that importing a field does not import FSM field""" + # Mock a field and object + field_mock = Mock(attribute='state') + obj_mock = Mock(_meta=Mock(fields=[Mock(name='state', spec=['name'], __class__=Mock)])) + + # Mock the super() method + super_mock = Mock() + self.resource.import_field = super_mock + + # Call the method with FSMField and non-FSMField + self.resource.import_field(field_mock, obj_mock, data={}, is_m2m=False) + + # Assert that super().import_field() is called only for non-FSMField + super_mock.assert_called_once_with(field_mock, obj_mock, data={}, is_m2m=False) + + + class TestDomainAdmin(MockEppLib, WebTest): # csrf checks do not work with WebTest. # We disable them here. TODO for another ticket. @@ -759,6 +796,15 @@ class TestDomainAdmin(MockEppLib, WebTest): def test_place_and_remove_hold_epp(self): raise + @override_settings(IS_PRODUCTION=True) + def test_prod_only_shows_export(self): + """Test that production environment only displays export""" + with less_console_noise(): + response = self.client.get("/admin/registrar/domain/") + self.assertContains(response, ">Export<") + # Now let's make sure the long description does not exist + self.assertNotContains(response, ">Import<") + def tearDown(self): super().tearDown() PublicContact.objects.all().delete() From 2d9f96c6d01b4759a35af31953215dc168466ef9 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 08:10:38 -0400 Subject: [PATCH 26/76] formatted for linter --- src/registrar/tests/test_admin.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 6dbff5d53..98ba994d8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -80,7 +80,7 @@ class TestFsmModelResource(TestCase): """Test initializing an instance of a class with a FSM field""" # Mock a row with FSMField data - row_data = {'state': 'ready'} + row_data = {"state": "ready"} self.resource._meta.model = Domain @@ -88,13 +88,13 @@ class TestFsmModelResource(TestCase): # Assert that the instance is initialized correctly self.assertIsInstance(instance, Domain) - self.assertEqual(instance.state, 'ready') + self.assertEqual(instance.state, "ready") def test_import_field(self): """Test that importing a field does not import FSM field""" # Mock a field and object - field_mock = Mock(attribute='state') - obj_mock = Mock(_meta=Mock(fields=[Mock(name='state', spec=['name'], __class__=Mock)])) + field_mock = Mock(attribute="state") + obj_mock = Mock(_meta=Mock(fields=[Mock(name="state", spec=["name"], __class__=Mock)])) # Mock the super() method super_mock = Mock() @@ -107,7 +107,6 @@ class TestFsmModelResource(TestCase): super_mock.assert_called_once_with(field_mock, obj_mock, data={}, is_m2m=False) - class TestDomainAdmin(MockEppLib, WebTest): # csrf checks do not work with WebTest. # We disable them here. TODO for another ticket. From aca85227ace3ef0152122e14052505333c70e73c Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 08:18:26 -0400 Subject: [PATCH 27/76] fixed tests --- src/registrar/tests/test_admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 98ba994d8..f901d5ce0 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -638,7 +638,7 @@ class TestDomainAdmin(MockEppLib, WebTest): response = self.client.get("/admin/registrar/domain/") # There are 4 template references to Federal (4) plus four references in the table # for our actual domain_request - self.assertContains(response, "Federal", count=48) + self.assertContains(response, "Federal", count=54) # This may be a bit more robust self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist @@ -1420,7 +1420,7 @@ class TestDomainRequestAdmin(MockEppLib): response = self.client.get("/admin/registrar/domainrequest/?generic_org_type__exact=federal") # There are 2 template references to Federal (4) and two in the results data # of the request - self.assertContains(response, "Federal", count=46) + self.assertContains(response, "Federal", count=52) # This may be a bit more robust self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist From cb5d9d2368505f99549be8f27c8034aea8282b1f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 May 2024 08:47:32 -0600 Subject: [PATCH 28/76] PR suggestions --- src/registrar/models/domain.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index f2640e8ec..4c9028bb4 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -846,7 +846,7 @@ class Domain(TimeStampedModel, DomainHelper): # get publicContact objects that have the matching # domain and type but a different id - # like in highlander we there can only be one + # like in highlander where there can only be one duplicate_contacts = PublicContact.objects.exclude(registry_id=contact.registry_id).filter( domain=self, contact_type=contact.contact_type ) @@ -1962,20 +1962,13 @@ class Domain(TimeStampedModel, DomainHelper): domain=self, ) - # If we find duplicates, log it and delete the newest ones. + # If we find duplicates, log it and delete the oldest ones. if db_contact.count() > 1: logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.") - # Q: Should we be deleting the newest or the oldest? Does it even matter? - oldest_duplicate = db_contact.order_by("created_at").first() + newest_duplicate = db_contact.order_by("-created_at").first() - # The linter wants this check on the id / oldest_duplicate, though in practice - # this should be otherwise impossible. - if oldest_duplicate is not None and hasattr(oldest_duplicate, "id"): - # Exclude the oldest - duplicates_to_delete = db_contact.exclude(id=oldest_duplicate.id) - else: - duplicates_to_delete = db_contact + duplicates_to_delete = db_contact.exclude(id=newest_duplicate.id) # type: ignore # Delete all duplicates duplicates_to_delete.delete() From 185dabe72dee459198a45bdd16053a671420859d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 May 2024 08:52:02 -0600 Subject: [PATCH 29/76] Fix migrations after merge with latest --- ...ogether.py => 0091_alter_publiccontact_unique_together.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/registrar/migrations/{0090_alter_publiccontact_unique_together.py => 0091_alter_publiccontact_unique_together.py} (73%) diff --git a/src/registrar/migrations/0090_alter_publiccontact_unique_together.py b/src/registrar/migrations/0091_alter_publiccontact_unique_together.py similarity index 73% rename from src/registrar/migrations/0090_alter_publiccontact_unique_together.py rename to src/registrar/migrations/0091_alter_publiccontact_unique_together.py index a476bfa04..ff5a18beb 100644 --- a/src/registrar/migrations/0090_alter_publiccontact_unique_together.py +++ b/src/registrar/migrations/0091_alter_publiccontact_unique_together.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-05-03 17:06 +# Generated by Django 4.2.10 on 2024-05-07 14:51 from django.db import migrations @@ -6,7 +6,7 @@ from django.db import migrations class Migration(migrations.Migration): dependencies = [ - ("registrar", "0089_user_verification_type"), + ("registrar", "0090_waffleflag"), ] operations = [ From fcef18658cb1335e5d0802aca9bf8016974e469f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 11:08:46 -0400 Subject: [PATCH 30/76] adding newline to a html template --- .../templates/admin/import_export/change_list_import_item.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/admin/import_export/change_list_import_item.html b/src/registrar/templates/admin/import_export/change_list_import_item.html index b640331cb..8255a8ba7 100644 --- a/src/registrar/templates/admin/import_export/change_list_import_item.html +++ b/src/registrar/templates/admin/import_export/change_list_import_item.html @@ -5,4 +5,4 @@ {% if not IS_PRODUCTION %}
  • {% trans "Import" %}
    • {% endif %} -{% endif %} \ No newline at end of file +{% endif %} From 8b1d692457a6ce66d1a5d4f2dac277d04dd4720d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 12:18:57 -0400 Subject: [PATCH 31/76] updated unit test --- src/registrar/tests/test_admin.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index f901d5ce0..b14016cf8 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -92,19 +92,27 @@ class TestFsmModelResource(TestCase): def test_import_field(self): """Test that importing a field does not import FSM field""" - # Mock a field and object - field_mock = Mock(attribute="state") - obj_mock = Mock(_meta=Mock(fields=[Mock(name="state", spec=["name"], __class__=Mock)])) - # Mock the super() method - super_mock = Mock() - self.resource.import_field = super_mock + # Mock a FSMField and a non-FSM-field + fsm_field_mock = Mock(attribute="state", column_name="state") + field_mock = Mock(attribute="name", column_name="name") + # Mock the data + data_mock = {"state": "unknown", "name": "test"} + # Define a mock Domain + obj = Domain(state=Domain.State.UNKNOWN, name="test") + + # Mock the save() method of fields so that we can test if save is called + # save() is only supposed to be called for non FSM fields + field_mock.save = Mock() + fsm_field_mock.save = Mock() # Call the method with FSMField and non-FSMField - self.resource.import_field(field_mock, obj_mock, data={}, is_m2m=False) + self.resource.import_field(fsm_field_mock, obj, data=data_mock, is_m2m=False) + self.resource.import_field(field_mock, obj, data=data_mock, is_m2m=False) - # Assert that super().import_field() is called only for non-FSMField - super_mock.assert_called_once_with(field_mock, obj_mock, data={}, is_m2m=False) + # Assert that field.save() in super().import_field() is called only for non-FSMField + field_mock.save.assert_called_once() + fsm_field_mock.save.assert_not_called() class TestDomainAdmin(MockEppLib, WebTest): @@ -893,7 +901,6 @@ class TestDomainAdmin(MockEppLib, WebTest): with less_console_noise(): response = self.client.get("/admin/registrar/domain/") self.assertContains(response, ">Export<") - # Now let's make sure the long description does not exist self.assertNotContains(response, ">Import<") def tearDown(self): From ab5b0be46641957c2985af3dff9e8296ab844517 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 13:23:30 -0400 Subject: [PATCH 32/76] included DraftDomain, Websites and Host --- docs/operations/import_export.md | 11 +++++---- src/registrar/admin.py | 41 ++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/docs/operations/import_export.md b/docs/operations/import_export.md index 3e57e5152..1b413809c 100644 --- a/docs/operations/import_export.md +++ b/docs/operations/import_export.md @@ -20,6 +20,9 @@ need to be exported: * DomainRequest * DomainInformation * DomainUserRole +* DraftDomain +* Websites +* Host ### Import @@ -37,9 +40,6 @@ Delete all rows from tables in the following order through django admin: * Domain * User (all but the current user) * Contact - -It may not be necessary, but advisable to also remove rows from these tables: - * Websites * DraftDomain * Host @@ -49,8 +49,11 @@ It may not be necessary, but advisable to also remove rows from these tables: Once target environment is prepared, files can be imported in the following order: -* User +* User (After importing User table, you need to delete all rows from Contact table before importing Contacts) * Contact +* Host +* DraftDomain +* Websites * Domain * DomainRequest * DomainInformation diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 356286936..040039e1e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -54,18 +54,15 @@ class FsmModelResource(resources.ModelResource): from data in the row.""" # Get fields which are fsm fields - fsm_fields = [] + fsm_fields = {} for f in self._meta.model._meta.fields: if isinstance(f, FSMField): if row and f.name in row: - fsm_fields.append((f.name, row[f.name])) + fsm_fields[f.name] = row[f.name] - # Convert fsm_fields fields_and_values to kwargs - kwargs = dict(fsm_fields) - - # Initialize model instance with kwargs - return self._meta.model(**kwargs) + # Initialize model instance with fsm_fields + return self._meta.model(**fsm_fields) def import_field(self, field, obj, data, is_m2m=False, **kwargs): """Overrides the import_field method of ModelResource. If the @@ -760,9 +757,17 @@ class HostIPInline(admin.StackedInline): model = models.HostIP -class MyHostAdmin(AuditedAdmin): +class HostResource(resources.ModelResource): + + class Meta: + model = models.Host + + +class MyHostAdmin(AuditedAdmin, ImportExportModelAdmin): """Custom host admin class to use our inlines.""" + resource_classes = [HostResource] + search_fields = ["name", "domain__name"] search_help_text = "Search by domain or host name." inlines = [HostIPInline] @@ -899,9 +904,17 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().change_view(request, object_id, form_url, extra_context=extra_context) -class WebsiteAdmin(ListHeaderAdmin): +class WebsiteResource(resources.ModelResource): + + class Meta: + model = models.Website + + +class WebsiteAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom website admin class.""" + resource_classes = [WebsiteResource] + # Search search_fields = [ "website", @@ -2139,9 +2152,17 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): return super().has_change_permission(request, obj) -class DraftDomainAdmin(ListHeaderAdmin): +class DraftDomainResource(resources.ModelResource): + + class Meta: + model = models.DraftDomain + + +class DraftDomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom draft domain admin class.""" + resource_classes = [DraftDomainResource] + search_fields = ["name"] search_help_text = "Search by draft domain name." From 5ee316b7afda229eb12d3bc82277e6d0ba65347b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 13:28:04 -0400 Subject: [PATCH 33/76] reformatted for linter --- 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 040039e1e..bcda7e048 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -762,7 +762,7 @@ class HostResource(resources.ModelResource): class Meta: model = models.Host - + class MyHostAdmin(AuditedAdmin, ImportExportModelAdmin): """Custom host admin class to use our inlines.""" From 943b14fe010a4a03aa5c31bb86911355969c7221 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 7 May 2024 13:51:03 -0400 Subject: [PATCH 34/76] updated documentation --- docs/operations/import_export.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/operations/import_export.md b/docs/operations/import_export.md index 1b413809c..897e8a01a 100644 --- a/docs/operations/import_export.md +++ b/docs/operations/import_export.md @@ -51,10 +51,10 @@ order: * User (After importing User table, you need to delete all rows from Contact table before importing Contacts) * Contact +* Domain * Host * DraftDomain * Websites -* Domain * DomainRequest * DomainInformation * UserDomainRole \ No newline at end of file From 78abd5b1f9abf06556b79713751ea65ec9cc29a3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 May 2024 14:49:48 -0600 Subject: [PATCH 35/76] Structure --- docs/developer/README.md | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 9421d5856..027fe9d1b 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -320,9 +320,6 @@ it may help to resync your laptop with time.nist.gov: sudo sntp -sS time.nist.gov ``` -## Connection pool -To handle our connection to the registry, we utilize a connection pool to keep a socket open to increase responsiveness. In order to accomplish this, we are utilizing a heavily modified version of the [geventconnpool](https://github.com/rasky/geventconnpool) library. - ### Settings The config for the connection pool exists inside the `settings.py` file. | Name | Purpose | @@ -333,20 +330,6 @@ The config for the connection pool exists inside the `settings.py` file. Consider updating the `POOL_TIMEOUT` or `POOL_KEEP_ALIVE` periods if the pool often restarts. If the pool only restarts after a period of inactivity, update `POOL_KEEP_ALIVE`. If it restarts during the EPP call itself, then `POOL_TIMEOUT` needs to be updated. -### Test if the connection pool is running -Our connection pool has a built-in `pool_status` object which you can call at anytime to assess the current connection status of the pool. Follow these steps to access it. - -1. `cf ssh getgov-{env-name} -i {instance-index}` -* env-name -> Which environment to target, e.g. `staging` -* instance-index -> Which instance to target. For instance, `cf ssh getgov-staging -i 0` -2. `/tmp/lifecycle/shell` -3. `./manage.py shell` -4. `from epplibwrapper import CLIENT as registry, commands` -5. `print(registry.pool_status.connection_success)` -* Should return true - -If you have multiple instances (staging for example), then repeat commands 1-5 for each instance you want to test. - ## Adding a S3 instance to your sandbox This can either be done through the CLI, or through the cloud.gov dashboard. Generally, it is better to do it through the dashboard as it handles app binding for you. @@ -378,4 +361,15 @@ You can view these variables by running the following command: cf env getgov-{app name} ``` -Then, copy the variables under the section labled `s3`. \ No newline at end of file +Then, copy the variables under the section labled `s3`. + +## Signals +Though minimally, our application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/) for a select few models to manage `user <---> contact` interaction. In particular, we use a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals). + +Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) would be sufficient. + +### When should you use signals? + +### Where should you use them? + +### Why use signals at all? \ No newline at end of file From 27ac39ce4ff10273aeb9cf29e5238deb88342f12 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 May 2024 14:55:30 -0600 Subject: [PATCH 36/76] The where --- docs/developer/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 027fe9d1b..42a4a489d 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -366,10 +366,11 @@ Then, copy the variables under the section labled `s3`. ## Signals Though minimally, our application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/) for a select few models to manage `user <---> contact` interaction. In particular, we use a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals). -Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) would be sufficient. +Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are be sufficient. ### When should you use signals? ### Where should you use them? +This project compiles signals in a unified location to maintain readability. If you are adding a signal, you should always define them in [signals.py](link to signals.py). The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). With the exception of rare circumstances (such as import loops), this should be adhered to for the reasons mentioned above. ### Why use signals at all? \ No newline at end of file From 61e39420cff1cbf200e6b56c0f0d79ecb7453c7f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 May 2024 15:11:09 -0600 Subject: [PATCH 37/76] Basic documentation --- docs/developer/README.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 42a4a489d..2840fd75e 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -369,8 +369,16 @@ Though minimally, our application uses [Django signals](https://docs.djangoproje Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are be sufficient. ### When should you use signals? +(TODO - do some prelim research on this) +Generally, you should use signals in two scenarios: +1. When you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions +2. You need to perform some logic before or after a model is saved to the DB. (TODO improve this section) ### Where should you use them? This project compiles signals in a unified location to maintain readability. If you are adding a signal, you should always define them in [signals.py](link to signals.py). The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). With the exception of rare circumstances (such as import loops), this should be adhered to for the reasons mentioned above. -### Why use signals at all? \ No newline at end of file +### How are we currently using signals? +To keep our signal usage coherent and well-documented, add to this document when a new function is added for ease of reference and use. + +#### Function handle_profile +This function hooks to the post_save event on the `User` model to sync the user object and the contact object. It will either create a new one, or update an existing one by linking the contact object to the incoming user object. \ No newline at end of file From 9311a41ac6ee5d883f58d0b548d1cc3841f9bf04 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 7 May 2024 16:31:01 -0700 Subject: [PATCH 38/76] Exclude 2 values from the domain request on the form --- src/registrar/forms/domain_request_wizard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 9d16a30de..0e9e87f9d 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -97,6 +97,7 @@ class OrganizationElectionForm(RegistrarForm): class OrganizationContactForm(RegistrarForm): # for federal agencies we also want to know the top-level agency. + excluded_agencies = ["gov Administration", "Non-Federal Agency"] federal_agency = forms.ModelChoiceField( label="Federal agency", # not required because this field won't be filled out unless @@ -104,9 +105,8 @@ class OrganizationContactForm(RegistrarForm): # if it has been filled in when required. # uncomment to see if modelChoiceField can be an arg later required=False, - queryset=FederalAgency.objects.all(), + queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies), empty_label="--Select--", - # choices=[("", "--Select--")] + DomainRequest.AGENCY_CHOICES, ) organization_name = forms.CharField( label="Organization name", From e32dff775feb5ec17ecf505e0c7d0a524e32e6c4 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Tue, 7 May 2024 17:05:14 -0700 Subject: [PATCH 39/76] Add tests verifying excluded federal agencies in domain request form --- src/registrar/tests/test_views_request.py | 48 ++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 272464133..3be733388 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -717,12 +717,58 @@ class DomainRequestTests(TestWithUser, WebTest): type_form["generic_org_type-generic_org_type"] = DomainRequest.OrganizationChoices.SPECIAL_DISTRICT self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) type_result = type_page.forms[0].submit() - # follow first redirect + # follow first redirectt self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) contact_page = type_result.follow() self.assertContains(contact_page, self.TITLES[Step.ABOUT_YOUR_ORGANIZATION]) + def test_federal_agency_dropdown_excludes_expected_values(self): + """The Federal Agency dropdown on a domain request form should not + include options for gov Administration or Non-Federal Agency""" + intro_page = self.app.get(reverse("domain-request:")) + # django-webtest does not handle cookie-based sessions well because it keeps + # resetting the session key on each new request, thus destroying the concept + # of a "session". We are going to do it manually, saving the session ID here + # and then setting the cookie on each request. + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + intro_form = intro_page.forms[0] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + intro_result = intro_form.submit() + + # follow first redirect + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + type_page = intro_result.follow() + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + + # ---- TYPE PAGE ---- + type_form = type_page.forms[0] + type_form["generic_org_type-generic_org_type"] = "federal" + + # test next button + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + type_result = type_form.submit() + + # ---- FEDERAL BRANCH PAGE ---- + # Follow the redirect to the next form page + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + federal_page = type_result.follow() + federal_form = federal_page.forms[0] + federal_form["organization_federal-federal_type"] = "executive" + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + federal_result = federal_form.submit() + + # ---- ORG CONTACT PAGE ---- + # Follow the redirect to the next form page + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + org_contact_page = federal_result.follow() + + # gov Administration and Non-Federal Agency should not be federal agency options + self.assertNotContains(org_contact_page, "gov Administration") + self.assertNotContains(org_contact_page, "Non-Federal Agency") + self.assertContains(org_contact_page, "General Services Administration") + def test_yes_no_contact_form_inits_blank_for_new_domain_request(self): """On the Other Contacts page, the yes/no form gets initialized with nothing selected for new domain requests""" From 8b2671e7f441ad58a486e3d3ef0d104ab66d7cba Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 09:26:22 -0600 Subject: [PATCH 40/76] Add documentation --- docs/developer/README.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 2840fd75e..d391c604f 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -368,17 +368,29 @@ Though minimally, our application uses [Django signals](https://docs.djangoproje Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are be sufficient. +In other words, signals + ### When should you use signals? -(TODO - do some prelim research on this) -Generally, you should use signals in two scenarios: -1. When you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions -2. You need to perform some logic before or after a model is saved to the DB. (TODO improve this section) +Generally, you would use signals when you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions. + +However, in most scenarios, if you can get away with not using signals - you should. + +Consider using signals when: +1. Synchronizing events across multiple models or areas of code. +2. Performing logic before or after saving a model to the database (when otherwise difficult through `save()`). +3. Encountering an import loop when overriding functions such as `save()`. +4. You are otherwise unable to achieve the intended behavior by overriding `save()` or `__init__` methods. +5. (Rare) Offloading tasks when using multi-threading. ### Where should you use them? -This project compiles signals in a unified location to maintain readability. If you are adding a signal, you should always define them in [signals.py](link to signals.py). The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). With the exception of rare circumstances (such as import loops), this should be adhered to for the reasons mentioned above. +This project compiles signals in a unified location to maintain readability. If you are adding a signal, you should always define them in [signals.py](../../src/registrar/signals.py). The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). With the exception of rare circumstancee, this should be adhered to for the reasons mentioned above. ### How are we currently using signals? To keep our signal usage coherent and well-documented, add to this document when a new function is added for ease of reference and use. -#### Function handle_profile -This function hooks to the post_save event on the `User` model to sync the user object and the contact object. It will either create a new one, or update an existing one by linking the contact object to the incoming user object. \ No newline at end of file +#### handle_profile +This function is triggered by the post_save event on the User model, designed to manage the synchronization between User and Contact entities. It operates under the following conditions: + +1. For New Users: Upon the creation of a new user, it checks for an existing `Contact` by email. If no matching contact is found, it creates a new Contact using the user's details from Login.gov. If a matching contact is found, it associates this contact with the user. In cases where multiple contacts with the same email exist, it logs a warning and associates the first contact found. + +2. For Existing Users: For users logging in subsequent times, the function ensures that any updates from Login.gov are applied to the associated User record. However, it does not alter any existing Contact records. From 9fefbf16c5d254b68f021a5f2af39a13f1de0291 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 09:37:23 -0600 Subject: [PATCH 41/76] Add documentation --- docs/developer/README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index d391c604f..50279b21b 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -368,12 +368,14 @@ Though minimally, our application uses [Django signals](https://docs.djangoproje Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are be sufficient. -In other words, signals +In other words, signals are a mechanism that allows different parts of an application to communicate with each other by sending and receiving notifications when said actions occur. + +When an event occurs (such as creating, updating, or deleting a record) signals can automatically trigger specific actions in response. This allows different parts of an application to stay synchronized without tightly coupling the component. ### When should you use signals? Generally, you would use signals when you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions. -However, in most scenarios, if you can get away with not using signals - you should. +However, in most scenarios, if you can get away with _not_ using signals - you should. The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). Consider using signals when: 1. Synchronizing events across multiple models or areas of code. @@ -383,9 +385,11 @@ Consider using signals when: 5. (Rare) Offloading tasks when using multi-threading. ### Where should you use them? -This project compiles signals in a unified location to maintain readability. If you are adding a signal, you should always define them in [signals.py](../../src/registrar/signals.py). The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). With the exception of rare circumstancee, this should be adhered to for the reasons mentioned above. +This project compiles signals in a unified location to maintain readability. If you are adding a signal, you should always define them in [signals.py](../../src/registrar/signals.py). Except under rare circumstances, this should be adhered to for the reasons mentioned above. ### How are we currently using signals? +At the time of writing, we currently only use signals for the Contact and User objects when synchronizing data returned from Login.gov. This is because the `Contact` object holds information that the user specified in our system, whereas the `User` object holds information that was specified in Login.gov. + To keep our signal usage coherent and well-documented, add to this document when a new function is added for ease of reference and use. #### handle_profile From a93ba61f3060d7a0712d8031a2445fbf15799504 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 09:38:02 -0600 Subject: [PATCH 42/76] Update README.md --- docs/developer/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 50279b21b..6e3e3acb2 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -364,7 +364,7 @@ cf env getgov-{app name} Then, copy the variables under the section labled `s3`. ## Signals -Though minimally, our application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/) for a select few models to manage `user <---> contact` interaction. In particular, we use a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals). +Though minimally, our application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/). In particular, we use a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals). Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are be sufficient. From cf7e4420939f137c002abf1a020e80482ef4a5c5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 09:46:59 -0600 Subject: [PATCH 43/76] Update docs/developer/README.md --- docs/developer/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 6e3e3acb2..7d8532bc8 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -366,7 +366,7 @@ Then, copy the variables under the section labled `s3`. ## Signals Though minimally, our application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/). In particular, we use a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals). -Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are be sufficient. +Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are sufficient. In other words, signals are a mechanism that allows different parts of an application to communicate with each other by sending and receiving notifications when said actions occur. From 278fc28ad2f40edf5ad6fd57ece29e1b12639b89 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 10:01:46 -0600 Subject: [PATCH 44/76] Add additional documentation --- docs/developer/README.md | 14 +++++++------- src/registrar/models/contact.py | 6 +++++- src/registrar/models/user.py | 2 ++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 6e3e3acb2..d0a8be136 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -364,18 +364,16 @@ cf env getgov-{app name} Then, copy the variables under the section labled `s3`. ## Signals -Though minimally, our application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/). In particular, we use a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals). +The application uses [Django signals](https://docs.djangoproject.com/en/5.0/topics/signals/). In particular, it uses a subset of prebuilt signals called [model signals](https://docs.djangoproject.com/en/5.0/ref/signals/#module-django.db.models.signals). -Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" For the vast majority of our use cases, [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) or [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) are be sufficient. +Per Django, signals "[...allow certain senders to notify a set of receivers that some action has taken place.](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch)" -In other words, signals are a mechanism that allows different parts of an application to communicate with each other by sending and receiving notifications when said actions occur. - -When an event occurs (such as creating, updating, or deleting a record) signals can automatically trigger specific actions in response. This allows different parts of an application to stay synchronized without tightly coupling the component. +In other words, signals are a mechanism that allows different parts of an application to communicate with each other by sending and receiving notifications when events occur. When an event occurs (such as creating, updating, or deleting a record), signals can automatically trigger specific actions in response. This allows different parts of an application to stay synchronized without tightly coupling the component. ### When should you use signals? Generally, you would use signals when you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions. -However, in most scenarios, if you can get away with _not_ using signals - you should. The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). +However, in most scenarios, if you can get away with avoiding signals - you should. The reasoning for this is that [signals give the appearance of loose coupling, but they can quickly lead to code that is hard to understand, adjust and debug](https://docs.djangoproject.com/en/5.0/topics/signals/#module-django.dispatch). Consider using signals when: 1. Synchronizing events across multiple models or areas of code. @@ -384,8 +382,10 @@ Consider using signals when: 4. You are otherwise unable to achieve the intended behavior by overriding `save()` or `__init__` methods. 5. (Rare) Offloading tasks when using multi-threading. +For the vast majority of use cases, the [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) and [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) signals are sufficient in terms of model-to-model management. + ### Where should you use them? -This project compiles signals in a unified location to maintain readability. If you are adding a signal, you should always define them in [signals.py](../../src/registrar/signals.py). Except under rare circumstances, this should be adhered to for the reasons mentioned above. +This project compiles signals in a unified location to maintain readability. If you are adding a signal or otherwise utilizing one, you should always define them in [signals.py](../../src/registrar/signals.py). Except under rare circumstances, this should be adhered to for the reasons mentioned above. ### How are we currently using signals? At the time of writing, we currently only use signals for the Contact and User objects when synchronizing data returned from Login.gov. This is because the `Contact` object holds information that the user specified in our system, whereas the `User` object holds information that was specified in Login.gov. diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 3ebd8bc3e..6b2344a57 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -6,7 +6,11 @@ from phonenumber_field.modelfields import PhoneNumberField # type: ignore class Contact(TimeStampedModel): - """Contact information follows a similar pattern for each contact.""" + """ + Contact information follows a similar pattern for each contact. + + This model uses signals [as defined in [signals.py](../../src/registrar/signals.py)]. + """ user = models.OneToOneField( "registrar.User", diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 5e4c88f63..9385b6855 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -22,6 +22,8 @@ class User(AbstractUser): """ A custom user model that performs identically to the default user model but can be customized later. + + This model uses signals [as defined in [signals.py](../../src/registrar/signals.py)]. """ class VerificationTypeChoices(models.TextChoices): From fad90aaee777c36d67ce0ec66a8f59b173a4c1fb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 10:12:42 -0600 Subject: [PATCH 45/76] Finish documentation --- docs/developer/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/developer/README.md b/docs/developer/README.md index d0a8be136..d293abf5b 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -398,3 +398,11 @@ This function is triggered by the post_save event on the User model, designed to 1. For New Users: Upon the creation of a new user, it checks for an existing `Contact` by email. If no matching contact is found, it creates a new Contact using the user's details from Login.gov. If a matching contact is found, it associates this contact with the user. In cases where multiple contacts with the same email exist, it logs a warning and associates the first contact found. 2. For Existing Users: For users logging in subsequent times, the function ensures that any updates from Login.gov are applied to the associated User record. However, it does not alter any existing Contact records. + +## Rules of use + +When using signals, try to adhere to these guidelines: +1. Document its usage in this readme (or another centralized location), as well as briefly on the underlying class it is associated with. For instance, since the `handle_profile` directly affects the class `Contact`, the class description notes this and links to [signals.py](../../src/registrar/signals.py). +2. Where possible, avoid chaining signals together (i.e. a signal that calls a signal). If this has to be done, clearly document the flow. +3. Minimize logic complexity within the signal as much as possible. +4. Don't use signals when you can use another method, such as an override of `save()` or `__init__`. \ No newline at end of file From 3cf0792d15212006065ef06d1956095ef7f208b4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 10:16:31 -0600 Subject: [PATCH 46/76] Update README.md --- docs/developer/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index d293abf5b..de4698d2b 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -379,8 +379,8 @@ Consider using signals when: 1. Synchronizing events across multiple models or areas of code. 2. Performing logic before or after saving a model to the database (when otherwise difficult through `save()`). 3. Encountering an import loop when overriding functions such as `save()`. -4. You are otherwise unable to achieve the intended behavior by overriding `save()` or `__init__` methods. -5. (Rare) Offloading tasks when using multi-threading. +4. You are otherwise unable to achieve the intended behavior by overrides or other means. +5. (Rare) Offloading tasks when multi-threading. For the vast majority of use cases, the [pre_save](https://docs.djangoproject.com/en/5.0/ref/signals/#pre-save) and [post_save](https://docs.djangoproject.com/en/5.0/ref/signals/#post-save) signals are sufficient in terms of model-to-model management. From d334f3729392143f9c00acd2c8ef7539d5d152e2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 10:17:20 -0600 Subject: [PATCH 47/76] Update README.md --- docs/developer/README.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index de4698d2b..7730b9314 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -370,6 +370,13 @@ Per Django, signals "[...allow certain senders to notify a set of receivers that In other words, signals are a mechanism that allows different parts of an application to communicate with each other by sending and receiving notifications when events occur. When an event occurs (such as creating, updating, or deleting a record), signals can automatically trigger specific actions in response. This allows different parts of an application to stay synchronized without tightly coupling the component. +## Rules of use +When using signals, try to adhere to these guidelines: +1. Document its usage in this readme (or another centralized location), as well as briefly on the underlying class it is associated with. For instance, since the `handle_profile` directly affects the class `Contact`, the class description notes this and links to [signals.py](../../src/registrar/signals.py). +2. Where possible, avoid chaining signals together (i.e. a signal that calls a signal). If this has to be done, clearly document the flow. +3. Minimize logic complexity within the signal as much as possible. +4. Don't use signals when you can use another method, such as an override of `save()` or `__init__`. + ### When should you use signals? Generally, you would use signals when you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions. @@ -398,11 +405,3 @@ This function is triggered by the post_save event on the User model, designed to 1. For New Users: Upon the creation of a new user, it checks for an existing `Contact` by email. If no matching contact is found, it creates a new Contact using the user's details from Login.gov. If a matching contact is found, it associates this contact with the user. In cases where multiple contacts with the same email exist, it logs a warning and associates the first contact found. 2. For Existing Users: For users logging in subsequent times, the function ensures that any updates from Login.gov are applied to the associated User record. However, it does not alter any existing Contact records. - -## Rules of use - -When using signals, try to adhere to these guidelines: -1. Document its usage in this readme (or another centralized location), as well as briefly on the underlying class it is associated with. For instance, since the `handle_profile` directly affects the class `Contact`, the class description notes this and links to [signals.py](../../src/registrar/signals.py). -2. Where possible, avoid chaining signals together (i.e. a signal that calls a signal). If this has to be done, clearly document the flow. -3. Minimize logic complexity within the signal as much as possible. -4. Don't use signals when you can use another method, such as an override of `save()` or `__init__`. \ No newline at end of file From adecbdcfc745619739af9284300938d321c7a513 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 10:24:09 -0600 Subject: [PATCH 48/76] Add brief overview on contact/user --- src/registrar/models/contact.py | 5 +++++ src/registrar/models/user.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 6b2344a57..28277df23 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -10,6 +10,11 @@ class Contact(TimeStampedModel): Contact information follows a similar pattern for each contact. This model uses signals [as defined in [signals.py](../../src/registrar/signals.py)]. + When a new user is created through Login.gov, a contact object will be created and + associated on the `user` field. + + If the `user` object already exists, the underlying user object + will be updated if any updates are made to it through Login.gov. """ user = models.OneToOneField( diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 9385b6855..38f9dc05b 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -24,6 +24,11 @@ class User(AbstractUser): but can be customized later. This model uses signals [as defined in [signals.py](../../src/registrar/signals.py)]. + When a new user is created through Login.gov, a contact object will be created and + associated on the contacts `user` field. + + If the `user` object already exists, said user object + will be updated if any updates are made to it through Login.gov. """ class VerificationTypeChoices(models.TextChoices): From 7c3e03a2405258cf0df7691e9559b00d04e79731 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 10:28:10 -0600 Subject: [PATCH 49/76] Linting --- src/registrar/models/contact.py | 6 +++--- src/registrar/models/user.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index 28277df23..a5a6ff16c 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -8,12 +8,12 @@ from phonenumber_field.modelfields import PhoneNumberField # type: ignore class Contact(TimeStampedModel): """ Contact information follows a similar pattern for each contact. - + This model uses signals [as defined in [signals.py](../../src/registrar/signals.py)]. - When a new user is created through Login.gov, a contact object will be created and + When a new user is created through Login.gov, a contact object will be created and associated on the `user` field. - If the `user` object already exists, the underlying user object + If the `user` object already exists, the underlying user object will be updated if any updates are made to it through Login.gov. """ diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 38f9dc05b..8a9fe425f 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -24,10 +24,10 @@ class User(AbstractUser): but can be customized later. This model uses signals [as defined in [signals.py](../../src/registrar/signals.py)]. - When a new user is created through Login.gov, a contact object will be created and + When a new user is created through Login.gov, a contact object will be created and associated on the contacts `user` field. - If the `user` object already exists, said user object + If the `user` object already exists, said user object will be updated if any updates are made to it through Login.gov. """ From 5f9edd78cbe59e30c8a7aa5a68e6a7639cf4ef59 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 11:35:25 -0600 Subject: [PATCH 50/76] Add migration --- ...ogether.py => 0093_alter_publiccontact_unique_together.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/registrar/migrations/{0091_alter_publiccontact_unique_together.py => 0093_alter_publiccontact_unique_together.py} (65%) diff --git a/src/registrar/migrations/0091_alter_publiccontact_unique_together.py b/src/registrar/migrations/0093_alter_publiccontact_unique_together.py similarity index 65% rename from src/registrar/migrations/0091_alter_publiccontact_unique_together.py rename to src/registrar/migrations/0093_alter_publiccontact_unique_together.py index ff5a18beb..08c71e122 100644 --- a/src/registrar/migrations/0091_alter_publiccontact_unique_together.py +++ b/src/registrar/migrations/0093_alter_publiccontact_unique_together.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-05-07 14:51 +# Generated by Django 4.2.10 on 2024-05-08 17:35 from django.db import migrations @@ -6,7 +6,7 @@ from django.db import migrations class Migration(migrations.Migration): dependencies = [ - ("registrar", "0090_waffleflag"), + ("registrar", "0092_rename_updated_federal_agency_domaininformation_federal_agency_and_more"), ] operations = [ From c52a7ee0a987a27eaff44f953b1e60ea450ac7e3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 13:19:53 -0600 Subject: [PATCH 51/76] Swap order --- docs/developer/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 7730b9314..4e6293854 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -372,10 +372,10 @@ In other words, signals are a mechanism that allows different parts of an applic ## Rules of use When using signals, try to adhere to these guidelines: -1. Document its usage in this readme (or another centralized location), as well as briefly on the underlying class it is associated with. For instance, since the `handle_profile` directly affects the class `Contact`, the class description notes this and links to [signals.py](../../src/registrar/signals.py). -2. Where possible, avoid chaining signals together (i.e. a signal that calls a signal). If this has to be done, clearly document the flow. -3. Minimize logic complexity within the signal as much as possible. -4. Don't use signals when you can use another method, such as an override of `save()` or `__init__`. +1. Don't use signals when you can use another method, such as an override of `save()` or `__init__`. +2. Document its usage in this readme (or another centralized location), as well as briefly on the underlying class it is associated with. For instance, since the `handle_profile` directly affects the class `Contact`, the class description notes this and links to [signals.py](../../src/registrar/signals.py). +3. Where possible, avoid chaining signals together (i.e. a signal that calls a signal). If this has to be done, clearly document the flow. +4. Minimize logic complexity within the signal as much as possible. ### When should you use signals? Generally, you would use signals when you want an event to be synchronized across multiple areas of code at once (such as with two models or more models at once) in a way that would otherwise be difficult to achieve by overriding functions. From 8bd8be7fc48472f16774f3d002068b8bad7c1a17 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 13:22:57 -0600 Subject: [PATCH 52/76] Update README.md --- docs/developer/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index 4e6293854..31a94e6e7 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -370,7 +370,7 @@ Per Django, signals "[...allow certain senders to notify a set of receivers that In other words, signals are a mechanism that allows different parts of an application to communicate with each other by sending and receiving notifications when events occur. When an event occurs (such as creating, updating, or deleting a record), signals can automatically trigger specific actions in response. This allows different parts of an application to stay synchronized without tightly coupling the component. -## Rules of use +### Rules of use When using signals, try to adhere to these guidelines: 1. Don't use signals when you can use another method, such as an override of `save()` or `__init__`. 2. Document its usage in this readme (or another centralized location), as well as briefly on the underlying class it is associated with. For instance, since the `handle_profile` directly affects the class `Contact`, the class description notes this and links to [signals.py](../../src/registrar/signals.py). From 6022cd051fd8e11887dc9e517f2be05b76b0df52 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 13:50:59 -0600 Subject: [PATCH 53/76] Add first ready on field --- src/registrar/utility/csv_export.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 8787f9e74..7fcbae475 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -96,6 +96,7 @@ def parse_row_for_domain( FIELDS = { "Domain name": domain.name, "Status": domain.get_state_display(), + "First ready on": domain.first_ready, "Expiration date": domain.expiration_date, "Domain type": domain_type, "Agency": domain_info.federal_agency, @@ -106,7 +107,6 @@ def parse_row_for_domain( "AO email": domain_info.authorizing_official.email if domain_info.authorizing_official else " ", "Security contact email": security_email, "Created at": domain.created_at, - "First ready": domain.first_ready, "Deleted": domain.deleted, } @@ -378,13 +378,17 @@ def write_csv_for_requests( def export_data_type_to_csv(csv_file): - """All domains report with extra columns""" + """ + All domains report with extra columns. + This maps to the "All domain metadata" button. + """ writer = csv.writer(csv_file) # define columns to include in export columns = [ "Domain name", "Status", + "First ready on", "Expiration date", "Domain type", "Agency", From 6a3eb005bcea300ad5271da6d7676468ce410c3b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 14:04:00 -0600 Subject: [PATCH 54/76] Update csv_export.py --- src/registrar/utility/csv_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 7fcbae475..f4a079d20 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -96,7 +96,7 @@ def parse_row_for_domain( FIELDS = { "Domain name": domain.name, "Status": domain.get_state_display(), - "First ready on": domain.first_ready, + "First ready on": domain.first_ready or "(blank)", "Expiration date": domain.expiration_date, "Domain type": domain_type, "Agency": domain_info.federal_agency, From f7208a80ea467d032a6cf57dc0ad0321fec7af61 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 14:36:08 -0600 Subject: [PATCH 55/76] Add unit test and move things around slightly --- src/registrar/tests/test_reports.py | 240 +++++++++++++++------------- 1 file changed, 125 insertions(+), 115 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index d918dda92..7a11c18c0 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -25,6 +25,56 @@ from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # typ from datetime import datetime from django.utils import timezone from .common import MockDb, MockEppLib, less_console_noise +from api.tests.common import less_console_noise_decorator + + +class HelperFunctions(MockDb): + """This asserts that 1=1. Its limited usefulness lies in making sure the helper methods stay healthy.""" + + def get_time_aware_date(self, date=datetime(2023, 11, 1)): + """Returns a time aware date""" + return timezone.make_aware(date) + + def test_get_default_start_date(self): + expected_date = self.get_time_aware_date() + actual_date = get_default_start_date() + self.assertEqual(actual_date, expected_date) + + def test_get_default_end_date(self): + # Note: You may need to mock timezone.now() for accurate testing + expected_date = timezone.now() + actual_date = get_default_end_date() + self.assertEqual(actual_date.date(), expected_date.date()) + + def test_get_sliced_domains(self): + """Should get fitered domains counts sliced by org type and election office.""" + + with less_console_noise(): + filter_condition = { + "domain__permissions__isnull": False, + "domain__first_ready__lte": self.end_date, + } + # Test with distinct + managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) + expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] + self.assertEqual(managed_domains_sliced_at_end_date, expected_content) + + # Test without distinct + managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) + expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] + self.assertEqual(managed_domains_sliced_at_end_date, expected_content) + + def test_get_sliced_requests(self): + """Should get fitered requests counts sliced by org type and election office.""" + + with less_console_noise(): + filter_condition = { + "status": DomainRequest.DomainRequestStatus.SUBMITTED, + "submission_date__lte": self.end_date, + } + submitted_requests_sliced_at_end_date = get_sliced_requests(filter_condition) + expected_content = [2, 2, 0, 0, 0, 0, 0, 0, 0, 0] + self.assertEqual(submitted_requests_sliced_at_end_date, expected_content) class CsvReportsTest(MockDb): @@ -194,84 +244,89 @@ class CsvReportsTest(MockDb): self.assertEqual(expected_file_content, response.content) -class ExportDataTest(MockDb, MockEppLib): +class ExportDataTest(HelperFunctions, MockEppLib): def setUp(self): super().setUp() def tearDown(self): super().tearDown() - - def test_export_domains_to_writer_security_emails(self): + + @less_console_noise_decorator + def test_export_domains_to_writer_security_emails_and_first_ready(self): """Test that export_domains_to_writer returns the - expected security email""" + expected security email and first_ready value""" + # Add security email information + self.domain_1.name = "defaultsecurity.gov" + self.domain_1.save() + # Invoke setter + self.domain_1.security_contact + # Invoke setter + self.domain_2.security_contact + # Invoke setter + self.domain_3.security_contact - with less_console_noise(): - # Add security email information - self.domain_1.name = "defaultsecurity.gov" - self.domain_1.save() - # Invoke setter - self.domain_1.security_contact - # Invoke setter - self.domain_2.security_contact - # Invoke setter - self.domain_3.security_contact - # Create a CSV file in memory - csv_file = StringIO() - writer = csv.writer(csv_file) - # Define columns, sort fields, and filter condition - columns = [ - "Domain name", - "Domain type", - "Agency", - "Organization name", - "City", - "State", - "AO", - "AO email", - "Security contact email", - "Status", - "Expiration date", - ] - sort_fields = ["domain__name"] - filter_condition = { - "domain__state__in": [ - Domain.State.READY, - Domain.State.DNS_NEEDED, - Domain.State.ON_HOLD, - ], - } - self.maxDiff = None - # Call the export functions - write_csv_for_domains( - writer, - columns, - sort_fields, - filter_condition, - should_get_domain_managers=False, - should_write_header=True, - ) + # Add a first ready date on the first domain. Leaving the others blank. + self.domain_1.first_ready = get_default_start_date() + self.domain_1.save() - # Reset the CSV file's position to the beginning - csv_file.seek(0) - # Read the content into a variable - csv_content = csv_file.read() - # We expect READY domains, - # sorted alphabetially by domain name - expected_content = ( - "Domain name,Domain type,Agency,Organization name,City,State,AO," - "AO email,Security contact email,Status,Expiration date\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,Ready\n" - "adomain2.gov,Interstate,(blank),Dns needed\n" - "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady\n" - "ddomain3.gov,Federal,Armed Forces Retirement Home,security@mail.gov,On hold,2023-11-15\n" - "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready\n" - "zdomain12.govInterstateReady\n" - ) - # Normalize line endings and remove commas, - # spaces and leading/trailing whitespace - csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() - expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - self.assertEqual(csv_content, expected_content) + # Create a CSV file in memory + csv_file = StringIO() + writer = csv.writer(csv_file) + # Define columns, sort fields, and filter condition + columns = [ + "Domain name", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "AO", + "AO email", + "Security contact email", + "Status", + "Expiration date", + "First ready on", + ] + sort_fields = ["domain__name"] + filter_condition = { + "domain__state__in": [ + Domain.State.READY, + Domain.State.DNS_NEEDED, + Domain.State.ON_HOLD, + ], + } + self.maxDiff = None + # Call the export functions + write_csv_for_domains( + writer, + columns, + sort_fields, + filter_condition, + should_get_domain_managers=False, + should_write_header=True, + ) + + # Reset the CSV file's position to the beginning + csv_file.seek(0) + # Read the content into a variable + csv_content = csv_file.read() + # We expect READY domains, + # sorted alphabetially by domain name + expected_content = ( + "Domain name,Domain type,Agency,Organization name,City,State,AO," + "AO email,Security contact email,Status,Expiration date, First ready on\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,Ready,2024-05-09\n" + "adomain2.gov,Interstate,(blank),Dns needed,(blank)\n" + "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady,2024-05-08\n" + "ddomain3.gov,Federal,Armed Forces Retirement Home,security@mail.gov,On hold,2023-11-15,(blank)\n" + "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready,2023-11-01\n" + "zdomain12.govInterstateReady,2024-05-08\n" + ) + # Normalize line endings and remove commas, + # spaces and leading/trailing whitespace + csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() + expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() + self.assertEqual(csv_content, expected_content) def test_write_csv_for_domains(self): """Test that write_body returns the @@ -692,48 +747,3 @@ class ExportDataTest(MockDb, MockEppLib): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) - - -class HelperFunctions(MockDb): - """This asserts that 1=1. Its limited usefulness lies in making sure the helper methods stay healthy.""" - - def test_get_default_start_date(self): - expected_date = timezone.make_aware(datetime(2023, 11, 1)) - actual_date = get_default_start_date() - self.assertEqual(actual_date, expected_date) - - def test_get_default_end_date(self): - # Note: You may need to mock timezone.now() for accurate testing - expected_date = timezone.now() - actual_date = get_default_end_date() - self.assertEqual(actual_date.date(), expected_date.date()) - - def test_get_sliced_domains(self): - """Should get fitered domains counts sliced by org type and election office.""" - - with less_console_noise(): - filter_condition = { - "domain__permissions__isnull": False, - "domain__first_ready__lte": self.end_date, - } - # Test with distinct - managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] - self.assertEqual(managed_domains_sliced_at_end_date, expected_content) - - # Test without distinct - managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] - self.assertEqual(managed_domains_sliced_at_end_date, expected_content) - - def test_get_sliced_requests(self): - """Should get fitered requests counts sliced by org type and election office.""" - - with less_console_noise(): - filter_condition = { - "status": DomainRequest.DomainRequestStatus.SUBMITTED, - "submission_date__lte": self.end_date, - } - submitted_requests_sliced_at_end_date = get_sliced_requests(filter_condition) - expected_content = [2, 2, 0, 0, 0, 0, 0, 0, 0, 0] - self.assertEqual(submitted_requests_sliced_at_end_date, expected_content) From 5c685a3fd6b39400ade3bed16b7f18713673abfa Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 14:41:19 -0600 Subject: [PATCH 56/76] Cleanup --- src/registrar/tests/common.py | 4 + src/registrar/tests/test_reports.py | 239 ++++++++++++++-------------- 2 files changed, 121 insertions(+), 122 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 3d9a147a2..ba1c75dce 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -746,6 +746,10 @@ class MockDb(TestCase): DomainInvitation.objects.all().delete() FederalAgency.objects.all().delete() + def get_time_aware_date(self, date=datetime(2023, 11, 1)): + """Returns a time aware date""" + return timezone.make_aware(date) + def mock_user(): """A simple user.""" diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 7a11c18c0..0c9e1d6c4 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -25,56 +25,6 @@ from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # typ from datetime import datetime from django.utils import timezone from .common import MockDb, MockEppLib, less_console_noise -from api.tests.common import less_console_noise_decorator - - -class HelperFunctions(MockDb): - """This asserts that 1=1. Its limited usefulness lies in making sure the helper methods stay healthy.""" - - def get_time_aware_date(self, date=datetime(2023, 11, 1)): - """Returns a time aware date""" - return timezone.make_aware(date) - - def test_get_default_start_date(self): - expected_date = self.get_time_aware_date() - actual_date = get_default_start_date() - self.assertEqual(actual_date, expected_date) - - def test_get_default_end_date(self): - # Note: You may need to mock timezone.now() for accurate testing - expected_date = timezone.now() - actual_date = get_default_end_date() - self.assertEqual(actual_date.date(), expected_date.date()) - - def test_get_sliced_domains(self): - """Should get fitered domains counts sliced by org type and election office.""" - - with less_console_noise(): - filter_condition = { - "domain__permissions__isnull": False, - "domain__first_ready__lte": self.end_date, - } - # Test with distinct - managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] - self.assertEqual(managed_domains_sliced_at_end_date, expected_content) - - # Test without distinct - managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] - self.assertEqual(managed_domains_sliced_at_end_date, expected_content) - - def test_get_sliced_requests(self): - """Should get fitered requests counts sliced by org type and election office.""" - - with less_console_noise(): - filter_condition = { - "status": DomainRequest.DomainRequestStatus.SUBMITTED, - "submission_date__lte": self.end_date, - } - submitted_requests_sliced_at_end_date = get_sliced_requests(filter_condition) - expected_content = [2, 2, 0, 0, 0, 0, 0, 0, 0, 0] - self.assertEqual(submitted_requests_sliced_at_end_date, expected_content) class CsvReportsTest(MockDb): @@ -244,89 +194,89 @@ class CsvReportsTest(MockDb): self.assertEqual(expected_file_content, response.content) -class ExportDataTest(HelperFunctions, MockEppLib): +class ExportDataTest(MockDb, MockEppLib): def setUp(self): super().setUp() def tearDown(self): super().tearDown() - - @less_console_noise_decorator + def test_export_domains_to_writer_security_emails_and_first_ready(self): """Test that export_domains_to_writer returns the expected security email and first_ready value""" - # Add security email information - self.domain_1.name = "defaultsecurity.gov" - self.domain_1.save() - # Invoke setter - self.domain_1.security_contact - # Invoke setter - self.domain_2.security_contact - # Invoke setter - self.domain_3.security_contact + with less_console_noise: + # Add security email information + self.domain_1.name = "defaultsecurity.gov" + self.domain_1.save() + # Invoke setter + self.domain_1.security_contact + # Invoke setter + self.domain_2.security_contact + # Invoke setter + self.domain_3.security_contact - # Add a first ready date on the first domain. Leaving the others blank. - self.domain_1.first_ready = get_default_start_date() - self.domain_1.save() + # Add a first ready date on the first domain. Leaving the others blank. + self.domain_1.first_ready = get_default_start_date() + self.domain_1.save() - # Create a CSV file in memory - csv_file = StringIO() - writer = csv.writer(csv_file) - # Define columns, sort fields, and filter condition - columns = [ - "Domain name", - "Domain type", - "Agency", - "Organization name", - "City", - "State", - "AO", - "AO email", - "Security contact email", - "Status", - "Expiration date", - "First ready on", - ] - sort_fields = ["domain__name"] - filter_condition = { - "domain__state__in": [ - Domain.State.READY, - Domain.State.DNS_NEEDED, - Domain.State.ON_HOLD, - ], - } - self.maxDiff = None - # Call the export functions - write_csv_for_domains( - writer, - columns, - sort_fields, - filter_condition, - should_get_domain_managers=False, - should_write_header=True, - ) + # Create a CSV file in memory + csv_file = StringIO() + writer = csv.writer(csv_file) + # Define columns, sort fields, and filter condition + columns = [ + "Domain name", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "AO", + "AO email", + "Security contact email", + "Status", + "Expiration date", + "First ready on", + ] + sort_fields = ["domain__name"] + filter_condition = { + "domain__state__in": [ + Domain.State.READY, + Domain.State.DNS_NEEDED, + Domain.State.ON_HOLD, + ], + } + self.maxDiff = None + # Call the export functions + write_csv_for_domains( + writer, + columns, + sort_fields, + filter_condition, + should_get_domain_managers=False, + should_write_header=True, + ) - # Reset the CSV file's position to the beginning - csv_file.seek(0) - # Read the content into a variable - csv_content = csv_file.read() - # We expect READY domains, - # sorted alphabetially by domain name - expected_content = ( - "Domain name,Domain type,Agency,Organization name,City,State,AO," - "AO email,Security contact email,Status,Expiration date, First ready on\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,Ready,2024-05-09\n" - "adomain2.gov,Interstate,(blank),Dns needed,(blank)\n" - "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady,2024-05-08\n" - "ddomain3.gov,Federal,Armed Forces Retirement Home,security@mail.gov,On hold,2023-11-15,(blank)\n" - "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready,2023-11-01\n" - "zdomain12.govInterstateReady,2024-05-08\n" - ) - # Normalize line endings and remove commas, - # spaces and leading/trailing whitespace - csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() - expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - self.assertEqual(csv_content, expected_content) + # Reset the CSV file's position to the beginning + csv_file.seek(0) + # Read the content into a variable + csv_content = csv_file.read() + # We expect READY domains, + # sorted alphabetially by domain name + expected_content = ( + "Domain name,Domain type,Agency,Organization name,City,State,AO," + "AO email,Security contact email,Status,Expiration date, First ready on\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,Ready,2024-05-09\n" + "adomain2.gov,Interstate,(blank),Dns needed,(blank)\n" + "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady,2024-05-08\n" + "ddomain3.gov,Federal,Armed Forces Retirement Home,security@mail.gov,On hold,2023-11-15,(blank)\n" + "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready,2023-11-01\n" + "zdomain12.govInterstateReady,2024-05-08\n" + ) + # Normalize line endings and remove commas, + # spaces and leading/trailing whitespace + csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() + expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() + self.assertEqual(csv_content, expected_content) def test_write_csv_for_domains(self): """Test that write_body returns the @@ -747,3 +697,48 @@ class ExportDataTest(HelperFunctions, MockEppLib): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) + + +class HelperFunctions(MockDb): + """This asserts that 1=1. Its limited usefulness lies in making sure the helper methods stay healthy.""" + + def test_get_default_start_date(self): + expected_date = self.get_time_aware_date() + actual_date = get_default_start_date() + self.assertEqual(actual_date, expected_date) + + def test_get_default_end_date(self): + # Note: You may need to mock timezone.now() for accurate testing + expected_date = timezone.now() + actual_date = get_default_end_date() + self.assertEqual(actual_date.date(), expected_date.date()) + + def test_get_sliced_domains(self): + """Should get fitered domains counts sliced by org type and election office.""" + + with less_console_noise(): + filter_condition = { + "domain__permissions__isnull": False, + "domain__first_ready__lte": self.end_date, + } + # Test with distinct + managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) + expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] + self.assertEqual(managed_domains_sliced_at_end_date, expected_content) + + # Test without distinct + managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) + expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] + self.assertEqual(managed_domains_sliced_at_end_date, expected_content) + + def test_get_sliced_requests(self): + """Should get fitered requests counts sliced by org type and election office.""" + + with less_console_noise(): + filter_condition = { + "status": DomainRequest.DomainRequestStatus.SUBMITTED, + "submission_date__lte": self.end_date, + } + submitted_requests_sliced_at_end_date = get_sliced_requests(filter_condition) + expected_content = [2, 2, 0, 0, 0, 0, 0, 0, 0, 0] + self.assertEqual(submitted_requests_sliced_at_end_date, expected_content) From fda321b02a3c4a544175bc24dac165ed06d4748d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 14:43:51 -0600 Subject: [PATCH 57/76] Update src/registrar/tests/test_reports.py --- src/registrar/tests/test_reports.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 0c9e1d6c4..7d88c9b30 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -204,6 +204,7 @@ class ExportDataTest(MockDb, MockEppLib): def test_export_domains_to_writer_security_emails_and_first_ready(self): """Test that export_domains_to_writer returns the expected security email and first_ready value""" + with less_console_noise: # Add security email information self.domain_1.name = "defaultsecurity.gov" From 460388ea57089fddcebfa17d12aa00caa1d57eb3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 14:46:24 -0600 Subject: [PATCH 58/76] Update test_reports.py --- src/registrar/tests/test_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 7d88c9b30..aa21635e3 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -205,7 +205,7 @@ class ExportDataTest(MockDb, MockEppLib): """Test that export_domains_to_writer returns the expected security email and first_ready value""" - with less_console_noise: + with less_console_noise(): # Add security email information self.domain_1.name = "defaultsecurity.gov" self.domain_1.save() From ceed417aebdaafbe4b80ba887aacfc34c9940f8b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 May 2024 14:52:16 -0600 Subject: [PATCH 59/76] Update test_reports.py --- src/registrar/tests/test_reports.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index aa21635e3..430e04d6c 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -22,7 +22,6 @@ from django.conf import settings from botocore.exceptions import ClientError import boto3_mocking from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # type: ignore -from datetime import datetime from django.utils import timezone from .common import MockDb, MockEppLib, less_console_noise From 112ea84bf6cc615e6cac7f3f6e48565169bbb7ad Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 8 May 2024 16:33:35 -0700 Subject: [PATCH 60/76] Fix spelling and add a comment --- src/registrar/tests/test_views_request.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 3be733388..2b577b41a 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -717,7 +717,7 @@ class DomainRequestTests(TestWithUser, WebTest): type_form["generic_org_type-generic_org_type"] = DomainRequest.OrganizationChoices.SPECIAL_DISTRICT self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) type_result = type_page.forms[0].submit() - # follow first redirectt + # follow first redirect self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) contact_page = type_result.follow() @@ -725,7 +725,7 @@ class DomainRequestTests(TestWithUser, WebTest): def test_federal_agency_dropdown_excludes_expected_values(self): """The Federal Agency dropdown on a domain request form should not - include options for gov Administration or Non-Federal Agency""" + include options for gov Administration and Non-Federal Agency""" intro_page = self.app.get(reverse("domain-request:")) # django-webtest does not handle cookie-based sessions well because it keeps # resetting the session key on each new request, thus destroying the concept @@ -767,6 +767,7 @@ class DomainRequestTests(TestWithUser, WebTest): # gov Administration and Non-Federal Agency should not be federal agency options self.assertNotContains(org_contact_page, "gov Administration") self.assertNotContains(org_contact_page, "Non-Federal Agency") + # make sure correct federal agency options still show up self.assertContains(org_contact_page, "General Services Administration") def test_yes_no_contact_form_inits_blank_for_new_domain_request(self): From a1ac715281a6f6bc17c5f282f2ae715e62989904 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 May 2024 08:17:43 -0600 Subject: [PATCH 61/76] Add blank check on expiration date --- src/registrar/utility/csv_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index f4a079d20..283c884f9 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -97,7 +97,7 @@ def parse_row_for_domain( "Domain name": domain.name, "Status": domain.get_state_display(), "First ready on": domain.first_ready or "(blank)", - "Expiration date": domain.expiration_date, + "Expiration date": domain.expiration_date or "(blank)", "Domain type": domain_type, "Agency": domain_info.federal_agency, "Organization name": domain_info.organization_name, From 4618f302f5a57f3bfb9117f5463174c8c8d974a1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 May 2024 09:21:53 -0600 Subject: [PATCH 62/76] Adjust unit tests to not be time sensitive The first ready field was using timezone.now() which is not good when directly testing that field. This is a minor refactor which just sets a preset time as "now" and adjusted the unit tests minorly to compensate --- src/registrar/tests/common.py | 54 +++++++++++++++++------------ src/registrar/tests/test_reports.py | 52 ++++++++++++++------------- 2 files changed, 58 insertions(+), 48 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index ba1c75dce..be7065403 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -97,6 +97,11 @@ def less_console_noise(output_stream=None): output_stream.close() +def get_time_aware_date(date=datetime(2023, 11, 1)): + """Returns a time aware date""" + return timezone.make_aware(date) + + class GenericTestHelper(TestCase): """A helper class that contains various helper functions for TestCases""" @@ -532,11 +537,9 @@ class MockDb(TestCase): username=username, first_name=first_name, last_name=last_name, email=email ) - # Create a time-aware current date - current_datetime = timezone.now() - # Extract the date part - current_date = current_datetime.date() + current_date = get_time_aware_date(datetime(2024, 4, 2)) # Create start and end dates using timedelta + self.end_date = current_date + timedelta(days=2) self.start_date = current_date - timedelta(days=2) @@ -544,22 +547,22 @@ class MockDb(TestCase): self.federal_agency_2, _ = FederalAgency.objects.get_or_create(agency="Armed Forces Retirement Home") self.domain_1, _ = Domain.objects.get_or_create( - name="cdomain1.gov", state=Domain.State.READY, first_ready=timezone.now() + name="cdomain1.gov", state=Domain.State.READY, first_ready=get_time_aware_date(datetime(2024, 4, 2)) ) self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED) self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_5, _ = Domain.objects.get_or_create( - name="bdomain5.gov", state=Domain.State.DELETED, deleted=timezone.make_aware(datetime(2023, 11, 1)) + name="bdomain5.gov", state=Domain.State.DELETED, deleted=get_time_aware_date(datetime(2023, 11, 1)) ) self.domain_6, _ = Domain.objects.get_or_create( - name="bdomain6.gov", state=Domain.State.DELETED, deleted=timezone.make_aware(datetime(1980, 10, 16)) + name="bdomain6.gov", state=Domain.State.DELETED, deleted=get_time_aware_date(datetime(1980, 10, 16)) ) self.domain_7, _ = Domain.objects.get_or_create( - name="xdomain7.gov", state=Domain.State.DELETED, deleted=timezone.now() + name="xdomain7.gov", state=Domain.State.DELETED, deleted=get_time_aware_date(datetime(2024, 4, 2)) ) self.domain_8, _ = Domain.objects.get_or_create( - name="sdomain8.gov", state=Domain.State.DELETED, deleted=timezone.now() + name="sdomain8.gov", state=Domain.State.DELETED, deleted=get_time_aware_date(datetime(2024, 4, 2)) ) # We use timezone.make_aware to sync to server time a datetime object with the current date (using date.today()) # and a specific time (using datetime.min.time()). @@ -567,19 +570,19 @@ class MockDb(TestCase): self.domain_9, _ = Domain.objects.get_or_create( name="zdomain9.gov", state=Domain.State.DELETED, - deleted=timezone.make_aware(datetime.combine(date.today() - timedelta(days=1), datetime.min.time())), + deleted=get_time_aware_date(datetime(2024, 4, 1)), ) # ready tomorrow self.domain_10, _ = Domain.objects.get_or_create( name="adomain10.gov", state=Domain.State.READY, - first_ready=timezone.make_aware(datetime.combine(date.today() + timedelta(days=1), datetime.min.time())), + first_ready=get_time_aware_date(datetime(2024, 4, 3)), ) self.domain_11, _ = Domain.objects.get_or_create( - name="cdomain11.gov", state=Domain.State.READY, first_ready=timezone.now() + name="cdomain11.gov", state=Domain.State.READY, first_ready=get_time_aware_date(datetime(2024, 4, 2)) ) self.domain_12, _ = Domain.objects.get_or_create( - name="zdomain12.gov", state=Domain.State.READY, first_ready=timezone.now() + name="zdomain12.gov", state=Domain.State.READY, first_ready=get_time_aware_date(datetime(2024, 4, 2)) ) self.domain_information_1, _ = DomainInformation.objects.get_or_create( @@ -716,23 +719,31 @@ class MockDb(TestCase): with less_console_noise(): self.domain_request_1 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.STARTED, name="city1.gov" + status=DomainRequest.DomainRequestStatus.STARTED, + name="city1.gov", ) self.domain_request_2 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.IN_REVIEW, name="city2.gov" + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + name="city2.gov", ) self.domain_request_3 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.STARTED, name="city3.gov" + status=DomainRequest.DomainRequestStatus.STARTED, + name="city3.gov", ) self.domain_request_4 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.STARTED, name="city4.gov" + status=DomainRequest.DomainRequestStatus.STARTED, + name="city4.gov", ) self.domain_request_5 = completed_domain_request( - status=DomainRequest.DomainRequestStatus.APPROVED, name="city5.gov" + status=DomainRequest.DomainRequestStatus.APPROVED, + name="city5.gov", ) self.domain_request_3.submit() - self.domain_request_3.save() self.domain_request_4.submit() + + self.domain_request_3.submission_date = get_time_aware_date(datetime(2024, 4, 2)) + self.domain_request_4.submission_date = get_time_aware_date(datetime(2024, 4, 2)) + self.domain_request_3.save() self.domain_request_4.save() def tearDown(self): @@ -746,10 +757,6 @@ class MockDb(TestCase): DomainInvitation.objects.all().delete() FederalAgency.objects.all().delete() - def get_time_aware_date(self, date=datetime(2023, 11, 1)): - """Returns a time aware date""" - return timezone.make_aware(date) - def mock_user(): """A simple user.""" @@ -877,6 +884,7 @@ def completed_domain_request( if organization_type: domain_request_kwargs["organization_type"] = organization_type + domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 430e04d6c..9923df85d 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -23,7 +23,7 @@ from botocore.exceptions import ClientError import boto3_mocking from registrar.utility.s3_bucket import S3ClientError, S3ClientErrorCodes # type: ignore from django.utils import timezone -from .common import MockDb, MockEppLib, less_console_noise +from .common import MockDb, MockEppLib, less_console_noise, get_time_aware_date class CsvReportsTest(MockDb): @@ -265,12 +265,12 @@ class ExportDataTest(MockDb, MockEppLib): expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,AO," "AO email,Security contact email,Status,Expiration date, First ready on\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,Ready,2024-05-09\n" - "adomain2.gov,Interstate,(blank),Dns needed,(blank)\n" - "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady,2024-05-08\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,Ready,(blank),2023-11-01\n" + "adomain2.gov,Interstate,(blank),Dns needed,(blank),(blank)\n" + "cdomain11.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank),2023-11-01\n" "ddomain3.gov,Federal,Armed Forces Retirement Home,security@mail.gov,On hold,2023-11-15,(blank)\n" - "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready,2023-11-01\n" - "zdomain12.govInterstateReady,2024-05-08\n" + "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready,(blank),2023-11-01\n" + "zdomain12.govInterstateReady,(blank),2023-11-01\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace @@ -474,19 +474,21 @@ class ExportDataTest(MockDb, MockEppLib): # Read the content into a variable csv_content = csv_file.read() - - # We expect READY domains first, created between today-2 and today+2, sorted by created_at then name - # and DELETED domains deleted between today-2 and today+2, sorted by deleted then name + self.maxDiff = None + # We expect READY domains first, created between day-2 and day+2, sorted by created_at then name + # and DELETED domains deleted between day-2 and day+2, sorted by deleted then name expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Status,Expiration date\n" - "cdomain1.gov,Federal-Executive,World War I Centennial Commission,,,,Ready,\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,,,,Ready,\n" - "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady\n" - "zdomain12.govInterstateReady\n" - "zdomain9.gov,Federal,Armed Forces Retirement Home,,,,Deleted,\n" - "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,Deleted,\n" - "xdomain7.gov,Federal,Armed Forces Retirement Home,,,,Deleted,\n" + "cdomain1.gov,Federal-Executive,World War I Centennial Commission,,,,Ready,(blank)\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,,,,Ready,(blank)\n" + "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady(blank)\n" + "zdomain12.govInterstateReady(blank)\n" + "bdomain5.gov,Federal,ArmedForcesRetirementHome,Deleted(blank)\n" + "bdomain6.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank)\n" + "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,Deleted,(blank)\n" + "xdomain7.gov,Federal,Armed Forces Retirement Home,,,,Deleted,(blank)\n" + "zdomain9.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank)\n" ) # Normalize line endings and remove commas, @@ -531,7 +533,7 @@ class ExportDataTest(MockDb, MockEppLib): Domain.State.ON_HOLD, ], } - self.maxDiff = None + # Call the export functions write_csv_for_domains( writer, @@ -553,14 +555,14 @@ class ExportDataTest(MockDb, MockEppLib): "Organization name,City,State,AO,AO email," "Security contact email,Domain manager 1,DM1 status,Domain manager 2,DM2 status," "Domain manager 3,DM3 status,Domain manager 4,DM4 status\n" - "adomain10.gov,Ready,,Federal,Armed Forces Retirement Home,,,, , ,squeaker@rocks.com, I\n" - "adomain2.gov,Dns needed,,Interstate,,,,, , , ,meoward@rocks.com, R,squeaker@rocks.com, I\n" - "cdomain11.govReadyFederal-ExecutiveWorldWarICentennialCommissionmeoward@rocks.comR\n" - "cdomain1.gov,Ready,,Federal - Executive,World War I Centennial Commission,,," + "adomain10.gov,Ready,(blank),Federal,Armed Forces Retirement Home,,,, , ,squeaker@rocks.com, I\n" + "adomain2.gov,Dns needed,(blank),Interstate,,,,, , , ,meoward@rocks.com, R,squeaker@rocks.com, I\n" + "cdomain11.govReady,(blank),Federal-ExecutiveWorldWarICentennialCommissionmeoward@rocks.comR\n" + "cdomain1.gov,Ready,(blank),Federal - Executive,World War I Centennial Commission,,," ", , , ,meoward@rocks.com,R,info@example.com,R,big_lebowski@dude.co,R," "woofwardthethird@rocks.com,I\n" - "ddomain3.gov,On hold,,Federal,Armed Forces Retirement Home,,,, , , ,,\n" - "zdomain12.govReadyInterstatemeoward@rocks.comR\n" + "ddomain3.gov,On hold,(blank),Federal,Armed Forces Retirement Home,,,, , , ,,\n" + "zdomain12.gov,Ready,(blank),Interstate,meoward@rocks.com,R\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace @@ -695,7 +697,7 @@ class ExportDataTest(MockDb, MockEppLib): # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - + print(f"what is the actual content {csv_content}") self.assertEqual(csv_content, expected_content) @@ -703,7 +705,7 @@ class HelperFunctions(MockDb): """This asserts that 1=1. Its limited usefulness lies in making sure the helper methods stay healthy.""" def test_get_default_start_date(self): - expected_date = self.get_time_aware_date() + expected_date = get_time_aware_date() actual_date = get_default_start_date() self.assertEqual(actual_date, expected_date) From 8af790293929a5eeb544e44dd81cbe697c88d741 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 May 2024 09:23:17 -0600 Subject: [PATCH 63/76] Remove max diff --- src/registrar/tests/test_admin.py | 1 - src/registrar/tests/test_reports.py | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 98c5df0ac..e85c2fc5e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2164,7 +2164,6 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "Yes, select ineligible status") def test_readonly_when_restricted_creator(self): - self.maxDiff = None with less_console_noise(): domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) with boto3_mocking.clients.handler_for("sesv2", self.mock_client): diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 9923df85d..bdfddf534 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -245,7 +245,7 @@ class ExportDataTest(MockDb, MockEppLib): Domain.State.ON_HOLD, ], } - self.maxDiff = None + # Call the export functions write_csv_for_domains( writer, @@ -474,7 +474,7 @@ class ExportDataTest(MockDb, MockEppLib): # Read the content into a variable csv_content = csv_file.read() - self.maxDiff = None + # We expect READY domains first, created between day-2 and day+2, sorted by created_at then name # and DELETED domains deleted between day-2 and day+2, sorted by deleted then name expected_content = ( @@ -587,7 +587,7 @@ class ExportDataTest(MockDb, MockEppLib): csv_file.seek(0) # Read the content into a variable csv_content = csv_file.read() - self.maxDiff = None + # We expect the READY domain names with the domain managers: Their counts, and listing at end_date. expected_content = ( "MANAGED DOMAINS COUNTS AT START DATE\n" @@ -630,7 +630,7 @@ class ExportDataTest(MockDb, MockEppLib): csv_file.seek(0) # Read the content into a variable csv_content = csv_file.read() - self.maxDiff = None + # We expect the READY domain names with the domain managers: Their counts, and listing at end_date. expected_content = ( "UNMANAGED DOMAINS AT START DATE\n" From 0ac79da907597028664ab513cb85dce0b8e9294d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 May 2024 09:23:41 -0600 Subject: [PATCH 64/76] Remove print --- src/registrar/tests/test_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index bdfddf534..7337db15d 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -697,7 +697,7 @@ class ExportDataTest(MockDb, MockEppLib): # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - print(f"what is the actual content {csv_content}") + self.assertEqual(csv_content, expected_content) From 55e47d03cddf7c9c6201a9b241a9cbedba57e910 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 9 May 2024 11:31:11 -0400 Subject: [PATCH 65/76] added HostIP --- docs/operations/import_export.md | 3 +++ src/registrar/admin.py | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/docs/operations/import_export.md b/docs/operations/import_export.md index 897e8a01a..e04a79cd2 100644 --- a/docs/operations/import_export.md +++ b/docs/operations/import_export.md @@ -23,6 +23,7 @@ need to be exported: * DraftDomain * Websites * Host +* HostIP ### Import @@ -42,6 +43,7 @@ Delete all rows from tables in the following order through django admin: * Contact * Websites * DraftDomain +* HostIP * Host #### Importing into Target Environment @@ -53,6 +55,7 @@ order: * Contact * Domain * Host +* HostIP * DraftDomain * Websites * DomainRequest diff --git a/src/registrar/admin.py b/src/registrar/admin.py index bcda7e048..ae6e02c28 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -773,6 +773,19 @@ class MyHostAdmin(AuditedAdmin, ImportExportModelAdmin): inlines = [HostIPInline] +class HostIpResource(resources.ModelResource): + + class Meta: + model = models.HostIP + + +class HostIpAdmin(AuditedAdmin, ImportExportModelAdmin): + """Custom host ip admin class""" + + resource_classes = [HostIpResource] + model = models.HostIP + + class ContactResource(resources.ModelResource): class Meta: @@ -2298,9 +2311,8 @@ admin.site.register(models.DomainInformation, DomainInformationAdmin) admin.site.register(models.Domain, DomainAdmin) admin.site.register(models.DraftDomain, DraftDomainAdmin) admin.site.register(models.FederalAgency, FederalAgencyAdmin) -# Host and HostIP removed from django admin because changes in admin -# do not propagate to registry and logic not applied admin.site.register(models.Host, MyHostAdmin) +admin.site.register(models.HostIP, HostIpAdmin) admin.site.register(models.Website, WebsiteAdmin) admin.site.register(models.PublicContact, PublicContactAdmin) admin.site.register(models.DomainRequest, DomainRequestAdmin) From e23c3eee52211450091e185f1aca0973188dd212 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 May 2024 13:56:13 -0600 Subject: [PATCH 66/76] Fix tests and lint --- src/registrar/tests/test_reports.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 7337db15d..e214ec0ee 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -265,12 +265,13 @@ class ExportDataTest(MockDb, MockEppLib): expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,AO," "AO email,Security contact email,Status,Expiration date, First ready on\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,Ready,(blank),2023-11-01\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,Ready,(blank),2024-04-03\n" "adomain2.gov,Interstate,(blank),Dns needed,(blank),(blank)\n" - "cdomain11.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank),2023-11-01\n" + "cdomain11.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank),2024-04-02\n" "ddomain3.gov,Federal,Armed Forces Retirement Home,security@mail.gov,On hold,2023-11-15,(blank)\n" - "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),Ready,(blank),2023-11-01\n" - "zdomain12.govInterstateReady,(blank),2023-11-01\n" + "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission," + "(blank),Ready,(blank),2023-11-01\n" + "zdomain12.govInterstateReady,(blank),2024-04-02\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace @@ -474,7 +475,7 @@ class ExportDataTest(MockDb, MockEppLib): # Read the content into a variable csv_content = csv_file.read() - + self.maxDiff = None # We expect READY domains first, created between day-2 and day+2, sorted by created_at then name # and DELETED domains deleted between day-2 and day+2, sorted by deleted then name expected_content = ( @@ -484,11 +485,9 @@ class ExportDataTest(MockDb, MockEppLib): "adomain10.gov,Federal,Armed Forces Retirement Home,,,,Ready,(blank)\n" "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady(blank)\n" "zdomain12.govInterstateReady(blank)\n" - "bdomain5.gov,Federal,ArmedForcesRetirementHome,Deleted(blank)\n" - "bdomain6.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank)\n" - "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,Deleted,(blank)\n" - "xdomain7.gov,Federal,Armed Forces Retirement Home,,,,Deleted,(blank)\n" "zdomain9.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank)\n" + "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,Deleted,(blank)\n" + "xdomain7.gov,FederalArmedForcesRetirementHome,Deleted,(blank)\n" ) # Normalize line endings and remove commas, From cbf5b12f9e43f71e683cbe19d559b1dca12a310e Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Thu, 9 May 2024 23:22:10 -0400 Subject: [PATCH 67/76] Update designer-onboarding.md --- .github/ISSUE_TEMPLATE/designer-onboarding.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/designer-onboarding.md b/.github/ISSUE_TEMPLATE/designer-onboarding.md index 461850b60..2a4cab3c2 100644 --- a/.github/ISSUE_TEMPLATE/designer-onboarding.md +++ b/.github/ISSUE_TEMPLATE/designer-onboarding.md @@ -1,6 +1,6 @@ --- name: Designer Onboarding -about: Onboarding steps for designers. +about: Onboarding steps for new designers joining the .gov team. title: 'Designer Onboarding: GH_HANDLE' labels: design, onboarding assignees: katherineosos From 6906f7b775c166c0934416a485427c5e1922b4ce Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Thu, 9 May 2024 23:23:28 -0400 Subject: [PATCH 68/76] Update developer-onboarding.md --- .github/ISSUE_TEMPLATE/developer-onboarding.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/developer-onboarding.md b/.github/ISSUE_TEMPLATE/developer-onboarding.md index 94b2a367d..4d231a039 100644 --- a/.github/ISSUE_TEMPLATE/developer-onboarding.md +++ b/.github/ISSUE_TEMPLATE/developer-onboarding.md @@ -1,6 +1,6 @@ --- name: Developer Onboarding -about: Onboarding steps for developers. +about: Onboarding steps for new developers joining the .gov team. title: 'Developer Onboarding: GH_HANDLE' labels: dev, onboarding assignees: abroddrick From 5673889a55e6b11f73ef968010d7f0dab4beac9e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 10 May 2024 07:21:31 -0400 Subject: [PATCH 69/76] updated templates with css --- src/registrar/assets/sass/_theme/_admin.scss | 4 + .../templates/admin/import_export/import.html | 191 ++++++++++++++++++ .../import_export/resource_fields_list.html | 21 ++ 3 files changed, 216 insertions(+) create mode 100644 src/registrar/templates/admin/import_export/import.html create mode 100644 src/registrar/templates/admin/import_export/resource_fields_list.html diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 9f5ea7a97..c716ad49c 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -717,6 +717,10 @@ div.dja__model-description{ } +.import_export_text { + color: var(--secondary); +} + .text-underline { text-decoration: underline !important; } diff --git a/src/registrar/templates/admin/import_export/import.html b/src/registrar/templates/admin/import_export/import.html new file mode 100644 index 000000000..ef1160a2d --- /dev/null +++ b/src/registrar/templates/admin/import_export/import.html @@ -0,0 +1,191 @@ +{% extends "admin/import_export/base.html" %} +{% load i18n %} +{% load admin_urls %} +{% load import_export_tags %} +{% load static %} + +{% block extrastyle %}{{ block.super }}{% endblock %} + +{% block extrahead %}{{ block.super }} + + {% if confirm_form %} + {{ confirm_form.media }} + {% else %} + {{ form.media }} + {% endif %} +{% endblock %} + +{% block breadcrumbs_last %} +{% trans "Import" %} +{% endblock %} + +{% block content %} + + {% if confirm_form %} + {% block confirm_import_form %} +
    + {% csrf_token %} + {{ confirm_form.as_p }} +

    + {% trans "Below is a preview of data to be imported. If you are satisfied with the results, click 'Confirm import'" %} +

    +
    + +
    +
    + {% endblock %} + {% else %} + {% block import_form %} +
    + {% csrf_token %} + + {% include "admin/import_export/resource_fields_list.html" with import_or_export="import" %} + {% block import_form_additional_info %}{% endblock %} + + {% block form_detail %} +
    + {% for field in form %} +
    + {{ field.errors }} + + {{ field.label_tag }} + + {{ field }} + + {% if field.field.help_text %} +

    {{ field.field.help_text|safe }}

    + {% endif %} +
    + {% endfor %} +
    + {% endblock %} + + {% block form_submit_button %} +
    + +
    + {% endblock %} +
    + {% endblock %} + {% endif %} + + {% if result %} + + {% if result.has_errors %} + {% block errors %} +

    {% trans "Errors" %}

    +
      + {% for error in result.base_errors %} +
    • + {{ error.error }} +
      {{ error.traceback|linebreaks }}
      +
      • + {% endfor %} + {% for line, errors in result.row_errors %} + {% for error in errors %} +
    • + {% trans "Line number" %}: {{ line }} - {{ error.error }} +
      {{ error.row.values|join:", " }}
      +
      {{ error.traceback|linebreaks }}
      +
      • + {% endfor %} + {% endfor %} +
    + {% endblock %} + + {% elif result.has_validation_errors %} + + {% block validation_errors %} +

    {% trans "Some rows failed to validate" %}

    + +

    {% trans "Please correct these errors in your data where possible, then reupload it using the form above." %}

    + + + + + + + {% for field in result.diff_headers %} + + {% endfor %} + + + + {% for row in result.invalid_rows %} + + + + {% for field in row.values %} + + {% endfor %} + + {% endfor %} + +
    {% trans "Row" %}{% trans "Errors" %}{{ field }}
    {{ row.number }} + {{ row.error_count }} +
    +
      + {% for field_name, error_list in row.field_specific_errors.items %} +
    • + {{ field_name }} +
        + {% for error in error_list %} +
      • {{ error }}
        • + {% endfor %} +
      +
      • + {% endfor %} + {% if row.non_field_specific_errors %} +
    • + {% trans "Non field specific" %} +
        + {% for error in row.non_field_specific_errors %} +
      • {{ error }}
        • + {% endfor %} +
      +
      • + {% endif %} +
    +
    +     		{{ field }}
    + {% endblock %} + + {% else %} + + {% block preview %} +

    {% trans "Preview" %}

    + + + + + + {% for field in result.diff_headers %} + + {% endfor %} + + + {% for row in result.valid_rows %} + + + {% for field in row.diff %} + + {% endfor %} + + {% endfor %} +
    {{ field }}
    + {% if row.import_type == 'new' %} + {% trans "New" %} + {% elif row.import_type == 'skip' %} + {% trans "Skipped" %} + {% elif row.import_type == 'delete' %} + {% trans "Delete" %} + {% elif row.import_type == 'update' %} + {% trans "Update" %} + {% endif %} + {{ field }}
    + {% endblock %} + + {% endif %} + + {% endif %} +{% endblock %} diff --git a/src/registrar/templates/admin/import_export/resource_fields_list.html b/src/registrar/templates/admin/import_export/resource_fields_list.html new file mode 100644 index 000000000..3f5483ea7 --- /dev/null +++ b/src/registrar/templates/admin/import_export/resource_fields_list.html @@ -0,0 +1,21 @@ +{% load i18n %} +{% block fields_help %} +

    + {% if import_or_export == "export" %} + {% trans "This exporter will export the following fields: " %} + {% elif import_or_export == "import" %} + {% trans "This importer will import the following fields: " %} + {% endif %} + + {% if fields_list|length <= 1 %} + {{ fields_list.0.1|join:", " }} + {% else %} +

    + {% for resource, fields in fields_list %} +
    {{ resource }}
    +
    {{ fields|join:", " }}
    + {% endfor %} +
    + {% endif %} +

    +{% endblock %} \ No newline at end of file From 645c85e1476c152e0a636b934eea855801f7544d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 10 May 2024 07:36:23 -0400 Subject: [PATCH 70/76] updated documentation and code comments --- docs/operations/import_export.md | 5 ++++- src/registrar/admin.py | 10 ++++++++++ src/registrar/models/utility/generic_helper.py | 6 ++++-- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/docs/operations/import_export.md b/docs/operations/import_export.md index e04a79cd2..7c3ee1159 100644 --- a/docs/operations/import_export.md +++ b/docs/operations/import_export.md @@ -60,4 +60,7 @@ order: * Websites * DomainRequest * DomainInformation -* UserDomainRole \ No newline at end of file +* UserDomainRole + +Optional step: +* Run fixtures to load fixture users back in \ No newline at end of file diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ae6e02c28..49574496d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -80,6 +80,7 @@ class FsmModelResource(resources.ModelResource): class UserResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.User @@ -758,6 +759,7 @@ class HostIPInline(admin.StackedInline): class HostResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.Host @@ -774,6 +776,7 @@ class MyHostAdmin(AuditedAdmin, ImportExportModelAdmin): class HostIpResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.HostIP @@ -787,6 +790,7 @@ class HostIpAdmin(AuditedAdmin, ImportExportModelAdmin): class ContactResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.Contact @@ -918,6 +922,7 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): class WebsiteResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.Website @@ -976,6 +981,7 @@ class WebsiteAdmin(ListHeaderAdmin, ImportExportModelAdmin): class UserDomainRoleResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.UserDomainRole @@ -1067,6 +1073,7 @@ class DomainInvitationAdmin(ListHeaderAdmin): class DomainInformationResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.DomainInformation @@ -1208,6 +1215,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): class DomainRequestResource(FsmModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.DomainRequest @@ -1761,6 +1769,7 @@ class DomainInformationInline(admin.StackedInline): class DomainResource(FsmModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.Domain @@ -2166,6 +2175,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): class DraftDomainResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" class Meta: model = models.DraftDomain diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 7d3586770..0befd6627 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -100,7 +100,7 @@ class CreateOrUpdateOrganizationTypeHelper: # Update the field self._update_fields(organization_type_needs_update, generic_org_type_needs_update) - def _handle_existing_instance(self, force_update_when_no_are_changes_found=False): + def _handle_existing_instance(self, force_update_when_no_changes_are_found=False): # == Init variables == # try: # Instance is already in the database, fetch its current state @@ -119,7 +119,7 @@ class CreateOrUpdateOrganizationTypeHelper: raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") elif not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): # No changes found - if force_update_when_no_are_changes_found: + if force_update_when_no_changes_are_found: # If we want to force an update anyway, we can treat this record like # its a new one in that we check for "None" values rather than changes. self._handle_new_instance() @@ -132,6 +132,8 @@ class CreateOrUpdateOrganizationTypeHelper: # Update the field self._update_fields(organization_type_needs_update, generic_org_type_needs_update) except self.sender.DoesNotExist: + # this exception should only be raised when import_export utility attempts to import + # a new row and already has an id pass def _update_fields(self, organization_type_needs_update, generic_org_type_needs_update): From b95c70ff83718382ee88113ec09bf4a78008acc1 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 10 May 2024 07:43:49 -0400 Subject: [PATCH 71/76] formatted for readability --- src/registrar/admin.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 49574496d..ab281c32f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -80,7 +80,8 @@ class FsmModelResource(resources.ModelResource): class UserResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.User @@ -759,7 +760,8 @@ class HostIPInline(admin.StackedInline): class HostResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.Host @@ -776,7 +778,8 @@ class MyHostAdmin(AuditedAdmin, ImportExportModelAdmin): class HostIpResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.HostIP @@ -790,7 +793,8 @@ class HostIpAdmin(AuditedAdmin, ImportExportModelAdmin): class ContactResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.Contact @@ -922,7 +926,8 @@ class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin): class WebsiteResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.Website @@ -981,7 +986,8 @@ class WebsiteAdmin(ListHeaderAdmin, ImportExportModelAdmin): class UserDomainRoleResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.UserDomainRole @@ -1073,7 +1079,8 @@ class DomainInvitationAdmin(ListHeaderAdmin): class DomainInformationResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.DomainInformation @@ -1215,7 +1222,8 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): class DomainRequestResource(FsmModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.DomainRequest @@ -1769,7 +1777,8 @@ class DomainInformationInline(admin.StackedInline): class DomainResource(FsmModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.Domain @@ -2175,7 +2184,8 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): class DraftDomainResource(resources.ModelResource): - """defines how each field in the referenced model should be mapped to the corresponding fields in the import/export file""" + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" class Meta: model = models.DraftDomain From 45af1f10e5e225075bc1ca6925526bd34b418a49 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 10 May 2024 08:33:34 -0600 Subject: [PATCH 72/76] Cleanup --- src/registrar/tests/test_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index e214ec0ee..4f308b2b6 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -475,7 +475,7 @@ class ExportDataTest(MockDb, MockEppLib): # Read the content into a variable csv_content = csv_file.read() - self.maxDiff = None + # We expect READY domains first, created between day-2 and day+2, sorted by created_at then name # and DELETED domains deleted between day-2 and day+2, sorted by deleted then name expected_content = ( From e30650abee974197b5edeab624f80964b7b54b8b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 14 May 2024 14:00:32 -0600 Subject: [PATCH 73/76] Set federal agency if none exist --- src/registrar/models/domain_request.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 09f2b9fab..57f901779 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -751,6 +751,10 @@ class DomainRequest(TimeStampedModel): domain request into an admin on that domain. It also triggers an email notification.""" + if self.federal_agency is None: + self.federal_agency = "Non-Federal Agency" + self.save() + # create the domain Domain = apps.get_model("registrar.Domain") From af20a9b52a6daca90c6b43c49bdb046c86b43fde Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 14 May 2024 14:50:17 -0600 Subject: [PATCH 74/76] Add unit test --- src/registrar/models/domain_request.py | 3 ++- src/registrar/tests/test_models.py | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 57f901779..2501cdc87 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -9,6 +9,7 @@ from django.db import models from django_fsm import FSMField, transition # type: ignore from django.utils import timezone from registrar.models.domain import Domain +from registrar.models.federal_agency import FederalAgency from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes @@ -752,7 +753,7 @@ class DomainRequest(TimeStampedModel): notification.""" if self.federal_agency is None: - self.federal_agency = "Non-Federal Agency" + self.federal_agency = FederalAgency.objects.filter(agency="Non-Federal Agency").first() self.save() # create the domain diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 1558ab310..847168356 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -12,6 +12,7 @@ from registrar.models import ( DraftDomain, DomainInvitation, UserDomainRole, + FederalAgency, ) import boto3_mocking @@ -75,6 +76,26 @@ class TestDomainRequest(TestCase): with less_console_noise(): return self.assertRaises(Exception, None, exception_type) + def test_federal_agency_set_to_non_federal_on_approve(self): + """Ensures that when the federal_agency field is 'none' when .approve() is called, + the field is set to the 'Non-Federal Agency' record""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + name="city2.gov", + federal_agency=None, + ) + + # Ensure that the federal agency is None + self.assertEqual(domain_request.federal_agency, None) + + # Approve the request + domain_request.approve() + self.assertEqual(domain_request.status, DomainRequest.DomainRequestStatus.APPROVED) + + # After approval, it should be "Non-Federal agency" + expected_federal_agency = FederalAgency.objects.filter(agency="Non-Federal Agency").first() + self.assertEqual(domain_request.federal_agency, expected_federal_agency) + def test_empty_create_fails(self): """Can't create a completely empty domain request. NOTE: something about theexception this test raises messes up with the From 70cd55f623cf3dd69131cd288fcb8302fe4dae7a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 15 May 2024 08:16:46 -0600 Subject: [PATCH 75/76] Fix unit test --- src/registrar/tests/test_admin.py | 2 +- src/registrar/tests/test_models.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 632099dde..4a6e76e3d 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -646,7 +646,7 @@ class TestDomainAdmin(MockEppLib, WebTest): response = self.client.get("/admin/registrar/domain/") # There are 4 template references to Federal (4) plus four references in the table # for our actual domain_request - self.assertContains(response, "Federal", count=54) + self.assertContains(response, "Federal", count=56) # This may be a bit more robust self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 847168356..fa074c3c6 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -963,6 +963,7 @@ class TestDomainInformation(TestCase): domain=domain, notes="test notes", domain_request=domain_request, + federal_agency=FederalAgency.objects.get(agency="Non-Federal Agency"), ).__dict__ # Test the two records for consistency From c25cbb808c03395acae5f2fa1794373feb250ef1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 15 May 2024 13:03:56 -0600 Subject: [PATCH 76/76] Create 0094_create_groups_v12.py --- .../migrations/0094_create_groups_v12.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/registrar/migrations/0094_create_groups_v12.py diff --git a/src/registrar/migrations/0094_create_groups_v12.py b/src/registrar/migrations/0094_create_groups_v12.py new file mode 100644 index 000000000..42106cdb5 --- /dev/null +++ b/src/registrar/migrations/0094_create_groups_v12.py @@ -0,0 +1,37 @@ +# This migration creates the create_full_access_group and create_cisa_analyst_group groups +# It is dependent on 0079 (which populates federal agencies) +# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS +# in the user_group model then: +# [NOT RECOMMENDED] +# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions +# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups +# step 3: fake run the latest migration in the migrations list +# [RECOMMENDED] +# Alternatively: +# step 1: duplicate the migration that loads data +# step 2: docker-compose exec app ./manage.py migrate + +from django.db import migrations +from registrar.models import UserGroup +from typing import Any + + +# For linting: RunPython expects a function reference, +# so let's give it one +def create_groups(apps, schema_editor) -> Any: + UserGroup.create_cisa_analyst_group(apps, schema_editor) + UserGroup.create_full_access_group(apps, schema_editor) + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0093_alter_publiccontact_unique_together"), + ] + + operations = [ + migrations.RunPython( + create_groups, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ]