From e33c7045c50f73edfb791ef76f563f4121767367 Mon Sep 17 00:00:00 2001 From: Sampo Tawast <5328394+sirtawast@users.noreply.github.com> Date: Mon, 14 Oct 2024 15:23:30 +0300 Subject: [PATCH] feat: include attachment additions to change set (hl-1209) (#3423) * refactor: general cleanup and helper function usage * feat: add attachment history changes * test: fix issues with refactor and attachment changes * feat: minor tweaks between attachment change sets and others --- .../applications/services/change_history.py | 99 +++++++++++++------ .../tests/test_applications_api.py | 8 +- .../handler/public/locales/en/common.json | 3 +- .../handler/public/locales/fi/common.json | 3 +- .../handler/public/locales/sv/common.json | 3 +- .../src/components/sidebar/ChangeSet.tsx | 34 ++++--- 6 files changed, 100 insertions(+), 50 deletions(-) diff --git a/backend/benefit/applications/services/change_history.py b/backend/benefit/applications/services/change_history.py index d2d473605f..fdce337424 100644 --- a/backend/benefit/applications/services/change_history.py +++ b/backend/benefit/applications/services/change_history.py @@ -36,19 +36,27 @@ def _get_handlers(as_ids: bool = False) -> list: ] -def _parse_change_values(value): +def _parse_change_values(value, field): if value is None: return None if isinstance(value, bool): return bool(value) + if field == "handler": + return User.objects.get(id=value).get_full_name() or "Unknown user" + return str(value) -def _format_change_dict(change: ModelChange, employee: bool) -> dict: +def _format_change_dict(change: ModelChange, relation_name: str = None) -> dict: + def _get_field_name(relation_name): + if relation_name: + return f"{relation_name}.{change.field}" + return f"{change.field}" + return { - "field": f"{'employee.' if employee else ''}{change.field}", - "old": _parse_change_values(change.old), - "new": _parse_change_values(change.new), + "field": _get_field_name(relation_name), + "old": _parse_change_values(change.old, change.field), + "new": _parse_change_values(change.new, change.field), } @@ -105,12 +113,15 @@ def _get_application_change_history_between_timestamps( changes = [] changes += [ - _format_change_dict(change, employee=False) + _format_change_dict( + change, + ) for change in application_delta.changes ] changes += [ - _format_change_dict(change, employee=True) for change in employee_delta.changes + _format_change_dict(change, relation_name="employee") + for change in employee_delta.changes ] if new_or_edited_attachments: for attachment in new_or_edited_attachments: @@ -205,11 +216,11 @@ def _get_change_set_base(new_record: ModelChange): def _filter_and_format_changes( - changes, excluded_fields, is_employee_change, delta_time=None + changes, excluded_fields, relation_name: str = None, delta_time=None ): return list( map( - lambda change: _format_change_dict(change, is_employee_change), + lambda change: _format_change_dict(change, relation_name), filter( # Only accept those changes that are within the threshold delta_time and/or not excluded field lambda change: not _is_history_change_excluded(change, excluded_fields) and ( @@ -223,6 +234,29 @@ def _filter_and_format_changes( ) +def _create_change_set(app_diff, employee_diffs): + change_set = _get_change_set_base(app_diff.new_record) + change_set["changes"] = _filter_and_format_changes( + app_diff.changes, + EXCLUDED_APPLICATION_FIELDS, + ) + + for employee_diff in employee_diffs: + delta_time = ( + employee_diff.new_record.history_date - app_diff.new_record.history_date + ).total_seconds() + change_set["changes"] += _filter_and_format_changes( + employee_diff.changes, EXCLUDED_EMPLOYEE_FIELDS, "employee", delta_time + ) + + return ( + change_set + if len(change_set["changes"]) > 0 + or (change_set["reason"] and len(change_set["reason"]) > 0) + else None + ) + + def get_application_change_history_made_by_handler(application: Application) -> list: """ Get application change history between the point when application is received and @@ -268,37 +302,40 @@ def get_application_change_history_made_by_handler(application: Application) -> diff = employee_history[i].diff_against(employee_history[i + 1]) employee_diffs.append(diff) - def create_change_set(app_diff, employee_diffs): - change_set = _get_change_set_base(app_diff.new_record) - change_set["changes"] = _filter_and_format_changes( - app_diff.changes, EXCLUDED_APPLICATION_FIELDS, False - ) - - for employee_diff in employee_diffs: - delta_time = ( - employee_diff.new_record.history_date - app_diff.new_record.history_date - ).total_seconds() - change_set["changes"] += _filter_and_format_changes( - employee_diff.changes, EXCLUDED_EMPLOYEE_FIELDS, True, delta_time - ) - - return ( - change_set - if len(change_set["changes"]) > 0 - or (change_set["reason"] and len(change_set["reason"]) > 0) - else None - ) - change_sets = list( filter( None, map( - lambda app_diff: create_change_set(app_diff, employee_diffs), + lambda app_diff: _create_change_set(app_diff, employee_diffs), application_diffs, ), ) ) + submitted_at = ( + ApplicationLogEntry.objects.filter( + application=application, to_status=ApplicationStatus.RECEIVED + ) + .order_by("-created_at") + .values("created_at")[:1] + ) + + attachment_diffs = [] + for attachment in application.attachments.all(): + for new_record in attachment.history.filter( + history_type="+", history_date__gte=submitted_at + ): + change_set_base = _get_change_set_base(new_record) + change_set_base["changes"] = [ + { + "field": "attachments", + "old": "+", + "new": f"{new_record.attachment_file} ({_(new_record.attachment_type)})", + } + ] + attachment_diffs.append(change_set_base) + change_sets += attachment_diffs + return change_sets diff --git a/backend/benefit/applications/tests/test_applications_api.py b/backend/benefit/applications/tests/test_applications_api.py index 1417b5ab63..ff69ffe220 100755 --- a/backend/benefit/applications/tests/test_applications_api.py +++ b/backend/benefit/applications/tests/test_applications_api.py @@ -2400,7 +2400,8 @@ def test_application_history_change_sets_for_handler( request, handler_api_client, application ): # Setup application to handling status - add_attachments_to_application(request, application) + with freeze_time("2021-01-01") as frozen_datetime: + add_attachments_to_application(request, application) payload = HandlerApplicationSerializer(application).data payload["status"] = ApplicationStatus.RECEIVED @@ -2506,9 +2507,8 @@ def update_application(application_payload): update_payloads.reverse() - # Add a row which gets inserted when application status changes to "handling" - user = get_client_user(handler_api_client) - update_payloads.append({"change_reason": None, "handler": str(user.id)}) + # Add a mock row which gets inserted when application status changes to "handling" + update_payloads.append({"change_reason": None, "handler": "Unknown user"}) assert len(changes) == len(update_payloads) diff --git a/frontend/benefit/handler/public/locales/en/common.json b/frontend/benefit/handler/public/locales/en/common.json index c30f3b2282..b30e17843b 100644 --- a/frontend/benefit/handler/public/locales/en/common.json +++ b/frontend/benefit/handler/public/locales/en/common.json @@ -1801,7 +1801,8 @@ "label": "Diaarinro" }, "attachments": { - "label": "Liite" + "label": "Liite", + "newAttachment": "Uusi liite" }, "handler": { "label": "Käsittelijä" diff --git a/frontend/benefit/handler/public/locales/fi/common.json b/frontend/benefit/handler/public/locales/fi/common.json index 4d576c96b4..a557755219 100644 --- a/frontend/benefit/handler/public/locales/fi/common.json +++ b/frontend/benefit/handler/public/locales/fi/common.json @@ -1800,7 +1800,8 @@ "label": "Diaarinro" }, "attachments": { - "label": "Liite" + "label": "Liite", + "newAttachment": "Uusi liite" }, "handler": { "label": "Käsittelijä" diff --git a/frontend/benefit/handler/public/locales/sv/common.json b/frontend/benefit/handler/public/locales/sv/common.json index c30f3b2282..b30e17843b 100644 --- a/frontend/benefit/handler/public/locales/sv/common.json +++ b/frontend/benefit/handler/public/locales/sv/common.json @@ -1801,7 +1801,8 @@ "label": "Diaarinro" }, "attachments": { - "label": "Liite" + "label": "Liite", + "newAttachment": "Uusi liite" }, "handler": { "label": "Käsittelijä" diff --git a/frontend/benefit/handler/src/components/sidebar/ChangeSet.tsx b/frontend/benefit/handler/src/components/sidebar/ChangeSet.tsx index 05bd094840..f5ed14ecc4 100644 --- a/frontend/benefit/handler/src/components/sidebar/ChangeSet.tsx +++ b/frontend/benefit/handler/src/components/sidebar/ChangeSet.tsx @@ -39,6 +39,10 @@ const ChangeSet: React.FC = ({ data }: ChangeSetProps) => { const { t } = useTranslation(); const { date, user, changes, reason } = data; + const isAttachmentChange = changes.find( + (change) => change.field === 'attachments' + ); + return ( <$ChangeSet> <$ChangeSetHeader> @@ -63,16 +67,18 @@ const ChangeSet: React.FC = ({ data }: ChangeSetProps) => { valueLengthsExceedRowLayout(t, change) ? 'column' : 'row' } > - <> - - {formatOrTranslateValue( - t, - change.old, - prepareChangeFieldName(change.field) - )} - - - + {!isAttachmentChange && ( + <> + + {formatOrTranslateValue( + t, + change.old, + prepareChangeFieldName(change.field) + )} + + + + )} {formatOrTranslateValue( t, @@ -85,7 +91,7 @@ const ChangeSet: React.FC = ({ data }: ChangeSetProps) => { ))} <$ChangeSetFooter> - {changes.length > 0 && ( + {changes.length > 1 && !isAttachmentChange && (

{t('common:changes.header.amountOfChanges', { amount: changes.length, @@ -96,7 +102,11 @@ const ChangeSet: React.FC = ({ data }: ChangeSetProps) => { <$ViewFieldBold> {t('common:applications.sections.fields.changeReason.placeholder')} - <$ViewField>{formatOrTranslateValue(t, reason)} + <$ViewField> + {isAttachmentChange + ? t('common:changes.fields.attachments.newAttachment') + : formatOrTranslateValue(t, reason)} + );