Merge pull request #290 from GSA/revert-289-wwang/code-refactoring

Revert "refactoring users, and remove unused page/fuctions"

training-front-end/src/components/AdminIndex.vue

@@ -0,0 +1,69 @@
+<script setup>
+  import { onBeforeMount, ref } from 'vue'
+  import { useStore } from '@nanostores/vue'
+  import { profile } from '../stores/user'
+  import USWDSAlert from './USWDSAlert.vue'
+
+  const authUser = useStore(profile)
+  const userList = ref([])
+  const error = ref()
+  const isAuthorized = ref(true)
+
+  function loadUsers() {
+    const usersEndpoint = `${import.meta.env.PUBLIC_API_BASE_URL}/api/v1/users`
+
+    fetch(usersEndpoint, {
+      method: "GET",
+      headers: { "Authorization": `Bearer ${authUser.value.jwt}` }
+    }).then((res) => {
+      if (res.status === 401) {
+        isAuthorized.value = false
+        error.value = {
+          name: "Unauthorized",
+          message: "You are not authorized to access this feature."
+        }
+      } else {
+        res.json().then((data) => {
+          userList.value = data
+        })
+      }
+    }).catch((err) => {
+      error.value = err
+    })
+  }
+
+  onBeforeMount(() => {
+    loadUsers()
+  })
+
+</script>
+
+<template>
+  <USWDSAlert
+    v-if="error"
+    status="warning"
+    :heading="error.name"
+  >
+    {{ error.message }}
+  </USWDSAlert>
+
+  <div v-if="isAuthorized">
+    <table class="usa-table">
+      <thead>
+        <th>Name</th>
+        <th>Email</th>
+        <th>Agency</th>
+      </thead>
+      <tbody>
+        <tr
+          v-for="user in userList"
+          :key="user.id"
+        >
+          <td>{{ user.name }}</td>
+          <td>{{ user.email }}</td>
+          <td>{{ user.agency_id }}</td>
+        </tr>
+      </tbody>
+    </table>
+  </div>
+</template>

training-front-end/src/components/AdminSearchUser.vue

@@ -13,7 +13,7 @@
   const PAGE_SIZE = 25
 
   const base_url = import.meta.env.PUBLIC_API_BASE_URL
-  const report_url = `${base_url}/api/v1/users/`
+  const report_url = `${base_url}/api/v1/users/search-users-by-name/`
   const update_url = `${base_url}/api/v1/users/edit-user-for-reporting/`
 
   const currentPage = ref(0)

training/api/api_v1/users.py

@@ -1,6 +1,7 @@
 import csv
 from io import StringIO
 import logging
+from typing import List
 from training.api.auth import RequireRole
 from fastapi import APIRouter, status, HTTPException, Response, Depends
 from training.schemas import User, UserCreate, UserSearchResult
