Merge pull request #1646 from bcgov/fix/daniel-remove-user-filter-but…

…ton-1634

fix: Remove Active filter button and Remove User ID Column

backend/lcfs/web/api/user/repo.py

@@ -1,9 +1,7 @@
-import structlog
 from typing import List
 
+import structlog
 from fastapi import Depends
-from fastapi_cache.decorator import cache
-from lcfs.db.models.user import UserLoginHistory
 from sqlalchemy import (
     and_,
     select,
@@ -14,42 +12,40 @@
     func,
     cast,
     String,
-    update,
 )
-from sqlalchemy.orm import joinedload
 from sqlalchemy.ext.asyncio import AsyncSession
-from lcfs.web.exception.exceptions import DataNotFoundException
+from sqlalchemy.orm import joinedload
 
-from lcfs.services.keycloak.dependencies import parse_external_username
-from lcfs.web.core.decorators import repo_handler
 from lcfs.db.dependencies import get_async_db_session
-from lcfs.db.models.user.UserProfile import UserProfile
-from lcfs.db.models.user.UserRole import UserRole
-from lcfs.db.models.organization.Organization import Organization
-from lcfs.db.models.user.Role import Role, RoleEnum
-from lcfs.db.models.transfer.TransferHistory import TransferHistory
-from lcfs.db.models.transfer.TransferStatus import TransferStatus
+from lcfs.db.models.admin_adjustment.AdminAdjustmentHistory import (
+    AdminAdjustmentHistory,
+)
+from lcfs.db.models.admin_adjustment.AdminAdjustmentStatus import AdminAdjustmentStatus
 from lcfs.db.models.initiative_agreement.InitiativeAgreementHistory import (
     InitiativeAgreementHistory,
 )
 from lcfs.db.models.initiative_agreement.InitiativeAgreementStatus import (
     InitiativeAgreementStatus,
 )
-from lcfs.db.models.admin_adjustment.AdminAdjustmentHistory import (
-    AdminAdjustmentHistory,
-)
-from lcfs.db.models.admin_adjustment.AdminAdjustmentStatus import AdminAdjustmentStatus
-from lcfs.web.api.user.schema import (
-    UserCreateSchema,
-    UserBaseSchema,
-    UserLoginHistorySchema,
-)
+from lcfs.db.models.organization.Organization import Organization
+from lcfs.db.models.transfer.TransferHistory import TransferHistory
+from lcfs.db.models.transfer.TransferStatus import TransferStatus
+from lcfs.db.models.user import UserLoginHistory
+from lcfs.db.models.user.Role import Role, RoleEnum
+from lcfs.db.models.user.UserProfile import UserProfile
+from lcfs.db.models.user.UserRole import UserRole
 from lcfs.web.api.base import (
     PaginationRequestSchema,
     camel_to_snake,
     apply_filter_conditions,
     get_field_for_filter,
 )
+from lcfs.web.api.user.schema import (
+    UserCreateSchema,
+    UserBaseSchema,
+    UserLoginHistorySchema,
+)
+from lcfs.web.core.decorators import repo_handler
 
 logger = structlog.get_logger(__name__)
 
@@ -68,7 +64,6 @@ def apply_filters(self, pagination, conditions):
             filter_type = filter.filter_type
 
             if filter.field == "role":
