Fix unit test, linting, cleanup

src/djangooidc/views.py

@@ -99,9 +99,18 @@ def login_callback(request):
             return CLIENT.create_authn_request(request.session)
         user = authenticate(request=request, **userinfo)
         if user:
-            # Set login metadata about this user
+
-            # (verification_type for instance)
+            # Fixture users kind of exist in a superposition of verification types,
-            _set_authenticated_user_metadata(user)
+            # because while the system "verified" them, if they login,
+            # we don't know how the user themselves was verified through login.gov until
+            # they actually try logging in. This edge-case only matters in non-production environments.
+            fixture_user = User.VerificationTypeChoices.FIXTURE_USER
+            is_fixture_user = user.verification_type and user.verification_type == fixture_user
+
+            # Set the verification type if it doesn't already exist or if its a fixture user
+            if not user.verification_type or is_fixture_user:
+                user.set_user_verification_type()
+                user.save()
 
             login(request, user)
 
@@ -131,26 +140,6 @@ def login_callback(request):
         return error_page(request, err)
 
 
-def _set_authenticated_user_metadata(user):
-    """Does checks on the recieved authenticated user from login_callback,
-    and updates fields accordingly."""
-    should_update_user = False
-    # Fixture users kind of exist in a superposition of verification types,
-    # because while the system "verified" them, if they login,
-    # we don't know how the user themselves was verified through login.gov until
-    # they actually try logging in. This edge-case only matters in non-production environments.
-    fixture_user = User.VerificationTypeChoices.FIXTURE_USER
-    is_fixture_user = user.verification_type and user.verification_type == fixture_user
-
-    # Set the verification type if it doesn't already exist or if its a fixture user
-    if not user.verification_type or is_fixture_user:
-        user.set_user_verification_type()
-        should_update_user = True
-
-    if should_update_user:
-        user.save()
-
-
 def _requires_step_up_auth(userinfo):
     """if User.needs_identity_verification and step_up_acr_value not in
     ial returned from callback, return True"""

src/registrar/assets/sass/_theme/_base.scss

@@ -171,7 +171,6 @@ abbr[title] {
   cursor: pointer;
 }
 
-// todo this class should ideally be renamed
 .input-with-edit-button {
   svg.usa-icon {
     width: 1.5em !important;
@@ -208,4 +207,4 @@ abbr[title] {
     background-color: white;
     padding-right: 8px;
   }
-}
+}

src/registrar/assets/sass/_theme/_forms.scss

@@ -31,7 +31,6 @@
 }
 
 .usa-form-readonly {
-  // todo update
   border-top: 2px #{$dhs-dark-gray-15} solid;
 
   .bold-usa-label label.usa-label{
@@ -53,6 +52,12 @@
   margin-top: unset;
 }
 
+@media (min-width: 35em) {
+  .usa-form--largest {
+      max-width: 35rem;
+  }
+}
+
 .usa-form-group--unstyled-error {
   margin-left: 0;
   padding-left: 0;
@@ -66,13 +71,6 @@ legend.float-left-tablet + button.float-right-tablet {
   }
 }
 
-@media (min-width: 35em) {
-  .usa-form--largest {
-      max-width: 35rem;
-  }
-}
-
-
 // Custom style for disabled inputs
 @media (prefers-color-scheme: light) {
   .usa-input:disabled, .usa-select:disabled, .usa-textarea:disabled {

src/registrar/assets/sass/_theme/_headers.scss

@@ -11,4 +11,4 @@
 
 .usa-logo button.usa-button--unstyled.disabled-button:hover{
     color: #{$dhs-dark-gray-85};
-}
+}

src/registrar/assets/sass/_theme/_tooltips.scss

@@ -27,4 +27,4 @@
 
 #extended-logo .usa-tooltip__body {
   font-weight: 400 !important;
-}
+}

src/registrar/config/urls.py

@@ -179,8 +179,6 @@ urlpatterns = [
         name="domain-users-add",
     ),
     path(
-        # We embed the current user ID here, but we have a permission check
-        # that ensures the user is who they say they are.
         "finish-profile-setup",
         views.FinishProfileSetupView.as_view(),
         name="finish-user-profile-setup",

src/registrar/fixtures_users.py

@@ -126,11 +126,11 @@ class UserFixture:
             "last_name": "Osos-Analyst",
             "email": "kosos@truss.works",
         },