@@ -29,8 +30,21 @@ def create_user(
     return db_user
 
 
-@router.patch("/users/edit-user-for-reporting", response_model=User)
-def edit_user_for_reporting(
+@router.get("/users", response_model=List[User])
+def get_users(
+    agency_id: int | None = None,
+    repo: UserRepository = Depends(user_repository),
+    user=Depends(RequireRole(["Admin"]))
+):
+    if agency_id:
+        return repo.find_by_agency(agency_id)
+    else:
+        # If agency_id is <= 0 or None, return all users:
+        return repo.find_all()
+
+
+@router.put("/users/edit-user-for-reporting", response_model=User)
+def edit_user_by_id(
     user_id: int,
     agency_id_list: list[int],
     repo: UserRepository = Depends(user_repository),
@@ -70,22 +84,15 @@ def download_report_csv(user=Depends(user_from_form), repo: UserRepository = Dep
     return Response(output.getvalue(), headers=headers, media_type='application/csv')
 
 
-'''
-Get/users is used to search users for admin portal, currently only support search by user name, it may have additional search criteira in future.
-page_number param is used to support UI pagination functionality.
-It returns UserSearchResult object with a list of users and total_count returned.
-'''
-
-
-@router.get("/users", response_model=UserSearchResult)
-def get_users(
-    name: str | None = None,
-    page_number: int = 1,
+@router.get("/users/search-users-by-name/{name}", response_model=UserSearchResult)
+def search_users_by_name(
+    name: str,
+    page_number: int,
     repo: UserRepository = Depends(user_repository),
     user=Depends(RequireRole(["Admin"]))
 ):
     try:
-        return repo.get_users(name, page_number)
+        return repo.search_users_by_name(name, page_number)
     except ValueError:
         raise HTTPException(
             status_code=status.HTTP_400_BAD_REQUEST,

training/repositories/user.py

@@ -19,7 +19,6 @@ def find_by_email(self, email: str) -> models.User | None:
     def find_by_agency(self, agency_id: int) -> list[models.User]:
         return self._session.query(models.User).filter(models.User.agency_id == agency_id).all()
 
-    # edit_user_for_reporting is function allow admin to modify specific user and assign report role and associate report agencies to the user
     def edit_user_for_reporting(self, user_id: int, report_agencies_list: list[int]) -> models.User:
         db_user = self._session.query(models.User).filter(models.User.id == user_id).first()
         if db_user is None:
@@ -31,13 +30,11 @@ def edit_user_for_reporting(self, user_id: int, report_agencies_list: list[int])
                 if report_role:
                     db_user.roles.append(report_role)
                 else:
-                    # if Report role is not in DB, add it to DB (should not happen if data is prepopulated properly via seed.py and no direct DB removal)
                     role = models.Role(name="Report")
                     self._session.add(role)
                     self._session.commit()
                     db_user.roles.append(role)
         else:
-            # if report_agencies_list =[], it means remove all user associated agencies and thus remove user report role.
             if len(report_role_exist) > 0:
                 db_user.roles = [obj for obj in db_user.roles if obj.name != "Report"]
         db_user.report_agencies.clear()
@@ -50,6 +47,9 @@ def edit_user_for_reporting(self, user_id: int, report_agencies_list: list[int])
         self._session.commit()
         return db_user
 
+    def get_admins_users(self) -> list[models.User]:
+        return self._session.query(models.User).filter(models.User.roles.any(name='Admin')).all()
+
     def get_user_quiz_completion_report(self, report_user_id: int) -> list[UserQuizCompletionReportData]:
         report_user = self.find_by_id(report_user_id)
         if report_user and report_user.report_agencies:
@@ -66,8 +66,7 @@ def get_user_quiz_completion_report(self, report_user_id: int) -> list[UserQuizC
         else:
             raise ValueError("Invalid Report User")
 
-    def get_users(self, name: str, page_number: int) -> UserSearchResult:
-        # currently UI only support search by name, future may provide more search criteria
+    def search_users_by_name(self, name: str, page_number: int) -> UserSearchResult:
         if (name and name.strip() != '' and page_number > 0):
             count = self._session.query(models.User).filter(models.User.name.ilike(f"%{name}%")).count()
             page_size = 25

training/tests/test_api_users.py

@@ -53,12 +53,36 @@ def test_create_user_duplicate(goodJWT, mock_user_repo: UserRepository):
 
 
 @patch('training.config.settings', 'JWT_SECRET', 'super_secret')
-def test_get_users(goodJWT, mock_user_repo: UserRepository):
+def test_get_users_all(goodJWT, mock_user_repo: UserRepository):
+    users = [UserSchemaFactory.build() for x in range(3)]
+    mock_user_repo.find_all.return_value = users
+    response = client.get(
+        "/api/v1/users",
+        headers={"Authorization": f"Bearer {goodJWT}"}
+    )
+    assert response.status_code == status.HTTP_200_OK
+    assert len(response.json()) == 3
+
+
+@patch('training.config.settings', 'JWT_SECRET', 'super_secret')
+def test_get_users_by_agency(goodJWT, mock_user_repo: UserRepository):
+    users = [UserSchemaFactory.build(agency_id=2) for x in range(5)]
+    mock_user_repo.find_by_agency.return_value = users
+    response = client.get(
+        "/api/v1/users?agency_id=2",
+        headers={"Authorization": f"Bearer {goodJWT}"}
+    )
+    assert response.status_code == status.HTTP_200_OK
+    assert len(response.json()) == 5
+
+
+@patch('training.config.settings', 'JWT_SECRET', 'super_secret')
+def test_search_users_by_name(goodJWT, mock_user_repo: UserRepository):
     users = [UserSchemaFactory.build(name="test name") for x in range(2)]
     user_search_result = UserSearchResult(users=users, total_count=2)
-    mock_user_repo.get_users.return_value = user_search_result
+    mock_user_repo.search_users_by_name.return_value = user_search_result
     response = client.get(
-        "/api/v1/users?name=test&page_number=1",
+        "/api/v1/users/search-users-by-name/test?page_number=1",
         headers={"Authorization": f"Bearer {goodJWT}"}
     )
     assert response.status_code == status.HTTP_200_OK
@@ -78,7 +102,7 @@ def test_edit_user_for_reporting(mock_user_repo: UserRepository, goodJWT: str):
     mock_user_repo.edit_user_for_reporting.return_value = user
     user_id = user.id
     URL = f"/api/v1/users/edit-user-for-reporting?user_id={user_id}"
-    response = client.patch(
+    response = client.put(
         URL,
         json=[3],
         headers={"Authorization": f"Bearer {goodJWT}"}

training/tests/test_user_repository.py

@@ -99,17 +99,18 @@ def test_invalid_edit_user_for_reporting(user_repo_with_data: UserRepository):
         user_repo_with_data.create(invalid_user_id, invalid_agency_id_list)
 
 
-def test_get_users(user_repo_with_data: UserRepository, valid_user_ids: List[int]):
+def test_search_users_by_name(user_repo_with_data: UserRepository, valid_user_ids: List[int]):
     valid_user_id = valid_user_ids[0]
     db_user = user_repo_with_data.find_by_id(valid_user_id)
     search_criteria = db_user.name[:-1]
-    result = user_repo_with_data.get_users(search_criteria, 1)
+    result = user_repo_with_data.search_users_by_name(search_criteria, 1)
     assert result is not None
     for item in result.users:
         assert search_criteria in item.name
 
 
-def test_invalid_get_users(user_repo_with_data: UserRepository):
+def test_invalid_search_users_by_name(user_repo_with_data: UserRepository):
     search_criteria = None
+
     with pytest.raises(Exception):
-        user_repo_with_data.get_users(search_criteria, 0)
+        user_repo_with_data.search_users_by_name(search_criteria, 0)