From de195650ea1f9fe3dc245125209140b226bd17bc Mon Sep 17 00:00:00 2001 From: jadmsaadaot Date: Tue, 11 Feb 2025 07:38:04 -0800 Subject: [PATCH] Fix submission history logging (#288) * Log package updated submission, failed and approved CR * log management plan fail * Invalidate submission history cache when package is updated * remove unused imports --- .../src/submit_api/enums/activity_type.py | 2 + .../src/submit_api/models/activity_log.py | 20 ++++++++ .../services/activity_log_service.py | 8 +--- .../services/consultation_record_service.py | 37 +++++++-------- .../services/management_plan_service.py | 14 ++++++ submit-api/src/submit_api/services/package.py | 46 +++++++++++-------- .../submit_api/services/submission_review.py | 29 +++++------- .../SubmissionHistory/HistoryTable.tsx | 2 +- submit-web/src/hooks/api/useActivtyLog.ts | 4 +- submit-web/src/hooks/api/useItems.ts | 8 ++++ submit-web/src/hooks/api/usePackages.ts | 8 ++++ 11 files changed, 110 insertions(+), 68 deletions(-) diff --git a/submit-api/src/submit_api/enums/activity_type.py b/submit-api/src/submit_api/enums/activity_type.py index 056aeb16..fa5130a4 100644 --- a/submit-api/src/submit_api/enums/activity_type.py +++ b/submit-api/src/submit_api/enums/activity_type.py @@ -33,6 +33,7 @@ class ActivityActionType(enum.Enum): """Enum for activity type statuses.""" ORIGINAL_SUBMISSION = "Original Submission" + UPDATED_SUBMISSION = "Updated Submission" START_CONSULTATION_CHECK = "Start Consultation Check" UPDATE_REQUESTED = "Update Requested" PASSED_CONSULTATION_CHECK = "Passed Consultation Check" @@ -43,3 +44,4 @@ class ActivityActionType(enum.Enum): MP_SATISFIED = "MP Satisfied" MP_REVIEW_REJECTED = "MP Review Rejected" REVISION_REQUIRED = "Revision Required" + MP_REVIEW_FAILED = "MP Review Failed" diff --git a/submit-api/src/submit_api/models/activity_log.py b/submit-api/src/submit_api/models/activity_log.py index d082d17e..2de1e242 100644 --- a/submit-api/src/submit_api/models/activity_log.py +++ b/submit-api/src/submit_api/models/activity_log.py @@ -10,6 +10,7 @@ from .base_model import BaseModel from .db import db +from ..enums.activity_type import VisibilityTypeEnum class ActivityLog(BaseModel): @@ -39,3 +40,22 @@ def to_dict(self): "actor_type": self.actor_type, "visibility": self.visibility } + + @classmethod + def get_activity_logs(cls, entity_type: str, entity_id: int, for_staff: bool = True) -> list: + """ + Retrieve activity logs for a specific entity type and ID. + + - `entity_type`: The type of entity (e.g., 'submission', 'file_upload'). + - `entity_id`: The specific entity ID. + - `for_staff`: If False, only public logs are returned. + + Returns a list of logs. + """ + query = cls.query.filter_by(entity_type=entity_type, entity_id=entity_id) + + if not for_staff: + query = query.filter(cls.visibility == VisibilityTypeEnum.PUBLIC.value) + + logs = query.order_by(cls.activity_at.asc()).all() + return logs diff --git a/submit-api/src/submit_api/services/activity_log_service.py b/submit-api/src/submit_api/services/activity_log_service.py index f76daeb6..23b4df5e 100644 --- a/submit-api/src/submit_api/services/activity_log_service.py +++ b/submit-api/src/submit_api/services/activity_log_service.py @@ -65,12 +65,7 @@ def get_activity_logs(entity_type: str, entity_id: int, for_staff: bool = True) Returns a list of logs. """ - query = ActivityLog.query.filter_by(entity_type=entity_type, entity_id=entity_id) - - if not for_staff: - query = query.filter(ActivityLog.visibility == VisibilityTypeEnum.PUBLIC.value) - - logs = query.order_by(ActivityLog.activity_at.desc()).all() + logs = ActivityLog.get_activity_logs(entity_type, entity_id, for_staff) return logs @staticmethod @@ -78,7 +73,6 @@ def _create_activity_log_object( # pylint: disable=too-many-arguments entity_type, entity_id, entity_version, action, actor_id, actor_type, visibility ) -> ActivityLog: """Creates an ActivityLog object without adding it to a session.""" - print(actor_id) return ActivityLog( entity_type=entity_type, entity_id=entity_id, diff --git a/submit-api/src/submit_api/services/consultation_record_service.py b/submit-api/src/submit_api/services/consultation_record_service.py index 493db5d6..4ec93967 100644 --- a/submit-api/src/submit_api/services/consultation_record_service.py +++ b/submit-api/src/submit_api/services/consultation_record_service.py @@ -5,7 +5,7 @@ from submit_api.enums.activity_type import ActivityActionType from submit_api.enums.item_status import ItemStatus -from submit_api.models import UpdateRequest +from submit_api.models import UpdateRequest, Package from submit_api.models import PackageMetadata, SubmissionReviewEntry from submit_api.models.package_metadata import PackageMetadataFields from submit_api.models.submission import SubmissionStatus @@ -36,7 +36,14 @@ def approve_consultation_record(cls, item, session): } session.add(item) session.add(package_metadata) - cls._log_activity_consultation_check(item, session, success=True) + package = Package.find_by_id(item.package_id) + ActivityLogService.log_activity( + entity_id=package.id, + action=ActivityActionType.PASSED_CONSULTATION_CHECK.value, + entity_version=package.version.version, + actor_id=TokenInfo.get_id(), + session=session + ) current_app.logger.info(f"Consultation record approved for item {item.id}.") current_app.logger.info(f"Starting MP review for package {item.package_id}.") @@ -45,30 +52,20 @@ def approve_consultation_record(cls, item, session): return item - @staticmethod - def _log_activity_consultation_check(item, session, success=True): - """Log activity for passing or failing the consultation check.""" - action_type = ( - ActivityActionType.PASSED_CONSULTATION_CHECK.value - if success else - ActivityActionType.FAILED_CONSULTATION_CHECK.value - ) - - ActivityLogService.log_activity( - entity_id=item.id, - action=action_type, - entity_version=item.package.version.version, - actor_id=TokenInfo.get_id(), - session=session - ) - @classmethod def reject_consultation_record(cls, item, session): """Reject consultation record.""" cls._update_submissions_status(item, SubmissionStatus.REJECTED, session) update_request_data = cls._prepare_update_request_data(item) cls._create_update_request(update_request_data, session) - cls._log_activity_consultation_check(item, session, success=False) + package = Package.find_by_id(item.package_id) + ActivityLogService.log_activity( + entity_id=package.id, + action=ActivityActionType.FAILED_CONSULTATION_CHECK.value, + entity_version=package.version.version, + actor_id=TokenInfo.get_id(), + session=session + ) session.flush() return item diff --git a/submit-api/src/submit_api/services/management_plan_service.py b/submit-api/src/submit_api/services/management_plan_service.py index 0c65eefc..d112116c 100644 --- a/submit-api/src/submit_api/services/management_plan_service.py +++ b/submit-api/src/submit_api/services/management_plan_service.py @@ -37,9 +37,23 @@ def reject_management_plan_form(cls, item, session): 'type': UpdateRequestType.REVIEW, } cls._create_mp_update_request(update_request_data, session) + cls._log_management_plan_rejection_activity(item, session) current_app.logger.info(f"Management plan form rejected for item {item.id}.") return item + @classmethod + def _log_management_plan_rejection_activity(cls, item, session): + """Log activity for management plan rejection.""" + current_app.logger.info(f"Logging activity for management plan rejection for package {item.package_id}.") + package = PackageModel.find_by_id(item.package_id) + ActivityLogService.log_activity( + entity_id=package.id, + action=ActivityActionType.MP_REVIEW_FAILED.value, + entity_version=package.version.version, + session=session + ) + current_app.logger.info(f"Activity logged for management plan rejection for package {package.id}.") + @classmethod def _update_item_status_mp_rejection(cls, item): """Update the status and review date of the item for rejection.""" diff --git a/submit-api/src/submit_api/services/package.py b/submit-api/src/submit_api/services/package.py index 278fbebd..03a3eb30 100644 --- a/submit-api/src/submit_api/services/package.py +++ b/submit-api/src/submit_api/services/package.py @@ -232,13 +232,11 @@ def _update_items_status(items, status, session): session.add(item) @staticmethod - def _update_mp_item(items, data, session): + def _update_mp_item(mp_item, data, session): """Update the status of all items in the package.""" - for item in items: - if item.type.name == SubmissionItemType.MANAGEMENT_PLAN_FORM.value: - item.status = data.get('status') - item.review_start_date = data.get('review_start_date') - session.add(item) + mp_item.status = data.get('status') + mp_item.review_start_date = data.get('review_start_date') + session.add(mp_item) @staticmethod def _update_cr_status(items, data, session): @@ -312,21 +310,8 @@ def submit_package(cls, package_id): else: submitted_package: PackageModel = cls._submit_package(package, session) - cls._log_activity_submission(package, submitted_package, session) - return submitted_package - @staticmethod - def _log_activity_submission(package, submitted_package, session): - """Log activity for package submission.""" - ActivityLogService.log_activity( - entity_id=package.id, - action=ActivityActionType.ORIGINAL_SUBMISSION.value, - actor_type=ActorTypeEnum.ENTITY.value, - entity_version=submitted_package.version.version, - session=session - ) - @classmethod def _submit_package(cls, package, session): """Submit the package by updating its status and items.""" @@ -337,6 +322,7 @@ def _submit_package(cls, package, session): cls.update_submission_status(package, ItemStatus.SUBMITTED.value, session) cls._deactivate_revision_required_requests(package, session) cls._create_email_queue_record(package, session) + cls._log_activity_submission(package, ActivityActionType.ORIGINAL_SUBMISSION.value, session) return package @classmethod @@ -352,8 +338,20 @@ def _resubmit_package(cls, package, session): cls._deactivate_revision_required_requests(package, session) cls._update_update_requests(session, package, status=UpdateRequestStatus.PENDING_REVIEW.value) cls._deactivate_reviews(package, session) + cls._log_activity_submission(package, ActivityActionType.UPDATED_SUBMISSION.value, session) return package + @staticmethod + def _log_activity_submission(package, action, session): + """Log activity for package submission.""" + ActivityLogService.log_activity( + entity_id=package.id, + action=action, + actor_type=ActorTypeEnum.ENTITY.value, + entity_version=package.version.version, + session=session + ) + @classmethod def _deactivate_reviews(cls, package, session): """Deactivate all reviews for the package.""" @@ -379,11 +377,19 @@ def start_mp_review(cls, package_id, _session=None): @classmethod def start_mp_review_process(cls, package, package_id, session): """Common logic for starting the review process.""" + mp_item = next((item for item in package.items + if item.type.name == SubmissionItemType.MANAGEMENT_PLAN_FORM.value), None) + if not mp_item: + current_app.logger.info(f"Management plan form not found in package {package_id}") + raise BadRequestError("Management plan form not found in package") + if mp_item.status == ItemStatus.UNDER_REVIEW: + current_app.logger.info(f"Management plan form {mp_item.id} is already under review") + return item_data = { 'status': ItemStatus.UNDER_REVIEW.value, 'review_start_date': datetime.utcnow().isoformat() } - cls._update_mp_item(package.items, item_data, session) + cls._update_mp_item(mp_item, item_data, session) cls._update_package_status(package_id, session, package) new_metadata = { PackageMetadataFields.REVIEW_START_DATE.value: item_data.get('review_start_date') diff --git a/submit-api/src/submit_api/services/submission_review.py b/submit-api/src/submit_api/services/submission_review.py index 533a524e..aa32d444 100644 --- a/submit-api/src/submit_api/services/submission_review.py +++ b/submit-api/src/submit_api/services/submission_review.py @@ -3,7 +3,6 @@ from flask import current_app -from submit_api.enums.activity_type import ActivityActionType from submit_api.exceptions import UnprocessableEntityError, ResourceNotFoundError from submit_api.models import Item as ItemModel from submit_api.models import Package as PackageModel @@ -170,26 +169,9 @@ def approve_submission(cls, item_id, session): approval_processor(item, session) cls._update_package_status(item.package_id, session) cls._update_item_submissions_status(SubmissionStatus.APPROVED, session, item=item) - current_app.logger.info(f"Submission item {item.id} approved.") return item - @staticmethod - def _log_activity_mp_review(item, session, approved=True): - """Log activity for approving or rejecting the Management Plan review.""" - action_type = ( - ActivityActionType.MP_APPROVED.value - if approved else - ActivityActionType.MP_REVIEW_REJECTED.value - ) - - ActivityLogService.log_activity( - entity_id=item.id, - action=action_type, - entity_version=item.package.version_id, - session=session - ) - @classmethod def reject_submission(cls, item_id, session): """Reject submission item.""" @@ -214,3 +196,14 @@ def _get_submission_item_rejection_processor(cls, item: ItemModel) -> callable: ) current_app.logger.debug(f"Rejection processor retrieved for item {item.id} of type {item_type}") return status_processor_map[item_type] + + @staticmethod + def _log_activity_mp_review(item, action, session): + """Log activity for approving or rejecting the Management Plan review.""" + package = PackageModel.find_by_id(item.package_id) + ActivityLogService.log_activity( + entity_id=package.id, + action=action, + entity_version=package.version.version, + session=session + ) diff --git a/submit-web/src/components/Submission/InfoBox/SubmissionHistory/HistoryTable.tsx b/submit-web/src/components/Submission/InfoBox/SubmissionHistory/HistoryTable.tsx index cdc0a6ae..23963492 100644 --- a/submit-web/src/components/Submission/InfoBox/SubmissionHistory/HistoryTable.tsx +++ b/submit-web/src/components/Submission/InfoBox/SubmissionHistory/HistoryTable.tsx @@ -92,7 +92,7 @@ type HistoryTableProps = { export const HistoryTable = ({ packageId }: HistoryTableProps) => { const { data: activityLogs, isPending: isLoading } = useGetAcitivityLogForAdmin({ - id: packageId, + id: Number(packageId), entityType: ACTIVITY_LOG_ENTITY_TYPE.PACKAGE, }); diff --git a/submit-web/src/hooks/api/useActivtyLog.ts b/submit-web/src/hooks/api/useActivtyLog.ts index b0d511b4..0a79e7d5 100644 --- a/submit-web/src/hooks/api/useActivtyLog.ts +++ b/submit-web/src/hooks/api/useActivtyLog.ts @@ -4,7 +4,7 @@ import { queryOptions, useQuery } from "@tanstack/react-query"; import { QUERY_KEY } from "./constants"; type GetAcitivityLogForAdminByIdParams = { - id: string; + id: number; entityType: string; }; const getAcitivityLogForAdminById = ({ @@ -17,7 +17,7 @@ const getAcitivityLogForAdminById = ({ }; type UseGetAcitivityLogForAdminByIdParams = { - id: string; + id: number; entityType: string; enabled?: boolean; }; diff --git a/submit-web/src/hooks/api/useItems.ts b/submit-web/src/hooks/api/useItems.ts index b282a54c..4b8f2719 100644 --- a/submit-web/src/hooks/api/useItems.ts +++ b/submit-web/src/hooks/api/useItems.ts @@ -8,6 +8,7 @@ import { } from "@tanstack/react-query"; import { QUERY_KEY } from "./constants"; import { SubmissionReview } from "@/models/SubmissionReview"; +import { ACTIVITY_LOG_ENTITY_TYPE } from "@/models/ActivityLog"; type GetSubmissionItemByIdParams = { itemId: number; @@ -117,6 +118,13 @@ export const useSaveSubmissionReview = ({ queryClient.removeQueries({ queryKey: [QUERY_KEY.ACCOUNT_PROJECTS], }); + queryClient.invalidateQueries({ + queryKey: [ + QUERY_KEY.ACTIVITY_LOGS, + ACTIVITY_LOG_ENTITY_TYPE.PACKAGE, + packageId, + ], + }); }, }); }; diff --git a/submit-web/src/hooks/api/usePackages.ts b/submit-web/src/hooks/api/usePackages.ts index 7a6240c7..9f392b65 100644 --- a/submit-web/src/hooks/api/usePackages.ts +++ b/submit-web/src/hooks/api/usePackages.ts @@ -8,6 +8,7 @@ import { import { Options } from "./types"; import { PackageVersion, SubmissionPackage } from "@/models/Package"; import { defaultUseQueryOptions, QUERY_KEY } from "./constants"; +import { ACTIVITY_LOG_ENTITY_TYPE } from "@/models/ActivityLog"; const createSubmissionPackage = ({ accountProjectId, @@ -180,6 +181,13 @@ export const useUpdateStateSubmissionPackage = (options?: Options) => { queryClient.invalidateQueries({ queryKey: [QUERY_KEY.ACCOUNT_PROJECTS], }); + queryClient.invalidateQueries({ + queryKey: [ + QUERY_KEY.ACTIVITY_LOGS, + ACTIVITY_LOG_ENTITY_TYPE.PACKAGE, + submissionPackage.id, + ], + }); if (options?.onSuccess) { options.onSuccess(); }