Merge remote-tracking branch 'origin/main' into rjm/680-admin-workshop

.github/ISSUE_TEMPLATE/bug.yml

@@ -43,7 +43,7 @@ body:
   - type: textarea
     id: environment
     attributes:
-      label: Environment (optional)
+      label: Environment
       description: |
         Where is this issue occurring? If related to development environment, list the relevant tool versions.
         
@@ -54,12 +54,12 @@ body:
   - type: textarea
     id: additional-context
     attributes:
-      label: Additional Context (optional)
+      label: Additional Context
       description: "Please include additional references (screenshots, design links, documentation, etc.) that are relevant"
   - type: textarea
     id: issue-links
     attributes:
-      label: Issue Links (optional)
+      label: Issue Links
       description: |
         What other issues does this story relate to and how?
         

.github/ISSUE_TEMPLATE/issue-default.yml

@@ -1,37 +1,34 @@
 name: Issue
-description: Provide a title for the issue you are describing 
+description: Capture uncategorized work or content
-title: "Please provide a clear title"
+
-labels: ["review"]
-# assignees:
-#   - kitsushadow
 body:
   - type: markdown
+    id: help
     attributes:
       value: |
-        Describe the ticket your are capturing in further detail.
+        > **Note**
+        > GitHub Issues use [GitHub Flavored Markdown](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax) for formatting.
   - type: textarea
-    id: why
+    id: issue
     attributes:
-      label: Ticket Description
+      label: Issue Description
-      description: Please provide details to accurately reflect why this ticket is being captured and also what is necessary to resolve.
+      description: |
-      placeholder: Provide details describing your lead up to needing this issue as well as any resolution or requirements for resolving or working on this more.
+        Describe the issue you are adding or content you are suggesting.
-      value: "While (working on or discussing) (issue #000 or domain request validation) I discovered there was (a missing workflow, an improvement, a missing feature). To resolve this (more research, a new feature, a new field, an interview) is required. Provide any links, screenshots, or mockups which would further detail the description."
+        Share any next steps that should be taken our outcomes that would be beneficial.
-    validations:
-      required: true
-  - type: dropdown
-    id: type
-    attributes:
-      label: Issue Type
-      description: Does this work require
-      options:
-        - discovery (Default)
-        - development
-        - design review
     validations:
       required: true
   - type: textarea
-    id: Dependencies
+    id: additional-context
     attributes:
-      label: Link dependent issues
+      label: Additional Context (optional)
-      description: If this ticket is dependent on another issue or blocks a current issue, please link.
+      description: "Include additional references (screenshots, design links, documentation, etc.) that are relevant"
-      render: shell
+  - type: textarea
+    id: issue-links
+    attributes:
+      label: Issue Links
+      description: |
+        What other issues does this story relate to and how?
+        
+        Example:
+        - 🚧 Blocked by: #123
+        - 🔄 Relates to: #234

.github/ISSUE_TEMPLATE/story.yml

@@ -47,12 +47,12 @@ body:
   - type: textarea
     id: additional-context
     attributes:
-      label: Additional Context (optional)
+      label: Additional Context
       description: "Please include additional references (screenshots, design links, documentation, etc.) that are relevant"
   - type: textarea
     id: issue-links
     attributes:
-      label: Issue Links (optional)
+      label: Issue Links
       description: |
         What other issues does this story relate to and how?
         

docs/architecture/decisions/0021-django-admin.md

@@ -0,0 +1,41 @@
+# 21. Use Django Admin for Application Management
+
+Date: 2023-06-22
+
+## Status
+
+Accepted
+
+## Context
+
+CISA needs a way to perform administrative actions to manage the new get.gov application as well as the .gov domain
+application requests submitted. Analysts need to be able to view, review, and approve domain applications. Other
+dashboard views, reports, searches (with filters and sorting) are also highly desired.
+
+## Decision
+
+Use Django's [Admin](https://docs.djangoproject.com/en/4.2/ref/contrib/admin/) site for administrative actions. Django
+Admin gives administrators all the powers we anticipate needing (and more), with relatively little overhead on the
+development team.
+
+## Consequences
+
+Django admin provides the team with a _huge_ head start on the creation of an administrator portal.
+
+While Django Admin is highly customizable, design and development will be constrained by what is possible within Django
+Admin.
+
+We anticipate that this will, overall, speed up the time to MVP compared to building a completely custom solution.
+
+Django Admin offers omnipotence for administrators out of the box, with direct access to database objects. This includes
+the ability to put the application and its data in an erroneous state, based on otherwise normal business rules/logic.
+
+In contrast to building an admin interface from scratch where development activities would predominantly
+involve _building up_, leveraging Django Admin will require carefully _pairing back_ the functionalities available to
+users such as analysts.
+
+While we anticipate that Django Admin will meet (or even exceed) the user needs that we are aware of today, it is still
+an open question whether Django Admin will be the long-term administrator tool of choice. A pivot away from Django Admin
+in the future would of course mean starting from scratch at a later date, and potentially juggling two separate admin
+portals for a period of time while a custom solution is incrementally developed. This would result in an overall 
+_increase_ to the total amount of time invested in building an administrator portal. 

src/registrar/admin.py

@@ -214,16 +214,26 @@ class DomainApplicationAdmin(ListHeaderAdmin):
             # Get the original application from the database
             original_obj = models.DomainApplication.objects.get(pk=obj.pk)
 
-            if (
+            if obj.status != original_obj.status:
-                obj.status != original_obj.status
+                if obj.status == models.DomainApplication.STARTED:
-                and obj.status == models.DomainApplication.INVESTIGATING
+                    # No conditions
-            ):
+                    pass
-                # This is a transition annotated method in model which will throw an
+                elif obj.status == models.DomainApplication.SUBMITTED:
-                # error if the condition is violated. To make this work, we need to
+                    # This is an fsm in model which will throw an error if the
-                # call it on the original object which has the right status value,
+                    # transition condition is violated, so we call it on the
-                # but pass the current object which contains the up-to-date data
+                    # original object which has the right status value, and pass
-                # for the email.
+                    # the updated object which contains the up-to-date data
-                original_obj.in_review(obj)
+                    # for the side effects (like an email send). Same
+                    # comment applies to original_obj method calls below.
+                    original_obj.submit(updated_domain_application=obj)
+                elif obj.status == models.DomainApplication.INVESTIGATING:
+                    original_obj.in_review(updated_domain_application=obj)
+                elif obj.status == models.DomainApplication.APPROVED:
+                    original_obj.approve(updated_domain_application=obj)
+                elif obj.status == models.DomainApplication.WITHDRAWN:
+                    original_obj.withdraw()
+                else:
+                    logger.warning("Unknown status selected in django admin")
 
         super().save_model(request, obj, form, change)
 

src/registrar/fixtures.py

@@ -52,6 +52,11 @@ class UserFixture:
             "first_name": "Cameron",
             "last_name": "Dixon",
         },