-        # {
+        {
-        #     "username": "2cc0cde8-8313-4a50-99d8-5882e71443e8",
+            "username": "2cc0cde8-8313-4a50-99d8-5882e71443e8",
-        #     "first_name": "Zander-Analyst",
+            "first_name": "Zander-Analyst",
-        #     "last_name": "Adkinson-Analyst",
+            "last_name": "Adkinson-Analyst",
-        # },
+        },
         {
             "username": "57ab5847-7789-49fe-a2f9-21d38076d699",
             "first_name": "Paul-Analyst",

src/registrar/models/contact.py

@@ -1,6 +1,7 @@
 from django.db import models
 
 from .utility.time_stamped_model import TimeStampedModel
+
 from phonenumber_field.modelfields import PhoneNumberField  # type: ignore
 
 

src/registrar/models/utility/generic_helper.py

@@ -272,6 +272,7 @@ class CreateOrUpdateOrganizationTypeHelper:
 def replace_url_queryparams(url_to_modify: str, query_params: dict[Any, list]):
     """
     Replaces the query parameters of a given URL.
+    Because this replaces them, this can be used to either add, delete, or modify.
 
     Args:
         url_to_modify (str): The URL whose query parameters need to be modified.

src/registrar/registrar_middleware.py

@@ -10,6 +10,23 @@ from waffle.decorators import flag_is_active
 from registrar.models.utility.generic_helper import replace_url_queryparams
 
 
+class NoCacheMiddleware:
+    """
+    Middleware to add Cache-control: no-cache to every response.
+
+    Used to force Cloudfront caching to leave us alone while we develop
+    better caching responses.
+    """
+
+    def __init__(self, get_response):
+        self.get_response = get_response
+
+    def __call__(self, request):
+        response = self.get_response(request)
+        response["Cache-Control"] = "no-cache"
+        return response
+
+
 class CheckUserProfileMiddleware:
     """
     Checks if the current user has finished_setup = False.
@@ -20,8 +37,14 @@ class CheckUserProfileMiddleware:
     def __init__(self, get_response):
         self.get_response = get_response
 
+        self.setup_page = reverse("finish-user-profile-setup")
+        self.logout_page = reverse("logout")
+        self.excluded_pages = [
+            self.setup_page,
+            self.logout_page,
+        ]
+
     def __call__(self, request):
-        """Code that gets executed on each request before the view is called"""
         response = self.get_response(request)
         return response
 
@@ -53,19 +76,13 @@ class CheckUserProfileMiddleware:
 
         Otherwise, we assume they want to go to the home page.
         """
-        setup_page = reverse("finish-user-profile-setup")
-        logout_page = reverse("logout")
-        excluded_pages = [
-            setup_page,
-            logout_page,
-        ]
 
         # In some cases, we don't want to redirect to home. This handles that.
         # Can easily be generalized if need be, but for now lets keep this easy to read.
         custom_redirect = "domain-request:" if request.path == "/request/" else None
 
         # Don't redirect on excluded pages (such as the setup page itself)
-        if not any(request.path.startswith(page) for page in excluded_pages):
+        if not any(request.path.startswith(page) for page in self.excluded_pages):
 
             # Preserve the original query parameters, and coerce them into a dict
             query_params = parse_qs(request.META["QUERY_STRING"])
@@ -75,28 +92,9 @@ class CheckUserProfileMiddleware:
                 query_params["redirect"] = custom_redirect
 
             # Add our new query param, while preserving old ones
-            if query_params:
+            new_setup_page = replace_url_queryparams(self.setup_page, query_params) if query_params else self.setup_page
-                setup_page = replace_url_queryparams(setup_page, query_params)
 
-            # Redirect to the setup page
+            return HttpResponseRedirect(new_setup_page)
-            return HttpResponseRedirect(setup_page)
         else:
             # Process the view as normal
             return None
-
-
-class NoCacheMiddleware:
-    """
-    Middleware to add Cache-control: no-cache to every response.
-
-    Used to force Cloudfront caching to leave us alone while we develop
-    better caching responses.
-    """
-
-    def __init__(self, get_response):
-        self.get_response = get_response
-
-    def __call__(self, request):
-        response = self.get_response(request)
-        response["Cache-Control"] = "no-cache"
-        return response

src/registrar/templates/includes/finish_profile_form.html

@@ -84,4 +84,4 @@
     </div>
 </form>
 
-{% endblock profile_form %}
+{% endblock profile_form %}

src/registrar/templates/includes/label_with_edit_button.html

@@ -11,4 +11,4 @@
         </svg>Edit
       </button>
     </div>
-</div>
+</div>

src/registrar/templates/includes/profile_form.html

@@ -48,4 +48,4 @@
 </fieldset>
 <button type="submit" class="usa-button">Save</button>
 </form>
-{% endblock profile_form %}
+{% endblock profile_form %}

src/registrar/tests/test_views.py

@@ -523,6 +523,10 @@ class HomeTests(TestWithUser):
 class FinishUserProfileTests(TestWithUser, WebTest):
     """A series of tests that target the finish setup page for user profile"""
 
+    # csrf checks do not work well with WebTest.
+    # We disable them here.
+    csrf_checks = False
+
     def setUp(self):
         super().setUp()
         self.user.title = None
@@ -556,7 +560,8 @@ class FinishUserProfileTests(TestWithUser, WebTest):
         """Tests that a new user is redirected to the profile setup page when profile_feature is on"""
         self.app.set_user(self.incomplete_user.username)
         with override_flag("profile_feature", active=True):
-            # This will redirect the user to the setup page
+            # This will redirect the user to the setup page.
+            # Follow implicity checks if our redirect is working.
             finish_setup_page = self.app.get(reverse("home")).follow()
             self._set_session_cookie()
 
@@ -578,10 +583,14 @@ class FinishUserProfileTests(TestWithUser, WebTest):
             finish_setup_form["phone"] = "(201) 555-0123"
             finish_setup_form["title"] = "CEO"
             finish_setup_form["last_name"] = "example"
-            completed_setup_page = self._submit_form_webtest(finish_setup_page.form, follow=True)
+            save_page = self._submit_form_webtest(finish_setup_form, follow=True)
 
-            self.assertEqual(completed_setup_page.status_code, 200)
+            self.assertEqual(save_page.status_code, 200)
-            # Assert that we're on the home page
+            self.assertContains(save_page, "Your profile has been updated.")
+
+            # Try to navigate back to the home page.
+            # This is the same as clicking the back button.
+            completed_setup_page = self.app.get(reverse("home"))
             self.assertContains(completed_setup_page, "Manage your domain")
 
     @less_console_noise_decorator