domain manager tests and linted

src/registrar/tests/test_admin.py

@@ -710,7 +710,6 @@ class TestPortfolioInvitationAdmin(TestCase):
     #     self.assertContains(response, "Simulated error message", msg_prefix="Expected error message not found.")
 
 
-
 class TestHostAdmin(TestCase):
     """Tests for the HostAdmin class as super user
 

src/registrar/tests/test_forms.py

@@ -18,7 +18,11 @@ from registrar.forms.domain_request_wizard import (
     AboutYourOrganizationForm,
 )
 from registrar.forms.domain import ContactForm
-from registrar.forms.portfolio import BasePortfolioMemberForm, PortfolioInvitedMemberForm, PortfolioMemberForm, PortfolioNewMemberForm
+from registrar.forms.portfolio import (
+    PortfolioInvitedMemberForm,
+    PortfolioMemberForm,
+    PortfolioNewMemberForm,
+)
 from registrar.models.portfolio import Portfolio
 from registrar.models.portfolio_invitation import PortfolioInvitation
 from registrar.models.user import User
@@ -423,7 +427,9 @@ class TestBasePortfolioMemberForms(TestCase):
     def setUp(self):
         super().setUp()
         self.user = create_user()
-        self.portfolio, _ = Portfolio.objects.get_or_create(creator_id=self.user.id, organization_name="Hotel California")
+        self.portfolio, _ = Portfolio.objects.get_or_create(
+            creator_id=self.user.id, organization_name="Hotel California"
+        )
 
     def tearDown(self):
         super().tearDown()
@@ -433,10 +439,10 @@ class TestBasePortfolioMemberForms(TestCase):
         User.objects.all().delete()
 
     def _assert_form_is_valid(self, form_class, data, instance=None):
-        if instance != None:
+        if instance is not None:
             form = form_class(data=data, instance=instance)
         else:
-            print('no instance')
+            print("no instance")
             form = form_class(data=data)
         self.assertTrue(form.is_valid(), f"Form {form_class.__name__} failed validation with data: {data}")
         return form
@@ -497,7 +503,9 @@ class TestBasePortfolioMemberForms(TestCase):
 
         These things are handled in the views."""
 
-        user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(portfolio=self.portfolio, user=self.user)
+        user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(
+            portfolio=self.portfolio, user=self.user
+        )
         portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(portfolio=self.portfolio, email="hi@ho")
 
         data = {
@@ -534,7 +542,9 @@ class TestBasePortfolioMemberForms(TestCase):
         """Test that the clean method correctly handles the special "no_access" value for members.
         We'll need to add a portfolio, which in the app is handled by the view post."""
 
-        user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(portfolio=self.portfolio, user=self.user)
+        user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(
+            portfolio=self.portfolio, user=self.user
+        )
         portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(portfolio=self.portfolio, email="hi@ho")
 
         data = {
@@ -550,7 +560,6 @@ class TestBasePortfolioMemberForms(TestCase):
         cleaned_data = form.cleaned_data
         self.assertEqual(cleaned_data["domain_request_permission_member"], None)
 
-
     def test_map_instance_to_initial_admin_role(self):
         """Test that instance data is correctly mapped to the initial form values for an admin role."""
         user_portfolio_permission = UserPortfolioPermission(

src/registrar/tests/test_views_domain.py

@@ -4,6 +4,8 @@ from unittest.mock import MagicMock, ANY, patch
 from django.conf import settings
 from django.urls import reverse
 from django.contrib.auth import get_user_model
+from registrar.models.portfolio_invitation import PortfolioInvitation
+from registrar.utility.email import EmailSendingError
 from waffle.testutils import override_flag
 from api.tests.common import less_console_noise_decorator
 from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
@@ -454,6 +456,7 @@ class TestDomainManagers(TestDomainOverview):
 
     def tearDown(self):
         """Ensure that the user has its original permissions"""
+        PortfolioInvitation.objects.all().delete()
         super().tearDown()
 
     @less_console_noise_decorator
@@ -486,7 +489,7 @@ class TestDomainManagers(TestDomainOverview):
     @less_console_noise_decorator
     def test_domain_user_add_form(self):
         """Adding an existing user works."""
-        other_user, _ = get_user_model().objects.get_or_create(email="mayor@igorville.gov")
+        get_user_model().objects.get_or_create(email="mayor@igorville.gov")
         add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id}))
         session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
 
