Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RPP / Verification / Connect the duplicate payment prevention service #7411

Closed
RadoslavGeorgiev opened this issue Oct 6, 2023 · 1 comment · Fixed by #7450
Closed

RPP / Verification / Connect the duplicate payment prevention service #7411

RadoslavGeorgiev opened this issue Oct 6, 2023 · 1 comment · Fixed by #7450
Assignees
Labels
category: core WC Payments core related issues, where it’s obvious. category: engineering For product engineering, architecture work, tech debt and so on. category: projects For any issues which are part of any project, including bugs, enhancements, etc. component: checkout Issues related to Checkout

Comments

@RadoslavGeorgiev
Copy link
Contributor

RadoslavGeorgiev commented Oct 6, 2023

Description

We should connect the new payment process with the WCPay\Duplicate_Payment_Prevention_Service (code).

To begin with, the service is relatively simple, and mostly independent. With that in mind, let's duplicate it to src under a new namespace, WCPay\Internal\Service\DuplicatePaymentPreventionService. There are too many small tweaks that need to be done on a relatively small and active piece of functionality, which is why it makes more sense to do it cleanly and independently.

  1. Copy the service to src and the new namespace.
  2. Add the service to WCPay\Internal\DependencyManagement\ServiceProvider\PaymentsServiceProvider.
  3. Remove the dependency on the gateway. $gateway->get_return_url() can be safely replaced with $order->get_checkout_order_received_url(). If coded properly, this should not be needed, as the gateway would be the one to generate those URLs.
  4. Instead of working directly with order objects, use the OrderService class. This will require new methods in the order service, seems like those would be:
    • get_intent_id( $order_id ) instead of accessing meta directly.
    • get_cart_hash( $order_id ) to compare orders directly.
    • is_paid( $order_id ) instead of $order->has_status( wc_get_is_paid_statuses() )
    • add_note( $order_id, $note )
    • delete( $order_id )
  5. Instead of using the legacy order service, add a proxy to update_order_status_from_intent to the new order service.

Now that the service is suitable for src, it will need to be implemented within the new payment process. This involves a few moving pieces:

  1. Linking all needed methods like maybe_update_session_processing_order, remove_session_processing_order, and clear_session_processing_order_after_landing_order_received_page in the right place. Those are essentially hooks to allow the service to store/clear the needed data.
  2. Detect and handle previously paid orders.
  3. Detect and handle successful intents.

Handling previously paid orders

Use check_against_session_processing_order to bail early if a similar order was already paid. This will require creating a new SimilarOrderDetectedState, and transitioning to it before any actions are performed. The gateway should know how to handle this state, especially regarding return URLs.

Ideally, the gateway should be able to do something like this:

// Entry into the new payment process.
$state = $state->process();

if ( $state instanceof SimilarOrderDetectedState ) {
	// Consider whether to use `wc_get_order()` directly here.
	$return_url = add_query_arg(
		DuplicatePaymentPreventionService::FLAG_PREVIOUS_ORDER_PAID,
		'yes',
		$this->get_return_url( wc_get_order( $context->get_previous_order_id() ) )
	);

	return [
		'result'   => 'success',
		'redirect' => $return_url,
	];
}

// This is already in place, handling the happy path.
if ( $state instanceof CompletedState ) {
	return [
		'result'   => 'success',
		'redirect' => $this->get_return_url( $order ),
	];
}

throw new Exception( 'Unknown state' );

Handling existing successful intents

Use check_payment_intent_attached_to_order_succeeded to use an existing intent, rather than creating a new one.

This one is a bit more complicated, as it will require splitting InitialState into InitialState (where verification and preparation are done), a PreparedState or VerifiedState where intents are created (this one will be skipped), and a Processed state. If there is an intent already, it should be loaded from the API, stored in the context, and then we'd transition straight to ProcessedState to complete post-processing.

Example:

class InitialState {
	public function process() {
		// ... preparation, other verifications.

		$previous_intent = $this->duplicate_payment_prevention_service->get_intent_attached_to_order_succeeded();
		if ( ! is_null( $previous_intent ) ) {
			$context->set_intent( $previous_intent );
			$next_state = $this->createState( ProcessedState::class );
			return $next_state->complete();
		}

		// ... Continue with customer creation/update, and intent creation.
		$next_state = $this->createSate( PreparedState::class );
		return $next_state->request_remote_object(); // name TBD.
	}
}

Acceptance criteria

  1. The DPPS lives in src.
  2. Both branches of the duplicate payment prevention service work.
  3. Everything fits into the right payment state.
  4. 100% unit tests coverage.

Testing instructions

Same as the standard duplicate payment duplication prevention service.

Dev notes

Something similar was already implemented in the states PoC: https://github.com/Automattic/woocommerce-payments/pull/6287/files#diff-3bb8d0eedea7f88f28b34f5a291c5bf8c29621f783eadbf0cde7498589569a77

Additional context

Epic: #6673
Project: paJDYF-9hL-p2

@RadoslavGeorgiev RadoslavGeorgiev added component: checkout Issues related to Checkout category: engineering For product engineering, architecture work, tech debt and so on. category: projects For any issues which are part of any project, including bugs, enhancements, etc. category: core WC Payments core related issues, where it’s obvious. labels Oct 6, 2023
@RadoslavGeorgiev RadoslavGeorgiev changed the title RPP / Verification / Connect the duplicate payment prevention RPP / Verification / Connect the duplicate payment prevention service Oct 7, 2023
@htdat
Copy link
Member

htdat commented Oct 17, 2023

I am going to add tests as well as fix Psalm errors. But it's ready to continue having more reviews. @RadoslavGeorgiev @Automattic/gamma @marcinbot

Update: this comment would go to PR instead #7450 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: core WC Payments core related issues, where it’s obvious. category: engineering For product engineering, architecture work, tech debt and so on. category: projects For any issues which are part of any project, including bugs, enhancements, etc. component: checkout Issues related to Checkout
Projects
None yet
2 participants