+        {
+            "username": "0353607a-cbba-47d2-98d7-e83dcd5b90ea",
+            "first_name": "Ryan",
+            "last_name": "Brooks",
+        },
     ]
 
     STAFF = [

src/registrar/models/domain_application.py

@@ -471,60 +471,42 @@ class DomainApplication(TimeStampedModel):
         except Exception:
             return ""
 
-    def _send_confirmation_email(self):
+    def _send_status_update_email(
-        """Send a confirmation email that this application was submitted.
+        self, new_status, email_template, email_template_subject
+    ):
+        """Send a atatus update email to the submitter.
 
         The email goes to the email address that the submitter gave as their
         contact information. If there is not submitter information, then do
         nothing.
         """
+
         if self.submitter is None or self.submitter.email is None:
             logger.warning(
-                "Cannot send confirmation email, no submitter email address."
+                f"Cannot send {new_status} email, no submitter email address."
             )
             return
         try:
             send_templated_email(
-                "emails/submission_confirmation.txt",
+                email_template,
-                "emails/submission_confirmation_subject.txt",
+                email_template_subject,
                 self.submitter.email,
                 context={"application": self},
             )
-            logger.info(
+            logger.info(f"The {new_status} email sent to: {self.submitter.email}")
-                f"Submission confirmation email sent to: {self.submitter.email}"
-            )
         except EmailSendingError:
             logger.warning("Failed to send confirmation email", exc_info=True)
 
-    def _send_in_review_email(self):
-        """Send an email that this application is now in review.
-
-        The email goes to the email address that the submitter gave as their
-        contact information. If there is not submitter information, then do
-        nothing.
-        """
-        if self.submitter is None or self.submitter.email is None:
-            logger.warning(
-                "Cannot send status change (in review) email,"
-                "no submitter email address."
-            )
-            return
-        try:
-            send_templated_email(
-                "emails/status_change_in_review.txt",
-                "emails/status_change_in_review_subject.txt",
-                self.submitter.email,
-                context={"application": self},
-            )
-            logging.info(f"In review email sent to: {self.submitter.email}")
-        except EmailSendingError:
-            logger.warning(
-                "Failed to send status change (in review) email", exc_info=True
-            )
-
     @transition(field="status", source=[STARTED, WITHDRAWN], target=SUBMITTED)
-    def submit(self):
+    def submit(self, updated_domain_application=None):
-        """Submit an application that is started."""
+        """Submit an application that is started.
+
+        As a side effect, an email notification is sent.
+
+        This method is called in admin.py on the original application
+        which has the correct status value, but is passed the changed
+        application which has the up-to-date data that we'll use
+        in the email."""
 
         # check our conditions here inside the `submit` method so that we
         # can raise more informative exceptions
@@ -540,17 +522,52 @@ class DomainApplication(TimeStampedModel):
         if not DraftDomain.string_could_be_domain(self.requested_domain.name):
             raise ValueError("Requested domain is not a valid domain name.")
 
-        # When an application is submitted, we need to send a confirmation email
+        if updated_domain_application is not None:
-        # This is a side-effect of the state transition
+            # A DomainApplication is being passed to this method (ie from admin)
-        self._send_confirmation_email()
+            updated_domain_application._send_status_update_email(
+                "submission confirmation",
+                "emails/submission_confirmation.txt",
+                "emails/submission_confirmation_subject.txt",
+            )
+        else:
+            # Or this method is called with the right application
+            # for context, ie from views/application.py
+            self._send_status_update_email(
+                "submission confirmation",
+                "emails/submission_confirmation.txt",
+                "emails/submission_confirmation_subject.txt",
+            )
+
+    @transition(field="status", source=SUBMITTED, target=INVESTIGATING)
+    def in_review(self, updated_domain_application):
+        """Investigate an application that has been submitted.
+
+        As a side effect, an email notification is sent.
+
+        This method is called in admin.py on the original application
+        which has the correct status value, but is passed the changed
+        application which has the up-to-date data that we'll use
+        in the email."""
+
+        updated_domain_application._send_status_update_email(
+            "application in review",
+            "emails/status_change_in_review.txt",
+            "emails/status_change_in_review_subject.txt",
+        )
 
     @transition(field="status", source=[SUBMITTED, INVESTIGATING], target=APPROVED)
-    def approve(self):
+    def approve(self, updated_domain_application=None):
         """Approve an application that has been submitted.
 
         This has substantial side-effects because it creates another database
         object for the approved Domain and makes the user who created the
-        application into an admin on that domain.
+        application into an admin on that domain. It also triggers an email
+        notification.
+
+        This method is called in admin.py on the original application
+        which has the correct status value, but is passed the changed
+        application which has the up-to-date data that we'll use
+        in the email.
         """
 
         # create the domain
@@ -570,18 +587,19 @@ class DomainApplication(TimeStampedModel):
             user=self.creator, domain=created_domain, role=UserDomainRole.Roles.ADMIN
         )
 
-    @transition(field="status", source=SUBMITTED, target=INVESTIGATING)
+        if updated_domain_application is not None:
-    def in_review(self, updated_domain_application):
+            # A DomainApplication is being passed to this method (ie from admin)
-        """Investigate an application that has been submitted.
+            updated_domain_application._send_status_update_email(
-
+                "application approved",
-        This method is called in admin.py on the original application
+                "emails/status_change_approved.txt",
-        which has the correct status value, but is passed the changed
+                "emails/status_change_approved_subject.txt",
-        application which has the up-to-date data that we'll use
+            )
-        in the email."""
+        else:
-
+            self._send_status_update_email(
-        # When an application is moved to in review, we need to send a
+                "application approved",
-        # confirmation email. This is a side-effect of the state transition
+                "emails/status_change_approved.txt",
-        updated_domain_application._send_in_review_email()
+                "emails/status_change_approved_subject.txt",
+            )
 
     @transition(field="status", source=[SUBMITTED, INVESTIGATING], target=WITHDRAWN)
     def withdraw(self):

src/registrar/templates/emails/status_change_approved.txt

@@ -0,0 +1,40 @@
+{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #}
+Hi {{ application.submitter.first_name }}.
+
+Congratulations! Your .gov domain request has been approved.
+
+DOMAIN REQUESTED: {{ application.requested_domain.name }}
+REQUEST RECEIVED ON: {{ application.updated_at|date }}
+REQUEST #: {{ application.id }}
+STATUS: In review
+
+Now that your .gov domain has been approved, there are a few more things to do before your domain can be used. 
+
+
+YOU MUST ADD DOMAIN NAME SERVER INFORMATION
+
+Before your .gov domain can be used, you have to connect it to your Domain Name System (DNS) hosting service. At this time, we don’t provide DNS hosting services.
+Go to the domain management page to add your domain name server information <https://registrar.get.gov/domain/{{ application.id }}/nameservers>.
+
+Get help with adding your domain name server information <https://get.gov/help/domain-management/#manage-dns-information-for-your-domain>.
+
+
+ADD DOMAIN MANAGERS, SECURITY EMAIL
+
+We strongly recommend that you add other points of contact who will help manage your domain. We also recommend that you provide a security email. This email will allow the public to report security issues on your domain. Security emails are made public. 
+
+Go to the domain management page to add domain contacts <https://registrar.get.gov/domain/{{ application.id }}/your-contact-information> and a security email <https://registrar.get.gov/domain/{{ application.id }}/security-email>. 
+
+Get help with managing your .gov domain <https://get.gov/help/domain-management/>.
+
+
+THANK YOU
+
+.Gov helps the public identify official, trusted information. Thank you for using a .gov domain.
+
+----------------------------------------------------------------
+
+The .gov team
+Contact us: <https://get.gov/contact/>
+Visit <https://get.gov>
+{% endautoescape %}

src/registrar/templates/emails/status_change_approved_subject.txt

@@ -0,0 +1 @@
+Your .gov domain request is approved

src/registrar/tests/test_admin.py

@@ -1,7 +1,7 @@
 from django.test import TestCase, RequestFactory, Client
 from django.contrib.admin.sites import AdminSite
 from registrar.admin import DomainApplicationAdmin, ListHeaderAdmin
-from registrar.models import DomainApplication, User
+from registrar.models import DomainApplication, DomainInformation, User
 from .common import completed_application
 from django.contrib.auth import get_user_model
 
@@ -31,7 +31,56 @@ class TestDomainApplicationAdmin(TestCase):
         )
 
     @boto3_mocking.patching
-    def test_save_model_sends_email_on_property_change(self):
+    def test_save_model_sends_submitted_email(self):
+        # make sure there is no user with this email
+        EMAIL = "mayor@igorville.gov"
+        User.objects.filter(email=EMAIL).delete()
+
+        mock_client = MagicMock()
+        mock_client_instance = mock_client.return_value
+
+        with boto3_mocking.clients.handler_for("sesv2", mock_client):
+            # Create a sample application
+            application = completed_application()
+
+            # Create a mock request
+            request = self.factory.post(
+                "/admin/registrar/domainapplication/{}/change/".format(application.pk)
+            )
+
+            # Create an instance of the model admin
+            model_admin = DomainApplicationAdmin(DomainApplication, self.site)
+
+            # Modify the application's property
+            application.status = DomainApplication.SUBMITTED
+
+            # Use the model admin's save_model method
+            model_admin.save_model(request, application, form=None, change=True)
+
+        # Access the arguments passed to send_email
+        call_args = mock_client_instance.send_email.call_args
+        args, kwargs = call_args
+
+        # Retrieve the email details from the arguments
+        from_email = kwargs.get("FromEmailAddress")
+        to_email = kwargs["Destination"]["ToAddresses"][0]
+        email_content = kwargs["Content"]
+        email_body = email_content["Simple"]["Body"]["Text"]["Data"]
+
+        # Assert or perform other checks on the email details
+        expected_string = "We received your .gov domain request."
+        self.assertEqual(from_email, settings.DEFAULT_FROM_EMAIL)
+        self.assertEqual(to_email, EMAIL)
+        self.assertIn(expected_string, email_body)
+
+        # Perform assertions on the mock call itself
+        mock_client_instance.send_email.assert_called_once()
+
+        # Cleanup
+        application.delete()
+
+    @boto3_mocking.patching
+    def test_save_model_sends_in_review_email(self):
         # make sure there is no user with this email
         EMAIL = "mayor@igorville.gov"
         User.objects.filter(email=EMAIL).delete()
@@ -68,7 +117,7 @@ class TestDomainApplicationAdmin(TestCase):
         email_body = email_content["Simple"]["Body"]["Text"]["Data"]
 
         # Assert or perform other checks on the email details
-        expected_string = "Your .gov domain request is being reviewed"
+        expected_string = "Your .gov domain request is being reviewed."
         self.assertEqual(from_email, settings.DEFAULT_FROM_EMAIL)
         self.assertEqual(to_email, EMAIL)
         self.assertIn(expected_string, email_body)
@@ -79,6 +128,57 @@ class TestDomainApplicationAdmin(TestCase):
         # Cleanup
         application.delete()
 
+    @boto3_mocking.patching
+    def test_save_model_sends_approved_email(self):
+        # make sure there is no user with this email
+        EMAIL = "mayor@igorville.gov"
+        User.objects.filter(email=EMAIL).delete()
+
+        mock_client = MagicMock()
+        mock_client_instance = mock_client.return_value
+
+        with boto3_mocking.clients.handler_for("sesv2", mock_client):
+            # Create a sample application
+            application = completed_application(status=DomainApplication.INVESTIGATING)
+
+            # Create a mock request
+            request = self.factory.post(
+                "/admin/registrar/domainapplication/{}/change/".format(application.pk)
+            )
+
+            # Create an instance of the model admin
+            model_admin = DomainApplicationAdmin(DomainApplication, self.site)
+
+            # Modify the application's property
+            application.status = DomainApplication.APPROVED
+
+            # Use the model admin's save_model method
+            model_admin.save_model(request, application, form=None, change=True)
+
+        # Access the arguments passed to send_email
+        call_args = mock_client_instance.send_email.call_args
+        args, kwargs = call_args
+
+        # Retrieve the email details from the arguments
+        from_email = kwargs.get("FromEmailAddress")
+        to_email = kwargs["Destination"]["ToAddresses"][0]
+        email_content = kwargs["Content"]
+        email_body = email_content["Simple"]["Body"]["Text"]["Data"]
+
+        # Assert or perform other checks on the email details
+        expected_string = "Congratulations! Your .gov domain request has been approved."
+        self.assertEqual(from_email, settings.DEFAULT_FROM_EMAIL)
+        self.assertEqual(to_email, EMAIL)
+        self.assertIn(expected_string, email_body)
+
+        # Perform assertions on the mock call itself
+        mock_client_instance.send_email.assert_called_once()
+
+        # Cleanup
+        if DomainInformation.objects.get(id=application.pk) is not None:
+            DomainInformation.objects.get(id=application.pk).delete()
+        application.delete()
+
     def test_changelist_view(self):
         # Have to get creative to get past linter
         p = "adminpassword"

src/registrar/tests/test_models.py

@@ -14,7 +14,8 @@ from registrar.models import (
 )
 
 import boto3_mocking  # type: ignore
-from .common import MockSESClient, less_console_noise
+from .common import MockSESClient, less_console_noise, completed_application
+from django_fsm import TransitionNotAllowed
 
 boto3_mocking.clients.register_handler("sesv2", MockSESClient)
 
@@ -134,6 +135,123 @@ class TestDomainApplication(TestCase):
             0,
         )
 
+    def test_transition_not_allowed_submitted_submitted(self):
+        """Create an application with status submitted and call submit
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.SUBMITTED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.submit()
+
+    def test_transition_not_allowed_investigating_submitted(self):
+        """Create an application with status investigating and call submit
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.INVESTIGATING)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.submit()
+
+    def test_transition_not_allowed_approved_submitted(self):
+        """Create an application with status approved and call submit
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.APPROVED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.submit()
+
+    def test_transition_not_allowed_started_investigating(self):
+        """Create an application with status started and call in_review
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.STARTED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.in_review()
+
+    def test_transition_not_allowed_investigating_investigating(self):
+        """Create an application with status investigating and call in_review
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.INVESTIGATING)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.in_review()
+
+    def test_transition_not_allowed_approved_investigating(self):
+        """Create an application with status approved and call in_review
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.APPROVED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.in_review()
+
+    def test_transition_not_allowed_withdrawn_investigating(self):
+        """Create an application with status withdrawn and call in_review
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.WITHDRAWN)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.in_review()
+
+    def test_transition_not_allowed_started_approved(self):
+        """Create an application with status started and call approve
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.STARTED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.approve()
+
+    def test_transition_not_allowed_approved_approved(self):
+        """Create an application with status approved and call approve
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.APPROVED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.approve()
+
+    def test_transition_not_allowed_withdrawn_approved(self):
+        """Create an application with status withdrawn and call approve
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.WITHDRAWN)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.approve()
+
+    def test_transition_not_allowed_started_withdrawn(self):
+        """Create an application with status started and call withdraw
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.STARTED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.withdraw()
+
+    def test_transition_not_allowed_approved_withdrawn(self):
+        """Create an application with status approved and call withdraw
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.APPROVED)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.withdraw()
+
+    def test_transition_not_allowed_withdrawn_withdrawn(self):
+        """Create an application with status withdrawn and call withdraw
+        against transition rules"""
+
+        application = completed_application(status=DomainApplication.WITHDRAWN)
+
+        with self.assertRaises(TransitionNotAllowed):
+            application.withdraw()
+
 
 class TestPermissions(TestCase):