[litterbox] 2162: Delete Submitter field and Your contact information page #2693

erinysong · 2024-08-29T22:46:26Z

Ticket

Resolves #2162

Changes

Remove submitter from registrar models
Remove Contacts who are only assigned as domain request submitters (as opposed to Senior Officials and Other Contacts)
Remove Your contact information page from domain and domain request view since Your contact information uses submitter data which is now deprecated in favor of storing contact info through user profile (saved as Creator in Domain and DomainRequest)
Remove submitter from fixtures
Remove references to submitter from models diagram
Remove tests involving submitter and refactor email tests to test new behavior of sending emails to creators per Change email sending to use Creator email not submitter on domain requests #2157

Context for reviewers

Now that the Creator field takes data from user profiles to associate domains and domain requests with a point of contact, we no longer need the submitter field and are removing it from the system.

Tagged @dotgov-designers since this involves removing a page that uses submitter data which we're removing.

Setup

Code Review Verification Steps

1. Test submitter no longer exists on /admin

Go to getgov-es /admin page. On the pages for Domain and DomainRequest, confirm that Submitter no longer exists below Creator as a field.

2. Test Your contact page no longer exists on registrar
Verify that Your contact information no longer exists as a page on the following:

Domain and Domain Request pages as a sidebar menu item and page

2. Domain Request form as a sidebar menu item and page

As the original developer, I have

Satisfied acceptance criteria and met development standards

Met the acceptance criteria, or will meet them in a subsequent PR
Created/modified automated tests
Added at least 2 developers as PR reviewers (only 1 will need to approve)
Messaged on Slack or in standup to notify the team that a PR is ready for review
Changes to “how we do things” are documented in READMEs and or onboarding guide
If any model was updated to modify/add/delete columns, makemigrations was ran and the associated migrations file has been commited.

Ensured code standards are met (Original Developer)

All new functions and methods are commented using plain language
Did dependency updates in Pipfile also get changed in requirements.txt?
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Add at least 1 designer as PR reviewer

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Pulled this branch locally and tested it
Reviewed this code and left comments
Checked that all code is adequately covered by tests
Made it clear which comments need to be addressed before this work is merged
If any model was updated to modify/add/delete columns, makemigrations was ran and the associated migrations file has been commited.

Ensured code standards are met (Code reviewer)

All new functions and methods are commented using plain language
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values
(Rarely needed) Did dependency updates in Pipfile also get changed in requirements.txt?

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist

As a designer reviewer, I have

Verified that the changes match the design intention

Checked that the design translated visually
Checked behavior
Checked different states (empty, one, some, error)
Checked for landmarks, page heading structure, and links
Tried to break the intended flow

Validated user-facing changes as a designer

Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers (check off which ones were used)
- Chrome
- Microsoft Edge
- FireFox
- Safari
(Rarely needed) Tested as both an analyst and applicant user

Screenshots

… es/2574-remove-submitter

github-actions · 2024-09-10T15:58:23Z

🥳 Successfully deployed to developer sandbox es.

github-actions · 2024-09-10T16:12:01Z

🥳 Successfully deployed to developer sandbox es.

zandercymatics

Getting some weird behavior with the email allow list when the profile_feature flag is turned off. Basically if you remove yourself from the email allow list and send out an email when the flag is off, it says it sends successfully (though it did not). I think its because of this logic that is still lingering.

zandercymatics · 2024-09-10T19:50:28Z

src/registrar/forms/domain_request_wizard.py

@@ -387,12 +387,12 @@ class PurposeForm(RegistrarForm):


 class YourContactForm(RegistrarForm):


I think you can safely delete this entire thing now since we are deleting the domain_your_contact_info page. It looks like we aren't using it anywhere.

zandercymatics · 2024-09-10T19:55:36Z

src/registrar/tests/test_admin_request.py

+        self.assert_email_is_accurate(
+            "ORGANIZATION ALREADY HAS A .GOV DOMAIN", 0, _creator.email, bcc_email_address=BCC_EMAIL
+        )
        self.assertEqual(len(self.mock_client.EMAILS_SENT), 1)


Suggested change

self.assert_email_is_accurate(

"ORGANIZATION ALREADY HAS A .GOV DOMAIN", 0, _creator.email, bcc_email_address=BCC_EMAIL

)

self.assertEqual(len(self.mock_client.EMAILS_SENT), 1)

self.assert_email_is_accurate(

"ORGANIZATION ALREADY HAS A .GOV DOMAIN", 0, EMAIL, bcc_email_address=BCC_EMAIL

)

self.assertEqual(len(self.mock_client.EMAILS_SENT), 1)

Can you change these tests too such that it tests on EMAIL rather than _creator.email?

zandercymatics · 2024-09-10T19:56:42Z

src/registrar/tests/test_emails.py

@@ -112,9 +111,7 @@ def test_submission_confirmation_other_contacts_spacing(self):
        _, kwargs = self.mock_client.send_email.call_args
        body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"]
        self.assertIn("Other employees from your organization:", body)
-        # spacing should be right between adjacent elements
-        self.assertRegex(body, r"5556\n\nOther employees")


@therealslimhsiehdy See above ^^

zandercymatics · 2024-09-10T20:01:04Z

src/registrar/tests/test_views_domain.py

-class TestDomainContactInformation(TestDomainOverview):
-    @less_console_noise_decorator
-    def test_domain_your_contact_information(self):
-        """Can load domain's your contact information page."""
-        page = self.client.get(reverse("domain-your-contact-information", kwargs={"pk": self.domain.id}))
-        self.assertContains(page, "Your contact information")
-
-    @less_console_noise_decorator
-    def test_domain_your_contact_information_content(self):
-        """Logged-in user's contact information appears on the page."""
-        self.user.first_name = "Testy"
-        self.user.save()
-        page = self.app.get(reverse("domain-your-contact-information", kwargs={"pk": self.domain.id}))
-        self.assertContains(page, "Testy")
-
-


Basically, we're removing the feature flag. But as of now, this page will appear on the domain view page even though it no longer exists. This will return an error is a dead link when you navigate to it @therealslimhsiehdy

github-actions · 2024-09-10T21:18:08Z

🥳 Successfully deployed to developer sandbox es.

zandercymatics

LGTM after your changes, but I'd suggest that Alysia briefly looks this over as well before merging. There are quite a few varying submitter "flows" so its possible that something was missed, though I tried to be as thorough as I could

… es/2162-delete-submitter-v2

github-actions · 2024-09-11T17:08:32Z

🥳 Successfully deployed to developer sandbox es.

Matt-Spence

I focused specifically on the migrations, they LGTM. I ran them in reverse a couple times to ensure if we need to revert that things will work fine. Only note: I agree with @zandercymatics's comment here that we should consider utilizing the export feature to save the data.

asaki222

LGTM

CocoByte

Zander was quite thorough. I downloaded the branch and found a couple areas I want to double-check on...

Don't we also want to get rid of Submitter in these areas?...

CocoByte

Nice work! Approved

therealslimhsiehdy · 2024-09-12T21:34:50Z

Zander was quite thorough. I downloaded the branch and found a couple areas I want to double-check on...

Don't we also want to get rid of Submitter in these areas?...

FYI for the JS section bc it's variable naming I decided against updating that for now. Great catch on the second screenshot -- I've updated it in this commit

github-actions · 2024-09-12T21:37:42Z

🥳 Successfully deployed to developer sandbox es.

github-actions · 2024-09-12T21:39:57Z

🥳 Successfully deployed to developer sandbox es.

github-actions · 2024-09-12T21:45:52Z

🥳 Successfully deployed to developer sandbox es.

github-actions · 2024-09-12T21:51:24Z

🥳 Successfully deployed to developer sandbox es.

erinysong and others added 28 commits August 26, 2024 11:23

Remove refs to submitter in domain request

f1c8f52

Remove submitter from models

2f2b2fe

Remove submitter from model tests

2713e59

Continued test cleanup

ff9e73d

Remove test_is_creator_complete test

11e44a3

Remove test test_home_deletes_domain_request_and_orphans

9a83287

Delete test_home_deletes_domain_request_and_orphans

d0b3dd7

Remove submitter from deleting orphaned contacts

2c43533

Delete outdated contact join tests

06ef54f

remove submitter from orphan tests

804d19f

Revert deleted contact shared

29de97d

Restore variable in orphan tests

5b3ae28

Update email status tests

2fcad73

Refactor email tests to deprecate submitter

a9ab882

Fix linting

a431b41

Remove submitter refs in docs

71d4435

Save progress

2d52e02

Remove stragglign file

171be9a

Test diagram update

