From 6bf992586368c85cec8ba58460fea070abd4de0b Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Fri, 10 Jan 2025 15:48:59 +0000 Subject: [PATCH 01/12] Added payment_service_items table to the audit_history table logs. Created template for move history events for rejecting/accepting payment_service_items --- migrations/app/migrations_manifest.txt | 1 + ...ory_table_for_payment_service_items.up.sql | 1 + src/constants/MoveHistory/Database/Tables.js | 1 + .../UpdatePaymentServiceItem.jsx | 28 +++++++++++++++++++ .../MoveHistory/EventTemplates/index.js | 1 + .../MoveHistory/UIDisplay/Operations.js | 1 + 6 files changed, 33 insertions(+) create mode 100644 migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql create mode 100644 src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItem.jsx diff --git a/migrations/app/migrations_manifest.txt b/migrations/app/migrations_manifest.txt index 9151f5ec112..6ee1de57c69 100644 --- a/migrations/app/migrations_manifest.txt +++ b/migrations/app/migrations_manifest.txt @@ -1066,3 +1066,4 @@ 20241230190638_remove_AK_zips_from_zip3.up.sql 20241230190647_add_missing_AK_zips_to_zip3_distances.up.sql 20250103180420_update_pricing_proc_to_use_local_price_variable.up.sql +20250109194140_create_audit_history_table_for_payment_service_items.up.sql diff --git a/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql new file mode 100644 index 00000000000..68c3a865112 --- /dev/null +++ b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql @@ -0,0 +1 @@ +SELECT add_audit_history_table('payment_service_items'); \ No newline at end of file diff --git a/src/constants/MoveHistory/Database/Tables.js b/src/constants/MoveHistory/Database/Tables.js index e097f569adf..7a96c2ba5aa 100644 --- a/src/constants/MoveHistory/Database/Tables.js +++ b/src/constants/MoveHistory/Database/Tables.js @@ -19,4 +19,5 @@ export default { moving_expenses: 'moving_expenses', progear_weight_tickets: 'progear_weight_tickets', gsr_appeals: 'gsr_appeals', + payment_service_items: 'payment_service_items', }; diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItem.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItem.jsx new file mode 100644 index 00000000000..0977bb649ca --- /dev/null +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItem.jsx @@ -0,0 +1,28 @@ +import React from 'react'; + +import o from 'constants/MoveHistory/UIDisplay/Operations'; +import a from 'constants/MoveHistory/Database/Actions'; +import t from 'constants/MoveHistory/Database/Tables'; +import { getMtoShipmentLabel } from 'utils/formatMtoShipment'; +import LabeledDetails from 'pages/Office/MoveHistory/LabeledDetails'; + +const formatChangedValues = (historyRecord) => { + const newChangedValues = { + ...historyRecord.changedValues, + ...getMtoShipmentLabel(historyRecord), + }; + + return { ...historyRecord, changedValues: newChangedValues }; +}; + +export default { + action: a.UPDATE, + eventName: o.updatePaymentServiceItem, + tableName: t.payment_service_items, + getEventNameDisplay: () => { + return
Updated Payment Service Item
; + }, + getDetails: (historyRecord) => { + return ; + }, +}; diff --git a/src/constants/MoveHistory/EventTemplates/index.js b/src/constants/MoveHistory/EventTemplates/index.js index afcbd6b7060..d1d582e6f16 100644 --- a/src/constants/MoveHistory/EventTemplates/index.js +++ b/src/constants/MoveHistory/EventTemplates/index.js @@ -109,3 +109,4 @@ export { default as moveCancelerPPMShipments } from './MoveCanceler/MoveCanceler export { default as cancelMove } from './CancelMove/CancelMove'; export { default as cancelMoveMTOShipments } from './CancelMove/CancelMoveMTOShipments'; export { default as cancelMovePPMShipments } from './CancelMove/CancelMovePPMShipments'; +export { default as updatePaymentServiceItem } from './UpdatePaymentServiceItem/UpdatePaymentServiceItem'; diff --git a/src/constants/MoveHistory/UIDisplay/Operations.js b/src/constants/MoveHistory/UIDisplay/Operations.js index 8ca4a3f5cd0..79f50d951aa 100644 --- a/src/constants/MoveHistory/UIDisplay/Operations.js +++ b/src/constants/MoveHistory/UIDisplay/Operations.js @@ -34,6 +34,7 @@ export default { updateOrder: 'updateOrder', // ghc.yaml updateOrders: 'updateOrders', // internal.yaml updatePaymentRequestStatus: 'updatePaymentRequestStatus', + updatePaymentServiceItem: 'updatePaymentServiceItem', updateReweigh: 'updateReweigh', updateServiceItemStatus: 'updateMTOServiceItemStatus', updateServiceItemSitEntryDate: 'updateServiceItemSitEntryDate', // ghc.yaml From c7489b91a7bef1d32e709d63fd5a6cd147af5bb4 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Tue, 14 Jan 2025 20:47:59 +0000 Subject: [PATCH 02/12] Changed migration to exclude some cols, added SQL to move_history_fetcher for payment_service_items, renamed move history event for consistency --- ...ory_table_for_payment_service_items.up.sql | 2 +- .../sql_scripts/move_history_fetcher.sql | 44 +++++++++++++++++++ ...jsx => UpdatePaymentServiceItemStatus.jsx} | 2 +- .../MoveHistory/EventTemplates/index.js | 2 +- .../MoveHistory/UIDisplay/Operations.js | 2 +- 5 files changed, 48 insertions(+), 4 deletions(-) rename src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/{UpdatePaymentServiceItem.jsx => UpdatePaymentServiceItemStatus.jsx} (94%) diff --git a/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql index 68c3a865112..1502e3302c8 100644 --- a/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql +++ b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql @@ -1 +1 @@ -SELECT add_audit_history_table('payment_service_items'); \ No newline at end of file +SELECT add_audit_history_table(target_table := 'payment_service_items', audit_rows := BOOLEAN 't', audit_query_text := BOOLEAN 't', ignored_cols := ARRAY['created_at', 'updated_at']); \ No newline at end of file diff --git a/pkg/assets/sql_scripts/move_history_fetcher.sql b/pkg/assets/sql_scripts/move_history_fetcher.sql index dacacf55d78..36584462a26 100644 --- a/pkg/assets/sql_scripts/move_history_fetcher.sql +++ b/pkg/assets/sql_scripts/move_history_fetcher.sql @@ -239,6 +239,45 @@ WITH move AS ( JOIN move_payment_requests ON move_payment_requests.id = audit_history.object_id WHERE audit_history.table_name = 'payment_requests' ), + move_payment_service_items AS ( + SELECT + jsonb_agg(jsonb_build_object( + 'name', re_services.name, + 'price', payment_service_items.price_cents::TEXT, + 'status', payment_service_items.status, + 'rejection_reason', payment_service_items.rejection_reason, + 'requested_at', payment_service_items.requested_at, + 'denied_at', payment_service_items.denied_at, + 'sent_to_gex_at', payment_service_items.sent_to_gex_at, + 'paid_at', payment_service_items.paid_at, + 'shipment_id', move_shipments.id::TEXT, + 'shipment_id_abbr', move_shipments.shipment_id_abbr, + 'shipment_type', move_shipments.shipment_type, + 'shipment_locator', move_shipments.shipment_locator + ) + )::TEXT AS context, + payment_service_items.id AS id + FROM + payment_requests + JOIN payment_service_items ON payment_service_items.payment_request_id = payment_requests.id + JOIN move_service_items ON move_service_items.id = payment_service_items.mto_service_item_id + LEFT JOIN move_shipments ON move_shipments.id = move_service_items.mto_shipment_id + JOIN re_services ON move_service_items.re_service_id = re_services.id + WHERE + payment_requests.move_id = (SELECT move.id FROM move) + GROUP BY + payment_service_items.id + ), + payment_service_items_logs AS ( + SELECT DISTINCT + audit_history.*, + context AS context, + NULL AS context_id + FROM + audit_history + JOIN move_payment_service_items ON move_payment_service_items.id = audit_history.object_id + WHERE audit_history.table_name = 'payment_service_items' + ), move_proof_of_service_docs AS ( SELECT proof_of_service_docs.*, @@ -698,6 +737,11 @@ WITH move AS ( FROM payment_requests_logs UNION + SELECT + * + FROM + payment_service_items_logs + UNION SELECT * FROM diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItem.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx similarity index 94% rename from src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItem.jsx rename to src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx index 0977bb649ca..cb7909334e6 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItem.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx @@ -17,7 +17,7 @@ const formatChangedValues = (historyRecord) => { export default { action: a.UPDATE, - eventName: o.updatePaymentServiceItem, + eventName: o.updatePaymentServiceItemStatus, tableName: t.payment_service_items, getEventNameDisplay: () => { return
Updated Payment Service Item
; diff --git a/src/constants/MoveHistory/EventTemplates/index.js b/src/constants/MoveHistory/EventTemplates/index.js index d1d582e6f16..9418f981424 100644 --- a/src/constants/MoveHistory/EventTemplates/index.js +++ b/src/constants/MoveHistory/EventTemplates/index.js @@ -109,4 +109,4 @@ export { default as moveCancelerPPMShipments } from './MoveCanceler/MoveCanceler export { default as cancelMove } from './CancelMove/CancelMove'; export { default as cancelMoveMTOShipments } from './CancelMove/CancelMoveMTOShipments'; export { default as cancelMovePPMShipments } from './CancelMove/CancelMovePPMShipments'; -export { default as updatePaymentServiceItem } from './UpdatePaymentServiceItem/UpdatePaymentServiceItem'; +export { default as updatePaymentServiceItemStatus } from './UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus'; diff --git a/src/constants/MoveHistory/UIDisplay/Operations.js b/src/constants/MoveHistory/UIDisplay/Operations.js index 79f50d951aa..c4b3c91decc 100644 --- a/src/constants/MoveHistory/UIDisplay/Operations.js +++ b/src/constants/MoveHistory/UIDisplay/Operations.js @@ -34,7 +34,7 @@ export default { updateOrder: 'updateOrder', // ghc.yaml updateOrders: 'updateOrders', // internal.yaml updatePaymentRequestStatus: 'updatePaymentRequestStatus', - updatePaymentServiceItem: 'updatePaymentServiceItem', + updatePaymentServiceItemStatus: 'updatePaymentServiceItemStatus', updateReweigh: 'updateReweigh', updateServiceItemStatus: 'updateMTOServiceItemStatus', updateServiceItemSitEntryDate: 'updateServiceItemSitEntryDate', // ghc.yaml From 3d248185e064b2e578daf7bf6048463459b34aaf Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Wed, 15 Jan 2025 16:26:19 +0000 Subject: [PATCH 03/12] Added tests for approved/denied payment service items. --- .../UpdatePaymentServiceItemStatus.test.jsx | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx new file mode 100644 index 00000000000..55d83e79c77 --- /dev/null +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx @@ -0,0 +1,60 @@ +import { render, screen } from '@testing-library/react'; + +import getTemplate from 'constants/MoveHistory/TemplateManager'; +import o from 'constants/MoveHistory/UIDisplay/Operations'; +import a from 'constants/MoveHistory/Database/Actions'; +import t from 'constants/MoveHistory/Database/Tables'; + +describe('When approving/rejecting a payment service item', () => { + const rejectPaymentServiceItemRecord = { + action: a.UPDATE, + actionTstampClk: '2025-01-10T19:44:31.255Z', + actionTstampStm: '2025-01-10T19:44:31.253Z', + actionTstampTx: '2025-01-10T19:44:31.220Z', + context: [ + { + shipment_type: 'PPM', + shipment_locator: 'RQ38D4-01', + shipment_id_abbr: 'f10be', + }, + ], + changedValues: { + reason: 'Some reason', + status: 'DENIED', + }, + eventName: o.updatePaymentServiceItemStatus, + tableName: t.payment_service_items, + id: '2419f1db-3f8b-4823-974f-9aa4edb753da', + objectId: 'eee30fb1-dc66-4821-a17c-2ecf431ceb9d', + oldValues: { + id: 'eee30fb1-dc66-4821-a17c-2ecf431ceb9d', + ppm_shipment_id: '86329c14-564b-4580-94b9-8a2e80bccefc', + reason: null, + status: null, + }, + }; + + const approvePaymentServiceItemRecord = { ...rejectPaymentServiceItemRecord }; + approvePaymentServiceItemRecord.changedValues = { + status: 'APPROVED', + }; + + it('displays an approved payment service item', () => { + const template = getTemplate(approvePaymentServiceItemRecord); + + render(template.getDetails(approvePaymentServiceItemRecord)); + expect(screen.getByText('PPM shipment #RQ38D4-01')).toBeInTheDocument(); + expect(screen.getByText('Status')).toBeInTheDocument(); + expect(screen.getByText(': APPROVED')).toBeInTheDocument(); + }); + + it('displays a rejected payment service item and the rejection reason', () => { + const template = getTemplate(rejectPaymentServiceItemRecord); + + render(template.getDetails(rejectPaymentServiceItemRecord)); + expect(screen.getByText('Status')).toBeInTheDocument(); + expect(screen.getByText(': DENIED')).toBeInTheDocument(); + expect(screen.getByText('Reason')).toBeInTheDocument(); + expect(screen.getByText(': Some reason')).toBeInTheDocument(); + }); +}); From 869cd88438ad19eefab1fff3830ebf1e5d0dfee2 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Tue, 21 Jan 2025 17:06:05 +0000 Subject: [PATCH 04/12] Fixed unintentional change to gitlab-ci.yml (again) --- .gitlab-ci.yml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a465b1690e7..f963dd16e4f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -46,14 +46,6 @@ variables: #postgres: &postgres postgres:16.4 redis: &redis redis:5.0.6 - #RUNNER_TAG: &runner_tag milmove - RUNNER_TAG: &runner_tag milmove - DOCKER_RUNNER_TAG: &docker_runner_tag eks_cluster_runner - - postgres: &postgres postgres:16.4 - #postgres: &postgres postgres:16.4 - redis: &redis redis:5.0.6 - stages: - pre_checks - build @@ -300,6 +292,7 @@ stages: yarn install --frozen-lockfile --cache-folder ~/.cache/yarn ./node_modules/.bin/playwright install + sast: stage: pre_checks tags: From fc6e31e1884539a2668f243169145a20f84924b3 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Tue, 21 Jan 2025 14:27:10 -0500 Subject: [PATCH 05/12] Updated template to use correct prefix when rejecting, approving, or updating a service item. Also updated field mapping to include "denied_at" field. --- src/constants/MoveHistory/Database/FieldMappings.js | 1 + .../UpdatePaymentServiceItemStatus.jsx | 12 ++++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/constants/MoveHistory/Database/FieldMappings.js b/src/constants/MoveHistory/Database/FieldMappings.js index 5b9ece8b735..195972b8213 100644 --- a/src/constants/MoveHistory/Database/FieldMappings.js +++ b/src/constants/MoveHistory/Database/FieldMappings.js @@ -148,4 +148,5 @@ export default { approvals_requested_at: 'Approvals requested at', approved_at: 'Approved at', counseling_office_name: 'Counseling office', + denied_at: 'Denied at', }; diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx index cb7909334e6..c136377f847 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx @@ -19,8 +19,16 @@ export default { action: a.UPDATE, eventName: o.updatePaymentServiceItemStatus, tableName: t.payment_service_items, - getEventNameDisplay: () => { - return
Updated Payment Service Item
; + getEventNameDisplay: (historyRecord) => { + let actionPrefix = ''; + if (historyRecord.changedValues.status === 'DENIED') { + actionPrefix = 'Rejected'; + } else if (historyRecord.changedValues.status === 'APPROVED') { + actionPrefix = 'Approved'; + } else { + actionPrefix = 'Updated'; + } + return
{actionPrefix} Payment Service Item
; }, getDetails: (historyRecord) => { return ; From 58da051762f3213cc079a9feb42dbec4f9cb9d0d Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Wed, 22 Jan 2025 17:38:53 -0500 Subject: [PATCH 06/12] Added rejection reason to submitted payment request items in audit log, removed a bunch of unnecessary change tracking for payment items (approved_at, denied_at) --- ...it_history_table_for_payment_service_items.up.sql | 2 +- pkg/assets/sql_scripts/move_history_fetcher.sql | 6 ++---- src/constants/MoveHistory/Database/FieldMappings.js | 1 - .../UpdatePaymentServiceItemStatus.jsx | 12 ++++++++++-- src/pages/Office/MoveHistory/PaymentDetails.jsx | 7 +++++++ .../Office/MoveHistory/PaymentDetails.module.scss | 8 ++++++++ 6 files changed, 28 insertions(+), 8 deletions(-) diff --git a/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql index 1502e3302c8..a5875998071 100644 --- a/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql +++ b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql @@ -1 +1 @@ -SELECT add_audit_history_table(target_table := 'payment_service_items', audit_rows := BOOLEAN 't', audit_query_text := BOOLEAN 't', ignored_cols := ARRAY['created_at', 'updated_at']); \ No newline at end of file +SELECT add_audit_history_table(target_table := 'payment_service_items', audit_rows := BOOLEAN 't', audit_query_text := BOOLEAN 't', ignored_cols := ARRAY['created_at', 'updated_at', 'denied_at', 'requested_at', 'sent_to_gex_at']); \ No newline at end of file diff --git a/pkg/assets/sql_scripts/move_history_fetcher.sql b/pkg/assets/sql_scripts/move_history_fetcher.sql index 36584462a26..431aa0ef856 100644 --- a/pkg/assets/sql_scripts/move_history_fetcher.sql +++ b/pkg/assets/sql_scripts/move_history_fetcher.sql @@ -212,7 +212,8 @@ WITH move AS ( 'shipment_id', move_shipments.id::TEXT, 'shipment_id_abbr', move_shipments.shipment_id_abbr, 'shipment_type', move_shipments.shipment_type, - 'shipment_locator', move_shipments.shipment_locator + 'shipment_locator', move_shipments.shipment_locator, + 'rejection_reason', payment_service_items.rejection_reason ) )::TEXT AS context, payment_requests.id AS id, @@ -246,9 +247,6 @@ WITH move AS ( 'price', payment_service_items.price_cents::TEXT, 'status', payment_service_items.status, 'rejection_reason', payment_service_items.rejection_reason, - 'requested_at', payment_service_items.requested_at, - 'denied_at', payment_service_items.denied_at, - 'sent_to_gex_at', payment_service_items.sent_to_gex_at, 'paid_at', payment_service_items.paid_at, 'shipment_id', move_shipments.id::TEXT, 'shipment_id_abbr', move_shipments.shipment_id_abbr, diff --git a/src/constants/MoveHistory/Database/FieldMappings.js b/src/constants/MoveHistory/Database/FieldMappings.js index 195972b8213..5b9ece8b735 100644 --- a/src/constants/MoveHistory/Database/FieldMappings.js +++ b/src/constants/MoveHistory/Database/FieldMappings.js @@ -148,5 +148,4 @@ export default { approvals_requested_at: 'Approvals requested at', approved_at: 'Approved at', counseling_office_name: 'Counseling office', - denied_at: 'Denied at', }; diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx index c136377f847..c2f7608d388 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx @@ -12,7 +12,15 @@ const formatChangedValues = (historyRecord) => { ...getMtoShipmentLabel(historyRecord), }; - return { ...historyRecord, changedValues: newChangedValues }; + // Removed unneeded values to avoid clutter in audit log + if (newChangedValues.status === 'APPROVED') { + delete newChangedValues.rejection_reason; + } + + delete newChangedValues.status; + const newHistoryRecord = { ...historyRecord }; + delete newHistoryRecord.changedValues.status; + return { ...newHistoryRecord, changedValues: newChangedValues }; }; export default { @@ -21,7 +29,7 @@ export default { tableName: t.payment_service_items, getEventNameDisplay: (historyRecord) => { let actionPrefix = ''; - if (historyRecord.changedValues.status === 'DENIED') { + if (historyRecord.changedValues.rejection_reason !== null || historyRecord.changedValues.status === 'REJECTED') { actionPrefix = 'Rejected'; } else if (historyRecord.changedValues.status === 'APPROVED') { actionPrefix = 'Approved'; diff --git a/src/pages/Office/MoveHistory/PaymentDetails.jsx b/src/pages/Office/MoveHistory/PaymentDetails.jsx index 65af4dbe90c..807828ec635 100644 --- a/src/pages/Office/MoveHistory/PaymentDetails.jsx +++ b/src/pages/Office/MoveHistory/PaymentDetails.jsx @@ -24,6 +24,13 @@ const filterContextStatus = (context, statusToFilter) => {
{value.name}
{price.toFixed(2)}
+
+ {value.status === 'DENIED' ? ( +
+ Rejection Reason: + {value?.rejection_reason} +
+ ) : null}
, ); } diff --git a/src/pages/Office/MoveHistory/PaymentDetails.module.scss b/src/pages/Office/MoveHistory/PaymentDetails.module.scss index ba62a69fd09..8e347f56909 100644 --- a/src/pages/Office/MoveHistory/PaymentDetails.module.scss +++ b/src/pages/Office/MoveHistory/PaymentDetails.module.scss @@ -13,6 +13,7 @@ font-weight: 500; display: flex; justify-content: space-between; + flex-wrap: wrap; } .statusRow { display: flex; @@ -24,4 +25,11 @@ .rejectTimes { color: $error; } + .break { + flex-basis: 100%; + height: 0; + } + .rejectionReason { + text-indent: 1rem; + } } From 0e2c5d0a412bfb7671804ed03fa8a00e66906375 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Thu, 23 Jan 2025 12:14:18 -0500 Subject: [PATCH 07/12] Added tests for new rejection reason in payment details and for event title text. Fixed a potential bad if check in the actionPrefix section. --- .../UpdatePaymentServiceItemStatus.jsx | 11 +++++++- .../UpdatePaymentServiceItemStatus.test.jsx | 25 +++++++++++++++---- .../MoveHistory/PaymentDetails.test.jsx | 24 +++++++++++++++++- 3 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx index c2f7608d388..a6cd5072ae0 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx @@ -29,7 +29,16 @@ export default { tableName: t.payment_service_items, getEventNameDisplay: (historyRecord) => { let actionPrefix = ''; - if (historyRecord.changedValues.rejection_reason !== null || historyRecord.changedValues.status === 'REJECTED') { + + /** + * IF there is a rejection_reason present in the changedValues, then either the reason was updated (in which case the status will be undefined) + * OR it was just rejected, wither way we want the rejected prefix, second || condition is a "just in case" check, not sure if there's a state + * where status would be updated but not rejection_reason + */ + if ( + ('rejection_reason' in historyRecord.changedValues && historyRecord.changedValues.rejection_reason !== null) || + historyRecord.changedValues.status === 'REJECTED' + ) { actionPrefix = 'Rejected'; } else if (historyRecord.changedValues.status === 'APPROVED') { actionPrefix = 'Approved'; diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx index 55d83e79c77..a3814f809f4 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx @@ -19,7 +19,7 @@ describe('When approving/rejecting a payment service item', () => { }, ], changedValues: { - reason: 'Some reason', + rejection_reason: 'Some reason', status: 'DENIED', }, eventName: o.updatePaymentServiceItemStatus, @@ -42,18 +42,33 @@ describe('When approving/rejecting a payment service item', () => { it('displays an approved payment service item', () => { const template = getTemplate(approvePaymentServiceItemRecord); + render(template.getEventNameDisplay(approvePaymentServiceItemRecord)); + expect(screen.getByText('Approved Payment Service Item')).toBeInTheDocument(); + render(template.getDetails(approvePaymentServiceItemRecord)); expect(screen.getByText('PPM shipment #RQ38D4-01')).toBeInTheDocument(); - expect(screen.getByText('Status')).toBeInTheDocument(); - expect(screen.getByText(': APPROVED')).toBeInTheDocument(); + }); + + it('displays an updated payment service item', () => { + const updatedServiceItemRecord = { ...approvePaymentServiceItemRecord }; + delete updatedServiceItemRecord.changedValues.status; + delete updatedServiceItemRecord.changedValues.rejection_reason; + const template = getTemplate(updatedServiceItemRecord); + + render(template.getEventNameDisplay(updatedServiceItemRecord)); + expect(screen.getByText('Updated Payment Service Item')).toBeInTheDocument(); + + render(template.getDetails(updatedServiceItemRecord)); + expect(screen.getByText('PPM shipment #RQ38D4-01')).toBeInTheDocument(); }); it('displays a rejected payment service item and the rejection reason', () => { const template = getTemplate(rejectPaymentServiceItemRecord); + render(template.getEventNameDisplay(rejectPaymentServiceItemRecord)); + expect(screen.getByText('Rejected Payment Service Item')).toBeInTheDocument(); + render(template.getDetails(rejectPaymentServiceItemRecord)); - expect(screen.getByText('Status')).toBeInTheDocument(); - expect(screen.getByText(': DENIED')).toBeInTheDocument(); expect(screen.getByText('Reason')).toBeInTheDocument(); expect(screen.getByText(': Some reason')).toBeInTheDocument(); }); diff --git a/src/pages/Office/MoveHistory/PaymentDetails.test.jsx b/src/pages/Office/MoveHistory/PaymentDetails.test.jsx index fca870071c2..f9f38895d70 100644 --- a/src/pages/Office/MoveHistory/PaymentDetails.test.jsx +++ b/src/pages/Office/MoveHistory/PaymentDetails.test.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { render, screen } from '@testing-library/react'; +import { render, screen, act } from '@testing-library/react'; import PaymentDetails from './PaymentDetails'; @@ -47,4 +47,26 @@ describe('PaymentDetails', () => { expect(screen.getByText(156.78, { exact: false })).toBeInTheDocument(); }); + + describe('rejected service items', () => { + const context = [ + { + name: 'Domestic uncrating', + price: '5555', + status: 'DENIED', + rejection_reason: 'some reason', + }, + ]; + it('renders a rejected service item and its rejection reason', async () => { + render(); + + expect(screen.getByText('Domestic uncrating')).toBeInTheDocument(); + + expect(screen.getByText('Rejection Reason:')).toBeInTheDocument(); + await act(() => { + screen.getByText('Rejection Reason:').click(); + }); + expect(screen.getByText('some reason')).toBeVisible(); + }); + }); }); From a8e1a8b76d8d3c97b62b98993e7bf250b151f7c3 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Thu, 23 Jan 2025 20:57:15 +0000 Subject: [PATCH 08/12] Updated go code and frontend code to catch edge case where if TIO clicked "cleared selection" on a service item it would show as rejected. Added unit test for this case. --- .../payment_service_item_status_updater.go | 19 +++++++++++-------- .../UpdatePaymentServiceItemStatus.jsx | 8 +++++--- .../UpdatePaymentServiceItemStatus.test.jsx | 13 +++++++++++++ 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/pkg/services/payment_service_item/payment_service_item_status_updater.go b/pkg/services/payment_service_item/payment_service_item_status_updater.go index b0d822ea7e2..0d064a6cc65 100644 --- a/pkg/services/payment_service_item/payment_service_item_status_updater.go +++ b/pkg/services/payment_service_item/payment_service_item_status_updater.go @@ -70,21 +70,24 @@ func (p *paymentServiceItemUpdater) updatePaymentServiceItem(appCtx appcontext.A return models.PaymentServiceItem{}, nil, verr } - // If we're denying this thing we want to make sure to update the DeniedAt field and nil out ApprovedAt. - if desiredStatus == models.PaymentServiceItemStatusDenied { + switch desiredStatus { + // when the user hits "clear selection" we want to clear all the fields + case models.PaymentServiceItemStatusRequested: + paymentServiceItem.RejectionReason = nil + paymentServiceItem.DeniedAt = nil + paymentServiceItem.ApprovedAt = nil + // if being denied, we want to nil out approvedAt and populate deniedAt + case models.PaymentServiceItemStatusDenied: paymentServiceItem.RejectionReason = rejectionReason paymentServiceItem.DeniedAt = models.TimePointer(time.Now()) paymentServiceItem.ApprovedAt = nil - paymentServiceItem.Status = desiredStatus - } - // If we're approving this thing then we don't want there to be a rejection reason - // We also will want to update the ApprovedAt field and nil out the DeniedAt field. - if desiredStatus == models.PaymentServiceItemStatusApproved { + // if being approved, populate approvedAt + case models.PaymentServiceItemStatusApproved: paymentServiceItem.RejectionReason = nil paymentServiceItem.DeniedAt = nil paymentServiceItem.ApprovedAt = models.TimePointer(time.Now()) - paymentServiceItem.Status = desiredStatus } + paymentServiceItem.Status = desiredStatus // Save the record verrs, err := appCtx.DB().ValidateAndSave(&paymentServiceItem) diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx index a6cd5072ae0..5f92c395e70 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx @@ -13,10 +13,9 @@ const formatChangedValues = (historyRecord) => { }; // Removed unneeded values to avoid clutter in audit log - if (newChangedValues.status === 'APPROVED') { + if (newChangedValues.status === 'APPROVED' || newChangedValues.status === 'REQUESTED') { delete newChangedValues.rejection_reason; } - delete newChangedValues.status; const newHistoryRecord = { ...historyRecord }; delete newHistoryRecord.changedValues.status; @@ -36,7 +35,10 @@ export default { * where status would be updated but not rejection_reason */ if ( - ('rejection_reason' in historyRecord.changedValues && historyRecord.changedValues.rejection_reason !== null) || + ('rejection_reason' in historyRecord.changedValues && + historyRecord.changedValues.rejection_reason !== null && + historyRecord.changedValues.status !== 'APPROVED' && + historyRecord.changedValues.status !== 'REQUESTED') || historyRecord.changedValues.status === 'REJECTED' ) { actionPrefix = 'Rejected'; diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx index a3814f809f4..1d0c3216a91 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.test.jsx @@ -72,4 +72,17 @@ describe('When approving/rejecting a payment service item', () => { expect(screen.getByText('Reason')).toBeInTheDocument(); expect(screen.getByText(': Some reason')).toBeInTheDocument(); }); + + it('displays a cleared payment service item with no unneeded information', () => { + const clearedServiceItem = rejectPaymentServiceItemRecord; + clearedServiceItem.changedValues.status = 'REQUESTED'; + const template = getTemplate(clearedServiceItem); + + render(template.getEventNameDisplay(clearedServiceItem)); + expect(screen.getByText('Updated Payment Service Item')).toBeInTheDocument(); + + render(template.getDetails(clearedServiceItem)); + expect(screen.queryByText('Reason')).not.toBeInTheDocument(); + expect(screen.queryByText(': Some reason')).not.toBeInTheDocument(); + }); }); From 976d82366b7aff4dba0824d2193264de28fcca41 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Thu, 23 Jan 2025 21:27:11 +0000 Subject: [PATCH 09/12] Undo backend code change --- .../payment_service_item_status_updater.go | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/pkg/services/payment_service_item/payment_service_item_status_updater.go b/pkg/services/payment_service_item/payment_service_item_status_updater.go index 0d064a6cc65..b0d822ea7e2 100644 --- a/pkg/services/payment_service_item/payment_service_item_status_updater.go +++ b/pkg/services/payment_service_item/payment_service_item_status_updater.go @@ -70,24 +70,21 @@ func (p *paymentServiceItemUpdater) updatePaymentServiceItem(appCtx appcontext.A return models.PaymentServiceItem{}, nil, verr } - switch desiredStatus { - // when the user hits "clear selection" we want to clear all the fields - case models.PaymentServiceItemStatusRequested: - paymentServiceItem.RejectionReason = nil - paymentServiceItem.DeniedAt = nil - paymentServiceItem.ApprovedAt = nil - // if being denied, we want to nil out approvedAt and populate deniedAt - case models.PaymentServiceItemStatusDenied: + // If we're denying this thing we want to make sure to update the DeniedAt field and nil out ApprovedAt. + if desiredStatus == models.PaymentServiceItemStatusDenied { paymentServiceItem.RejectionReason = rejectionReason paymentServiceItem.DeniedAt = models.TimePointer(time.Now()) paymentServiceItem.ApprovedAt = nil - // if being approved, populate approvedAt - case models.PaymentServiceItemStatusApproved: + paymentServiceItem.Status = desiredStatus + } + // If we're approving this thing then we don't want there to be a rejection reason + // We also will want to update the ApprovedAt field and nil out the DeniedAt field. + if desiredStatus == models.PaymentServiceItemStatusApproved { paymentServiceItem.RejectionReason = nil paymentServiceItem.DeniedAt = nil paymentServiceItem.ApprovedAt = models.TimePointer(time.Now()) + paymentServiceItem.Status = desiredStatus } - paymentServiceItem.Status = desiredStatus // Save the record verrs, err := appCtx.DB().ValidateAndSave(&paymentServiceItem) From abafe052c62d07728a3c7828d9a8ba85c262d7d1 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Fri, 24 Jan 2025 15:10:49 +0000 Subject: [PATCH 10/12] Use constant variables instead of strings --- .../UpdatePaymentServiceItemStatus.jsx | 5 +++-- src/pages/Office/MoveHistory/PaymentDetails.jsx | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx index 5f92c395e70..8733ec0bc30 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx @@ -5,6 +5,7 @@ import a from 'constants/MoveHistory/Database/Actions'; import t from 'constants/MoveHistory/Database/Tables'; import { getMtoShipmentLabel } from 'utils/formatMtoShipment'; import LabeledDetails from 'pages/Office/MoveHistory/LabeledDetails'; +import { PAYMENT_SERVICE_ITEM_STATUS } from 'shared/constants'; const formatChangedValues = (historyRecord) => { const newChangedValues = { @@ -39,10 +40,10 @@ export default { historyRecord.changedValues.rejection_reason !== null && historyRecord.changedValues.status !== 'APPROVED' && historyRecord.changedValues.status !== 'REQUESTED') || - historyRecord.changedValues.status === 'REJECTED' + historyRecord.changedValues.status === PAYMENT_SERVICE_ITEM_STATUS.DENIED ) { actionPrefix = 'Rejected'; - } else if (historyRecord.changedValues.status === 'APPROVED') { + } else if (historyRecord.changedValues.status === PAYMENT_SERVICE_ITEM_STATUS.APPROVED) { actionPrefix = 'Approved'; } else { actionPrefix = 'Updated'; diff --git a/src/pages/Office/MoveHistory/PaymentDetails.jsx b/src/pages/Office/MoveHistory/PaymentDetails.jsx index 807828ec635..f1b38320d23 100644 --- a/src/pages/Office/MoveHistory/PaymentDetails.jsx +++ b/src/pages/Office/MoveHistory/PaymentDetails.jsx @@ -25,7 +25,7 @@ const filterContextStatus = (context, statusToFilter) => {
{value.name}
{price.toFixed(2)}
- {value.status === 'DENIED' ? ( + {value.status === PAYMENT_SERVICE_ITEM_STATUS.DENIED ? (
Rejection Reason: {value?.rejection_reason} From 0efebf99693707eaa20bf62921467ca851783674 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Fri, 24 Jan 2025 15:10:49 +0000 Subject: [PATCH 11/12] Use constant variables instead of strings --- .../UpdatePaymentServiceItemStatus.jsx | 5 +++-- src/pages/Office/MoveHistory/PaymentDetails.jsx | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx index 5f92c395e70..8733ec0bc30 100644 --- a/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx +++ b/src/constants/MoveHistory/EventTemplates/UpdatePaymentServiceItem/UpdatePaymentServiceItemStatus.jsx @@ -5,6 +5,7 @@ import a from 'constants/MoveHistory/Database/Actions'; import t from 'constants/MoveHistory/Database/Tables'; import { getMtoShipmentLabel } from 'utils/formatMtoShipment'; import LabeledDetails from 'pages/Office/MoveHistory/LabeledDetails'; +import { PAYMENT_SERVICE_ITEM_STATUS } from 'shared/constants'; const formatChangedValues = (historyRecord) => { const newChangedValues = { @@ -39,10 +40,10 @@ export default { historyRecord.changedValues.rejection_reason !== null && historyRecord.changedValues.status !== 'APPROVED' && historyRecord.changedValues.status !== 'REQUESTED') || - historyRecord.changedValues.status === 'REJECTED' + historyRecord.changedValues.status === PAYMENT_SERVICE_ITEM_STATUS.DENIED ) { actionPrefix = 'Rejected'; - } else if (historyRecord.changedValues.status === 'APPROVED') { + } else if (historyRecord.changedValues.status === PAYMENT_SERVICE_ITEM_STATUS.APPROVED) { actionPrefix = 'Approved'; } else { actionPrefix = 'Updated'; diff --git a/src/pages/Office/MoveHistory/PaymentDetails.jsx b/src/pages/Office/MoveHistory/PaymentDetails.jsx index 807828ec635..f1b38320d23 100644 --- a/src/pages/Office/MoveHistory/PaymentDetails.jsx +++ b/src/pages/Office/MoveHistory/PaymentDetails.jsx @@ -25,7 +25,7 @@ const filterContextStatus = (context, statusToFilter) => {
{value.name}
{price.toFixed(2)}
- {value.status === 'DENIED' ? ( + {value.status === PAYMENT_SERVICE_ITEM_STATUS.DENIED ? (
Rejection Reason: {value?.rejection_reason} From 115d4c57821311b77b63ff9d3c52f2f9fd90c195 Mon Sep 17 00:00:00 2001 From: Brooklyn Welsh Date: Tue, 28 Jan 2025 21:34:25 +0000 Subject: [PATCH 12/12] Fixed missing ignored col in migration --- ..._create_audit_history_table_for_payment_service_items.up.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql index a5875998071..8318441ef8d 100644 --- a/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql +++ b/migrations/app/schema/20250109194140_create_audit_history_table_for_payment_service_items.up.sql @@ -1 +1 @@ -SELECT add_audit_history_table(target_table := 'payment_service_items', audit_rows := BOOLEAN 't', audit_query_text := BOOLEAN 't', ignored_cols := ARRAY['created_at', 'updated_at', 'denied_at', 'requested_at', 'sent_to_gex_at']); \ No newline at end of file +SELECT add_audit_history_table(target_table := 'payment_service_items', audit_rows := BOOLEAN 't', audit_query_text := BOOLEAN 't', ignored_cols := ARRAY['created_at', 'updated_at', 'denied_at', 'requested_at', 'sent_to_gex_at', 'approved_at']); \ No newline at end of file