Skip to content

quality-debt: inc/checkout/class-checkout-pages.php — PR #380 review feedback (high) #402

Description

@superdav42

Unactioned Review Feedback

Source PR: #380
File: inc/checkout/class-checkout-pages.php
Reviewers: coderabbit
Findings: 1
Max severity: high


HIGH: coderabbit (coderabbitai[bot])

File: inc/checkout/class-checkout-pages.php:692
⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== wp_lostpassword_url callsites with non-empty argument =="
rg -nP --type=php 'wp_lostpassword_url\s*\(\s*[^)\s][^)]*\)' -C2

echo
echo "== filter_lostpassword_url implementation =="
rg -nP --type=php -C20 'function\s+filter_lostpassword_url\s*\('

echo
echo "== usages of \$redirect inside inc/checkout/class-checkout-pages.php =="
rg -nP '\$redirect' inc/checkout/class-checkout-pages.php -n -C2

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 5889


🏁 Script executed:

# Verify the current implementation of filter_lostpassword_url in the subsite branch
sed -n '644,682p' inc/checkout/class-checkout-pages.php

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 1269


🏁 Script executed:

# Check the WordPress filter hook documentation and how it passes the redirect parameter
rg -A5 'apply_filters.*lostpassword_url' --type=php

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 63


🏁 Script executed:

# Verify if there's any redirect_to handling in the custom login form or password reset flow
rg -n 'redirect_to|redirect_url' views/checkout/partials/inline-login-prompt.php

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 63


🏁 Script executed:

# Check if filter_login_url (the main site branch) actually preserves redirect properly
sed -n '589,620p' inc/checkout/class-checkout-pages.php

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 1241


🏁 Script executed:

# Verify WordPress filter hook behavior by checking how wp_lostpassword_url calls the filter
rg -n 'wp_lostpassword_url' --type=php -B5 -A5 | head -50

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 3465


🏁 Script executed:

# Check if there's any other redirect handling in the custom login form that might compensate
rg -n 'action.*lostpassword' views/checkout -A10 -B5

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 63


Preserve redirect_to parameter when filtering lost-password URLs on subsites.

The method accepts $redirect (line 644) and documents it in the docstring (line 641). The main site branch (line 655) properly passes it to filter_login_url(), but the subsite branch (lines 658–681) ignores it entirely. This creates inconsistent behavior: callers passing a redirect target (e.g., views/checkout/partials/inline-login-prompt.php:52) will have that parameter silently dropped on subsites.

Proposed fix
$subsite_lostpassword_url = add_query_arg(
 	'action',
 	'lostpassword',
-	remove_query_arg(['action', 'error', 'checkemail'], $current_url)
+	remove_query_arg(['action', 'error', 'checkemail', 'redirect_to'], $current_url)
 );
+
+if ( ! empty($redirect)) {
+	$subsite_lostpassword_url = add_query_arg('redirect_to', $redirect, $subsite_lostpassword_url);
+}
 
 return $subsite_lostpassword_url;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	public function filter_lostpassword_url($lostpassword_url, $redirect = '') {

		if ( ! did_action('wp_loaded')) {
			return $lostpassword_url;
		}

		/*
		 * On the main site, use the custom login page (same as filter_login_url).
		 * Pass an empty string for $force_reauth since it's not relevant here.
		 */
		if (is_main_site()) {
			return $this->filter_login_url($lostpassword_url, $redirect, false);
		}

		/*
		 * On subsites, keep the user on their own domain.
		 *
		 * wp_lostpassword_url() generates a URL pointing to the main site's
		 * wp-login.php. Instead, we build the URL from the current page so
		 * the user stays on the subsite throughout the password reset flow.
		 *
		 * wu_get_current_url() returns the current page URL. We strip any
		 * existing action/error params and add action=lostpassword so the
		 * custom login form element renders the lost-password view.
		 */
		$current_url = wu_get_current_url();

		if ( ! $current_url) {
			return $lostpassword_url;
		}

		$subsite_lostpassword_url = add_query_arg(
			'action',
			'lostpassword',
			remove_query_arg(['action', 'error', 'checkemail', 'redirect_to'], $current_url)
		);

		if ( ! empty($redirect)) {
			$subsite_lostpassword_url = add_query_arg('redirect_to', $redirect, $subsite_lostpassword_url);
		}

		return $subsite_lostpassword_url;
	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@inc/checkout/class-checkout-pages.php` around lines 644 - 682, The
filter_lostpassword_url method drops the $redirect parameter for subsites; when
building $subsite_lostpassword_url (using wu_get_current_url(), remove_query_arg
and add_query_arg) ensure you preserve and append the redirect target by adding
a redirect_to query arg when $redirect is non-empty (or pass $redirect through
to add_query_arg alongside the existing action param), so callers that provide
$redirect receive the redirect_to param on subsites just like the main-site
branch does via filter_login_url.

✅ Addressed in commits 85bce43 to 8fce1c0

View comment


Auto-generated by quality-feedback-helper.sh scan-merged. Review each finding and either fix the code or dismiss with a reason.

Metadata

Metadata

Assignees

Labels

priority:highHigh severity — significant quality issuequality-debtUnactioned review feedback from merged PRssource:review-feedbackAuto-created by quality-feedback-helper.sh

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions