Pr suggestions ( minus tests )

src/registrar/assets/js/get-gov.js

@@ -1913,9 +1913,7 @@ class MembersTable extends LoadTableBase {
           const memberList = document.querySelector('.members__table tbody');
           memberList.innerHTML = '';
 
-          if (data.members)
+          data.members.forEach(member => {
-          {
-            data.members.forEach(member => {
             // const actionUrl = domain.action_url;
             const member_name = member.name;
             const member_email = member.email;
@@ -1946,31 +1944,8 @@ class MembersTable extends LoadTableBase {
                 </a>
               </td>
             `;
-
             memberList.appendChild(row);
           });
-        }
-        else
-        {
-          //TODO: error message?
-          const row = document.createElement('tr');
-          row.innerHTML = `
-          <th scope="row" role="rowheader" data-label="member name">
-            ERROR
-          </th>
-          <td data-sort-value="test" data-label="name">
-            ERROR
-          </td>
-        `;
-
-          memberList.appendChild(row);
-        }
-
-
-
-
-          // initialize tool tips immediately after the associated DOM elements are added
-          initializeTooltips();
 
           // Do not scroll on first page load
           if (scroll)
@@ -1992,8 +1967,8 @@ class MembersTable extends LoadTableBase {
           this.currentSortBy = sortBy;
           this.currentOrder = order;
           this.currentSearchTerm = searchTerm;
-        })
+      })
-        .catch(error => console.error('Error fetching members:', error));
+      .catch(error => console.error('Error fetching members:', error));
   }
 }
 

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

@@ -1,6 +1,5 @@
 @use "uswds-core" as *;
 
-
 // Only apply this custom wrapping to desktop
 @include at-media(desktop) {
   .usa-tooltip--registrar .usa-tooltip__body {

src/registrar/context_processors.py

@@ -97,5 +97,5 @@ def portfolio_permissions(request):
 
 
 def is_widescreen_mode(request):
-    widescreen_paths = ["/domains/", "/requests/"]
+    widescreen_paths = ["/domains/", "/requests/", "/members/"]
     return {"is_widescreen_mode": any(path in request.path for path in widescreen_paths) or request.path == "/"}

src/registrar/templates/includes/header_extended.html

@@ -93,7 +93,7 @@
 
             {% if has_organization_members_flag and has_view_members_portfolio_permission %}
             <li class="usa-nav__primary-item">
-                <a href="/members/" class="usa-nav-link">
+                <a href="/members/" class="usa-nav-link {% if path|is_members_subpage %} usa-current{% endif %}">
                     Members
                 </a>
             </li>

src/registrar/templates/includes/members_table.html

@@ -3,16 +3,10 @@
 {% comment %} Stores the json endpoint in a url for easier access {% endcomment %}
 {% url 'get_portfolio_members_json' as url %}
 <span id="get_members_json_url" class="display-none">{{url}}</span>
-<section class="section-outlined members margin-top-0{% if portfolio %} section-outlined--border-base-light{% endif %}" id="members">
+<section class="section-outlined members margin-top-0 section-outlined--border-base-light" id="members">
-  <div class="section-outlined__header margin-bottom-3 {% if not portfolio %} section-outlined__header--no-portfolio justify-content-space-between{% else %} grid-row{% endif %}">
+  <div class="section-outlined__header margin-bottom-3 grid-row">
-      {% if not portfolio %}
-        <h2 id="members-header" class="display-inline-block">Members</h2>
-      {% else %}
-        <!-- Embedding the portfolio value in a data attribute -->
-        <span id="portfolio-js-value" data-portfolio="{{ portfolio.id }}"></span>
-      {% endif %}
       <!-- ---------- SEARCH ---------- -->
-      <div class="section-outlined__search {% if portfolio %} mobile:grid-col-12 desktop:grid-col-6{% endif %}">
+      <div class="section-outlined__search mobile:grid-col-12 desktop:grid-col-6">
         <section aria-label="Members search component" class="margin-top-2">
           <form class="usa-search usa-search--small" method="POST" role="search">
             {% csrf_token %}
@@ -28,7 +22,7 @@
               id="members__search-field"
               type="search"
               name="search"
-              placeholder="Search by domain name"
+              placeholder="Search by member name"
             />
             <button class="usa-button" type="submit" id="members__search-field-submit">
               <img
@@ -40,120 +34,7 @@
           </form>
         </section>
       </div>
-      {% comment %}
-      <!-- ---------- Export as CSV ---------- -->
-      Note - the following if check will need to be done in javascript.
-      This is because you can dynamically delete these fields.
-      {% if portfolio_members and portfolio_members|length > 0 %}
-      <!-- <div class="section-outlined__utility-button mobile-lg:padding-right-105 {% if portfolio %} mobile:grid-col-12 desktop:grid-col-6 desktop:padding-left-3{% endif %}">
-        <section aria-label="Members report component" class="margin-top-205">
-          <a href="{% url 'export_data_type_user' %}" class="usa-button usa-button--unstyled usa-button--with-icon" role="button">
-            <svg class="usa-icon usa-icon--big" aria-hidden="true" focusable="false" role="img" width="24" height="24">
-              <use xlink:href="{%static 'img/sprite.svg'%}#file_download"></use>
-            </svg>Export as CSV
-          </a>
-        </section>
-      </div> -->
-      {% endif %}
-      {% endcomment %}
     </div>
-    {% if portfolio %}
-
-    <!-- ---------- FILTERING ---------- -->
-    <!-- <div class="display-flex flex-align-center"> 
-      <span class="margin-right-2 margin-top-neg-1 usa-prose text-base-darker">Filter by</span>
-      <div class="usa-accordion usa-accordion--select margin-right-2">
-        <div class="usa-accordion__heading">
-          <button
-            type="button"
-            class="usa-button usa-button--small padding--8-8-9 usa-button--outline usa-button--filter usa-accordion__button"
-            aria-expanded="false"
-            aria-controls="filter-status"
-          >
-            <span class="filter-indicator text-bold display-none"></span> Status
-            <svg class="usa-icon top-2px" aria-hidden="true" focusable="false" role="img" width="24">
-              <use xlink:href="/public/img/sprite.svg#expand_more"></use>
-            </svg>
-          </button>
-        </div>
-        <div id="filter-status" class="usa-accordion__content usa-prose shadow-1">
-          <h2>Status</h2>
-          <fieldset class="usa-fieldset margin-top-0">
-            <legend class="usa-legend">Select to apply <span class="sr-only">status</span> filter</legend>
-            <div class="usa-checkbox">
-              <input
-                class="usa-checkbox__input"
-                id="filter-status-dns-needed"
-                type="checkbox"
-                name="filter-status"
-                value="unknown"
-              />
-              <label class="usa-checkbox__label" for="filter-status-dns-needed"
-                >DNS Needed</label
-              >
-            </div>
-            <div class="usa-checkbox">
-              <input
-                class="usa-checkbox__input"
-                id="filter-status-ready"
-                type="checkbox"
-                name="filter-status"
-                value="ready"
-              />
-              <label class="usa-checkbox__label" for="filter-status-ready"
-                >Ready</label
-              >
-            </div>
-            <div class="usa-checkbox">
-              <input
-                class="usa-checkbox__input"
-                id="filter-status-on-hold"
-                type="checkbox"
-                name="filter-status"
-                value="on hold"
-              />
-              <label class="usa-checkbox__label" for="filter-status-on-hold"
-                >On hold</label
-              >
-            </div>
-            <div class="usa-checkbox">
-              <input
-                class="usa-checkbox__input"
-                id="filter-status-expired"
-                type="checkbox"
-                name="filter-status"
-                value="expired"
-              />
-              <label class="usa-checkbox__label" for="filter-status-expired"
-                >Expired</label
-              >
-            </div>
-            <div class="usa-checkbox">
-              <input
-                class="usa-checkbox__input"
-                id="filter-status-deleted"
-                type="checkbox"
-                name="filter-status"
-                value="deleted"
-              />
-              <label class="usa-checkbox__label" for="filter-status-deleted"
-                >Deleted</label
-              >
-            </div>
-          </fieldset>
-        </div>
-      </div>
-      <button
-        type="button"
-        class="usa-button usa-button--small padding--8-12-9-12 usa-button--outline usa-button--filter members__reset-filters display-none"
-      >
-        Clear filters
-        <svg class="usa-icon top-1px" aria-hidden="true" focusable="false" role="img" width="24">
-          <use xlink:href="/public/img/sprite.svg#close"></use>
-        </svg>
-      </button>
-    </div> -->
-    {% endif %}
 
     <!-- ---------- MAIN TABLE ---------- -->
     <div class="members__table-wrapper display-none usa-table-container--scrollable margin-top-0" tabindex="0">

src/registrar/templatetags/custom_filters.py

@@ -239,3 +239,14 @@ def is_portfolio_subpage(path):
         "senior-official",
     ]
     return get_url_name(path) in url_names
+
+
+@register.filter(name="is_members_subpage")
+def is_members_subpage(path):
+    """Checks if the given page is a subpage of portfolio.
+    Takes a path name, like '/organization/'."""
+    # Since our pages aren't unified under a common path, we need this approach for now.
+    url_names = [
+        "members",
+    ]
+    return get_url_name(path) in url_names

src/registrar/views/portfolio_members_json.py

@@ -14,98 +14,55 @@ def get_portfolio_members_json(request):
     """Given the current request,
     get all members that are associated with the given portfolio"""
 
-    objects = get_member_objects_from_request(request)
+    member_ids = get_member_ids_from_request(request)
-    if objects is not None:
+    objects = User.objects.filter(id__in=member_ids)
-        member_ids = objects.values_list("id", flat=True)
-
-        portfolio = request.session.get("portfolio")
-        admin_ids = UserPortfolioPermission.objects.filter(
-            portfolio=portfolio,
-            roles__overlap=[
-                UserPortfolioRoleChoices.ORGANIZATION_ADMIN,
-            ],
-        ).values_list("user__id", flat=True)
-        portfolio_invitation_emails = PortfolioInvitation.objects.filter(portfolio=portfolio).values_list(
-            "email", flat=True
-        )
-
-        unfiltered_total = member_ids.count()
-
-        objects = apply_search(objects, request)
-        # objects = apply_status_filter(objects, request)
-        objects = apply_sorting(objects, request)
-
-        paginator = Paginator(objects, 10)
-        page_number = request.GET.get("page", 1)
-        page_obj = paginator.get_page(page_number)
-        members = [
-            serialize_members(request, member, request.user, admin_ids, portfolio_invitation_emails)
-            for member in page_obj.object_list
-        ]
-
-        # If you're wondering where these JSON values are used, check out the class "MembersTable"
-        # in get-gov.js (specifically the "loadTable" function).
-        #
-        # The way this JSON gets passed to get-gov.js is via ajax, which depends on our HTML and Url.py
-        # files having routes to this json file.  Specifically, "get_portfolio_members_json" is embedded
-        # in members_table.html as a string, which is then referenced in get-gov.js to grab the appropriate
-        # path in url.py.  In short, make sure that both members_table.html and url.py have references to
-        # this json function in order for all of this to work.
-        #
-        # HELPFUL TIP:  You can easily test this json file's output by visiting
-        # http://localhost:8080/get-portfolio-members-json/
-        return JsonResponse(
-            {
-                "members": members,
-                "has_next": page_obj.has_next(),
-                "has_previous": page_obj.has_previous(),
-                "page": page_obj.number,
-                "num_pages": paginator.num_pages,
-                "total": paginator.count,
-                "unfiltered_total": unfiltered_total,
-            }
-        )
-
-    else:
-        return JsonResponse(
-            {
-                "members": [],
-                "has_next": False,
-                "has_previous": False,
-                "page": 0,
-                "num_pages": 0,
-                "total": 0,
-                "unfiltered_total": 0,
-            }
-        )
-
-
-def get_member_objects_from_request(request):
-    """Given the current request,
-    get all members that are associated with the given portfolio"""
-
-    # portfolio = request.GET.get("portfolio") #TODO: WHY DOESN"T THIS WORK?? It is empty
-    # TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f'portfolio = {portfolio}')  # TODO: delete me
 
     portfolio = request.session.get("portfolio")
+    admin_ids = UserPortfolioPermission.objects.filter(
+        portfolio=portfolio,
+        roles__overlap=[
+            UserPortfolioRoleChoices.ORGANIZATION_ADMIN,
+        ],
+    ).values_list("user__id", flat=True)
+    portfolio_invitation_emails = PortfolioInvitation.objects.filter(portfolio=portfolio).values_list(
+        "email", flat=True
+    )
 
+    unfiltered_total = member_ids.count()
+
+    objects = apply_search(objects, request)
+    # objects = apply_status_filter(objects, request)
+    objects = apply_sorting(objects, request)
+
+    paginator = Paginator(objects, 10)
+    page_number = request.GET.get("page", 1)
+    page_obj = paginator.get_page(page_number)
+    members = [
+        serialize_members(request, member, request.user, admin_ids, portfolio_invitation_emails)
+        for member in page_obj.object_list
+    ]
+
+    return JsonResponse(
+        {
+            "members": members,
+            "page": page_obj.number,
+            "num_pages": paginator.num_pages,
+            "has_previous": page_obj.has_previous(),
+            "has_next": page_obj.has_next(),
+            "total": paginator.count,
+            "unfiltered_total": unfiltered_total,
+        }
+    )
+
+
+def get_member_ids_from_request(request):
+    """Given the current request,
+    get all members that are associated with the given portfolio"""
+    portfolio = request.session.get("portfolio")
+    member_ids = None
     if portfolio:
-        # TODO: Permissions??
+        member_ids = UserPortfolioPermission.objects.filter(portfolio=portfolio).values_list("user__id", flat=True)
-        # if request.user.is_org_user(request) and request.user.has_view_all_requests_portfolio_permission(portfolio):
+    return member_ids
-        #     filter_condition = Q(portfolio=portfolio)
-        # else:
-        #     filter_condition = Q(portfolio=portfolio, creator=request.user)
-
-        permissions = UserPortfolioPermission.objects.filter(portfolio=portfolio)
-
-        portfolio_invitation_emails = PortfolioInvitation.objects.filter(portfolio=portfolio).values_list(
-            "email", flat=True
-        )
-
-        members = User.objects.filter(
-            Q(portfolio_permissions__in=permissions) | Q(email__in=portfolio_invitation_emails)
-        )
-        return members
 
 
 def apply_search(queryset, request):
@@ -121,21 +78,6 @@ def apply_search(queryset, request):
     return queryset
 
 
-# def apply_status_filter(queryset, request):
-#     status_param = request.GET.get("status")
-#     if status_param:
-#         status_list = status_param.split(",")
-#         statuses = [status for status in status_list if status in DomainRequest.DomainRequestStatus.values]
-#         # Construct Q objects for statuses that can be queried through ORM
-#         status_query = Q()
-#         if statuses:
-#             status_query |= Q(status__in=statuses)
-#         # Apply the combined query
-#         queryset = queryset.filter(status_query)
-
-#     return queryset
-
-
 def apply_sorting(queryset, request):
     sort_by = request.GET.get("sort_by", "id")  # Default to 'id'
     order = request.GET.get("order", "asc")  # Default to 'asc'
@@ -151,14 +93,13 @@ def serialize_members(request, member, user, admin_ids, portfolio_invitation_ema
     # ------- VIEW ONLY
     # If not view_only (the user has permissions to edit/manage users), show the gear icon with "Manage" link.
     # If view_only (the user only has view user permissions), show the "View" link (no gear icon).
+    # We check on user_group_permision to account for the upcoming "Manage portfolio" button on admin.
     user_can_edit_other_users = False
-    user_edit_permissions = ["registrar.full_access_permission", "registrar.change_user"]
+    for user_group_permission in ["registrar.full_access_permission", "registrar.change_user"]:
-    index = 0
+        if user.has_perm(user_group_permission):
-    while not user_can_edit_other_users and index < len(user_edit_permissions):
-        perm = user_edit_permissions[index]
-        if user.has_perm(perm):
             user_can_edit_other_users = True
-        index += 1
+            break
+
     view_only = not user.has_edit_members_portfolio_permission(portfolio) or not user_can_edit_other_users
 
     # ------- USER STATUSES

src/registrar/views/portfolios.py

@@ -48,20 +48,7 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View):
 
     def get(self, request):
         """Add additional context data to the template."""
-
+        return render(request, "portfolio_members.html")
-        if self.request.user.is_authenticated:
-            request.session["new_request"] = True
-
-        # We can override the base class. This view only needs this item.
-        context = {}
-        portfolio = self.request.session.get("portfolio")
-        if portfolio:
-            # ------- Gets all members
-            member_ids = UserPortfolioPermission.objects.filter(portfolio=portfolio).values_list("user__id", flat=True)
-
-            all_members = User.objects.filter(id__in=member_ids)
-            context["portfolio_members"] = all_members
-        return render(request, "portfolio_members.html", context)
 
 
 class PortfolioNoDomainsView(NoPortfolioDomainsPermissionView, View):