98a50fb

Revert Pipfile.lock

dd99f8f

Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…

eac8363

… es/2574-remove-submitter

Correct migrations

f46cc14

Update diagrams

ce0befc

remove unused INVESTIGATOR_NOT_SUBMITTER error code

bf00f20

Revert requirements.txt

fde8723

Update migrations

1bfc2df

Revert settings.py

f41c6b3

Delete submitter contacts

d879044

erinysong requested a review from abroddrick as a code owner August 29, 2024 22:46

Add delete domain info migration

be54f8c

therealslimhsiehdy assigned asaki222 Sep 10, 2024

Address feedback

d0ae331

Update unit tests

c909c2a

zandercymatics requested changes Sep 10, 2024

View reviewed changes

Remove your contact information pointers and update tests

f3ae6f8

zandercymatics approved these changes Sep 11, 2024

View reviewed changes

Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…

171b47c

… es/2162-delete-submitter-v2

CocoByte self-assigned this Sep 12, 2024

Matt-Spence approved these changes Sep 12, 2024

View reviewed changes

asaki222 approved these changes Sep 12, 2024

View reviewed changes

CocoByte requested changes Sep 12, 2024

View reviewed changes

therealslimhsiehdy added 2 commits September 12, 2024 14:31

Remove extraneous submitter

e5a0f94

Merge branch 'main' into es/2162-delete-submitter-v2

5c32a19

therealslimhsiehdy requested a review from CocoByte September 12, 2024 21:33

CocoByte approved these changes Sep 12, 2024

View reviewed changes

Fix linter

1eac50b

Update migration numerics

5f3b1b1

therealslimhsiehdy assigned Matt-Spence Sep 12, 2024

Remove line from unit test - no submitter anymore for domain request

289f7bd

therealslimhsiehdy merged commit c1daa45 into main Sep 13, 2024
10 checks passed

therealslimhsiehdy deleted the es/2162-delete-submitter-v2 branch September 13, 2024 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[litterbox] 2162: Delete Submitter field and Your contact information page #2693

[litterbox] 2162: Delete Submitter field and Your contact information page #2693

erinysong commented Aug 29, 2024 •

edited by witha-c

Loading

github-actions bot commented Sep 10, 2024

github-actions bot commented Sep 10, 2024

zandercymatics left a comment

zandercymatics Sep 10, 2024

zandercymatics Sep 10, 2024

zandercymatics Sep 10, 2024

zandercymatics Sep 10, 2024

github-actions bot commented Sep 10, 2024

zandercymatics left a comment •

edited

Loading

github-actions bot commented Sep 11, 2024

Matt-Spence left a comment

asaki222 left a comment

CocoByte left a comment

CocoByte left a comment

therealslimhsiehdy commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

		@@ -387,12 +387,12 @@ class PurposeForm(RegistrarForm):


		class YourContactForm(RegistrarForm):

[litterbox] 2162: Delete Submitter field and Your contact information page #2693

[litterbox] 2162: Delete Submitter field and Your contact information page #2693

Conversation

erinysong commented Aug 29, 2024 • edited by witha-c Loading

Ticket

Changes

Context for reviewers

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Ensured code standards are met (Original Developer)

Validated user-facing changes (if applicable)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Ensured code standards are met (Code reviewer)

Validated user-facing changes as a developer

As a designer reviewer, I have

Verified that the changes match the design intention

Validated user-facing changes as a designer

Screenshots

github-actions bot commented Sep 10, 2024

github-actions bot commented Sep 10, 2024

zandercymatics left a comment

Choose a reason for hiding this comment

zandercymatics Sep 10, 2024

Choose a reason for hiding this comment

zandercymatics Sep 10, 2024

Choose a reason for hiding this comment

zandercymatics Sep 10, 2024

Choose a reason for hiding this comment

zandercymatics Sep 10, 2024

Choose a reason for hiding this comment

github-actions bot commented Sep 10, 2024

zandercymatics left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 11, 2024

Matt-Spence left a comment

Choose a reason for hiding this comment

asaki222 left a comment

Choose a reason for hiding this comment

CocoByte left a comment

Choose a reason for hiding this comment

CocoByte left a comment

Choose a reason for hiding this comment

therealslimhsiehdy commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

erinysong commented Aug 29, 2024 •

edited by witha-c

Loading

zandercymatics left a comment •

edited

Loading