From e6e0c2c416857763d83fef8be032613e73fee0a0 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Fri, 6 Oct 2023 12:16:20 -0400 Subject: [PATCH 01/18] Content revision, add :emoji: names --- .github/ISSUE_TEMPLATE/issue-default.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/issue-default.yml b/.github/ISSUE_TEMPLATE/issue-default.yml index 27ec10415..2252845bf 100644 --- a/.github/ISSUE_TEMPLATE/issue-default.yml +++ b/.github/ISSUE_TEMPLATE/issue-default.yml @@ -6,13 +6,13 @@ body: id: title-help attributes: value: | - > Titles should be short, descriptive, and compelling. + > Titles should be short, descriptive, and compelling. Use sentence case. - type: textarea id: issue-description attributes: label: Issue description and context description: | - Describe the issue so that someone who wasn't present for its discovery can understand the problem and why it matters. Use full sentences, plain language, and good [formatting](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax). Share desired outcomes or potential next steps. Images or links to other content/context (like documents or Slack discussions) are welcome. + Describe the issue so that someone who wasn't present for its discovery can understand why it matters. Use full sentences, plain language, and good [formatting](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax). Screenshots and links to documents/discussions are welcome. validations: required: true - type: textarea @@ -20,13 +20,13 @@ body: attributes: label: Acceptance criteria description: "If known, share 1-3 statements that would need to be true for this issue to be considered resolved. Use a [task list](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/about-task-lists#creating-task-lists) if appropriate." - placeholder: "- [ ] The button does the thing." + placeholder: "- [ ]" - type: textarea id: links-to-other-issues attributes: label: Links to other issues description: | - Add the issue #number of other issues this relates to and how (e.g., 🚧 Blocks, ⛔️ Is blocked by, 🔄 Relates to). + "Add issue #numbers this relates to and how (e.g., 🚧 :construction: Blocks, ⛔️ :no_entry: Is blocked by, 🔄 :repeat: Relates to)." placeholder: 🔄 Relates to... - type: markdown id: note From 003db40e58d73a205d027fd8674b89a716370708 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 12 Oct 2023 15:32:10 -0400 Subject: [PATCH 02/18] use shortened names for orgs in the choicefield which satisfies the admin requirement, replace short name with a long name in the user facing app in the form, summary page, manage app page --- src/registrar/admin.py | 4 +- src/registrar/forms/application_wizard.py | 3 +- ...napplication_organization_type_and_more.py | 52 +++++++++++++++++ src/registrar/models/domain_application.py | 56 ++++++++++++++----- src/registrar/models/domain_information.py | 1 + .../templates/application_review.html | 9 ++- .../templates/application_status.html | 6 +- src/registrar/templatetags/custom_filters.py | 15 +++++ src/registrar/tests/test_admin.py | 17 ++++++ src/registrar/tests/test_views.py | 28 +++++++++- 10 files changed, 171 insertions(+), 20 deletions(-) create mode 100644 src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 174500f28..aef56e0b3 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -219,9 +219,9 @@ class MyUserAdmin(BaseUserAdmin): # (which should in theory be the ONLY group) def group(self, obj): if obj.groups.filter(name="full_access_group").exists(): - return "Full access" + return "full_access_group" elif obj.groups.filter(name="cisa_analysts_group").exists(): - return "Analyst" + return "cisa_analysts_group" return "" def get_list_display(self, request): diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 516683247..2fd78cdd8 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -153,7 +153,8 @@ class RegistrarFormSet(forms.BaseFormSet): class OrganizationTypeForm(RegistrarForm): organization_type = forms.ChoiceField( - choices=DomainApplication.OrganizationChoices.choices, + # use the long names in the application form + choices=DomainApplication.OrganizationChoicesVerbose.choices, widget=forms.RadioSelect, error_messages={"required": "Select the type of organization you represent."}, ) diff --git a/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py b/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py new file mode 100644 index 000000000..bdbca82d8 --- /dev/null +++ b/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py @@ -0,0 +1,52 @@ +# Generated by Django 4.2.1 on 2023-10-12 19:30 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0037_create_groups_v01"), + ] + + operations = [ + migrations.AlterField( + model_name="domainapplication", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of organization", + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of Organization", + max_length=255, + null=True, + ), + ), + ] diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 68429d381..a4752aa88 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -105,28 +105,57 @@ class DomainApplication(TimeStampedModel): ARMED_FORCES_AP = "AP", "Armed Forces Pacific (AP)" class OrganizationChoices(models.TextChoices): + + """ + Primary organization choices: + For use in django admin + Keys need to match OrganizationChoicesVerbose + """ + + FEDERAL = "federal", "Federal" + INTERSTATE = "interstate", "Interstate" + STATE_OR_TERRITORY = "state_or_territory", "State or territory" + TRIBAL = "tribal", "Tribal" + COUNTY = "county", "County" + CITY = "city", "City" + SPECIAL_DISTRICT = "special_district", "Special district" + SCHOOL_DISTRICT = "school_district", "School district" + + class OrganizationChoicesVerbose(models.TextChoices): + + """ + Secondary organization choices + For use in the application form and on the templates + Keys need to match OrganizationChoices + """ + FEDERAL = ( "federal", - "Federal: an agency of the U.S. government's executive, legislative, " - "or judicial branches", + "Federal: an agency of the U.S. government's executive, " + "legislative, or judicial branches", ) INTERSTATE = "interstate", "Interstate: an organization of two or more states" - STATE_OR_TERRITORY = "state_or_territory", ( - "State or territory: one of the 50 U.S. states, the District of " - "Columbia, American Samoa, Guam, Northern Mariana Islands, " - "Puerto Rico, or the U.S. Virgin Islands" + STATE_OR_TERRITORY = ( + "state_or_territory", + "State or territory: one of the 50 U.S. states, the District of Columbia, " + "American Samoa, Guam, Northern Mariana Islands, Puerto Rico, or the U.S. " + "Virgin Islands", ) - TRIBAL = "tribal", ( - "Tribal: a tribal government recognized by the federal or " - "a state government" + TRIBAL = ( + "tribal", + "Tribal: a tribal government recognized by the federal or a state " + "government", ) COUNTY = "county", "County: a county, parish, or borough" CITY = "city", "City: a city, town, township, village, etc." - SPECIAL_DISTRICT = "special_district", ( - "Special district: an independent organization within a single state" + SPECIAL_DISTRICT = ( + "special_district", + "Special district: an independent organization within a single state", ) - SCHOOL_DISTRICT = "school_district", ( - "School district: a school district that is not part of a local government" + SCHOOL_DISTRICT = ( + "school_district", + "School district: a school district that is not part of a local " + "government", ) class BranchChoices(models.TextChoices): @@ -297,6 +326,7 @@ class DomainApplication(TimeStampedModel): # ##### data fields from the initial form ##### organization_type = models.CharField( max_length=255, + # use the short names in Django admin choices=OrganizationChoices.choices, null=True, blank=True, diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 3b93aff48..d2bc5c53d 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -21,6 +21,7 @@ class DomainInformation(TimeStampedModel): StateTerritoryChoices = DomainApplication.StateTerritoryChoices + # use the short names in Django admin OrganizationChoices = DomainApplication.OrganizationChoices BranchChoices = DomainApplication.BranchChoices diff --git a/src/registrar/templates/application_review.html b/src/registrar/templates/application_review.html index be81303b8..6a4dcbffd 100644 --- a/src/registrar/templates/application_review.html +++ b/src/registrar/templates/application_review.html @@ -1,5 +1,6 @@ {% extends 'application_form.html' %} {% load static url_helpers %} +{% load custom_filters %} {% block form_required_fields_help_text %} {# there are no required fields on this page so don't show this #} @@ -26,7 +27,13 @@
{{ form_titles|get_item:step }}
{% if step == Step.ORGANIZATION_TYPE %} - {{ application.get_organization_type_display|default:"Incomplete" }} + {% if application.organization_type is not None %} + {% with long_org_type=application.organization_type|get_organization_long_name %} + {{ long_org_type }} + {% endwith %} + {% else %} + Incomplete + {% endif %} {% endif %} {% if step == Step.TRIBAL_GOVERNMENT %} {{ application.tribe_name|default:"Incomplete" }} diff --git a/src/registrar/templates/application_status.html b/src/registrar/templates/application_status.html index a68c07c8a..79d0f7ff9 100644 --- a/src/registrar/templates/application_status.html +++ b/src/registrar/templates/application_status.html @@ -1,5 +1,7 @@ {% extends 'base.html' %} +{% load custom_filters %} + {% block title %}Domain request status | {{ domainapplication.requested_domain.name }} | {% endblock %} {% load static url_helpers %} @@ -50,7 +52,9 @@

