From ade98452307775239a396791023b4b853ea770db Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Mon, 30 Dec 2024 14:22:42 -0800 Subject: [PATCH 1/8] fix: added missing database function and update counts --- .../versions/2024-12-30-13-54_9329e38396e1.py | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py diff --git a/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py b/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py new file mode 100644 index 000000000..b1165d656 --- /dev/null +++ b/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py @@ -0,0 +1,84 @@ +"""add update_count_transfers_in_progress db function and update existing counts + +Revision ID: 9329e38396e1 +Revises: 5fbcb508c1be +Create Date: 2024-12-30 13:54:09.361644 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "9329e38396e1" +down_revision = "5fbcb508c1be" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + # Create or replace the trigger function without considering Draft transfers + op.execute( + """ + CREATE OR REPLACE FUNCTION update_count_transfers_in_progress() + RETURNS TRIGGER AS $$ + BEGIN + -- Update count_transfers_in_progress for from_organization_id and to_organization_id + UPDATE organization o + SET count_transfers_in_progress = ( + SELECT COUNT(DISTINCT t.transfer_id) + FROM transfer t + WHERE + t.current_status_id IN (3, 4) + AND (t.from_organization_id = o.organization_id OR t.to_organization_id = o.organization_id) + ) + WHERE o.organization_id = COALESCE(NEW.from_organization_id, OLD.from_organization_id) + OR o.organization_id = COALESCE(NEW.to_organization_id, OLD.to_organization_id); + + RETURN NEW; + END; + $$ LANGUAGE plpgsql; + """ + ) + + # Create the trigger + op.execute( + """ + CREATE TRIGGER update_count_transfers_in_progress_trigger + AFTER INSERT OR UPDATE OR DELETE ON transfer + FOR EACH ROW + EXECUTE FUNCTION update_count_transfers_in_progress(); + """ + ) + + # Update existing counts for all organizations by aggregating both sent and received transfers without double-counting + op.execute( + """ + UPDATE organization o + SET count_transfers_in_progress = COALESCE(sub.total_transfer_count, 0) + FROM ( + SELECT + org.organization_id, + COUNT(DISTINCT t.transfer_id) AS total_transfer_count + FROM + organization org + LEFT JOIN transfer t ON org.organization_id = t.from_organization_id + OR org.organization_id = t.to_organization_id + WHERE + t.current_status_id IN (3, 4) + GROUP BY + org.organization_id + ) sub + WHERE + o.organization_id = sub.organization_id; + """ + ) + + +def downgrade() -> None: + # Drop the trigger + op.execute( + "DROP TRIGGER IF EXISTS update_count_transfers_in_progress_trigger ON transfer;" + ) + # Drop the trigger function + op.execute("DROP FUNCTION IF EXISTS update_count_transfers_in_progress();") From 3265b7cef95ab0be618ab0c89cdf437e2775ccba Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Mon, 30 Dec 2024 16:32:59 -0800 Subject: [PATCH 2/8] 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 31d637835059dcfa4ba344262534a4873390b9a6 Mon Sep 17 00:00:00 2001 From: Daniel Haselhan Date: Thu, 2 Jan 2025 10:57:54 -0800 Subject: [PATCH 3/8] fix: Add autofocus to Number and Text Editors * Auto focus when it renders to require 1 less click * Similar functionality already implemented in Autocomplete editor. --- .../BCDataGrid/components/Editors/NumberEditor.jsx | 10 ++++++++-- .../BCDataGrid/components/Editors/TextCellEditor.jsx | 12 +++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/BCDataGrid/components/Editors/NumberEditor.jsx b/frontend/src/components/BCDataGrid/components/Editors/NumberEditor.jsx index bbfc263d5..54bc8e396 100644 --- a/frontend/src/components/BCDataGrid/components/Editors/NumberEditor.jsx +++ b/frontend/src/components/BCDataGrid/components/Editors/NumberEditor.jsx @@ -1,4 +1,4 @@ -import { forwardRef, useImperativeHandle, useRef, useState } from 'react' +import { forwardRef, useEffect, useImperativeHandle, useRef } from 'react' import { TextField } from '@mui/material' import { styled } from '@mui/material/styles' @@ -24,6 +24,12 @@ export const NumberEditor = forwardRef( ({ value, onValueChange, eventKey, rowIndex, column, ...props }, ref) => { const inputRef = useRef(null) + useEffect(() => { + if (inputRef) { + inputRef.current.focus() + } + }, []) + useImperativeHandle(ref, () => { return { getValue() { @@ -58,7 +64,7 @@ export const NumberEditor = forwardRef( return ( { onValueChange(event.target.value) } + + const inputRef = useRef(null) + + useEffect(() => { + if (inputRef) { + inputRef.current.focus() + } + }, []) + return ( <> {() => ( Date: Tue, 31 Dec 2024 07:28:17 -0800 Subject: [PATCH 4/8] fix: resolve 404 error. --- .../audit_log/test_audit_log_services.py | 12 +++++++--- .../test_compliance_report_services.py | 22 +++++++++++++++---- backend/lcfs/web/api/audit_log/services.py | 3 --- .../web/api/compliance_report/services.py | 5 +---- backend/lcfs/web/api/notification/repo.py | 9 +++----- backend/lcfs/web/api/organization/services.py | 5 +---- .../lcfs/web/api/organizations/services.py | 3 --- backend/lcfs/web/api/transaction/services.py | 3 --- 8 files changed, 32 insertions(+), 30 deletions(-) diff --git a/backend/lcfs/tests/audit_log/test_audit_log_services.py b/backend/lcfs/tests/audit_log/test_audit_log_services.py index 92dbee85a..a5e545bc4 100644 --- a/backend/lcfs/tests/audit_log/test_audit_log_services.py +++ b/backend/lcfs/tests/audit_log/test_audit_log_services.py @@ -72,9 +72,15 @@ async def test_get_audit_logs_paginated_no_data(audit_log_service, mock_repo): pagination = PaginationRequestSchema(page=1, size=10, filters=[], sort_orders=[]) mock_repo.get_audit_logs_paginated.return_value = ([], 0) - # Act & Assert - with pytest.raises(DataNotFoundException): - await audit_log_service.get_audit_logs_paginated(pagination) + # Act + result = await audit_log_service.get_audit_logs_paginated(pagination) + + # Assert + assert result.audit_logs == [], "Should return an empty list of audit logs" + assert result.pagination.total == 0, "Total should be zero if no records" + assert result.pagination.page == pagination.page + assert result.pagination.size == pagination.size + assert result.pagination.total_pages == 0 @pytest.mark.anyio diff --git a/backend/lcfs/tests/compliance_report/test_compliance_report_services.py b/backend/lcfs/tests/compliance_report/test_compliance_report_services.py index 9300d2918..2c39bbe2d 100644 --- a/backend/lcfs/tests/compliance_report/test_compliance_report_services.py +++ b/backend/lcfs/tests/compliance_report/test_compliance_report_services.py @@ -116,16 +116,30 @@ async def test_get_compliance_reports_paginated_success( async def test_get_compliance_reports_paginated_not_found( compliance_report_service, mock_repo ): + # Arrange pagination_mock = AsyncMock() pagination_mock.page = 1 pagination_mock.size = 10 + pagination_mock.filters = [] + pagination_mock.sort_orders = [] + # Mock the repository to return no records mock_repo.get_reports_paginated.return_value = ([], 0) - with pytest.raises(DataNotFoundException): - await compliance_report_service.get_compliance_reports_paginated( - pagination_mock - ) + # Act + result = await compliance_report_service.get_compliance_reports_paginated( + pagination_mock + ) + + # Assert: Verify the service returns an empty list and correct pagination metadata + assert result.reports == [], "Expected no compliance reports to be returned" + assert result.pagination.total == 0, "Expected total=0 when there are no records" + assert result.pagination.page == 1, "Page should match the requested page" + assert result.pagination.size == 10, "Size should match the requested size" + assert result.pagination.total_pages == 0, "0 records should yield 0 total_pages" + + # Also verify our repo was called exactly once + mock_repo.get_reports_paginated.assert_called_once() @pytest.mark.anyio diff --git a/backend/lcfs/web/api/audit_log/services.py b/backend/lcfs/web/api/audit_log/services.py index 8a1a81529..d3ff1347a 100644 --- a/backend/lcfs/web/api/audit_log/services.py +++ b/backend/lcfs/web/api/audit_log/services.py @@ -76,9 +76,6 @@ async def get_audit_logs_paginated( offset, limit, conditions, pagination.sort_orders ) - if not audit_logs: - raise DataNotFoundException("No audit logs found") - processed_audit_logs = [] for audit_log in audit_logs: # Extract the changed_fields as a comma-separated string diff --git a/backend/lcfs/web/api/compliance_report/services.py b/backend/lcfs/web/api/compliance_report/services.py index 880a94be7..bc3588f97 100644 --- a/backend/lcfs/web/api/compliance_report/services.py +++ b/backend/lcfs/web/api/compliance_report/services.py @@ -171,10 +171,7 @@ async def get_compliance_reports_paginated( pagination, organization_id ) - if not reports: - raise DataNotFoundException("No compliance reports found.") - - if bceid_user: + if bceid_user and reports: reports = self._mask_report_status(reports) return ComplianceReportListSchema( diff --git a/backend/lcfs/web/api/notification/repo.py b/backend/lcfs/web/api/notification/repo.py index bd9d874fa..10b238604 100644 --- a/backend/lcfs/web/api/notification/repo.py +++ b/backend/lcfs/web/api/notification/repo.py @@ -124,7 +124,9 @@ def _apply_notification_filters( ) ) elif filter.field == "transaction_id": - field = get_field_for_filter(NotificationMessage, 'related_transaction_id') + field = get_field_for_filter( + NotificationMessage, "related_transaction_id" + ) conditions.append( apply_filter_conditions( field, filter_value, filter_option, filter_type @@ -353,11 +355,6 @@ async def get_notification_channel_subscriptions_by_user( result = await self.db.execute(query) subscriptions = result.scalars().all() - if not subscriptions: - raise DataNotFoundException( - f"Channel subscriptions not found for user id: '{user_profile_id}'" - ) - return subscriptions @repo_handler diff --git a/backend/lcfs/web/api/organization/services.py b/backend/lcfs/web/api/organization/services.py index 5ebf3573d..9371c6347 100644 --- a/backend/lcfs/web/api/organization/services.py +++ b/backend/lcfs/web/api/organization/services.py @@ -102,7 +102,7 @@ def apply_transaction_filters(self, pagination, conditions): # For non-date filters, use the standard filter value filter_value = filter.filter - if field.description == 'transaction_type': + if field.description == "transaction_type": filter_value = filter_value.replace(" ", "").lower() filter_option = filter.type @@ -169,9 +169,6 @@ async def get_transactions_paginated( ) ) - if not transactions: - raise DataNotFoundException("Transactions not found") - return { "transactions": [ TransactionViewSchema.model_validate(transaction) diff --git a/backend/lcfs/web/api/organizations/services.py b/backend/lcfs/web/api/organizations/services.py index 35c2155a3..5014760d5 100644 --- a/backend/lcfs/web/api/organizations/services.py +++ b/backend/lcfs/web/api/organizations/services.py @@ -258,9 +258,6 @@ async def get_organizations( offset, limit, conditions, pagination ) - if not organizations: - raise DataNotFoundException("Organizations not found") - return OrganizationListSchema( organizations=organizations, pagination=PaginationResponseSchema( diff --git a/backend/lcfs/web/api/transaction/services.py b/backend/lcfs/web/api/transaction/services.py index 9c2bf0011..1abdfc2f0 100644 --- a/backend/lcfs/web/api/transaction/services.py +++ b/backend/lcfs/web/api/transaction/services.py @@ -121,9 +121,6 @@ async def get_transactions_paginated( offset, limit, conditions, pagination.sort_orders, None ) - if not transactions: - raise DataNotFoundException("Transactions not found") - return { "transactions": [ TransactionViewSchema.model_validate(transaction) From a641c97918dae454cb50fa24492d23fdb6d85feb Mon Sep 17 00:00:00 2001 From: Kevin Hashimoto Date: Mon, 30 Dec 2024 16:23:09 -0800 Subject: [PATCH 5/8] fix: grid width for user activity --- frontend/src/views/Admin/AdminMenu/components/UserActivity.jsx | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/views/Admin/AdminMenu/components/UserActivity.jsx b/frontend/src/views/Admin/AdminMenu/components/UserActivity.jsx index d2bf96cfb..532214e8e 100644 --- a/frontend/src/views/Admin/AdminMenu/components/UserActivity.jsx +++ b/frontend/src/views/Admin/AdminMenu/components/UserActivity.jsx @@ -60,6 +60,7 @@ export const UserActivity = () => { getRowId={getRowId} overlayNoRowsTemplate={t('admin:activitiesNotFound')} autoSizeStrategy={{ + type: 'fitGridWidth', defaultMinWidth: 50, defaultMaxWidth: 600 }} From 396c402e7e5c919a2513fd4b094d7c46c93ac7e5 Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Fri, 3 Jan 2025 11:51:40 -0800 Subject: [PATCH 6/8] 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 97b778849a553a02fdc5ba9fee302bf37e07fde0 Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Fri, 3 Jan 2025 14:13:50 -0800 Subject: [PATCH 7/8] fix: migration order --- .../db/migrations/versions/2024-12-30-13-54_9329e38396e1.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py b/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py index b1165d656..561e1460e 100644 --- a/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py +++ b/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py @@ -1,7 +1,7 @@ """add update_count_transfers_in_progress db function and update existing counts Revision ID: 9329e38396e1 -Revises: 5fbcb508c1be +Revises: ab04810d4d7c Create Date: 2024-12-30 13:54:09.361644 """ @@ -11,7 +11,7 @@ # revision identifiers, used by Alembic. revision = "9329e38396e1" -down_revision = "5fbcb508c1be" +down_revision = "ab04810d4d7c" branch_labels = None depends_on = None From 57fa7fe5946957f0fb941d06446bf41dbed977b0 Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Fri, 3 Jan 2025 14:39:02 -0800 Subject: [PATCH 8/8] fix: migration order --- .../db/migrations/versions/2024-12-30-13-54_9329e38396e1.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py b/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py index 561e1460e..6d00c6db8 100644 --- a/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py +++ b/backend/lcfs/db/migrations/versions/2024-12-30-13-54_9329e38396e1.py @@ -1,7 +1,7 @@ """add update_count_transfers_in_progress db function and update existing counts Revision ID: 9329e38396e1 -Revises: ab04810d4d7c +Revises: d9cdd9fca0ce Create Date: 2024-12-30 13:54:09.361644 """ @@ -11,7 +11,7 @@ # revision identifiers, used by Alembic. revision = "9329e38396e1" -down_revision = "ab04810d4d7c" +down_revision = "d9cdd9fca0ce" branch_labels = None depends_on = None