@@ -509,6 +512,148 @@ class TestDomainManagers(TestDomainOverview):
         success_page = success_result.follow()
         self.assertContains(success_page, "mayor@igorville.gov")
 
+    @boto3_mocking.patching
+    @override_flag("organization_feature", active=True)
+    @less_console_noise_decorator
+    @patch("registrar.views.domain.send_portfolio_invitation_email")
+    @patch("registrar.views.domain.send_domain_invitation_email")
+    def test_domain_user_add_form_sends_portfolio_invitation(self, mock_send_domain_email, mock_send_portfolio_email):
+        """Adding an existing user works and sends portfolio invitation when
+        user is not member of portfolio."""
+        get_user_model().objects.get_or_create(email="mayor@igorville.gov")
+        add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id}))
+        session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
+
+        add_page.form["email"] = "mayor@igorville.gov"
+
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+
+        success_result = add_page.form.submit()
+
+        self.assertEqual(success_result.status_code, 302)
+        self.assertEqual(
+            success_result["Location"],
+            reverse("domain-users", kwargs={"pk": self.domain.id}),
+        )
+
+        # Verify that the invitation emails were sent
+        mock_send_portfolio_email.assert_called_once_with(
+            email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio
+        )
+        mock_send_domain_email.assert_called_once()
+        call_args = mock_send_domain_email.call_args.kwargs
+        self.assertEqual(call_args["email"], "mayor@igorville.gov")
+        self.assertEqual(call_args["requestor"], self.user)
+        self.assertEqual(call_args["domain"], self.domain)
+        self.assertIsNone(call_args.get("is_member_of_different_org"))
+
+        # Assert that the PortfolioInvitation is created
+        portfolio_invitation = PortfolioInvitation.objects.filter(
+            email="mayor@igorville.gov", portfolio=self.portfolio
+        ).first()
+        self.assertIsNotNone(portfolio_invitation, "Portfolio invitation should be created.")
+        self.assertEqual(portfolio_invitation.email, "mayor@igorville.gov")
+        self.assertEqual(portfolio_invitation.portfolio, self.portfolio)
+
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+        success_page = success_result.follow()
+        self.assertContains(success_page, "mayor@igorville.gov")
+
+    @boto3_mocking.patching
+    @override_flag("organization_feature", active=True)
+    @less_console_noise_decorator
+    @patch("registrar.views.domain.send_portfolio_invitation_email")
+    @patch("registrar.views.domain.send_domain_invitation_email")
+    def test_domain_user_add_form_doesnt_send_portfolio_invitation_if_already_member(
+        self, mock_send_domain_email, mock_send_portfolio_email
+    ):
+        """Adding an existing user works and sends portfolio invitation when
+        user is not member of portfolio."""
+        other_user, _ = get_user_model().objects.get_or_create(email="mayor@igorville.gov")
+        UserPortfolioPermission.objects.get_or_create(
+            user=other_user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+        add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id}))
+        session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
+
+        add_page.form["email"] = "mayor@igorville.gov"
+
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+
+        success_result = add_page.form.submit()
+
+        self.assertEqual(success_result.status_code, 302)
+        self.assertEqual(
+            success_result["Location"],
+            reverse("domain-users", kwargs={"pk": self.domain.id}),
+        )
+
+        # Verify that the invitation emails were sent
+        mock_send_portfolio_email.assert_not_called()
+        mock_send_domain_email.assert_called_once()
+        call_args = mock_send_domain_email.call_args.kwargs
+        self.assertEqual(call_args["email"], "mayor@igorville.gov")
+        self.assertEqual(call_args["requestor"], self.user)
+        self.assertEqual(call_args["domain"], self.domain)
+        self.assertIsNone(call_args.get("is_member_of_different_org"))
+
+        # Assert that no PortfolioInvitation is created
+        portfolio_invitation_exists = PortfolioInvitation.objects.filter(
+            email="mayor@igorville.gov", portfolio=self.portfolio
+        ).exists()
+        self.assertFalse(
+            portfolio_invitation_exists, "Portfolio invitation should not be created when the user is already a member."
+        )
+
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+        success_page = success_result.follow()
+        self.assertContains(success_page, "mayor@igorville.gov")
+
+    @boto3_mocking.patching
+    @override_flag("organization_feature", active=True)
+    @less_console_noise_decorator
+    @patch("registrar.views.domain.send_portfolio_invitation_email")
+    @patch("registrar.views.domain.send_domain_invitation_email")
+    def test_domain_user_add_form_sends_portfolio_invitation_raises_email_sending_error(
+        self, mock_send_domain_email, mock_send_portfolio_email
+    ):
+        """Adding an existing user works and attempts to send portfolio invitation when
+        user is not member of portfolio and send raises an error."""
+        mock_send_portfolio_email.side_effect = EmailSendingError("Failed to send email.")
+        get_user_model().objects.get_or_create(email="mayor@igorville.gov")
+        add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id}))
+        session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
+
+        add_page.form["email"] = "mayor@igorville.gov"
+
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+
+        success_result = add_page.form.submit()
+
+        self.assertEqual(success_result.status_code, 302)
+        self.assertEqual(
+            success_result["Location"],
+            reverse("domain-users", kwargs={"pk": self.domain.id}),
+        )
+
+        # Verify that the invitation emails were sent
+        mock_send_portfolio_email.assert_called_once_with(
+            email="mayor@igorville.gov", requestor=self.user, portfolio=self.portfolio
+        )
+        mock_send_domain_email.assert_not_called()
+
+        # Assert that no PortfolioInvitation is created
+        portfolio_invitation_exists = PortfolioInvitation.objects.filter(
+            email="mayor@igorville.gov", portfolio=self.portfolio
+        ).exists()
+        self.assertFalse(
+            portfolio_invitation_exists, "Portfolio invitation should not be created when email fails to send."
+        )
+
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+        success_page = success_result.follow()
+        self.assertContains(success_page, "Could not send email invitation.")
+
     @boto3_mocking.patching
     @less_console_noise_decorator
     def test_domain_invitation_created(self):
