From 3265b7cef95ab0be618ab0c89cdf437e2775ccba Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Mon, 30 Dec 2024 16:32:59 -0800 Subject: [PATCH 1/6] feat: updated org names endpoint to accept only_registered param --- backend/lcfs/web/api/organizations/views.py | 20 +++++++++---------- frontend/src/hooks/useOrganizations.js | 11 ++++++---- .../components/TransactionDetails.jsx | 4 ++-- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/backend/lcfs/web/api/organizations/views.py b/backend/lcfs/web/api/organizations/views.py index 8ff755fc9..9f3e8f517 100644 --- a/backend/lcfs/web/api/organizations/views.py +++ b/backend/lcfs/web/api/organizations/views.py @@ -162,25 +162,23 @@ async def get_organization_types( return await service.get_organization_types() -# TODO review security of this endpoint around returning balances -# for all organizations @router.get( "/names/", response_model=List[OrganizationSummaryResponseSchema], status_code=status.HTTP_200_OK, ) -@cache(expire=1) # cache for 1 hour -@view_handler(["*"]) +@cache(expire=1) # Cache for 1 hour +@view_handler( + [RoleEnum.GOVERNMENT] +) # Ensure only government can access this endpoint because it returns balances async def get_organization_names( - request: Request, service: OrganizationsService = Depends() + request: Request, + only_registered: bool = Query(True), + service: OrganizationsService = Depends(), ): - """Fetch all organization names""" - - # Set the default sorting order + """Fetch all organization names.""" order_by = ("name", "asc") - - # Call the service with only_registered set to True to fetch only registered organizations - return await service.get_organization_names(True, order_by) + return await service.get_organization_names(only_registered, order_by) @router.get( diff --git a/frontend/src/hooks/useOrganizations.js b/frontend/src/hooks/useOrganizations.js index 7cc6f4bfc..e665e145b 100644 --- a/frontend/src/hooks/useOrganizations.js +++ b/frontend/src/hooks/useOrganizations.js @@ -11,13 +11,16 @@ export const useOrganizationStatuses = (options) => { }) } -export const useOrganizationNames = (options) => { +export const useOrganizationNames = (onlyRegistered = true, options) => { const client = useApiService() return useQuery({ - queryKey: ['organization-names'], - queryFn: async () => (await client.get('/organizations/names/')).data, - ...options + queryKey: ['organization-names', onlyRegistered], + queryFn: async () => { + const response = await client.get(`/organizations/names/?only_registered=${onlyRegistered}`) + return response.data + }, + ...options, }) } diff --git a/frontend/src/views/Transactions/components/TransactionDetails.jsx b/frontend/src/views/Transactions/components/TransactionDetails.jsx index 81fdd1b3b..1d674d651 100644 --- a/frontend/src/views/Transactions/components/TransactionDetails.jsx +++ b/frontend/src/views/Transactions/components/TransactionDetails.jsx @@ -16,7 +16,7 @@ import { } from '@mui/material' import { dateFormatter, numberFormatter } from '@/utils/formatters' import { useFormContext, Controller } from 'react-hook-form' -import { useRegExtOrgs } from '@/hooks/useOrganizations' +import { useOrganizationNames } from '@/hooks/useOrganizations' import { useOrganizationBalance } from '@/hooks/useOrganization' import Loading from '@/components/Loading' import { @@ -34,7 +34,7 @@ export const TransactionDetails = ({ transactionId, isEditable }) => { control } = useFormContext() - const { data: orgData } = useRegExtOrgs() + const { data: orgData } = useOrganizationNames(false) const organizations = orgData?.map((org) => ({ value: parseInt(org.organizationId), From 396c402e7e5c919a2513fd4b094d7c46c93ac7e5 Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Fri, 3 Jan 2025 11:51:40 -0800 Subject: [PATCH 2/6] fix: tests updated with new hook --- .../__tests__/AddEditViewTransaction.test.jsx | 12 ++++++------ .../components/__tests__/TransactionDetails.test.jsx | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frontend/src/views/Transactions/__tests__/AddEditViewTransaction.test.jsx b/frontend/src/views/Transactions/__tests__/AddEditViewTransaction.test.jsx index 7a8be97a6..1b129f3e4 100644 --- a/frontend/src/views/Transactions/__tests__/AddEditViewTransaction.test.jsx +++ b/frontend/src/views/Transactions/__tests__/AddEditViewTransaction.test.jsx @@ -19,7 +19,7 @@ import { useCreateUpdateInitiativeAgreement, useInitiativeAgreement } from '@/hooks/useInitiativeAgreement' -import { useRegExtOrgs } from '@/hooks/useOrganizations' +import { useOrganizationNames } from '@/hooks/useOrganizations' import { useOrganizationBalance } from '@/hooks/useOrganization' import { useTransactionMutation } from '../transactionMutation' import { TRANSACTION_STATUSES } from '@/constants/statuses' @@ -110,7 +110,7 @@ vi.mock('@fortawesome/react-fontawesome', () => ({ // Mock the hooks vi.mock('@/hooks/useOrganizations', () => ({ - useRegExtOrgs: vi.fn().mockReturnValue({ + useOrganizationNames: vi.fn().mockReturnValue({ data: [ { organizationId: 1, @@ -281,7 +281,7 @@ describe('AddEditViewTransaction Component Tests', () => { state: null }) - useRegExtOrgs.mockReturnValue({ + useOrganizationNames.mockReturnValue({ data: [ { organizationId: 1, @@ -356,7 +356,7 @@ describe('AddEditViewTransaction Component Tests', () => { mutate: vi.fn(), isLoading: false }) - useRegExtOrgs.mockReturnValue({ + useOrganizationNames.mockReturnValue({ data: [], isLoading: false, isFetched: true, @@ -425,7 +425,7 @@ describe('AddEditViewTransaction Component Tests', () => { isLoadingError: false }) - useRegExtOrgs.mockReturnValue({ + useOrganizationNames.mockReturnValue({ data: [], isLoading: false, isFetched: true, @@ -500,7 +500,7 @@ describe('AddEditViewTransaction Component Tests', () => { isLoading: false }) - useRegExtOrgs.mockReturnValue({ + useOrganizationNames.mockReturnValue({ data: [], isLoading: false, isFetched: true, diff --git a/frontend/src/views/Transactions/components/__tests__/TransactionDetails.test.jsx b/frontend/src/views/Transactions/components/__tests__/TransactionDetails.test.jsx index 6782f7ca3..70a7eccce 100644 --- a/frontend/src/views/Transactions/components/__tests__/TransactionDetails.test.jsx +++ b/frontend/src/views/Transactions/components/__tests__/TransactionDetails.test.jsx @@ -3,7 +3,7 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest' import { TransactionDetails } from '../TransactionDetails' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { ThemeProvider } from '@mui/material' -import { useRegExtOrgs } from '@/hooks/useOrganizations' +import { useOrganizationNames } from '@/hooks/useOrganizations' import { useOrganizationBalance } from '@/hooks/useOrganization' import theme from '@/themes' import { FormProvider, useForm } from 'react-hook-form' @@ -65,7 +65,7 @@ describe('TransactionDetails Component', () => { beforeEach(() => { vi.clearAllMocks() - useRegExtOrgs.mockReturnValue({ + useOrganizationNames.mockReturnValue({ data: mockOrganizations, isLoading: false }) From a09c855494e45a96080266e75ebd72809e01f51b Mon Sep 17 00:00:00 2001 From: Daniel Haselhan Date: Fri, 3 Jan 2025 14:11:37 -0800 Subject: [PATCH 3/6] feat: Add nice renderer for login status * Use new renderer with pill style instead of checkboxes * Minor fixes to User Login Table, using ID as string and removing invalid cell data types. --- frontend/src/utils/grid/cellRenderers.jsx | 29 +++++++++++++++++++ .../AdminMenu/components/UserLoginHistory.jsx | 2 +- .../Admin/AdminMenu/components/_schema.js | 24 ++++----------- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/frontend/src/utils/grid/cellRenderers.jsx b/frontend/src/utils/grid/cellRenderers.jsx index 7563007f4..7cbf25016 100644 --- a/frontend/src/utils/grid/cellRenderers.jsx +++ b/frontend/src/utils/grid/cellRenderers.jsx @@ -65,6 +65,35 @@ export const StatusRenderer = (props) => { ) } +export const LoginStatusRenderer = (props) => { + return ( + + + + ) +} + export const OrgStatusRenderer = (props) => { const location = useLocation() const statusArr = getAllOrganizationStatuses() diff --git a/frontend/src/views/Admin/AdminMenu/components/UserLoginHistory.jsx b/frontend/src/views/Admin/AdminMenu/components/UserLoginHistory.jsx index 3f3fbc452..7be94f126 100644 --- a/frontend/src/views/Admin/AdminMenu/components/UserLoginHistory.jsx +++ b/frontend/src/views/Admin/AdminMenu/components/UserLoginHistory.jsx @@ -10,7 +10,7 @@ export const UserLoginHistory = () => { const { t } = useTranslation(['common', 'admin']) const getRowId = useCallback((params) => { - return params.data.userLoginHistoryId + return params.data.userLoginHistoryId.toString() }, []) return ( diff --git a/frontend/src/views/Admin/AdminMenu/components/_schema.js b/frontend/src/views/Admin/AdminMenu/components/_schema.js index 7b46ba5bb..1c8093fe5 100644 --- a/frontend/src/views/Admin/AdminMenu/components/_schema.js +++ b/frontend/src/views/Admin/AdminMenu/components/_schema.js @@ -5,6 +5,7 @@ import { } from '@/utils/formatters' import { LinkRenderer, + LoginStatusRenderer, RoleRenderer, StatusRenderer } from '@/utils/grid/cellRenderers' @@ -113,15 +114,6 @@ export const usersColumnDefs = (t) => [ } ] -export const usersDefaultColDef = { - resizable: true, - sortable: true, - filter: true, - minWidth: 300, - floatingFilter: true, // enables the filter boxes under the header label - suppressHeaderMenuButton: true // suppresses the menu button appearing next to the Header Label -} - export const idirUserDefaultFilter = [ { filterType: 'text', type: 'blank', field: 'organizationId', filter: '' } ] @@ -175,28 +167,24 @@ export const userLoginHistoryColDefs = (t) => [ }, { field: 'keycloakEmail', - headerName: t('admin:userLoginHistoryColLabels.keycloakEmail'), - cellDataType: 'string' + headerName: t('admin:userLoginHistoryColLabels.keycloakEmail') }, { field: 'keycloakUserId', - headerName: t('admin:userLoginHistoryColLabels.keycloakUserId'), - cellDataType: 'string' + headerName: t('admin:userLoginHistoryColLabels.keycloakUserId') }, { field: 'externalUsername', - headerName: t('admin:userLoginHistoryColLabels.externalUsername'), - cellDataType: 'string' + headerName: t('admin:userLoginHistoryColLabels.externalUsername') }, { field: 'isLoginSuccessful', headerName: t('admin:userLoginHistoryColLabels.isLoginSuccessful'), - cellDataType: 'boolean' + cellRenderer: LoginStatusRenderer }, { field: 'loginErrorMessage', - headerName: t('admin:userLoginHistoryColLabels.loginErrorMessage'), - cellDataType: 'string' + headerName: t('admin:userLoginHistoryColLabels.loginErrorMessage') }, { field: 'createDate', From 1324a4a3f926cab2a89188399ded7515e022f7f8 Mon Sep 17 00:00:00 2001 From: prv-proton Date: Fri, 3 Jan 2025 14:11:57 -0800 Subject: [PATCH 4/6] Fix: LCFS - Fix ETL Role Mapping Between TFRS and LCFS #1594 --- etl/nifi_scripts/user.groovy | 179 ++++++++++++++++++++++++----------- 1 file changed, 126 insertions(+), 53 deletions(-) diff --git a/etl/nifi_scripts/user.groovy b/etl/nifi_scripts/user.groovy index 77fb9bdf7..bc8910f11 100644 --- a/etl/nifi_scripts/user.groovy +++ b/etl/nifi_scripts/user.groovy @@ -1,8 +1,9 @@ import java.sql.Connection import java.sql.PreparedStatement import java.sql.ResultSet +import groovy.json.JsonSlurper -log.warn("**** STARTING USER ETL ****") +log.warn('**** STARTING USER ETL ****') // SQL query to extract user profiles def userProfileQuery = """ @@ -17,33 +18,98 @@ def userProfileQuery = """ first_name, last_name, is_active, - CASE WHEN organization_id = 1 THEN null ELSE organization_id END as organization_id + CASE WHEN organization_id = 1 THEN NULL ELSE organization_id END as organization_id FROM public.user; """ // SQL query to extract user roles def userRoleQuery = """ - SELECT ur.user_id as user_profile_id, - CASE - WHEN r.name = 'Admin' THEN 'ADMINISTRATOR' - WHEN r.name = 'GovUser' THEN 'ANALYST' - WHEN r.name IN ('GovDirector', 'GovDeputyDirector') THEN 'DIRECTOR' - WHEN r.name = 'GovComplianceManager' THEN 'COMPLIANCE_MANAGER' - WHEN r.name = 'FSAdmin' THEN 'MANAGE_USERS' - WHEN r.name = 'FSUser' THEN 'TRANSFER' - WHEN r.name = 'FSManager' THEN 'SIGNING_AUTHORITY' - WHEN r.name = 'FSNoAccess' THEN 'READ_ONLY' - WHEN r.name = 'ComplianceReporting' THEN 'COMPLIANCE_REPORTING' - ELSE NULL - END AS role_name - FROM public.user u - INNER JOIN user_role ur ON ur.user_id = u.id - INNER JOIN role r ON r.id = ur.role_id - WHERE r.name NOT IN ('FSDocSubmit', 'GovDoc'); + WITH RoleData AS ( + SELECT ur.user_id AS user_profile_id, + CASE + -- Government Roles + WHEN u.organization_id = 1 THEN + CASE + WHEN r.name = 'Admin' THEN 'ADMINISTRATOR' + WHEN r.name = 'GovUser' THEN 'ANALYST' + WHEN r.name IN ('GovDirector', 'GovDeputyDirector') THEN 'DIRECTOR' + WHEN r.name = 'GovComplianceManager' THEN 'COMPLIANCE_MANAGER' + END + -- Supplier Roles + WHEN u.organization_id > 1 THEN + CASE + WHEN r.name = 'FSAdmin' THEN 'MANAGE_USERS' + WHEN r.name = 'FSUser' THEN 'TRANSFER' + WHEN r.name = 'FSManager' THEN 'SIGNING_AUTHORITY' + WHEN r.name = 'FSNoAccess' THEN 'READ_ONLY' + WHEN r.name = 'ComplianceReporting' THEN 'COMPLIANCE_REPORTING' + END + END AS role_name, + u.organization_id + FROM public.user u + INNER JOIN user_role ur ON ur.user_id = u.id + INNER JOIN role r ON r.id = ur.role_id + WHERE r.name NOT IN ('FSDocSubmit', 'GovDoc') + ), + FilteredRoles AS ( + SELECT user_profile_id, + organization_id, + ARRAY_AGG(role_name) AS roles + FROM RoleData + WHERE role_name IS NOT NULL + GROUP BY user_profile_id, organization_id + ), + ProcessedRoles AS ( + SELECT + user_profile_id, + CASE + -- Rule 1: Government Users + WHEN organization_id = 1 THEN + CASE + -- Retain Administrator and one prioritized gov role + WHEN 'ADMINISTRATOR' = ANY(roles) THEN + ARRAY_REMOVE(ARRAY[ + 'ADMINISTRATOR', + CASE + WHEN 'DIRECTOR' = ANY(roles) THEN 'DIRECTOR' + WHEN 'COMPLIANCE_MANAGER' = ANY(roles) THEN 'COMPLIANCE_MANAGER' + WHEN 'ANALYST' = ANY(roles) THEN 'ANALYST' + END + ], NULL) + -- Priority among gov roles (no Administrator) + ELSE ARRAY_REMOVE(ARRAY[ + CASE + WHEN 'DIRECTOR' = ANY(roles) THEN 'DIRECTOR' + WHEN 'COMPLIANCE_MANAGER' = ANY(roles) THEN 'COMPLIANCE_MANAGER' + WHEN 'ANALYST' = ANY(roles) THEN 'ANALYST' + END + ], NULL) + END + -- Rule 2: Supplier Users + WHEN organization_id > 1 THEN + CASE + -- Remove government roles and retain Read Only if it exists + WHEN 'READ_ONLY' = ANY(roles) THEN + ARRAY['READ_ONLY'] + ELSE ARRAY( + SELECT UNNEST(roles) + EXCEPT + SELECT UNNEST(ARRAY['ADMINISTRATOR', 'ANALYST', 'DIRECTOR', 'COMPLIANCE_MANAGER']) + ) + END + END AS filtered_roles, + organization_id + FROM FilteredRoles + ) + SELECT + user_profile_id, + organization_id, + array_to_string(filtered_roles, ',') as roles_string + FROM ProcessedRoles; """ // SQL queries to insert user profiles and roles into destination tables with ON CONFLICT handling -def insertUserProfileSQL = """ +def insertUserProfileSQL = ''' INSERT INTO user_profile (user_profile_id, keycloak_user_id, keycloak_email, keycloak_username, email, title, phone, mobile_phone, first_name, last_name, is_active, organization_id) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (user_profile_id) DO UPDATE @@ -58,17 +124,17 @@ def insertUserProfileSQL = """ last_name = EXCLUDED.last_name, is_active = EXCLUDED.is_active, organization_id = EXCLUDED.organization_id; -""" +''' -def insertUserRoleSQL = """ +def insertUserRoleSQL = ''' INSERT INTO user_role (user_profile_id, role_id) VALUES (?, (SELECT role_id FROM role WHERE name = ?::role_enum)) ON CONFLICT (user_profile_id, role_id) DO NOTHING; -""" +''' // Fetch connections to both source and destination databases -def sourceDbcpService = context.controllerServiceLookup.getControllerService("3245b078-0192-1000-ffff-ffffba20c1eb") -def destinationDbcpService = context.controllerServiceLookup.getControllerService("3244bf63-0192-1000-ffff-ffffc8ec6d93") +def sourceDbcpService = context.controllerServiceLookup.getControllerService('3245b078-0192-1000-ffff-ffffba20c1eb') +def destinationDbcpService = context.controllerServiceLookup.getControllerService('3244bf63-0192-1000-ffff-ffffc8ec6d93') Connection sourceConn = null Connection destinationConn = null @@ -88,18 +154,18 @@ try { // Process the result set for user profiles while (userProfileResultSet.next()) { - def userProfileId = userProfileResultSet.getInt("user_profile_id") - def keycloakUserId = userProfileResultSet.getString("keycloak_user_id") - def keycloakEmail = userProfileResultSet.getString("keycloak_email") - def keycloakUsername = userProfileResultSet.getString("keycloak_username") - def email = userProfileResultSet.getString("email") - def title = userProfileResultSet.getString("title") - def phone = userProfileResultSet.getString("phone") - def mobilePhone = userProfileResultSet.getString("mobile_phone") - def firstName = userProfileResultSet.getString("first_name") - def lastName = userProfileResultSet.getString("last_name") - def isActive = userProfileResultSet.getBoolean("is_active") - def organizationId = userProfileResultSet.getObject("organization_id") // Nullable + def userProfileId = userProfileResultSet.getInt('user_profile_id') + def keycloakUserId = userProfileResultSet.getString('keycloak_user_id') + def keycloakEmail = userProfileResultSet.getString('keycloak_email') + def keycloakUsername = userProfileResultSet.getString('keycloak_username') + def email = userProfileResultSet.getString('email') + def title = userProfileResultSet.getString('title') + def phone = userProfileResultSet.getString('phone') + def mobilePhone = userProfileResultSet.getString('mobile_phone') + def firstName = userProfileResultSet.getString('first_name') + def lastName = userProfileResultSet.getString('last_name') + def isActive = userProfileResultSet.getBoolean('is_active') + def organizationId = userProfileResultSet.getObject('organization_id') // Nullable // Bind values to the prepared statement insertUserProfileStmt.setInt(1, userProfileId) @@ -136,25 +202,32 @@ try { PreparedStatement sourceRoleStmt = sourceConn.prepareStatement(userRoleQuery) ResultSet userRoleResultSet = sourceRoleStmt.executeQuery() - // Process the result set for user roles while (userRoleResultSet.next()) { - def userProfileId = userRoleResultSet.getInt("user_profile_id") - def roleName = userRoleResultSet.getString("role_name") - - // Bind values to the prepared statement - insertUserRoleStmt.setInt(1, userProfileId) - insertUserRoleStmt.setString(2, roleName) - - // Execute the insert/update for user roles - insertUserRoleStmt.executeUpdate() + def userProfileId = userRoleResultSet.getInt('user_profile_id') + def rolesString = userRoleResultSet.getString('roles_string') + + if (rolesString) { + def roles = rolesString.split(',') + + roles.each { role -> + try { + insertUserRoleStmt.setInt(1, userProfileId) + insertUserRoleStmt.setString(2, role) + insertUserRoleStmt.executeUpdate() + log.info("Successfully inserted role ${role} for user ${userProfileId}") + } catch (Exception e) { + log.error("Failed to insert role ${role} for user ${userProfileId}: ${e.message}") + } + } + } else { + log.warn("No roles found for user ${userProfileId}") + } } - } catch (Exception e) { - log.error("Error occurred while processing data", e) + log.error('Error occurred during ETL process', e) } finally { - // Close the connections - if (sourceConn != null) sourceConn.close() - if (destinationConn != null) destinationConn.close() + if (sourceConn) sourceConn.close() + if (destinationConn) destinationConn.close() } -log.warn("**** COMPLETED USER ETL ****") +log.warn('**** COMPLETED USER ETL ****') From 6b6217377b79f70e91b11e11678aa7c5bd651cf9 Mon Sep 17 00:00:00 2001 From: prv-proton Date: Fri, 3 Jan 2025 14:20:25 -0800 Subject: [PATCH 5/6] updates --- etl/nifi_scripts/user.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/etl/nifi_scripts/user.groovy b/etl/nifi_scripts/user.groovy index bc8910f11..4c6ce85c2 100644 --- a/etl/nifi_scripts/user.groovy +++ b/etl/nifi_scripts/user.groovy @@ -60,7 +60,7 @@ def userRoleQuery = """ GROUP BY user_profile_id, organization_id ), ProcessedRoles AS ( - SELECT + SELECT user_profile_id, CASE -- Rule 1: Government Users @@ -88,9 +88,9 @@ def userRoleQuery = """ -- Rule 2: Supplier Users WHEN organization_id > 1 THEN CASE - -- Remove government roles and retain Read Only if it exists + -- Return empty array if READ_ONLY exists WHEN 'READ_ONLY' = ANY(roles) THEN - ARRAY['READ_ONLY'] + ARRAY[]::text[] ELSE ARRAY( SELECT UNNEST(roles) EXCEPT From 545615e080b708ca94f68df017abf3e4b060301c Mon Sep 17 00:00:00 2001 From: Daniel Haselhan Date: Fri, 3 Jan 2025 15:43:18 -0800 Subject: [PATCH 6/6] Code Review Feedback Use bright red --- frontend/src/utils/grid/cellRenderers.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/utils/grid/cellRenderers.jsx b/frontend/src/utils/grid/cellRenderers.jsx index 7cbf25016..642ba9fed 100644 --- a/frontend/src/utils/grid/cellRenderers.jsx +++ b/frontend/src/utils/grid/cellRenderers.jsx @@ -74,7 +74,7 @@ export const LoginStatusRenderer = (props) => { >