Summary of your domain request

{% with heading_level='h3' %} - {% include "includes/summary_item.html" with title='Type of organization' value=domainapplication.get_organization_type_display heading_level=heading_level %} + {% with long_org_type=domainapplication.organization_type|get_organization_long_name %} + {% include "includes/summary_item.html" with title='Type of organization' value=long_org_type heading_level=heading_level %} + {% endwith %} {% if domainapplication.tribe_name %} {% include "includes/summary_item.html" with title='Tribal government' value=domainapplication.tribe_name heading_level=heading_level %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 3614db18e..e90c3166d 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -1,5 +1,6 @@ from django import template import re +from registrar.models.domain_application import DomainApplication register = template.Library() @@ -48,3 +49,17 @@ def contains_checkbox(html_list): if re.search(r']*type="checkbox"', html_string): return True return False + + +@register.filter +def get_organization_long_name(organization_type): + organization_choices_dict = {} + + for name, value in DomainApplication.OrganizationChoicesVerbose.choices: + organization_choices_dict[name] = value + + long_form_type = organization_choices_dict[organization_type] + if long_form_type is not None: + return long_form_type + + return "Error" diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 51ace34f7..b5827d3e9 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -300,6 +300,23 @@ class TestDomainApplicationAdmin(MockEppLib): self.superuser = create_superuser() self.staffuser = create_user() + def test_short_org_name_in_applications_list(self): + """ + Make sure the short name is displaying in admin on the list page + """ + self.client.force_login(self.superuser) + completed_application() + response = self.client.get("/admin/registrar/domainapplication/") + # There are 3 template references to Federal (3) plus one reference in the table + # for our actual application + self.assertContains(response, "Federal", count=4) + # This may be a bit more robust + self.assertContains( + response, 'Federal', count=1 + ) + # Now let's make sure the long description does not exist + self.assertNotContains(response, "Federal: an agency of the U.S. government") + @boto3_mocking.patching def test_save_model_sends_submitted_email(self): # make sure there is no user with this email diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 2194b42db..32a22916e 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -141,9 +141,12 @@ class DomainApplicationTests(TestWithUser, WebTest): @boto3_mocking.patching def test_application_form_submission(self): - """Can fill out the entire form and submit. + """ + Can fill out the entire form and submit. As we add additional form pages, we need to include them here to make this test work. + + This test also looks for the long organization name on the summary page. """ num_pages_tested = 0 # elections, type_of_work, tribal_government, no_other_contacts @@ -427,7 +430,8 @@ class DomainApplicationTests(TestWithUser, WebTest): review_form = review_page.form # Review page contains all the previously entered data - self.assertContains(review_page, "Federal") + # Let's make sure the long org name is displayed + self.assertContains(review_page, "Federal: an agency of the U.S. government") self.assertContains(review_page, "Executive") self.assertContains(review_page, "Testorg") self.assertContains(review_page, "address 1") @@ -1065,6 +1069,26 @@ class DomainApplicationTests(TestWithUser, WebTest): # page = self.app.get(url) # self.assertNotContains(page, "VALUE") + def test_long_org_name_in_application(self): + """ + Make sure the long name is displaying in the application form, + org step + """ + request = self.app.get(reverse("application:")).follow() + self.assertContains(request, "Federal: an agency of the U.S. government") + + def test_long_org_name_in_application_manage(self): + """ + Make sure the long name is displaying in the application summary + page (manage your application) + """ + completed_application(status=DomainApplication.SUBMITTED, user=self.user) + home_page = self.app.get("/") + self.assertContains(home_page, "city.gov") + # click the "Edit" link + detail_page = home_page.click("Manage") + self.assertContains(detail_page, "Federal: an agency of the U.S. government") + class TestWithDomainPermissions(TestWithUser): def setUp(self): From 7b13dd4b3a68e973e0d6a204a3ff3398dff47737 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 12 Oct 2023 16:14:51 -0400 Subject: [PATCH 03/18] trigger PR pipeline --- src/registrar/models/domain_application.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index a4752aa88..4da32ad18 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -107,9 +107,9 @@ class DomainApplication(TimeStampedModel): class OrganizationChoices(models.TextChoices): """ - Primary organization choices: - For use in django admin - Keys need to match OrganizationChoicesVerbose + Primary organization choices: + For use in django admin + Keys need to match OrganizationChoicesVerbose """ FEDERAL = "federal", "Federal" @@ -124,9 +124,9 @@ class DomainApplication(TimeStampedModel): class OrganizationChoicesVerbose(models.TextChoices): """ - Secondary organization choices - For use in the application form and on the templates - Keys need to match OrganizationChoices + Secondary organization choices + For use in the application form and on the templates + Keys need to match OrganizationChoices """ FEDERAL = ( From 6709ac9ddb91259f060657970086c05c50cde98e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 13 Oct 2023 16:20:38 -0400 Subject: [PATCH 04/18] trick the sandbox by deleting a conflicting migration --- .../migrations/0035_alter_user_options.py | 42 ++++++++++++++- ...napplication_organization_type_and_more.py | 52 ------------------- 2 files changed, 41 insertions(+), 53 deletions(-) delete mode 100644 src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py diff --git a/src/registrar/migrations/0035_alter_user_options.py b/src/registrar/migrations/0035_alter_user_options.py index 7ed81cdf5..b76f07c5d 100644 --- a/src/registrar/migrations/0035_alter_user_options.py +++ b/src/registrar/migrations/0035_alter_user_options.py @@ -1,6 +1,6 @@ # Generated by Django 4.2.1 on 2023-09-27 18:53 -from django.db import migrations +from django.db import migrations, models class Migration(migrations.Migration): @@ -18,4 +18,44 @@ class Migration(migrations.Migration): ] }, ), + migrations.AlterField( + model_name="domainapplication", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of organization", + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of Organization", + max_length=255, + null=True, + ), + ), ] diff --git a/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py b/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py deleted file mode 100644 index bdbca82d8..000000000 --- a/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py +++ /dev/null @@ -1,52 +0,0 @@ -# Generated by Django 4.2.1 on 2023-10-12 19:30 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("registrar", "0037_create_groups_v01"), - ] - - operations = [ - migrations.AlterField( - model_name="domainapplication", - name="organization_type", - field=models.CharField( - blank=True, - choices=[ - ("federal", "Federal"), - ("interstate", "Interstate"), - ("state_or_territory", "State or territory"), - ("tribal", "Tribal"), - ("county", "County"), - ("city", "City"), - ("special_district", "Special district"), - ("school_district", "School district"), - ], - help_text="Type of organization", - max_length=255, - null=True, - ), - ), - migrations.AlterField( - model_name="domaininformation", - name="organization_type", - field=models.CharField( - blank=True, - choices=[ - ("federal", "Federal"), - ("interstate", "Interstate"), - ("state_or_territory", "State or territory"), - ("tribal", "Tribal"), - ("county", "County"), - ("city", "City"), - ("special_district", "Special district"), - ("school_district", "School district"), - ], - help_text="Type of Organization", - max_length=255, - null=True, - ), - ), - ] From a2574124ffaae7eccaf3d94bb3e67aefdffe2ec7 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 13 Oct 2023 16:27:23 -0400 Subject: [PATCH 05/18] return migrations to pervious state --- .../migrations/0035_alter_user_options.py | 42 +-------------- ...napplication_organization_type_and_more.py | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 41 deletions(-) create mode 100644 src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py diff --git a/src/registrar/migrations/0035_alter_user_options.py b/src/registrar/migrations/0035_alter_user_options.py index b76f07c5d..7ed81cdf5 100644 --- a/src/registrar/migrations/0035_alter_user_options.py +++ b/src/registrar/migrations/0035_alter_user_options.py @@ -1,6 +1,6 @@ # Generated by Django 4.2.1 on 2023-09-27 18:53 -from django.db import migrations, models +from django.db import migrations class Migration(migrations.Migration): @@ -18,44 +18,4 @@ class Migration(migrations.Migration): ] }, ), - migrations.AlterField( - model_name="domainapplication", - name="organization_type", - field=models.CharField( - blank=True, - choices=[ - ("federal", "Federal"), - ("interstate", "Interstate"), - ("state_or_territory", "State or territory"), - ("tribal", "Tribal"), - ("county", "County"), - ("city", "City"), - ("special_district", "Special district"), - ("school_district", "School district"), - ], - help_text="Type of organization", - max_length=255, - null=True, - ), - ), - migrations.AlterField( - model_name="domaininformation", - name="organization_type", - field=models.CharField( - blank=True, - choices=[ - ("federal", "Federal"), - ("interstate", "Interstate"), - ("state_or_territory", "State or territory"), - ("tribal", "Tribal"), - ("county", "County"), - ("city", "City"), - ("special_district", "Special district"), - ("school_district", "School district"), - ], - help_text="Type of Organization", - max_length=255, - null=True, - ), - ), ] diff --git a/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py b/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py new file mode 100644 index 000000000..a06ea0451 --- /dev/null +++ b/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py @@ -0,0 +1,52 @@ +# Generated by Django 4.2.1 on 2023-10-13 20:26 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0037_create_groups_v01"), + ] + + operations = [ + migrations.AlterField( + model_name="domainapplication", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of organization", + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of Organization", + max_length=255, + null=True, + ), + ), + ] From 5e787eeed324f8789c8a1e3e9dcf0694d13d588a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 13 Oct 2023 16:30:29 -0400 Subject: [PATCH 06/18] merge diverging migrations --- src/registrar/migrations/0039_merge_20231013_2029.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/registrar/migrations/0039_merge_20231013_2029.py diff --git a/src/registrar/migrations/0039_merge_20231013_2029.py b/src/registrar/migrations/0039_merge_20231013_2029.py new file mode 100644 index 000000000..aed231bdc --- /dev/null +++ b/src/registrar/migrations/0039_merge_20231013_2029.py @@ -0,0 +1,12 @@ +# Generated by Django 4.2.1 on 2023-10-13 20:29 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0038_alter_domainapplication_organization_type_and_more"), + ("registrar", "0038_create_groups_v02"), + ] + + operations = [] From 372ead1121372d885b19fc56fce641f1cb6b7541 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 13 Oct 2023 17:05:33 -0400 Subject: [PATCH 07/18] refactor org literal mapping to dict, tweak error handing, lint --- src/registrar/models/domain_application.py | 12 ++++++------ src/registrar/templatetags/custom_filters.py | 17 ++++++++++------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 4da32ad18..a4752aa88 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -107,9 +107,9 @@ class DomainApplication(TimeStampedModel): class OrganizationChoices(models.TextChoices): """ - Primary organization choices: - For use in django admin - Keys need to match OrganizationChoicesVerbose + Primary organization choices: + For use in django admin + Keys need to match OrganizationChoicesVerbose """ FEDERAL = "federal", "Federal" @@ -124,9 +124,9 @@ class DomainApplication(TimeStampedModel): class OrganizationChoicesVerbose(models.TextChoices): """ - Secondary organization choices - For use in the application form and on the templates - Keys need to match OrganizationChoices + Secondary organization choices + For use in the application form and on the templates + Keys need to match OrganizationChoices """ FEDERAL = ( diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index e90c3166d..158b7269e 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -1,8 +1,10 @@ +import logging from django import template import re from registrar.models.domain_application import DomainApplication register = template.Library() +logger = logging.getLogger(__name__) @register.filter(name="extract_value") @@ -53,13 +55,14 @@ def contains_checkbox(html_list): @register.filter def get_organization_long_name(organization_type): - organization_choices_dict = {} - - for name, value in DomainApplication.OrganizationChoicesVerbose.choices: - organization_choices_dict[name] = value + # https://gist.github.com/OmenApps/3eef60ba4204f3d1842d9d7477efcce1#file-django_choices-txt-L28 + organization_choices_dict = dict( + DomainApplication.OrganizationChoicesVerbose.choices + ) long_form_type = organization_choices_dict[organization_type] - if long_form_type is not None: - return long_form_type + if long_form_type is None: + logger.error("Organization type error, triggered by a template's custom filter") + return "Error" - return "Error" + return long_form_type From 02a9c98a570f0c1d58a5d47f53e947990065a081 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 18 Oct 2023 17:49:49 -0400 Subject: [PATCH 08/18] fix org in domain table and write a unit test for it --- src/registrar/admin.py | 2 +- src/registrar/tests/test_admin.py | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 88f0de869..417d23708 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -721,7 +721,7 @@ class DomainAdmin(ListHeaderAdmin): ] def organization_type(self, obj): - return obj.domain_info.organization_type + return obj.domain_info.get_organization_type_display() organization_type.admin_order_field = ( # type: ignore "domain_info__organization_type" diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index b5827d3e9..805e97171 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -52,6 +52,26 @@ class TestDomainAdmin(MockEppLib): self.factory = RequestFactory() super().setUp() + def test_short_org_name_in_domains_list(self): + """ + Make sure the short name is displaying in admin on the list page + """ + self.client.force_login(self.superuser) + application = completed_application(status=DomainApplication.IN_REVIEW) + application.approve() + + response = self.client.get("/admin/registrar/domain/") + + # There are 3 template references to Federal (3) plus one reference in the table + # for our actual application + self.assertContains(response, "Federal", count=4) + # This may be a bit more robust + self.assertContains( + response, 'Federal', count=1 + ) + # Now let's make sure the long description does not exist + self.assertNotContains(response, "Federal: an agency of the U.S. government") + @skip("Why did this test stop working, and is is a good test") def test_place_and_remove_hold(self): domain = create_ready_domain() @@ -243,8 +263,11 @@ class TestDomainAdmin(MockEppLib): raise def tearDown(self): - User.objects.all().delete() super().tearDown() + Domain.objects.all().delete() + DomainInformation.objects.all().delete() + DomainApplication.objects.all().delete() + User.objects.all().delete() class TestDomainApplicationAdminForm(TestCase): From d01bebec4110dc247fd3a882a86cdd07c9a9af4f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 18 Oct 2023 18:03:22 -0400 Subject: [PATCH 09/18] merge conflicting migrations --- src/registrar/migrations/0040_merge_20231018_2203.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/registrar/migrations/0040_merge_20231018_2203.py diff --git a/src/registrar/migrations/0040_merge_20231018_2203.py b/src/registrar/migrations/0040_merge_20231018_2203.py new file mode 100644 index 000000000..fad679098 --- /dev/null +++ b/src/registrar/migrations/0040_merge_20231018_2203.py @@ -0,0 +1,12 @@ +# Generated by Django 4.2.1 on 2023-10-18 22:03 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0039_alter_transitiondomain_status"), + ("registrar", "0039_merge_20231013_2029"), + ] + + operations = [] From 15ecaa83e70f0c40f8a5a06dc0f506a54967cbdc Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Fri, 20 Oct 2023 06:33:11 -0400 Subject: [PATCH 10/18] Separate additional context from the description --- .github/ISSUE_TEMPLATE/issue-default.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/issue-default.yml b/.github/ISSUE_TEMPLATE/issue-default.yml index 2252845bf..701742f72 100644 --- a/.github/ISSUE_TEMPLATE/issue-default.yml +++ b/.github/ISSUE_TEMPLATE/issue-default.yml @@ -10,9 +10,9 @@ body: - type: textarea id: issue-description attributes: - label: Issue description and context + label: Issue description description: | - Describe the issue so that someone who wasn't present for its discovery can understand why it matters. Use full sentences, plain language, and good [formatting](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax). Screenshots and links to documents/discussions are welcome. + Describe the issue so that someone who wasn't present for its discovery can understand why it matters. Use full sentences, plain language, and good [formatting](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax). validations: required: true - type: textarea @@ -21,6 +21,11 @@ body: label: Acceptance criteria description: "If known, share 1-3 statements that would need to be true for this issue to be considered resolved. Use a [task list](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/about-task-lists#creating-task-lists) if appropriate." placeholder: "- [ ]" + - type: textarea + id: additional-context + attributes: + label: Additional context + description: "Share any other thoughts, like how this might be implemented or fixed. Screenshots and links to documents/discussions are welcome." - type: textarea id: links-to-other-issues attributes: @@ -32,4 +37,5 @@ body: id: note attributes: value: | - > We may edit this issue's text to document our understanding and clarify the product work. + > We may edit the text in this issue to document our understanding and clarify the product work. + From 0b087a81517c4e2a66e14844de18adeb2441ad41 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Fri, 20 Oct 2023 06:38:34 -0400 Subject: [PATCH 11/18] fix spacing --- .github/ISSUE_TEMPLATE/issue-default.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/issue-default.yml b/.github/ISSUE_TEMPLATE/issue-default.yml index 701742f72..26384ceda 100644 --- a/.github/ISSUE_TEMPLATE/issue-default.yml +++ b/.github/ISSUE_TEMPLATE/issue-default.yml @@ -24,8 +24,8 @@ body: - type: textarea id: additional-context attributes: - label: Additional context - description: "Share any other thoughts, like how this might be implemented or fixed. Screenshots and links to documents/discussions are welcome." + label: Additional context + description: "Share any other thoughts, like how this might be implemented or fixed. Screenshots and links to documents/discussions are welcome." - type: textarea id: links-to-other-issues attributes: From 34e0ce955ab54384b2404fac4ef4adafc493f49d Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 20 Oct 2023 11:51:11 -0400 Subject: [PATCH 12/18] clean up a bad comment --- src/registrar/templatetags/custom_filters.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 158b7269e..14e2c9e3e 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -55,11 +55,9 @@ def contains_checkbox(html_list): @register.filter def get_organization_long_name(organization_type): - # https://gist.github.com/OmenApps/3eef60ba4204f3d1842d9d7477efcce1#file-django_choices-txt-L28 organization_choices_dict = dict( DomainApplication.OrganizationChoicesVerbose.choices ) - long_form_type = organization_choices_dict[organization_type] if long_form_type is None: logger.error("Organization type error, triggered by a template's custom filter") From 3cfdacccfc3bc5aaf7aa71eb97da959b2cf1fc44 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 20 Oct 2023 12:00:17 -0500 Subject: [PATCH 13/18] Change User management to Domain managers --- src/registrar/templates/domain_detail.html | 2 +- src/registrar/templates/domain_sidebar.html | 2 +- src/registrar/templates/domain_users.html | 17 +++++++++++++++-- src/registrar/tests/test_views.py | 2 +- src/registrar/views/domain.py | 2 +- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index e0d672093..4ddbd673a 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -52,7 +52,7 @@ {% include "includes/summary_item.html" with title='Security email' value='None provided' edit_link=url %} {% endif %} {% url 'domain-users' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='User management' users='true' list=True value=domain.permissions.all edit_link=url %} + {% include "includes/summary_item.html" with title='Domain managers' users='true' list=True value=domain.permissions.all edit_link=url %}
{% endblock %} {# domain_content #} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 1acd87eeb..ac45ad04c 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -100,7 +100,7 @@ - User management + Domain managers diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 22b9d18d1..5cb7acffd 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -1,10 +1,23 @@ {% extends "domain_base.html" %} {% load static %} -{% block title %}User management | {{ domain.name }} | {% endblock %} +{% block title %}Domain managers | {{ domain.name }} | {% endblock %} {% block domain_content %} -

User management

+

Domain managers

+ +

+ Domain managers can update all information related to a domain within the + .gov registrar, including contact details, authorizing official, security + email, and DNS name servers. +

+ +
    +
  • There is no limit to the number of domain managers you can add.
    • +
  • After adding a domain manager, an email invitation will be sent to that user with + instructions on how to set up an account.
    • +
  • To remove a domain manager, contact us for assistance. +
{% if domain.permissions %}
diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 0e8f895af..8ad855433 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1204,7 +1204,7 @@ class TestDomainUserManagement(TestDomainOverview): response = self.client.get( reverse("domain-users", kwargs={"pk": self.domain.id}) ) - self.assertContains(response, "User management") + self.assertContains(response, "Domain managers") def test_domain_user_management_add_link(self): """Button to get to user add page works.""" diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index aa71a7551..d9b671a65 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -656,7 +656,7 @@ class DomainSecurityEmailView(DomainFormBaseView): class DomainUsersView(DomainBaseView): - """User management page in the domain details.""" + """Domain managers page in the domain details.""" template_name = "domain_users.html" From a0911b46f84e7b16df9c0d38de591799a99c91e0 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 20 Oct 2023 13:28:36 -0500 Subject: [PATCH 14/18] load public_site_url helper --- src/registrar/templates/domain_users.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 5cb7acffd..f66eef5a6 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -1,5 +1,5 @@ {% extends "domain_base.html" %} -{% load static %} +{% load static url_helpers %} {% block title %}Domain managers | {{ domain.name }} | {% endblock %} From 7f153f77ed70147c39e021ab02d85498c1cb4bf2 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 20 Oct 2023 13:46:51 -0500 Subject: [PATCH 15/18] Review feedback: rename tests to match page name --- src/registrar/tests/test_views.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 8ad855433..1262347a1 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1199,14 +1199,14 @@ class TestDomainOverview(TestWithDomainPermissions, WebTest): self.assertEqual(response.status_code, 403) -class TestDomainUserManagement(TestDomainOverview): - def test_domain_user_management(self): +class TestDomainManagers(TestDomainOverview): + def test_domain_managers(self): response = self.client.get( reverse("domain-users", kwargs={"pk": self.domain.id}) ) self.assertContains(response, "Domain managers") - def test_domain_user_management_add_link(self): + def test_domain_managers_add_link(self): """Button to get to user add page works.""" management_page = self.app.get( reverse("domain-users", kwargs={"pk": self.domain.id}) From 020f2614d0e7c727d0261a4f9bf3da0c22fbff3e Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Fri, 20 Oct 2023 15:46:51 -0400 Subject: [PATCH 16/18] Update issue-default.yml --- .github/ISSUE_TEMPLATE/issue-default.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/issue-default.yml b/.github/ISSUE_TEMPLATE/issue-default.yml index 26384ceda..3a34b2943 100644 --- a/.github/ISSUE_TEMPLATE/issue-default.yml +++ b/.github/ISSUE_TEMPLATE/issue-default.yml @@ -21,9 +21,9 @@ body: label: Acceptance criteria description: "If known, share 1-3 statements that would need to be true for this issue to be considered resolved. Use a [task list](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/about-task-lists#creating-task-lists) if appropriate." placeholder: "- [ ]" - - type: textarea - id: additional-context - attributes: + - type: textarea + id: additional-context + attributes: label: Additional context description: "Share any other thoughts, like how this might be implemented or fixed. Screenshots and links to documents/discussions are welcome." - type: textarea From b8b12bad6ea2f1330ef840cdf1318795d7876fae Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 20 Oct 2023 17:09:50 -0400 Subject: [PATCH 17/18] Clean up migrations --- ...napplication_organization_type_and_more.py | 52 ------------------- .../migrations/0039_merge_20231013_2029.py | 12 ----- .../migrations/0040_merge_20231018_2203.py | 12 ----- 3 files changed, 76 deletions(-) delete mode 100644 src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py delete mode 100644 src/registrar/migrations/0039_merge_20231013_2029.py delete mode 100644 src/registrar/migrations/0040_merge_20231018_2203.py diff --git a/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py b/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py deleted file mode 100644 index a06ea0451..000000000 --- a/src/registrar/migrations/0038_alter_domainapplication_organization_type_and_more.py +++ /dev/null @@ -1,52 +0,0 @@ -# Generated by Django 4.2.1 on 2023-10-13 20:26 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("registrar", "0037_create_groups_v01"), - ] - - operations = [ - migrations.AlterField( - model_name="domainapplication", - name="organization_type", - field=models.CharField( - blank=True, - choices=[ - ("federal", "Federal"), - ("interstate", "Interstate"), - ("state_or_territory", "State or territory"), - ("tribal", "Tribal"), - ("county", "County"), - ("city", "City"), - ("special_district", "Special district"), - ("school_district", "School district"), - ], - help_text="Type of organization", - max_length=255, - null=True, - ), - ), - migrations.AlterField( - model_name="domaininformation", - name="organization_type", - field=models.CharField( - blank=True, - choices=[ - ("federal", "Federal"), - ("interstate", "Interstate"), - ("state_or_territory", "State or territory"), - ("tribal", "Tribal"), - ("county", "County"), - ("city", "City"), - ("special_district", "Special district"), - ("school_district", "School district"), - ], - help_text="Type of Organization", - max_length=255, - null=True, - ), - ), - ] diff --git a/src/registrar/migrations/0039_merge_20231013_2029.py b/src/registrar/migrations/0039_merge_20231013_2029.py deleted file mode 100644 index aed231bdc..000000000 --- a/src/registrar/migrations/0039_merge_20231013_2029.py +++ /dev/null @@ -1,12 +0,0 @@ -# Generated by Django 4.2.1 on 2023-10-13 20:29 - -from django.db import migrations - - -class Migration(migrations.Migration): - dependencies = [ - ("registrar", "0038_alter_domainapplication_organization_type_and_more"), - ("registrar", "0038_create_groups_v02"), - ] - - operations = [] diff --git a/src/registrar/migrations/0040_merge_20231018_2203.py b/src/registrar/migrations/0040_merge_20231018_2203.py deleted file mode 100644 index fad679098..000000000 --- a/src/registrar/migrations/0040_merge_20231018_2203.py +++ /dev/null @@ -1,12 +0,0 @@ -# Generated by Django 4.2.1 on 2023-10-18 22:03 - -from django.db import migrations - - -class Migration(migrations.Migration): - dependencies = [ - ("registrar", "0039_alter_transitiondomain_status"), - ("registrar", "0039_merge_20231013_2029"), - ] - - operations = [] From d5b21435ddea84afd8fb821c065a3606a37f1726 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 20 Oct 2023 17:11:00 -0400 Subject: [PATCH 18/18] re-create org names migration --- ...napplication_organization_type_and_more.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 src/registrar/migrations/0041_alter_domainapplication_organization_type_and_more.py diff --git a/src/registrar/migrations/0041_alter_domainapplication_organization_type_and_more.py b/src/registrar/migrations/0041_alter_domainapplication_organization_type_and_more.py new file mode 100644 index 000000000..07cfe0e77 --- /dev/null +++ b/src/registrar/migrations/0041_alter_domainapplication_organization_type_and_more.py @@ -0,0 +1,52 @@ +# Generated by Django 4.2.1 on 2023-10-20 21:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0040_alter_userdomainrole_role"), + ] + + operations = [ + migrations.AlterField( + model_name="domainapplication", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of organization", + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of Organization", + max_length=255, + null=True, + ), + ), + ]