From f5007901465c4d1018fe05e16af39d6dffbfedf0 Mon Sep 17 00:00:00 2001 From: bruce aldridge Date: Tue, 3 Dec 2024 10:34:07 +1300 Subject: [PATCH] Update inquiry order notes to use inquiry specific content (#9828) Co-authored-by: Nagesh Pai Co-authored-by: Nagesh Pai <4162931+nagpai@users.noreply.github.com> --- changelog/fix-9612-inquiry-order-note | 4 ++ includes/class-wc-payments-order-service.php | 48 +++++++++++-- ...wc-payments-webhook-processing-service.php | 3 +- .../test-class-wc-payments-order-service.php | 71 ++++++++++++++++++- ...wc-payments-webhook-processing-service.php | 3 +- 5 files changed, 119 insertions(+), 10 deletions(-) create mode 100644 changelog/fix-9612-inquiry-order-note diff --git a/changelog/fix-9612-inquiry-order-note b/changelog/fix-9612-inquiry-order-note new file mode 100644 index 00000000000..3fce0a23430 --- /dev/null +++ b/changelog/fix-9612-inquiry-order-note @@ -0,0 +1,4 @@ +Significance: minor +Type: fix + +Order notes for inquiries have clearer content. diff --git a/includes/class-wc-payments-order-service.php b/includes/class-wc-payments-order-service.php index 195dbe115a7..c563877e830 100644 --- a/includes/class-wc-payments-order-service.php +++ b/includes/class-wc-payments-order-service.php @@ -314,15 +314,17 @@ public function mark_order_blocked_for_fraud( $order, $intent_id, $intent_status * @param string $amount The disputed amount – formatted currency value. * @param string $reason The reason for the dispute – human-readable text. * @param string $due_by The deadline for responding to the dispute - formatted date string. + * @param string $status The status of the dispute. * * @return void */ - public function mark_payment_dispute_created( $order, $charge_id, $amount, $reason, $due_by ) { + public function mark_payment_dispute_created( $order, $charge_id, $amount, $reason, $due_by, $status = '' ) { if ( ! is_a( $order, 'WC_Order' ) ) { return; } - $note = $this->generate_dispute_created_note( $charge_id, $amount, $reason, $due_by ); + $is_inquiry = strpos( $status, 'warning_' ) === 0; + $note = $this->generate_dispute_created_note( $charge_id, $amount, $reason, $due_by, $is_inquiry ); if ( $this->order_note_exists( $order, $note ) ) { return; } @@ -346,7 +348,8 @@ public function mark_payment_dispute_closed( $order, $charge_id, $status ) { return; } - $note = $this->generate_dispute_closed_note( $charge_id, $status ); + $is_inquiry = strpos( $status, 'warning_' ) === 0; + $note = $this->generate_dispute_closed_note( $charge_id, $status, $is_inquiry ); if ( $this->order_note_exists( $order, $note ) ) { return; @@ -1643,15 +1646,32 @@ private function generate_fraud_blocked_note( $order ): string { * @param string $amount The disputed amount – formatted currency value. * @param string $reason The reason for the dispute – human-readable text. * @param string $due_by The deadline for responding to the dispute - formatted date string. + * @param bool $is_inquiry Whether the dispute is an inquiry or not. * * @return string Note content. */ - private function generate_dispute_created_note( $charge_id, $amount, $reason, $due_by ) { + private function generate_dispute_created_note( $charge_id, $amount, $reason, $due_by, $is_inquiry = false ) { $dispute_url = $this->compose_dispute_url( $charge_id ); // Get merchant-friendly dispute reason description. $reason = WC_Payments_Utils::get_dispute_reason_description( $reason ); + if ( $is_inquiry ) { + return sprintf( + WC_Payments_Utils::esc_interpolated_html( + /* translators: %1: the disputed amount and currency; %2: the dispute reason; %3 the deadline date for responding to the inquiry */ + __( 'A payment inquiry has been raised for %1$s with reason "%2$s". Response due by %3$s.', 'woocommerce-payments' ), + [ + 'a' => '', + ] + ), + $amount, + $reason, + $due_by, + $dispute_url + ); + } + return sprintf( WC_Payments_Utils::esc_interpolated_html( /* translators: %1: the disputed amount and currency; %2: the dispute reason; %3 the deadline date for responding to dispute */ @@ -1672,15 +1692,31 @@ private function generate_dispute_created_note( $charge_id, $amount, $reason, $d * * @param string $charge_id The ID of the disputed charge associated with this order. * @param string $status The status of the dispute. + * @param bool $is_inquiry Whether the dispute is an inquiry or not. * * @return string Note content. */ - private function generate_dispute_closed_note( $charge_id, $status ) { + private function generate_dispute_closed_note( $charge_id, $status, $is_inquiry = false ) { $dispute_url = $this->compose_dispute_url( $charge_id ); + + if ( $is_inquiry ) { + return sprintf( + WC_Payments_Utils::esc_interpolated_html( + /* translators: %1: the dispute status */ + __( 'Payment inquiry has been closed with status %1$s. See payment status for more details.', 'woocommerce-payments' ), + [ + 'a' => '', + ] + ), + $status, + $dispute_url + ); + } + return sprintf( WC_Payments_Utils::esc_interpolated_html( /* translators: %1: the dispute status */ - __( 'Payment dispute has been closed with status %1$s. See dispute overview for more details.', 'woocommerce-payments' ), + __( 'Dispute has been closed with status %1$s. See dispute overview for more details.', 'woocommerce-payments' ), [ 'a' => '', ] diff --git a/includes/class-wc-payments-webhook-processing-service.php b/includes/class-wc-payments-webhook-processing-service.php index 0cad2ffe950..d9b0333c765 100644 --- a/includes/class-wc-payments-webhook-processing-service.php +++ b/includes/class-wc-payments-webhook-processing-service.php @@ -535,6 +535,7 @@ private function process_webhook_dispute_created( $event_body ) { $reason = $this->read_webhook_property( $event_object, 'reason' ); $amount_raw = $this->read_webhook_property( $event_object, 'amount' ); $evidence = $this->read_webhook_property( $event_object, 'evidence_details' ); + $status = $this->read_webhook_property( $event_object, 'status' ); $due_by = $this->read_webhook_property( $evidence, 'due_by' ); $order = $this->wcpay_db->order_from_charge_id( $charge_id ); @@ -558,7 +559,7 @@ private function process_webhook_dispute_created( $event_body ) { ); } - $this->order_service->mark_payment_dispute_created( $order, $charge_id, $amount, $reason, $due_by ); + $this->order_service->mark_payment_dispute_created( $order, $charge_id, $amount, $reason, $due_by, $status ); // Clear dispute caches to trigger a fetch of new data. $this->database_cache->delete( DATABASE_CACHE::DISPUTE_STATUS_COUNTS_KEY ); diff --git a/tests/unit/test-class-wc-payments-order-service.php b/tests/unit/test-class-wc-payments-order-service.php index d7c6ca45c0d..2eeaa50864e 100644 --- a/tests/unit/test-class-wc-payments-order-service.php +++ b/tests/unit/test-class-wc-payments-order-service.php @@ -867,6 +867,44 @@ public function test_mark_payment_dispute_created() { $this->assertCount( 2, $notes_2 ); } + + /** + * Tests if the payment was updated to show inquiry created. + */ + public function test_mark_payment_dispute_created_for_inquiry() { + // Arrange: Set the charge_id and reason, and the order status. + $charge_id = 'ch_123'; + $amount = '$123.45'; + $reason = 'product_not_received'; + $deadline = 'June 7, 2023'; + $order_status = Order_Status::ON_HOLD; + $dispute_status = 'warning_needs_response'; + + // Act: Attempt to mark payment dispute created. + $this->order_service->mark_payment_dispute_created( $this->order, $charge_id, $amount, $reason, $deadline, $dispute_status ); + + // Assert: Check that the order status was updated to on-hold status. + $this->assertTrue( $this->order->has_status( [ $order_status ] ) ); + + $notes = wc_get_order_notes( [ 'order_id' => $this->order->get_id() ] ); + + // Assert: Check that dispute order note was added with relevant info and link to dispute detail. + $this->assertStringNotContainsString( 'Payment has been disputed', $notes[0]->content ); + $this->assertStringContainsString( 'inquiry', $notes[0]->content ); + $this->assertStringContainsString( $amount, $notes[0]->content ); + $this->assertStringContainsString( 'Product not received', $notes[0]->content ); + $this->assertStringContainsString( $deadline, $notes[0]->content ); + $this->assertStringContainsString( '%2Fpayments%2Ftransactions%2Fdetails&id=ch_123" target="_blank" rel="noopener noreferrer">Response due by', $notes[0]->content ); + + // Assert: Check that order status change note was added. + $this->assertStringContainsString( 'Pending payment to On hold', $notes[1]->content ); + + // Assert: Applying the same data multiple times does not cause duplicate actions. + $this->order_service->mark_payment_dispute_created( $this->order, $charge_id, $amount, $reason, $deadline, $dispute_status ); + $notes_2 = wc_get_order_notes( [ 'order_id' => $this->order->get_id() ] ); + $this->assertCount( 2, $notes_2 ); + } + /** * Tests to make sure mark_payment_dispute_created exits if the order is invalid. */ @@ -909,7 +947,7 @@ public function test_mark_payment_dispute_closed_with_status_won() { // Assert: Check that the notes were updated. $notes = wc_get_order_notes( [ 'order_id' => $this->order->get_id() ] ); $this->assertStringContainsString( 'Pending payment to Completed', $notes[1]->content ); - $this->assertStringContainsString( 'Payment dispute has been closed with status won', $notes[0]->content ); + $this->assertStringContainsString( 'Dispute has been closed with status won', $notes[0]->content ); $this->assertStringContainsString( '%2Fpayments%2Ftransactions%2Fdetails&id=ch_123" target="_blank" rel="noopener noreferrer">dispute overview', $notes[0]->content ); // Assert: Applying the same data multiple times does not cause duplicate actions. @@ -937,7 +975,7 @@ public function test_mark_payment_dispute_closed_with_status_lost() { // Assert: Check that the notes were updated. $notes = wc_get_order_notes( [ 'order_id' => $this->order->get_id() ] ); $this->assertStringContainsString( 'On hold to Refunded', $notes[1]->content ); - $this->assertStringContainsString( 'Payment dispute has been closed with status lost', $notes[0]->content ); + $this->assertStringContainsString( 'Dispute has been closed with status lost', $notes[0]->content ); $this->assertStringContainsString( '%2Fpayments%2Ftransactions%2Fdetails&id=ch_123" target="_blank" rel="noopener noreferrer">dispute overview', $notes[0]->content ); // Assert: Check for created refund, and the amount is correct. @@ -951,6 +989,35 @@ public function test_mark_payment_dispute_closed_with_status_lost() { $this->assertCount( 3, $notes_2 ); } + + /** + * Tests if the order note was added to show inquiry closed. + */ + public function test_mark_payment_dispute_closed_with_status_warning_closed() { + // Arrange: Set the charge_id, dispute status, the order status, and update the order status. + $charge_id = 'ch_123'; + $status = 'warning_closed'; + $order_status = Order_Status::COMPLETED; + $this->order->update_status( Order_Status::ON_HOLD ); // When a dispute is created, the order status is changed to On Hold. + + // Act: Attempt to mark payment dispute created. + $this->order_service->mark_payment_dispute_closed( $this->order, $charge_id, $status ); + + // Assert: Check that the order status was left in on-hold status. + $this->assertTrue( $this->order->has_status( [ $order_status ] ) ); + + // Assert: Check that the notes were updated. + $notes = wc_get_order_notes( [ 'order_id' => $this->order->get_id() ] ); + $this->assertStringNotContainsString( 'Dispute has been closed with status won', $notes[0]->content ); + $this->assertStringContainsString( 'inquiry', $notes[0]->content ); + $this->assertStringContainsString( '%2Fpayments%2Ftransactions%2Fdetails&id=ch_123" target="_blank" rel="noopener noreferrer">payment status', $notes[0]->content ); + + // Assert: Applying the same data multiple times does not cause duplicate actions. + $this->order_service->mark_payment_dispute_closed( $this->order, $charge_id, $status ); + $notes_2 = wc_get_order_notes( [ 'order_id' => $this->order->get_id() ] ); + $this->assertCount( 3, $notes_2 ); + } + /** * Tests to make sure mark_payment_dispute_closed exits if the order is invalid. */ diff --git a/tests/unit/test-class-wc-payments-webhook-processing-service.php b/tests/unit/test-class-wc-payments-webhook-processing-service.php index d376b1491fd..2acb5c318ad 100644 --- a/tests/unit/test-class-wc-payments-webhook-processing-service.php +++ b/tests/unit/test-class-wc-payments-webhook-processing-service.php @@ -1240,6 +1240,7 @@ public function test_dispute_created_order_note() { 'charge' => 'test_charge_id', 'reason' => 'test_reason', 'amount' => 9900, + 'status' => 'test_status', 'evidence_details' => [ 'due_by' => 'test_due_by', ], @@ -1289,7 +1290,7 @@ public function test_dispute_closed_order_note() { ->method( 'add_order_note' ) ->with( $this->matchesRegularExpression( - '/Payment dispute has been closed with status test_status/' + '/Dispute has been closed with status test_status/' ) );