use css for read only styles

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

@@ -82,3 +82,13 @@ legend.float-left-tablet + button.float-right-tablet {
     color: var(--close-button-hover-bg);
   }
 }
+
+.read-only-label {
+  font-size: size('body', 'sm');
+  color: color('primary');
+  margin-bottom: units(0.5);
+}
+
+.read-only-value {
+  margin-top: units(0);
+}

src/registrar/migrations/0114_alter_user_portfolio_additional_permissions_and_more.py

@@ -0,0 +1,52 @@
+# Generated by Django 4.2.10 on 2024-07-30 02:51
+
+import django.contrib.postgres.fields
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("registrar", "0113_user_portfolio_user_portfolio_additional_permissions_and_more"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="user",
+            name="portfolio_additional_permissions",
+            field=django.contrib.postgres.fields.ArrayField(
+                base_field=models.CharField(
+                    choices=[
+                        ("view_all_domains", "View all domains and domain reports"),
+                        ("view_managed_domains", "View managed domains"),
+                        ("edit_domains", "User is a manager on a domain"),
+                        ("view_member", "View members"),
+                        ("edit_member", "Create and edit members"),
+                        ("view_all_requests", "View all requests"),
+                        ("view_created_requests", "View created requests"),
+                        ("edit_requests", "Create and edit requests"),
+                        ("edit_portfolio", "Edit organization"),
+                    ],
+                    max_length=50,
+                ),
+                blank=True,
+                help_text="Select one or more additional permissions.",
+                null=True,
+                size=None,
+            ),
+        ),
+        migrations.AlterField(
+            model_name="user",
+            name="portfolio_roles",
+            field=django.contrib.postgres.fields.ArrayField(
+                base_field=models.CharField(
+                    choices=[("organization_admin", "Admin"), ("organization_admin_read_only", "Admin read only")],
+                    max_length=50,
+                ),
+                blank=True,
+                help_text="Select one or more roles.",
+                null=True,
+                size=None,
+            ),
+        ),
+    ]

src/registrar/models/user.py

@@ -69,7 +69,7 @@ class User(AbstractUser):
 
         ORGANIZATION_ADMIN = "organization_admin", "Admin"
         ORGANIZATION_ADMIN_READ_ONLY = "organization_admin_read_only", "Admin read only"
-        ORGANIZATION_MEMBER = "organization_member", "Member"
+        # ORGANIZATION_MEMBER is an abstract role where user.portfolio is true
 
     class UserPortfolioPermissionChoices(models.TextChoices):
         """ """
@@ -89,7 +89,7 @@ class User(AbstractUser):
         VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests"
         EDIT_REQUESTS = "edit_requests", "Create and edit requests"
 
-        VIEW_PORTFOLIO = "view_portfolio", "View organization"
+        # VIEW_PORTFOLIO is an abstract permission that returns true when user.portfolio is true
         EDIT_PORTFOLIO = "edit_portfolio", "Edit organization"
 
     PORTFOLIO_ROLE_PERMISSIONS = {
@@ -99,17 +99,12 @@ class User(AbstractUser):
             UserPortfolioPermissionChoices.EDIT_MEMBER,
             UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS,
             UserPortfolioPermissionChoices.EDIT_REQUESTS,
-            UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
             UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
         ],
         UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [
             UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS,
             UserPortfolioPermissionChoices.VIEW_MEMBER,
             UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS,
-            UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
-        ],
-        UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
-            UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
         ],
     }
 
@@ -280,10 +275,14 @@ class User(AbstractUser):
 
         return portfolio_permission in portfolio_permissions
 
-    # the methods below are checks for individual portfolio permissions.  they are defined here
+    # the methods below are checks for individual portfolio permissions. They are defined here
     # to make them easier to call elsewhere throughout the application
     def has_base_portfolio_permission(self):
-        return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO)
+        """Base role/permission, the user is simply linked to a portfolio"""
+        return self.portfolio is not None
+
+    def has_edit_org_portfolio_permission(self):
+        return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO)
 
     def has_domains_portfolio_permission(self):
         return (

src/registrar/templates/includes/finish_profile_form.html

@@ -35,7 +35,7 @@
 
         <input type="hidden" name="redirect" value="{{ form.initial.redirect }}">
 
-        {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable usa-form-editable--no-border padding-top-2" %}
+        {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable usa-form-editable--no-border padding-top-2" %}
             {% input_with_errors form.full_name %}
         {% endwith %}
         
@@ -54,7 +54,7 @@
         </div>
 
         {% public_site_url "help/account-management/#get-help-with-login.gov" as login_help_url %}
-        {% with show_readonly=True add_class="display-none" group_classes="usa-form-editable usa-form-editable padding-top-2 bold-usa-label" %}
+        {% with toggleable_input=True add_class="display-none" group_classes="usa-form-editable usa-form-editable padding-top-2 bold-usa-label" %}
             {% with link_href=login_help_url %}
                 {% with sublabel_text="We recommend using your work email for your .gov account. If the wrong email is displayed below, you’ll need to update your Login.gov account and log back in. Get help with your Login.gov account." %}
                     {% with link_text="Get help with your Login.gov account" target_blank=True do_not_show_max_chars=True %}
@@ -64,11 +64,11 @@
             {% endwith %}
         {% endwith %}
 
-        {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %}
+        {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %}
             {% input_with_errors form.title %}
         {% endwith %}
 
-        {% with show_edit_button=True show_readonly=True group_classes="usa-form-editable padding-top-2" %}
+        {% with toggleable_input=True toggleable_label=True group_classes="usa-form-editable padding-top-2" %}
             {% with add_class="usa-input--medium" %}
                 {% input_with_errors form.phone %}
             {% endwith %}

src/registrar/templates/includes/input_read_only.html

@@ -0,0 +1,7 @@
+{% comment %}
+Template include for read-only form fields 
+{% endcomment %}
+
+
+<h4 class="read-only-label">{{ field.label }}</h4>
+<p class="read-only-value">{{ field.value }}</p>

src/registrar/templates/includes/input_with_errors.html

@@ -27,8 +27,8 @@ error messages, if necessary.
 {% endif %}
 
   {% if not field.widget_type == "checkbox" %}
-    {% if show_edit_button %}
+    {% if toggleable_label %}
-      {% include "includes/label_with_edit_button.html" with bold_label=True %}
+      {% include "includes/toggleable_label.html" with bold_label=True %}
     {% else %}
       {% include "django/forms/label.html" %}
     {% endif %}
@@ -63,8 +63,8 @@ error messages, if necessary.
     <div class="display-flex flex-align-center">
   {% endif %}
 
-  {% if show_readonly %}
+  {% if toggleable_input %}
-    {% include "includes/readonly_input.html" %}
+    {% include "includes/toggleable_input.html" %}
   {% endif %}
 
   {# this is the input field, itself #}

src/registrar/templates/portfolio_organization.html

@@ -23,35 +23,27 @@
 
         <p>The name of your federal agency will be publicly listed as the domain registrant.</p>
 
+        {% if has_edit_org_portfolio_permission %}
             <p>
                 The federal agency for your organization can’t be updated here.
                 To suggest an update, email <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>.
             </p>
 
             {% include "includes/form_errors.html" with form=form %}
-
             {% include "includes/required_fields.html" %}
-
             <form class="usa-form usa-form--large" method="post" novalidate id="form-container">
                 {% csrf_token %}
-
+                <h4 class="read-only-label">Federal agency</h4>
-            <p>
+                <p class="read-only-value">
-                <strong class="text-primary display-block margin-bottom-1">Federal agency</strong>
                     {{ portfolio }}
                 </p>
-
                 {% input_with_errors form.address_line1 %}
-
                 {% input_with_errors form.address_line2 %}
-
                 {% input_with_errors form.city %}
-
                 {% input_with_errors form.state_territory %}
-
                 {% with add_class="usa-input--small" %}
                     {% input_with_errors form.zipcode %}
                 {% endwith %}
-
                 <button
                 type="submit"
                 class="usa-button"
@@ -59,6 +51,28 @@
                 Save
                 </button>
             </form>
+        {% else %}
+            <h4 class="read-only-label">Federal agency</h4>
+            <p class="read-only-value">
+                {{ portfolio }}
+            </p>
+            {% if form.address_line1.value is not None %}
+                {% include "includes/input_read_only.html" with field=form.address_line1 %}
+            {% endif %}
+            {% if form.address_line2.value is not None %}
+                {% include "includes/input_read_only.html" with field=form.address_line2 %}
+            {% endif %}
+            {% if form.city.value is not None %}
+                {% include "includes/input_read_only.html" with field=form.city %}
+            {% endif %}
+            {% if form.state_territory.value is not None %}
+                {% include "includes/input_read_only.html" with field=form.state_territory %}
+            {% endif %}
+            {% if form.zipcode.value is not None %}
+                {% include "includes/input_read_only.html" with field=form.zipcode %}
+            {% endif %}
+        {% endif %}
+
     </div>
 </div>
 {% endblock %}

src/registrar/templatetags/field_helpers.py

@@ -26,7 +26,7 @@ def input_with_errors(context, field=None):  # noqa: C901
         add_group_class: append to input element's surrounding tag's `class` attribute
         attr_* - adds or replaces any single html attribute for the input
         add_error_attr_* - like `attr_*` but only if field.errors is not empty
-        show_edit_button: shows a simple edit button, and adds display-none to the input field.
+        toggleable_input: shows a simple edit button, and adds display-none to the input field.
 
     Example usage:
         ```
@@ -92,7 +92,7 @@ def input_with_errors(context, field=None):  # noqa: C901
         elif key == "add_group_class":
             group_classes.append(value)
 
-        elif key == "show_edit_button":
+        elif key == "toggleable_input":
             # Hide the primary input field.
             # Used such that we can toggle it with JS
             if "display-none" not in classes:

src/registrar/tests/test_views_portfolio.py

@@ -37,25 +37,10 @@ class TestPortfolio(WebTest):
         User.objects.all().delete()
         super().tearDown()
 
-    @less_console_noise_decorator
-    def test_middleware_does_not_redirect_if_no_permission(self):
-        """Test that user with no portfolio permission is not redirected when attempting to access home"""
-        self.app.set_user(self.user.username)
-        self.user.portfolio = self.portfolio
-        self.user.save()
-        self.user.refresh_from_db()
-        with override_flag("organization_feature", active=True):
-            # This will redirect the user to the portfolio page.
-            # Follow implicity checks if our redirect is working.
-            portfolio_page = self.app.get(reverse("home"))
-            # Assert that we're on the right page
-            self.assertNotContains(portfolio_page, self.portfolio.organization_name)
-
     @less_console_noise_decorator
     def test_middleware_does_not_redirect_if_no_portfolio(self):
         """Test that user with no assigned portfolio is not redirected when attempting to access home"""
         self.app.set_user(self.user.username)
-        self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO]
         self.user.save()
         self.user.refresh_from_db()
         with override_flag("organization_feature", active=True):
@@ -67,10 +52,9 @@ class TestPortfolio(WebTest):
 
     @less_console_noise_decorator
     def test_middleware_redirects_to_portfolio_organization_page(self):
-        """Test that user with VIEW_PORTFOLIO is redirected to portfolio organization page"""
+        """Test that user with a portfolio is redirected to portfolio organization page"""
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
-        self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO]
         self.user.save()
         self.user.refresh_from_db()
         with override_flag("organization_feature", active=True):
@@ -83,11 +67,10 @@ class TestPortfolio(WebTest):
 
     @less_console_noise_decorator
     def test_middleware_redirects_to_portfolio_domains_page(self):
-        """Test that user with VIEW_PORTFOLIO and VIEW_ALL_DOMAINS is redirected to portfolio domains page"""
+        """Test that user with a portfolio and VIEW_ALL_DOMAINS is redirected to portfolio domains page"""
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
         self.user.portfolio_additional_permissions = [
-            User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
             User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS,
         ]
         self.user.save()
@@ -134,20 +117,47 @@ class TestPortfolio(WebTest):
             self.assertEqual(response.status_code, 403)
 
     @less_console_noise_decorator
-    def test_portfolio_organization_page_403_when_user_not_have_permission(self):
+    def test_portfolio_organization_page_read_only(self):
-        """Test that user without proper permission is not allowed access to portfolio organization page"""
+        """Test that user with a portfolio can access the portfolio organization page, read only"""
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
+        self.portfolio.city = "Los Angeles"
+        self.portfolio.save()
         self.user.save()
         self.user.refresh_from_db()
         with override_flag("organization_feature", active=True):
-            # This will redirect the user to the portfolio page.
+            response = self.app.get(reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk}))
-            # Follow implicity checks if our redirect is working.
+            # Assert the response is a 200
-            response = self.app.get(
+            self.assertEqual(response.status_code, 200)
-                reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk}), status=403
+            # The label for Federal agency will always be a h4
-            )
+            self.assertContains(response, '<h4 class="usa-prose text-primary margin-bottom-05">Federal agency</h4>')
-            # Assert the response is a 403 Forbidden
+            # The read only label for city will be a h4
-            self.assertEqual(response.status_code, 403)
+            self.assertContains(response, '<h4 class="usa-prose text-primary margin-bottom-05">City</h4>')
+            self.assertNotContains(response, 'for="id_city"')
+            self.assertContains(response, '<p class="margin-top-0">Los Angeles</p>')
+
+    @less_console_noise_decorator
+    def test_portfolio_organization_page_edit_access(self):
+        """Test that user with a portfolio can access the portfolio organization page, read only"""
+        self.app.set_user(self.user.username)
+        self.user.portfolio = self.portfolio
+        self.user.portfolio_additional_permissions = [
+            User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
+        ]
+        self.portfolio.city = "Los Angeles"
+        self.portfolio.save()
+        self.user.save()
+        self.user.refresh_from_db()
+        with override_flag("organization_feature", active=True):
+            response = self.app.get(reverse("portfolio-organization", kwargs={"portfolio_id": self.portfolio.pk}))
+            # Assert the response is a 200
+            self.assertEqual(response.status_code, 200)
+            # The label for Federal agency will always be a h4
+            self.assertContains(response, '<h4 class="usa-prose text-primary margin-bottom-05">Federal agency</h4>')
+            # The read only label for city will be a h4
+            self.assertNotContains(response, '<h4 class="usa-prose text-primary margin-bottom-05">City</h4>')
+            self.assertNotContains(response, '<p class="margin-top-0">Los Angeles</p>>')
+            self.assertContains(response, 'for="id_city"')
 
     @less_console_noise_decorator
     def test_navigation_links_hidden_when_user_not_have_permission(self):
@@ -155,7 +165,6 @@ class TestPortfolio(WebTest):
         self.app.set_user(self.user.username)
         self.user.portfolio = self.portfolio
         self.user.portfolio_additional_permissions = [
-            User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
             User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS,
             User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS,
         ]
@@ -176,9 +185,9 @@ class TestPortfolio(WebTest):
                 portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk})
             )
 
-            # reducing portfolio permissions to just VIEW_PORTFOLIO, which should remove domains
+            # removing non-basic portfolio perms, which should remove domains
             # and domain requests from nav
-            self.user.portfolio_additional_permissions = [User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO]
+            self.user.portfolio_additional_permissions = []
             self.user.save()
             self.user.refresh_from_db()
 
@@ -194,16 +203,13 @@ class TestPortfolio(WebTest):
                 portfolio_page, reverse("portfolio-domain-requests", kwargs={"portfolio_id": self.portfolio.pk})
             )
 
-
+    @less_console_noise_decorator
-class TestPortfolioOrganization(TestPortfolio):
-
     def test_portfolio_org_name(self):
         """Can load portfolio's org name page."""
         with override_flag("organization_feature", active=True):
             self.app.set_user(self.user.username)
             self.user.portfolio = self.portfolio
             self.user.portfolio_additional_permissions = [
-                User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
                 User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
             ]
             self.user.save()
@@ -214,13 +220,13 @@ class TestPortfolioOrganization(TestPortfolio):
                 page, "The name of your federal agency will be publicly listed as the domain registrant."
             )
 
+    @less_console_noise_decorator
     def test_domain_org_name_address_content(self):
         """Org name and address information appears on the page."""
         with override_flag("organization_feature", active=True):
             self.app.set_user(self.user.username)
             self.user.portfolio = self.portfolio
             self.user.portfolio_additional_permissions = [
-                User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
                 User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
             ]
             self.user.save()
@@ -232,13 +238,13 @@ class TestPortfolioOrganization(TestPortfolio):
             # Once in the sidenav, once in the main nav, once in the form
             self.assertContains(page, "Hotel California", count=3)
 
+    @less_console_noise_decorator
     def test_domain_org_name_address_form(self):
         """Submitting changes works on the org name address page."""
         with override_flag("organization_feature", active=True):
             self.app.set_user(self.user.username)
             self.user.portfolio = self.portfolio
             self.user.portfolio_additional_permissions = [
-                User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
                 User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
             ]
             self.user.save()

src/registrar/views/portfolios.py

@@ -64,6 +64,9 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin):
         """Add additional context data to the template."""
         context = super().get_context_data(**kwargs)
         # no need to add portfolio to request context here
+
+        context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission()
+
         context["has_profile_feature_flag"] = flag_is_active(self.request, "profile_feature")
         context["has_organization_feature_flag"] = flag_is_active(self.request, "organization_feature")
         return context