@@ -757,7 +902,9 @@ class TestDomainManagers(TestDomainOverview):
             self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
             add_page.form.submit()
 
-            expected_message_content = f"Can't send invitation email. No email is associated with the account for 'test_user'."
+            expected_message_content = (
+                "Can't send invitation email. No email is associated with the account for 'test_user'."
+            )
 
             # Assert that the error message was called with the correct argument
             mock_error.assert_called_once_with(

src/registrar/tests/test_views_portfolio.py

@@ -20,7 +20,6 @@ from registrar.models.user_portfolio_permission import UserPortfolioPermission
 from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
 from registrar.tests.test_views import TestWithUser
 from registrar.utility.email import EmailSendingError
-from registrar.utility.email_invitations import send_portfolio_invitation_email
 from registrar.utility.errors import MissingEmailError
 from .common import MockSESClient, completed_domain_request, create_test_user, create_user
 from waffle.testutils import override_flag
@@ -2635,7 +2634,7 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
             # assert that portfolio invitation is not created
             self.assertFalse(
                 PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(),
-                "Portfolio invitation should not be created when an Exception occurs."
+                "Portfolio invitation should not be created when an Exception occurs.",
             )
 
             # Check that an email was not sent
@@ -2711,13 +2710,11 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
             # assert that response is a redirect to reverse("members")
             self.assertRedirects(response, reverse("members"))
             # assert that messages contains message, "Could not send email invitation"
-            mock_warning.assert_called_once_with(
+            mock_warning.assert_called_once_with(response.wsgi_request, "Could not send email invitation.")
-                response.wsgi_request, "Could not send email invitation."
-            )
             # assert that portfolio invitation is not created
             self.assertFalse(
                 PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(),
-                "Portfolio invitation should not be created when an EmailSendingError occurs."
+                "Portfolio invitation should not be created when an EmailSendingError occurs.",
             )
 
     @less_console_noise_decorator
@@ -2753,12 +2750,13 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
             self.assertRedirects(response, reverse("members"))
             # assert that messages contains message, "Could not send email invitation"
             mock_error.assert_called_once_with(
-                response.wsgi_request, "Can't send invitation email. No email is associated with the account for 'test_user'."
+                response.wsgi_request,
+                "Can't send invitation email. No email is associated with the account for 'test_user'.",
             )
             # assert that portfolio invitation is not created
             self.assertFalse(
                 PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(),
-                "Portfolio invitation should not be created when a MissingEmailError occurs."
+                "Portfolio invitation should not be created when a MissingEmailError occurs.",
             )
 
     @less_console_noise_decorator
@@ -2793,13 +2791,11 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
             # assert that response is a redirect to reverse("members")
             self.assertRedirects(response, reverse("members"))
             # assert that messages contains message, "Could not send email invitation"
-            mock_warning.assert_called_once_with(
+            mock_warning.assert_called_once_with(response.wsgi_request, "Could not send email invitation.")
-                response.wsgi_request, "Could not send email invitation."
-            )
             # assert that portfolio invitation is not created
             self.assertFalse(
                 PortfolioInvitation.objects.filter(email=self.new_member_email, portfolio=self.portfolio).exists(),
-                "Portfolio invitation should not be created when an Exception occurs."
+                "Portfolio invitation should not be created when an Exception occurs.",
             )
 
     @less_console_noise_decorator
@@ -2828,7 +2824,14 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
         self.assertEqual(response.status_code, 200)
 
         # verify messages
-        self.assertContains(response, "This user is already assigned to a portfolio invitation. Based on current waffle flag settings, users cannot be assigned to multiple portfolios.")
+        self.assertContains(
+            response,
+            (
+                "This user is already assigned to a portfolio invitation. "
+                "Based on current waffle flag settings, users cannot be assigned "
+                "to multiple portfolios."
+            ),
+        )
 
         # Validate Database has not changed
         invite_count_after = PortfolioInvitation.objects.count()
@@ -2863,7 +2866,14 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest):
         self.assertEqual(response.status_code, 200)
 
         # Verify messages
-        self.assertContains(response, "This user is already assigned to a portfolio. Based on current waffle flag settings, users cannot be assigned to multiple portfolios.")
+        self.assertContains(
+            response,
+            (
+                "This user is already assigned to a portfolio. "
+                "Based on current waffle flag settings, users cannot be "
+                "assigned to multiple portfolios."
+            ),
+        )
 
         # Validate Database has not changed
         invite_count_after = PortfolioInvitation.objects.count()
@@ -3015,7 +3025,9 @@ class TestEditPortfolioMemberView(WebTest):
         """Tests an admin removing their own admin role redirects to home.
 
         Removing the admin role will remove both view and edit members permissions.
-        Note: The user can remove the edit members permissions but as long as they stay in admin role, they will at least still have view members permissions."""
+        Note: The user can remove the edit members permissions but as long as they
+        stay in admin role, they will at least still have view members permissions.
+        """
 
         self.client.force_login(self.user)
 

src/registrar/views/domain.py

@@ -1333,13 +1333,13 @@ class DomainAddUserView(DomainFormBaseView):
         elif isinstance(exception, IntegrityError):
             messages.warning(self.request, f"{email} is already a manager for this domain")
         else:
-            logger.warning("Could not send email invitation (Other Exception)", self.object, exc_info=True)
+            logger.warning("Could not send email invitation (Other Exception)", exc_info=True)
             messages.warning(self.request, "Could not send email invitation.")
 
     def _handle_portfolio_exceptions(self, exception, email, portfolio):
         """Handle exceptions raised during the process."""
         if isinstance(exception, EmailSendingError):
-            logger.warning("Could not send email invitation (EmailSendingError)", portfolio, exc_info=True)
+            logger.warning("Could not send email invitation (EmailSendingError)", exc_info=True)
             messages.warning(self.request, "Could not send email invitation.")
         elif isinstance(exception, MissingEmailError):
             messages.error(self.request, str(exception))
@@ -1348,7 +1348,7 @@ class DomainAddUserView(DomainFormBaseView):
                 exc_info=True,
             )
         else:
-            logger.warning("Could not send email invitation (Other Exception)", portfolio, exc_info=True)
+            logger.warning("Could not send email invitation (Other Exception)", exc_info=True)
             messages.warning(self.request, "Could not send email invitation.")