-                field = get_field_for_filter(Role, "name")
                 role_filter_present = True
                 conditions.append(
                     Role.name.in_(
@@ -624,7 +619,9 @@ def _apply_login_history_filters(self, query, pagination):
                 filter_type = filter.filter_type
                 if filter.field == "is_login_successful":
                     filter_option = "true" if filter_value == "Success" else "false"
-                    field = get_field_for_filter(UserLoginHistory, "is_login_successful")
+                    field = get_field_for_filter(
+                        UserLoginHistory, "is_login_successful"
+                    )
                 elif filter.field is not None:
                     field = get_field_for_filter(UserLoginHistory, filter.field)
                 if field is not None:

backend/lcfs/web/api/user/schema.py

@@ -85,7 +85,6 @@ class UserLoginHistorySchema(BaseSchema):
     user_login_history_id: int
     keycloak_email: str
     external_username: str
-    keycloak_user_id: str
     is_login_successful: bool
     login_error_message: Optional[str] = None
     create_date: datetime

frontend/src/assets/locales/en/organization.json

@@ -16,8 +16,6 @@
   "newUsrBtn": "New user",
   "orgDownloadBtn": "Download organization information",
   "userDownloadBtn": "Download user information",
-  "active": "Active",
-  "inctive": "Inactive",
   "legalNameLabel": "Legal name of organization",
   "operatingNameLabel": "Operating name of organization",
   "sameAsLegalNameLabel": "Same as legal name",
@@ -35,11 +33,8 @@
   "cntryLabel": "Country",
   "bcAddrLabel": "Address in B.C. (at which records are maintained)",
   "sameAddrLabel": "Same as address for service",
-  "contactMsg": "to update address informations.",
-  "activeUsersBtn": "Active users",
-  "inactiveUsersBtn": "Inactive users",
-  "showInactiveUsersBtn": "Show inactive users",
-  "showActiveUsersBtn": "Show active users",
+  "contactMsg": "to update address information.",
+  "usersLabel": "Users",
   "addOrgBtn": "Add organization",
   "addOrgTitle": "Add organization",
   "editOrgTitle": "Edit organization",

frontend/src/views/Admin/AdminMenu/components/_schema.js

@@ -83,7 +83,7 @@ export const usersColumnDefs = (t) => [
     headerName: t('admin:userColLabels.status'),
     valueGetter: (params) => params.data.isActive,
     filterParams: {
-      textMatcher: (filter) => {
+      textMatcher: () => {
         return true
       }
     },
@@ -210,10 +210,6 @@ export const userLoginHistoryColDefs = (t) => [
     field: 'keycloakEmail',
     headerName: t('admin:userLoginHistoryColLabels.keycloakEmail')
   },
-  {
-    field: 'keycloakUserId',
-    headerName: t('admin:userLoginHistoryColLabels.keycloakUserId')
-  },
   {
     field: 'externalUsername',
     headerName: t('admin:userLoginHistoryColLabels.externalUsername')

frontend/src/views/Organizations/ViewOrganization/ViewOrganization.jsx

@@ -26,7 +26,6 @@ import { LinkRenderer } from '@/utils/grid/cellRenderers.jsx'
 
 export const ViewOrganization = () => {
   const { t } = useTranslation(['common', 'org'])
-  const [showActive, setShowActive] = useState(true)
   const [alertMessage, setAlertMessage] = useState('')
   const [alertSeverity, setAlertSeverity] = useState('info')
 
@@ -56,12 +55,8 @@ export const ViewOrganization = () => {
 
   const [gridKey, setGridKey] = useState(`users-grid-${orgID}-active`)
   const handleGridKey = useCallback(() => {
-    if (showActive) {
-      setGridKey(`users-grid-${orgID}-active`)
-    } else {
-      setGridKey(`users-grid-${orgID}-inactive`)
-    }
-  }, [showActive, orgID])
+    setGridKey(`users-grid-${orgID}`)
+  }, [orgID])
 
   const gridOptions = {
     overlayNoRowsTemplate: 'No users found',
@@ -94,21 +89,6 @@ export const ViewOrganization = () => {
 
   const gridRef = useRef()
 
-  useEffect(() => {
-    if (gridRef.current) {
-      // clear any previous filters
-      localStorage.removeItem(`${gridKey}-filter`)
-      const statusFilter = gridRef?.current?.api?.getFilterInstance('isActive')
-      if (statusFilter) {
-        statusFilter.setModel({
-          type: 'equals',
-          filter: showActive ? 'Active' : 'Inactive'
-        })
-      }
-      gridRef?.current?.api?.onFilterChanged()
-    }
-  }, [showActive, gridKey])
-
   useEffect(() => {
     if (location.state?.message) {
       setAlertMessage(location.state.message)
@@ -226,105 +206,49 @@ export const ViewOrganization = () => {
         }}
         my={2}
       >
-        {showActive ? (
-          <>
-            <BCBox component="div">
-              <Role roles={[roles.administrator, roles.manage_users]}>
-                <BCButton
-                  variant="contained"
-                  size="small"
-                  color="primary"
-                  sx={{
-                    textTransform: 'none',
-                    marginRight: '8px',
-                    marginBottom: '8px'
-                  }}
-                  startIcon={
-                    <FontAwesomeIcon
-                      icon={faCirclePlus}
-                      className="small-icon"
-                    />
-                  }
-                  onClick={() =>
-                    !isCurrentUserLoading && hasRoles(roles.government)
-                      ? navigate(
-                          ROUTES.ORGANIZATIONS_ADDUSER.replace(':orgID', orgID)
-                        )
-                      : navigate(ROUTES.ORGANIZATION_ADDUSER)
-                  }
-                >
-                  <BCTypography variant="button">
-                    {t('org:newUsrBtn')}
-                  </BCTypography>
-                </BCButton>
-              </Role>
-              <BCButton
-                variant="outlined"
-                size="small"
-                color="primary"
-                data-test="show-inactive-btn"
-                sx={{
-                  textTransform: 'none',
-                  marginRight: '8px',
-                  marginBottom: '8px',
-                  whiteSpace: 'nowrap'
-                }}
-                onClick={() => setShowActive(false)}
-              >
-                <BCTypography variant="button">
-                  {t('org:showInactiveUsersBtn')}
-                </BCTypography>
-              </BCButton>
-            </BCBox>
-            <BCTypography
-              variant="h5"
-              mt={1}
-              color="primary"
-              data-test="active-users-heading"
-            >
-              {t('org:activeUsersBtn')}
-            </BCTypography>
-          </>
-        ) : (
-          <>
+        <BCBox component="div">
+          <Role roles={[roles.administrator, roles.manage_users]}>
             <BCButton
-              variant="outlined"
+              variant="contained"
               size="small"
               color="primary"
-              data-test="show-active-btn"
               sx={{
                 textTransform: 'none',
                 marginRight: '8px',
-                marginBottom: '8px',
-                whiteSpace: 'nowrap'
+                marginBottom: '8px'
               }}
-              onClick={() => setShowActive(true)}
+              startIcon={
+                <FontAwesomeIcon icon={faCirclePlus} className="small-icon" />
+              }
+              onClick={() =>
+                !isCurrentUserLoading && hasRoles(roles.government)
+                  ? navigate(
+                      ROUTES.ORGANIZATIONS_ADDUSER.replace(':orgID', orgID)
+                    )
+                  : navigate(ROUTES.ORGANIZATION_ADDUSER)
+              }
             >
-              <BCTypography variant="subtitle2">
-                {t('org:showActiveUsersBtn')}
-              </BCTypography>
+              <BCTypography variant="button">{t('org:newUsrBtn')}</BCTypography>
             </BCButton>
-            <BCTypography
-              variant="h5"
-              mt={1}
-              color="primary"
-              data-test="inactive-users-heading"
-            >
-              {t('org:inactiveUsersBtn')}
-            </BCTypography>
-          </>
-        )}
+          </Role>
+        </BCBox>
+        <BCTypography
+          variant="h5"
+          mt={1}
+          color="primary"
+          data-test="active-users-heading"
+        >
+          {t('org:usersLabel')}
+        </BCTypography>
       </BCBox>
       <BCBox sx={{ height: '100%', width: '100%' }}>
         <BCDataGridServer
           gridRef={gridRef}
-          apiEndpoint={apiRoutes.orgUsers
-            .replace(
-              ':orgID',
-              orgID || currentUser?.organization?.organizationId
-            )
-            .concat(showActive ? '?status=Active' : '?status=Inactive')}
-          apiData={'users'}
+          apiEndpoint={apiRoutes.orgUsers.replace(
+            ':orgID',
+            orgID || currentUser?.organization?.organizationId
+          )}
+          apiData="users"
           columnDefs={getUserColumnDefs(t)}
           gridKey={gridKey}
           getRowId={getRowId}

frontend/src/views/Organizations/ViewOrganization/__tests__/ViewOrganization.test.jsx

@@ -138,14 +138,4 @@ describe('ViewOrganization Component Tests', () => {
     expect(screen.getByText(/1,000/i)).toBeInTheDocument()
     expect(screen.getByText(/500/i)).toBeInTheDocument()
   })
-
-  it('toggles user visibility when toggling active/inactive users', async () => {
-    const showInactiveButton = screen.getByTestId('show-inactive-btn')
-    fireEvent.click(showInactiveButton)
-    expect(screen.getByTestId('inactive-users-heading')).toBeInTheDocument()
-
-    const showActiveButton = screen.getByTestId('show-active-btn')
-    fireEvent.click(showActiveButton)
-    expect(screen.getByTestId('active-users-heading')).toBeInTheDocument()
-  })
 })

frontend/src/views/Organizations/ViewOrganization/_schema.js

@@ -3,7 +3,6 @@ import { LinkRenderer, OrgStatusRenderer } from '@/utils/grid/cellRenderers'
 import { BCSelectFloatingFilter } from '@/components/BCDataGrid/components'
 import { useOrganizationStatuses } from '@/hooks/useOrganizations'
 import { usersColumnDefs } from '@/views/Admin/AdminMenu/components/_schema'
-import { t } from 'i18next'
 
 export const organizationsColDefs = (t) => [
   {
@@ -23,8 +22,6 @@ export const organizationsColDefs = (t) => [
     width: 300,
     valueGetter: (params) =>
       params.data.totalBalance - Math.abs(params.data.reservedBalance),
-    // Temporary measures
-    // filter: 'agNumberColumnFilter',
     filter: false,
     sortable: false
   },
@@ -36,8 +33,6 @@ export const organizationsColDefs = (t) => [
     valueGetter: (params) => Math.abs(params.data.reservedBalance),
     width: 300,
     cellRenderer: LinkRenderer,
-    // Temporary measures
-    // filter: 'agNumberColumnFilter',
     filter: false,
     sortable: false
   },
@@ -49,6 +44,7 @@ export const organizationsColDefs = (t) => [
     valueGetter: (params) => params.data.orgStatus.status,
     cellRenderer: OrgStatusRenderer,
     cellClass: 'vertical-middle',
+    filter: true,
     floatingFilterComponent: BCSelectFloatingFilter,
     floatingFilterComponentParams: {
       valueKey: 'status',
@@ -60,13 +56,11 @@ export const organizationsColDefs = (t) => [
 ]
 
 export const getUserColumnDefs = (t) => {
-  const colDefs = usersColumnDefs(t).map((colDef) => {
+  return usersColumnDefs(t).map((colDef) => {
     if (colDef.field === 'isActive') {
       return {
         ...colDef,
-        sortable: false,
-        suppressHeaderMenuButton: true,
-        floatingFilter: false
+        sortable: false
       }
     } else if (colDef.field === 'role') {
       // pick only supplier roles
@@ -76,11 +70,6 @@ export const getUserColumnDefs = (t) => {
     }
     return colDef
   })
-  return colDefs
 }
 
-export const usersColDefs = getUserColumnDefs(t)
 export const defaultSortModel = [{ field: 'firstName', direction: 'asc' }]
-export const defaultFilterModel = [
-  { filterType: 'text', type: 'equals', field: 'isActive', filter: 'Active' }
-]