From afbc6723c0af644566c2a372d156b086df68456a Mon Sep 17 00:00:00 2001 From: Hamed Valiollahi Date: Mon, 6 Jan 2025 08:26:34 -0800 Subject: [PATCH 1/6] feat: refine report history display to show only meaningful entries --- .../components/AssessmentCard.jsx | 45 +++++++++++-------- .../components/HistoryCard.jsx | 6 +++ 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx b/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx index 9c55d4117..2479499c7 100644 --- a/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx +++ b/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx @@ -1,3 +1,4 @@ +import { useMemo } from 'react' import BCButton from '@/components/BCButton' import BCTypography from '@/components/BCTypography' import BCWidgetCard from '@/components/BCWidgetCard/BCWidgetCard' @@ -23,7 +24,7 @@ export const AssessmentCard = ({ currentStatus, complianceReportId, alertRef, - chain + chain = [] }) => { const { t } = useTranslation(['report']) const navigate = useNavigate() @@ -52,6 +53,14 @@ export const AssessmentCard = ({ } }) + const filteredChain = useMemo(() => { + return chain.filter((report) => { + const isDraft = report.currentStatus.status === COMPLIANCE_REPORT_STATUSES.DRAFT + const hasHistory = report.history && report.history.length > 0 + return !isDraft && hasHistory + }) + }, [chain]) + return ( + dangerouslySetInnerHTML={{ + __html: t('report:assessmentLn1', { + name: orgData.name, + hasMet: hasMet ? 'has met' : 'has not met' + }) + }} + /> @@ -129,19 +138,19 @@ export const AssessmentCard = ({ + dangerouslySetInnerHTML={{ + __html: t('report:assessmentLn2', { + name: orgData.name, + hasMet: hasMet ? 'has met' : 'has not met' + }) + }} + /> )} - {!!chain.length && ( + {filteredChain.length > 0 && ( <> {t('report:reportHistory')} - {chain.map((report) => ( + {filteredChain.map((report) => ( ))} @@ -165,7 +174,7 @@ export const AssessmentCard = ({ sx={{ paddingTop: '16px' }} component="div" variant="body4" - > + > {t('report:supplementalWarning')} diff --git a/frontend/src/views/ComplianceReports/components/HistoryCard.jsx b/frontend/src/views/ComplianceReports/components/HistoryCard.jsx index fb8a755a8..5500db4fb 100644 --- a/frontend/src/views/ComplianceReports/components/HistoryCard.jsx +++ b/frontend/src/views/ComplianceReports/components/HistoryCard.jsx @@ -69,6 +69,12 @@ export const HistoryCard = ({ report }) => { }) .filter((item) => item.status.status !== COMPLIANCE_REPORT_STATUSES.DRAFT) }, [isGovernmentUser, report.history]) + + // If there's no content to show, return null + if (filteredHistory.length === 0) { + return null + } + return ( Date: Mon, 6 Jan 2025 09:27:43 -0800 Subject: [PATCH 2/6] refactor: improve the code --- .../components/HistoryCard.jsx | 61 +++++++++---------- 1 file changed, 29 insertions(+), 32 deletions(-) diff --git a/frontend/src/views/ComplianceReports/components/HistoryCard.jsx b/frontend/src/views/ComplianceReports/components/HistoryCard.jsx index 5500db4fb..ebb4203a9 100644 --- a/frontend/src/views/ComplianceReports/components/HistoryCard.jsx +++ b/frontend/src/views/ComplianceReports/components/HistoryCard.jsx @@ -70,11 +70,6 @@ export const HistoryCard = ({ report }) => { .filter((item) => item.status.status !== COMPLIANCE_REPORT_STATUSES.DRAFT) }, [isGovernmentUser, report.history]) - // If there's no content to show, return null - if (filteredHistory.length === 0) { - return null - } - return ( { : {report.currentStatus.status} - - - {filteredHistory.map((item, index) => ( - - - - - - ))} - - + {filteredHistory.length > 0 && ( + + + {filteredHistory.map((item, index) => ( + + + + + + ))} + + + )} ) } From 51156e803415a1e03b61fce15335f9b1d3cf1cd3 Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Mon, 6 Jan 2025 19:30:18 -0800 Subject: [PATCH 3/6] fix: org nickname, user auth field mappings updated --- etl/nifi_scripts/compliance_report.groovy | 5 +- etl/nifi_scripts/user.groovy | 88 +++++++++++++++++++---- 2 files changed, 78 insertions(+), 15 deletions(-) diff --git a/etl/nifi_scripts/compliance_report.groovy b/etl/nifi_scripts/compliance_report.groovy index 18ea947a5..b9e5e8fe2 100644 --- a/etl/nifi_scripts/compliance_report.groovy +++ b/etl/nifi_scripts/compliance_report.groovy @@ -849,6 +849,9 @@ def prepareStatements(Connection conn) { * @return Inserted compliance_report_id. */ def insertComplianceReport(PreparedStatement stmt, Map report, String groupUuid, int version, String supplementalInitiator, String reportingFrequency, Integer currentStatusId, String createUser, String updateUser) { + // Generate nickname based on version + def nickname = (version == 0) ? "Original Report" : "Supplemental Report ${version}" + stmt.setInt(1, report.compliance_period_id) stmt.setInt(2, report.organization_id) @@ -862,7 +865,7 @@ def insertComplianceReport(PreparedStatement stmt, Map report, String groupUuid, stmt.setInt(5, version) stmt.setObject(6, supplementalInitiator) stmt.setString(7, reportingFrequency) - stmt.setString(8, report.nickname) + stmt.setString(8, nickname) stmt.setString(9, report.supplemental_note) stmt.setString(10, createUser) stmt.setTimestamp(11, report.create_timestamp ?: Timestamp.valueOf("1970-01-01 00:00:00")) diff --git a/etl/nifi_scripts/user.groovy b/etl/nifi_scripts/user.groovy index 4c6ce85c2..49128e7af 100644 --- a/etl/nifi_scripts/user.groovy +++ b/etl/nifi_scripts/user.groovy @@ -5,23 +5,83 @@ import groovy.json.JsonSlurper log.warn('**** STARTING USER ETL ****') -// SQL query to extract user profiles def userProfileQuery = """ - SELECT id as user_profile_id, - keycloak_user_id, - COALESCE(NULLIF(email, ''), 'test@gov.bc.ca') as keycloak_email, - username as keycloak_username, - COALESCE(NULLIF(email, ''), 'test@gov.bc.ca') as email, - title, - phone, - cell_phone as mobile_phone, - first_name, - last_name, - is_active, - CASE WHEN organization_id = 1 THEN NULL ELSE organization_id END as organization_id - FROM public.user; + WITH ranked_users AS ( + SELECT + u.id AS user_profile_id, + u.keycloak_user_id, + + -- If external_username is empty or null, make it null for easier handling + CASE WHEN COALESCE(NULLIF(ucr.external_username, ''), NULL) IS NULL THEN NULL + ELSE ucr.external_username + END AS raw_external_username, + + -- Use a window function to identify duplicates within each external_username group + ROW_NUMBER() OVER ( + PARTITION BY COALESCE(NULLIF(ucr.external_username, ''), '___EMPTY___') + ORDER BY + CASE WHEN u.keycloak_user_id IS NOT NULL THEN 0 ELSE 1 END, + u.id + ) AS occurrence, + + COALESCE(NULLIF(ucr.keycloak_email, ''), u.email) AS keycloak_email, + COALESCE(NULLIF(u.email, ''), '') AS email, + u.title, + u.phone, + u.cell_phone AS mobile_phone, + u.first_name, + u.last_name, + u.is_active, + CASE WHEN u.organization_id = 1 THEN NULL ELSE u.organization_id END AS organization_id + FROM public.user u + LEFT JOIN user_creation_request ucr ON ucr.user_id = u.id + ), + resolved_users AS ( + SELECT + user_profile_id, + keycloak_user_id, + + CASE + -- 1) No external_username => "FIXME" + WHEN raw_external_username IS NULL THEN + CONCAT('FIXME', occurrence) + + -- 2) Duplicate external_username => add "_" + WHEN occurrence > 1 THEN + CONCAT(raw_external_username, '_', occurrence) + + -- 3) Unique or first occurrence => use raw_external_username + ELSE raw_external_username + END AS keycloak_username, + + keycloak_email, + email, + title, + phone, + mobile_phone, + first_name, + last_name, + is_active, + organization_id + FROM ranked_users + ) + SELECT + user_profile_id, + keycloak_user_id, + keycloak_username, + keycloak_email, + email, + title, + phone, + mobile_phone, + first_name, + last_name, + is_active, + organization_id + FROM resolved_users; """ + // SQL query to extract user roles def userRoleQuery = """ WITH RoleData AS ( From 116bee8e429527c8ec0b193b2dec054c732591f5 Mon Sep 17 00:00:00 2001 From: Hamed Valiollahi Date: Tue, 7 Jan 2025 11:37:14 -0800 Subject: [PATCH 4/6] fix: address null error when saving admin user roles in production --- backend/lcfs/web/api/user/repo.py | 55 +++++++++++++++++++------------ 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/backend/lcfs/web/api/user/repo.py b/backend/lcfs/web/api/user/repo.py index f55e0eab4..4035ebd99 100644 --- a/backend/lcfs/web/api/user/repo.py +++ b/backend/lcfs/web/api/user/repo.py @@ -352,31 +352,46 @@ async def create_user( async def update_user( self, user: UserProfile, user_update: UserCreateSchema ) -> None: + """ + Update an existing UserProfile with new data. + """ + + # Extract incoming data from the Pydantic schema user_data = user_update.model_dump() - updated_user_profile = UserProfile(**user_update.model_dump(exclude={"roles"})) - roles = user_data.pop("roles", {}) + new_roles = user_data.pop("roles", {}) + + # Update basic fields directly + user.email = user_update.email + user.title = user_update.title + user.first_name = user_update.first_name + user.last_name = user_update.last_name + user.is_active = user_update.is_active + user.keycloak_email = user_update.keycloak_email + user.keycloak_username = user_update.keycloak_username + user.phone = user_update.phone + user.mobile_phone = user_update.mobile_phone + # Find the RoleEnum member corresponding to each role - new_roles = [ - role_enum for role_enum in RoleEnum if role_enum.value.lower() in roles - ] + new_role_enums = [] + for role_str in new_roles: + try: + name_str = role_str.title() + role_enum = RoleEnum(name_str) + new_role_enums.append(role_enum) + except ValueError: + pass + # Create a set for faster membership checks existing_roles_set = set(user.role_names) - # Update the user object with the new data - user.email = updated_user_profile.email - user.title = updated_user_profile.title - user.first_name = updated_user_profile.first_name - user.last_name = updated_user_profile.last_name - user.is_active = updated_user_profile.is_active - user.keycloak_email = updated_user_profile.keycloak_email - user.keycloak_username = updated_user_profile.keycloak_username - user.phone = updated_user_profile.phone - user.mobile_phone = updated_user_profile.mobile_phone - if user.organization: - await self.update_bceid_roles(user, new_roles, existing_roles_set) + # BCEID logic + await self.update_bceid_roles(user, new_role_enums, existing_roles_set) else: - await self.update_idir_roles(user, new_roles, existing_roles_set) + # IDIR logic + await self.update_idir_roles(user, new_role_enums, existing_roles_set) + + # Add the updated user to the session self.db.add(user) return user @@ -669,9 +684,7 @@ async def create_login_history(self, user: UserProfile): self.db.add(login_history) @repo_handler - async def update_email( - self, user_profile_id: int, email: str - ) -> UserProfile: + async def update_email(self, user_profile_id: int, email: str) -> UserProfile: # Fetch the user profile query = select(UserProfile).where( UserProfile.user_profile_id == user_profile_id From 4dbcc888743f3d85856b2ad1fb8fbf4775b4d2d0 Mon Sep 17 00:00:00 2001 From: Daniel Haselhan Date: Wed, 8 Jan 2025 15:37:05 -0800 Subject: [PATCH 5/6] fix: FE Cache Fixes FE dockerfile was copying node_modules AFTER running NPM install meaning it used local node_modules over its own. Add a .dockerignore to prevent this. Use explicit copy instead of copy * to easier track and see layer changes --- frontend/.dockerignore | 3 +++ frontend/Dockerfile.dev | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 frontend/.dockerignore diff --git a/frontend/.dockerignore b/frontend/.dockerignore new file mode 100644 index 000000000..32dade485 --- /dev/null +++ b/frontend/.dockerignore @@ -0,0 +1,3 @@ +node_modules +.git +.DS_Store \ No newline at end of file diff --git a/frontend/Dockerfile.dev b/frontend/Dockerfile.dev index 913618d8e..d6004197b 100644 --- a/frontend/Dockerfile.dev +++ b/frontend/Dockerfile.dev @@ -5,10 +5,11 @@ FROM node:20 WORKDIR /app # Copy package files and install dependencies -COPY package*.json ./ +COPY package.json ./ +COPY package-lock.json ./ RUN npm install -# Copy other source files +# Copy other source files - node modules is excluded in .dockerignore COPY . . # Expose port 3000 for the app From e7b8b08a7b5a6ba9fd2be59eb17993e7ddf76468 Mon Sep 17 00:00:00 2001 From: Hamed Valiollahi Date: Thu, 9 Jan 2025 08:54:27 -0800 Subject: [PATCH 6/6] refactor: improved the code --- .../components/AssessmentCard.jsx | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx b/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx index 2479499c7..f790ec833 100644 --- a/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx +++ b/frontend/src/views/ComplianceReports/components/AssessmentCard.jsx @@ -24,7 +24,7 @@ export const AssessmentCard = ({ currentStatus, complianceReportId, alertRef, - chain = [] + chain }) => { const { t } = useTranslation(['report']) const navigate = useNavigate() @@ -55,9 +55,8 @@ export const AssessmentCard = ({ const filteredChain = useMemo(() => { return chain.filter((report) => { - const isDraft = report.currentStatus.status === COMPLIANCE_REPORT_STATUSES.DRAFT const hasHistory = report.history && report.history.length > 0 - return !isDraft && hasHistory + return hasHistory }) }, [chain]) @@ -117,13 +116,13 @@ export const AssessmentCard = ({ + dangerouslySetInnerHTML={{ + __html: t('report:assessmentLn1', { + name: orgData.name, + hasMet: hasMet ? 'has met' : 'has not met' + }) + }} + /> @@ -138,33 +137,34 @@ export const AssessmentCard = ({ + dangerouslySetInnerHTML={{ + __html: t('report:assessmentLn2', { + name: orgData.name, + hasMet: hasMet ? 'has met' : 'has not met' + }) + }} + /> )} - {filteredChain.length > 0 && ( - <> - - {t('report:reportHistory')} - - {filteredChain.map((report) => ( - - ))} - - )} + {filteredChain.length > 0 && + currentStatus !== COMPLIANCE_REPORT_STATUSES.DRAFT && ( + <> + + {t('report:reportHistory')} + + {filteredChain.map((report) => ( + + ))} + + )} {isFeatureEnabled(FEATURE_FLAGS.SUPPLEMENTAL_REPORTING) && @@ -174,7 +174,7 @@ export const AssessmentCard = ({ sx={{ paddingTop: '16px' }} component="div" variant="body4" - > + > {t('report:supplementalWarning')}