From 2a1c2ccc934e3bd22af2b7fccf1e3b71fb063256 Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Fri, 15 Nov 2024 11:55:18 -0800 Subject: [PATCH 01/12] Improved privacy request error handling --- ...e_default_timestamps_for_execution_logs.py | 32 +++++++++++++++ src/fides/api/graph/traversal.py | 2 +- src/fides/api/models/privacy_request.py | 31 +++++++++----- .../privacy_request/request_runner_service.py | 24 +++++------ src/fides/api/task/create_request_tasks.py | 11 +++-- src/fides/api/task/execute_request_tasks.py | 40 +++++++++---------- src/fides/api/task/graph_task.py | 8 ++-- src/fides/api/task/task_resources.py | 2 +- src/fides/api/tasks/storage.py | 2 +- src/fides/api/util/logger.py | 2 +- src/fides/api/util/logger_context_utils.py | 29 +++++++++++--- 11 files changed, 117 insertions(+), 66 deletions(-) create mode 100644 src/fides/api/alembic/migrations/versions/c90d46f6d3f2_update_default_timestamps_for_execution_logs.py diff --git a/src/fides/api/alembic/migrations/versions/c90d46f6d3f2_update_default_timestamps_for_execution_logs.py b/src/fides/api/alembic/migrations/versions/c90d46f6d3f2_update_default_timestamps_for_execution_logs.py new file mode 100644 index 0000000000..506d6f2a86 --- /dev/null +++ b/src/fides/api/alembic/migrations/versions/c90d46f6d3f2_update_default_timestamps_for_execution_logs.py @@ -0,0 +1,32 @@ +"""update default timestamps for execution logs + +Revision ID: c90d46f6d3f2 +Revises: abc53a00755b +Create Date: 2024-11-15 17:29:29.934671 + +""" + +from alembic import op +from sqlalchemy import text + +# revision identifiers, used by Alembic. +revision = "c90d46f6d3f2" +down_revision = "abc53a00755b" +branch_labels = None +depends_on = None + + +def upgrade(): + op.alter_column( + "executionlog", "created_at", server_default=text("clock_timestamp()") + ) + + op.alter_column( + "executionlog", "updated_at", server_default=text("clock_timestamp()") + ) + + +def downgrade(): + op.alter_column("executionlog", "created_at", server_default=text("now()")) + + op.alter_column("executionlog", "updated_at", server_default=text("now()")) diff --git a/src/fides/api/graph/traversal.py b/src/fides/api/graph/traversal.py index a770b0aebc..938f165fbe 100644 --- a/src/fides/api/graph/traversal.py +++ b/src/fides/api/graph/traversal.py @@ -322,7 +322,7 @@ def traverse( # pylint: disable=R0914 """ if environment: logger.info( - "starting traversal", + "Starting traversal", ) remaining_node_keys: Set[CollectionAddress] = set( self.traversal_node_dict.keys() diff --git a/src/fides/api/models/privacy_request.py b/src/fides/api/models/privacy_request.py index e010a6a6eb..f5c1b6fd03 100644 --- a/src/fides/api/models/privacy_request.py +++ b/src/fides/api/models/privacy_request.py @@ -5,7 +5,7 @@ import json from datetime import datetime, timedelta from enum import Enum as EnumType -from typing import Any, Dict, List, Optional, Set, Tuple, Union +from typing import Any, Dict, List, Optional, Set, Tuple, Type, Union from celery.result import AsyncResult from loguru import logger @@ -24,6 +24,7 @@ from sqlalchemy.ext.mutable import MutableDict, MutableList from sqlalchemy.orm import Query, RelationshipProperty, Session, backref, relationship from sqlalchemy.orm.dynamic import AppenderQuery +from sqlalchemy.sql import text from sqlalchemy_utils.types.encrypted.encrypted_type import ( AesGcmEngine, StringEncryptedType, @@ -1053,7 +1054,7 @@ def start_processing(self, db: Session) -> None: """Dispatches this PrivacyRequest throughout the Fidesops System""" if self.started_processing_at is None: self.started_processing_at = datetime.utcnow() - if self.status == PrivacyRequestStatus.pending: + if self.status in [PrivacyRequestStatus.pending, PrivacyRequestStatus.approved]: self.status = PrivacyRequestStatus.in_processing self.save(db=db) @@ -1825,6 +1826,20 @@ class ExecutionLog(Base): index=True, ) + # Use clock_timestamp() instead of NOW() to get the actual current time at row creation, + # regardless of transaction state. This prevents timestamp caching within transactions + # and ensures more accurate creation times. + # https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-CURRENT + + created_at = Column( + DateTime(timezone=True), server_default=text("clock_timestamp()") + ) + updated_at = Column( + DateTime(timezone=True), + server_default=text("clock_timestamp()"), + onupdate=text("clock_timestamp()"), + ) + def can_run_checkpoint( request_checkpoint: CurrentStep, from_checkpoint: Optional[CurrentStep] = None @@ -2056,11 +2071,9 @@ def upstream_tasks_complete(self, db: Session, should_log: bool = False) -> bool if not tasks_complete and should_log: logger.debug( - "Upstream tasks incomplete for {} task {}. Privacy Request: {}, Request Task {}.", + "Upstream tasks incomplete for {} task {}.", self.action_type.value, self.collection_address, - self.privacy_request_id, - self.id, ) return tasks_complete @@ -2088,24 +2101,20 @@ def request_task_running(self, should_log: bool = False) -> bool: if should_log: logger.debug( - "Celery Task ID {} found for {} task {}. Privacy Request: {}, Request Task {}.", + "Celery Task ID {} found for {} task {}.", celery_task_id, self.action_type.value, self.collection_address, - self.privacy_request_id, - self.id, ) task_in_flight: bool = celery_tasks_in_flight([celery_task_id]) if task_in_flight and should_log: logger.debug( - "Celery Task {} already processing for {} task {}. Privacy Request: {}, Request Task {}.", + "Celery Task {} already processing for {} task {}.", celery_task_id, self.action_type.value, self.collection_address, - self.privacy_request_id, - self.id, ) return task_in_flight diff --git a/src/fides/api/service/privacy_request/request_runner_service.py b/src/fides/api/service/privacy_request/request_runner_service.py index dcafa71ac7..c4e1e1587f 100644 --- a/src/fides/api/service/privacy_request/request_runner_service.py +++ b/src/fides/api/service/privacy_request/request_runner_service.py @@ -69,6 +69,7 @@ from fides.api.util.cache import cache_task_tracking_key from fides.api.util.collection_util import Row from fides.api.util.logger import Pii, _log_exception, _log_warning +from fides.api.util.logger_context_utils import LoggerContextKeys, log_context from fides.common.api.v1.urn_registry import ( PRIVACY_REQUEST_TRANSFER_TO_PARENT, V1_URL_PREFIX, @@ -272,14 +273,13 @@ def upload_access_results( # pylint: disable=R0912 return download_urls +@log_context(capture_args={"privacy_request_id": LoggerContextKeys.privacy_request_id}) def queue_privacy_request( privacy_request_id: str, from_webhook_id: Optional[str] = None, from_step: Optional[str] = None, ) -> str: - logger.info( - "Queueing privacy request {} from step {}", privacy_request_id, from_step - ) + logger.info("Queueing privacy request from step {}", from_step) task = run_privacy_request.delay( privacy_request_id=privacy_request_id, from_webhook_id=from_webhook_id, @@ -291,6 +291,7 @@ def queue_privacy_request( @celery_app.task(base=DatabaseTask, bind=True) +@log_context(capture_args={"privacy_request_id": LoggerContextKeys.privacy_request_id}) def run_privacy_request( self: DatabaseTask, privacy_request_id: str, @@ -320,18 +321,14 @@ def run_privacy_request( ) if privacy_request.status == PrivacyRequestStatus.canceled: - logger.info( - "Terminating privacy request {}: request canceled.", privacy_request.id - ) + logger.info("Terminating privacy request: request canceled.") return if privacy_request.deleted_at is not None: - logger.info( - "Terminating privacy request {}: request deleted.", privacy_request.id - ) + logger.info("Terminating privacy request: request deleted.") return - logger.info("Dispatching privacy request {}", privacy_request.id) + logger.info("Dispatching privacy request") privacy_request.start_processing(session) policy = privacy_request.policy @@ -523,10 +520,7 @@ def run_privacy_request( ): privacy_request.cache_failed_checkpoint_details(CurrentStep.email_post_send) privacy_request.pause_processing_for_email_send(session) - logger.info( - "Privacy request '{}' exiting: awaiting email send.", - privacy_request.id, - ) + logger.info("Privacy request exiting: awaiting email send.") return # Post Webhooks CHECKPOINT @@ -590,7 +584,7 @@ def run_privacy_request( }, ) privacy_request.status = PrivacyRequestStatus.complete - logger.info("Privacy request {} run completed.", privacy_request.id) + logger.info("Privacy request run completed.") privacy_request.save(db=session) diff --git a/src/fides/api/task/create_request_tasks.py b/src/fides/api/task/create_request_tasks.py index ae680d97f1..1c1ec63e48 100644 --- a/src/fides/api/task/create_request_tasks.py +++ b/src/fides/api/task/create_request_tasks.py @@ -33,6 +33,7 @@ from fides.api.schemas.policy import ActionType from fides.api.task.deprecated_graph_task import format_data_use_map_for_caching from fides.api.task.execute_request_tasks import log_task_queued, queue_request_task +from fides.api.util.logger_context_utils import log_context def _add_edge_if_no_nodes( @@ -422,6 +423,7 @@ def collect_tasks_fn( data[tn.address] = tn +@log_context def run_access_request( privacy_request: PrivacyRequest, policy: Policy, @@ -449,7 +451,7 @@ def run_access_request( ) else: try: - logger.info("Building access graph for {}", privacy_request.id) + logger.info("Building access graph") traversal: Traversal = Traversal(graph, identity) # Traversal.traverse populates traversal_nodes in place, adding parents and children to each traversal_node. @@ -497,6 +499,7 @@ def run_access_request( return ready_tasks +@log_context def run_erasure_request( # pylint: disable = too-many-arguments privacy_request: PrivacyRequest, session: Session, @@ -519,6 +522,7 @@ def run_erasure_request( # pylint: disable = too-many-arguments return ready_tasks +@log_context def run_consent_request( # pylint: disable = too-many-arguments privacy_request: PrivacyRequest, graph: DatasetGraph, @@ -543,7 +547,7 @@ def run_consent_request( # pylint: disable = too-many-arguments session, privacy_request, ActionType.consent ) else: - logger.info("Building consent graph for {}", privacy_request.id) + logger.info("Building consent graph") traversal_nodes: Dict[CollectionAddress, TraversalNode] = {} # Unlike erasure and access graphs, we don't call traversal.traverse, but build a simpler # graph that just has one node per dataset @@ -586,10 +590,9 @@ def get_existing_ready_tasks( if ready: logger.info( - "Found existing {} task(s) ready to reprocess: {}. Privacy Request: {}", + "Found existing {} task(s) ready to reprocess: {}.", action_type.value, [t.collection_address for t in ready], - privacy_request.id, ) return ready return ready diff --git a/src/fides/api/task/execute_request_tasks.py b/src/fides/api/task/execute_request_tasks.py index 61e500044a..5662f4cddf 100644 --- a/src/fides/api/task/execute_request_tasks.py +++ b/src/fides/api/task/execute_request_tasks.py @@ -30,6 +30,7 @@ from fides.api.tasks import DatabaseTask, celery_app from fides.api.util.cache import cache_task_tracking_key from fides.api.util.collection_util import Row +from fides.api.util.logger_context_utils import LoggerContextKeys, log_context # DSR 3.0 task functions @@ -74,7 +75,7 @@ def run_prerequisite_task_checks( # the node is already queued. if not request_task.upstream_tasks_complete(session, should_log=False): raise UpstreamTasksNotReady( - f"Cannot start {request_task.action_type} task {request_task.collection_address}. Privacy Request: {privacy_request.id}, Request Task {request_task.id}. Waiting for upstream tasks to finish." + f"Cannot start {request_task.action_type} task {request_task.collection_address}. Waiting for upstream tasks to finish." ) return privacy_request, request_task, upstream_results @@ -93,9 +94,7 @@ def create_graph_task( except Exception as exc: logger.debug( - "Cannot execute task - error loading task from database. Privacy Request: {}, Request Task {}. Exception {}", - request_task.privacy_request_id, - request_task.id, + "Cannot execute task - error loading task from database. Exception {}", str(exc), ) # Normally the GraphTask takes care of creating the ExecutionLog, but in this case we can't create it in the first place! @@ -128,10 +127,8 @@ def can_run_task_body( complete or this is a root/terminator node""" if request_task.is_terminator_task: logger.info( - "Terminator {} task reached. Privacy Request: {}, Request Task {}", + "Terminator {} task reached.", request_task.action_type.value, - request_task.privacy_request_id, - request_task.id, ) return False if request_task.is_root_task: @@ -139,12 +136,10 @@ def can_run_task_body( return False if request_task.status != ExecutionLogStatus.pending: logger_method(request_task)( - "Skipping {} task {} with status {}. Privacy Request: {}, Request Task {}", + "Skipping {} task {} with status {}.", request_task.action_type.value, request_task.collection_address, request_task.status.value, - request_task.privacy_request_id, - request_task.id, ) return False @@ -191,6 +186,7 @@ def queue_downstream_tasks( @celery_app.task(base=DatabaseTask, bind=True) +@log_context(capture_args={"privacy_request_task_id": LoggerContextKeys.task_id}) def run_access_node( self: DatabaseTask, privacy_request_id: str, @@ -234,6 +230,7 @@ def run_access_node( graph_task.access_request(*upstream_access_data) log_task_complete(request_task) + with self.get_new_session() as session: queue_downstream_tasks( session, request_task, @@ -241,7 +238,8 @@ def run_access_node( CurrentStep.upload_access, privacy_request_proceed, ) - return + + return @celery_app.task(base=DatabaseTask, bind=True) @@ -280,6 +278,7 @@ def run_erasure_node( log_task_complete(request_task) + with self.get_new_session() as session: queue_downstream_tasks( session, request_task, @@ -287,7 +286,8 @@ def run_erasure_node( CurrentStep.finalize_erasure, privacy_request_proceed, ) - return + + return @celery_app.task(base=DatabaseTask, bind=True) @@ -328,6 +328,7 @@ def run_consent_node( log_task_complete(request_task) + with self.get_new_session() as session: queue_downstream_tasks( session, request_task, @@ -335,7 +336,8 @@ def run_consent_node( CurrentStep.finalize_consent, privacy_request_proceed, ) - return + + return def logger_method(request_task: RequestTask) -> Callable: @@ -350,24 +352,20 @@ def logger_method(request_task: RequestTask) -> Callable: def log_task_starting(request_task: RequestTask) -> None: """Convenience method for logging task start""" logger_method(request_task)( - "Starting '{}' task {} with current status '{}'. Privacy Request: {}, Request Task {}", + "Starting '{}' task {} with current status '{}'.", request_task.action_type, request_task.collection_address, request_task.status.value, - request_task.privacy_request_id, - request_task.id, ) def log_task_complete(request_task: RequestTask) -> None: """Convenience method for logging task completion""" logger.info( - "{} task {} is {}. Privacy Request: {}, Request Task {}", + "{} task {} is {}.", request_task.action_type.value.capitalize(), request_task.collection_address, request_task.status.value, - request_task.privacy_request_id, - request_task.id, ) @@ -416,10 +414,8 @@ def queue_request_task( def log_task_queued(request_task: RequestTask, location: str) -> None: """Helper for logging that tasks are queued""" logger_method(request_task)( - "Queuing {} task {} from {}. Privacy Request: {}, Request Task {}", + "Queuing {} task {} from {}.", request_task.action_type.value, request_task.collection_address, location, - request_task.privacy_request_id, - request_task.id, ) diff --git a/src/fides/api/task/graph_task.py b/src/fides/api/task/graph_task.py index 8ea09445b8..c2e15580eb 100644 --- a/src/fides/api/task/graph_task.py +++ b/src/fides/api/task/graph_task.py @@ -390,7 +390,7 @@ def update_status( def log_start(self, action_type: ActionType) -> None: """Task start activities""" - logger.info("Starting {}, node {}", self.resources.request.id, self.key) + logger.info("Starting node {}", self.key) self.update_status( "starting", [], action_type, ExecutionLogStatus.in_processing @@ -398,7 +398,7 @@ def log_start(self, action_type: ActionType) -> None: def log_retry(self, action_type: ActionType) -> None: """Task retry activities""" - logger.info("Retrying {}, node {}", self.resources.request.id, self.key) + logger.info("Retrying node {}", self.key) self.update_status("retrying", [], action_type, ExecutionLogStatus.retrying) @@ -406,7 +406,7 @@ def log_awaiting_processing( self, action_type: ActionType, ex: Optional[BaseException] ) -> None: """On paused activities""" - logger.info("Pausing {}, node {}", self.resources.request.id, self.key) + logger.info("Pausing node {}", self.key) self.update_status( str(ex), [], action_type, ExecutionLogStatus.awaiting_processing @@ -414,7 +414,7 @@ def log_awaiting_processing( def log_skipped(self, action_type: ActionType, ex: str) -> None: """Log that a collection was skipped. For now, this is because a collection has been disabled.""" - logger.info("Skipping {}, node {}", self.resources.request.id, self.key) + logger.info("Skipping node {}", self.key) if action_type == ActionType.consent and self.request_task.id: self.request_task.consent_sent = False self.update_status(str(ex), [], action_type, ExecutionLogStatus.skipped) diff --git a/src/fides/api/task/task_resources.py b/src/fides/api/task/task_resources.py index 8808a3e7c6..c04169a5f2 100644 --- a/src/fides/api/task/task_resources.py +++ b/src/fides/api/task/task_resources.py @@ -199,5 +199,5 @@ def close(self) -> None: self.connections.close() to close connections to External Databases. This is really important to avoid opening up too many connections. """ - logger.debug("Closing all task resources for {}", self.request.id) + logger.debug("Closing all task resources") self.connections.close() diff --git a/src/fides/api/tasks/storage.py b/src/fides/api/tasks/storage.py index 85fd3f1d4f..0148dc355c 100644 --- a/src/fides/api/tasks/storage.py +++ b/src/fides/api/tasks/storage.py @@ -64,7 +64,7 @@ def write_to_in_memory_buffer( :param data: Dict :param request_id: str, The privacy request id """ - logger.info("Writing data to in-memory buffer") + logger.debug("Writing data to in-memory buffer") if resp_format == ResponseFormat.json.value: json_str = json.dumps(data, indent=2, default=storage_json_encoder) diff --git a/src/fides/api/util/logger.py b/src/fides/api/util/logger.py index a09d8a018e..432be55944 100644 --- a/src/fides/api/util/logger.py +++ b/src/fides/api/util/logger.py @@ -83,7 +83,7 @@ def create_handler_dicts( } extra_dict = { **standard_dict, - "format": log_format + " | {extra}", + "format": log_format + " | {extra}", "filter": lambda logRecord: bool(logRecord["extra"]), } return [standard_dict, extra_dict] diff --git a/src/fides/api/util/logger_context_utils.py b/src/fides/api/util/logger_context_utils.py index 7ad80b74d4..de83738c78 100644 --- a/src/fides/api/util/logger_context_utils.py +++ b/src/fides/api/util/logger_context_utils.py @@ -16,7 +16,7 @@ from fides.config import CONFIG -class LoggerContextKeys(Enum): +class LoggerContextKeys(str, Enum): action_type = "action_type" status_code = "status_code" body = "body" @@ -29,9 +29,10 @@ class LoggerContextKeys(Enum): response = "response" system_key = "system_key" url = "url" + task_id = "task_id" -class ErrorGroup(Enum): +class ErrorGroup(str, Enum): """A collection of user-friendly error labels to be used in contextualized logs.""" network_error = "NetworkError" @@ -57,18 +58,34 @@ def get_log_context(self) -> Dict[LoggerContextKeys, Any]: def log_context( - _func: Optional[Callable] = None, **additional_context: Any + _func: Optional[Callable] = None, + capture_args: Optional[dict[str, LoggerContextKeys]] = None, + **additional_context: Any, ) -> Callable: """ - A decorator that adds context information to log messages. It extracts context from - the arguments of the decorated function and from any specified additional context. - Optional additional context is provided through keyword arguments. + A decorator that adds context information to log messages. It extracts: + 1. Values from function arguments, mapping them to standardized context names via capture_args dict + 2. Context from any Contextualizable arguments + 3. Additional context provided as decorator kwargs + + Example: + @log_context(capture_args={"user_id": "standard_user_id"}, tenant="example") + def process_user(user_id: str) -> None: + + Logs will include standard_user_id= and tenant="example" """ def decorator(func: Callable) -> Callable: @wraps(func) def wrapper(*args: Any, **kwargs: Any) -> Any: context = dict(additional_context) + + # extract specified param values from kwargs + if capture_args: + for arg_name, context_name in capture_args.items(): + if arg_name in kwargs: + context[context_name.value] = kwargs[arg_name] + for arg in args: if isinstance(arg, Contextualizable): arg_context = arg.get_log_context() From 3cbd1652e4711946146cd85dd104bfdf91010304 Mon Sep 17 00:00:00 2001 From: Adrian Galvan Date: Sun, 17 Nov 2024 20:38:57 -0800 Subject: [PATCH 02/12] Privacy request detail page improvements Consistent privacy request logging New traversal entry in activity timeline --- .../features/common/RequestStatusBadge.tsx | 15 +- .../privacy-requests/PrivacyRequest.tsx | 59 +++++-- .../events-and-logs/ActivityTimeline.tsx | 136 ++++++++-------- .../events-and-logs/EventDetails.tsx | 150 ------------------ .../events-and-logs/EventLog.tsx | 2 +- .../events-and-logs/EventsAndLogs.tsx | 11 +- .../events-and-logs/LogDrawer.tsx | 97 +++++++++++ .../events-and-logs/TimelineEntry.tsx | 50 ++++++ src/fides/api/graph/traversal.py | 10 +- src/fides/api/models/privacy_request.py | 27 +++- .../privacy_request/request_runner_service.py | 14 +- src/fides/api/task/create_request_tasks.py | 8 + src/fides/api/task/deprecated_graph_task.py | 9 ++ src/fides/api/util/logger.py | 2 +- 14 files changed, 333 insertions(+), 257 deletions(-) delete mode 100644 clients/admin-ui/src/features/privacy-requests/events-and-logs/EventDetails.tsx create mode 100644 clients/admin-ui/src/features/privacy-requests/events-and-logs/LogDrawer.tsx create mode 100644 clients/admin-ui/src/features/privacy-requests/events-and-logs/TimelineEntry.tsx diff --git a/clients/admin-ui/src/features/common/RequestStatusBadge.tsx b/clients/admin-ui/src/features/common/RequestStatusBadge.tsx index 528db81541..00b7687430 100644 --- a/clients/admin-ui/src/features/common/RequestStatusBadge.tsx +++ b/clients/admin-ui/src/features/common/RequestStatusBadge.tsx @@ -1,4 +1,4 @@ -import { Badge, BadgeProps } from "fidesui"; +import { Badge, BadgeProps, Spinner } from "fidesui"; import { PrivacyRequestStatus } from "~/types/api"; @@ -65,7 +65,18 @@ const RequestStatusBadge = ({ status }: RequestBadgeProps) => ( textAlign="center" data-testid="request-status-badge" > - {statusPropMap[status].label} + + {statusPropMap[status].label} + {status === PrivacyRequestStatus.IN_PROCESSING && ( + + )} + ); diff --git a/clients/admin-ui/src/features/privacy-requests/PrivacyRequest.tsx b/clients/admin-ui/src/features/privacy-requests/PrivacyRequest.tsx index 91565b84ec..306831ac39 100644 --- a/clients/admin-ui/src/features/privacy-requests/PrivacyRequest.tsx +++ b/clients/admin-ui/src/features/privacy-requests/PrivacyRequest.tsx @@ -1,4 +1,8 @@ import { Box, VStack } from "fidesui"; +import { useMemo } from "react"; + +import { useGetAllPrivacyRequestsQuery } from "~/features/privacy-requests"; +import { PrivacyRequestStatus } from "~/types/api"; import EventsAndLogs from "./events-and-logs/EventsAndLogs"; import ManualProcessingList from "./manual-processing/ManualProcessingList"; @@ -10,23 +14,46 @@ type PrivacyRequestProps = { data: PrivacyRequestEntity; }; -const PrivacyRequest = ({ data: subjectRequest }: PrivacyRequestProps) => ( - - - - - - - - {subjectRequest.status === "requires_input" && ( +const PrivacyRequest = ({ data: initialData }: PrivacyRequestProps) => { + const queryOptions = useMemo( + () => ({ + id: initialData.id, + verbose: true, + }), + [initialData.id], + ); + + // Poll for the latest privacy request data while the status is approved or in processing + const { data: latestData } = useGetAllPrivacyRequestsQuery(queryOptions, { + pollingInterval: + initialData.status === PrivacyRequestStatus.APPROVED || + initialData.status === PrivacyRequestStatus.IN_PROCESSING + ? 2000 + : 0, + skip: !initialData.id, + }); + + // Use latest data if available, otherwise use initial data + const subjectRequest = latestData?.items[0] ?? initialData; + + return ( + + + + + + + + {subjectRequest.status === PrivacyRequestStatus.REQUIRES_INPUT && ( + + + + )} - + - )} - - - - -); + + ); +}; export default PrivacyRequest; diff --git a/clients/admin-ui/src/features/privacy-requests/events-and-logs/ActivityTimeline.tsx b/clients/admin-ui/src/features/privacy-requests/events-and-logs/ActivityTimeline.tsx index 5a8bf21b19..adac33dcf3 100644 --- a/clients/admin-ui/src/features/privacy-requests/events-and-logs/ActivityTimeline.tsx +++ b/clients/admin-ui/src/features/privacy-requests/events-and-logs/ActivityTimeline.tsx @@ -1,20 +1,14 @@ -import { - Box, - ErrorWarningIcon, - Flex, - GreenCheckCircleIcon, - Text, -} from "fidesui"; +import { Box, Text, useDisclosure } from "fidesui"; import { ExecutionLog, PrivacyRequestEntity } from "privacy-requests/types"; -import React from "react"; +import React, { useEffect, useState } from "react"; import { ExecutionLogStatus } from "~/types/api"; -import { EventData } from "./EventDetails"; +import LogDrawer from "./LogDrawer"; +import TimelineEntry from "./TimelineEntry"; type ActivityTimelineProps = { subjectRequest: PrivacyRequestEntity; - setEventDetails: (d: EventData) => void; }; const hasUnresolvedError = (entries: ExecutionLog[]) => { @@ -35,70 +29,86 @@ const hasUnresolvedError = (entries: ExecutionLog[]) => { }); // Check if any of the latest entries for the collections have an error status. - return Object.values(groupedByCollection).some( - (entry) => entry.status === ExecutionLogStatus.ERROR, - ); + return Object.values(groupedByCollection).some((entry) => { + // For entries with a collection_name, check for later complete status + if (entry.collection_name) { + const latestComplete = entries.find( + (e) => + e.status.toLowerCase() === "complete" && + !e.collection_name && // completion entries have null collection_name + new Date(e.updated_at) > new Date(entry.updated_at), + ); + + return !latestComplete && entry.status === ExecutionLogStatus.ERROR; + } + + return entry.status === ExecutionLogStatus.ERROR; + }); }; -const ActivityTimeline = ({ - subjectRequest, - setEventDetails, -}: ActivityTimelineProps) => { - const { results } = subjectRequest; +const ActivityTimeline = ({ subjectRequest }: ActivityTimelineProps) => { + const { isOpen, onOpen, onClose } = useDisclosure(); + const [currentLogs, setCurrentLogs] = useState([]); + const [currentKey, setCurrentKey] = useState(""); + const [isViewingError, setViewingError] = useState(false); + const [errorMessage, setErrorMessage] = useState(""); + const { results } = subjectRequest; const resultKeys = results ? Object.keys(results) : []; - const timelineEntries = resultKeys.map((key, index) => ( - - - - {hasUnresolvedError(results![key]) ? ( - - ) : ( - - )} - - {index === resultKeys.length - 1 ? null : ( - - )} - - - {key} - - - { - setEventDetails({ - key, - logs: results![key], - }); - }} - > - View Details - - - )); + // Update currentLogs when results change and we have a selected key + useEffect(() => { + if (currentKey && results && results[currentKey]) { + setCurrentLogs(results[currentKey]); + } + }, [results, currentKey]); + + const openErrorPanel = (message: string) => { + setErrorMessage(message); + setViewingError(true); + }; + + const closeErrorPanel = () => { + setViewingError(false); + }; + + const closeDrawer = () => { + if (isViewingError) { + closeErrorPanel(); + } + setCurrentKey(""); + onClose(); + }; + + const showLogs = (key: string, logs: ExecutionLog[]) => { + setCurrentKey(key); + setCurrentLogs(logs); + onOpen(); + }; return ( Activity timeline - {timelineEntries} + {resultKeys.map((key, index) => ( + showLogs(key, results![key])} + /> + ))} + ); }; diff --git a/clients/admin-ui/src/features/privacy-requests/events-and-logs/EventDetails.tsx b/clients/admin-ui/src/features/privacy-requests/events-and-logs/EventDetails.tsx deleted file mode 100644 index ebd87bb2fc..0000000000 --- a/clients/admin-ui/src/features/privacy-requests/events-and-logs/EventDetails.tsx +++ /dev/null @@ -1,150 +0,0 @@ -import { - AntButton as Button, - ArrowBackIcon, - Box, - CloseSolidIcon, - Divider, - Drawer, - DrawerBody, - DrawerContent, - DrawerHeader, - DrawerOverlay, - Flex, - Text, - useDisclosure, -} from "fidesui"; -import { ExecutionLog } from "privacy-requests/types"; -import React, { useState } from "react"; - -import EventError from "./EventError"; -import EventLog from "./EventLog"; - -export type EventData = { - key: string; - logs: ExecutionLog[]; -}; - -type EventDetailsProps = { - eventData: EventData | undefined; -}; - -const EventDetails = ({ eventData }: EventDetailsProps) => { - const { isOpen, onOpen, onClose } = useDisclosure(); - - const [isViewingError, setViewingError] = useState(false); - const [errorMessage, setErrorMessage] = useState(""); - - const openErrorPanel = (message: string) => { - setErrorMessage(message); - setViewingError(true); - }; - - const closeErrorPanel = () => { - setViewingError(false); - }; - - const closeDrawer = () => { - if (isViewingError) { - closeErrorPanel(); - } - - onClose(); - }; - - const headerText = isViewingError ? "Event detail" : "Event log"; - if (eventData === undefined) { - return null; - } - return ( - - - Event Details - - - - {eventData.key} - - - - { - onOpen(); - }} - > - View Log - - - - - - - - - {isViewingError && ( -