Fix. Integrations. GiveWP now has bot detector scripts in iframes.

[alexandergull]

https://app.doboard.com/1/task/48285

[codecov]

Codecov Report

❌ Patch coverage is 27.53623% with 50 lines in your changes missing coverage. Please review.
✅ Project coverage is 14.54%. Comparing base (4beeecc) to head (e0a1f50).

Files with missing lines	Patch %	Lines
...pam/ScriptsIntegration/ScriptIntegrationPlugin.php	0.00%	19 Missing ⚠️
...k/Antispam/ScriptsIntegration/FluentFormScript.php	0.00%	12 Missing ⚠️
...ntalk/Antispam/ScriptsIntegration/GiveWPScript.php	0.00%	11 Missing ⚠️
.../ScriptsIntegration/CleantalkScriptsIntegrator.php	76.00%	6 Missing ⚠️
cleantalk.php	0.00%	2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (27.53%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff              @@
##                fix     #790      +/-   ##
============================================
+ Coverage     12.46%   14.54%   +2.07%     
- Complexity     5549     5581      +32     
============================================
  Files           263      267       +4     
  Lines         32472    27289    -5183     
============================================
- Hits           4049     3970      -79     
+ Misses        28423    23319    -5104     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

This PR introduces a small “scripts integration” framework to centralize conditional script injection for specific 3rd‑party plugins, and wires it into the plugin’s public (non-admin) execution path. It also adds a GiveWP-specific integration intended to ensure bot detector scripts are present for GiveWP donation form views (including iframe routes).

Changes:

• Added CleantalkScriptsIntegrator to select eligible integrations and register their hooks.
• Added a ScriptIntegrationPlugin base class plus concrete integrations for GiveWP and FluentForm.
• Replaced the inline FluentForm-specific hook wiring in cleantalk.php with the new integrator, and added PHPUnit coverage for the integrator.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/Antispam/Integrations/TestScriptsIntegrator.php	Adds tests for plugin selection and hook registration in the new scripts integrator.
lib/Cleantalk/Antispam/ScriptsIntegration/ScriptIntegrationPlugin.php	Introduces a base class for script integrations with common activation/context checks.
lib/Cleantalk/Antispam/ScriptsIntegration/GiveWPScript.php	Adds a GiveWP hook integration to enqueue the bot detector script.
lib/Cleantalk/Antispam/ScriptsIntegration/FluentFormScript.php	Moves FluentForm script injection logic into an integration class.
lib/Cleantalk/Antispam/ScriptsIntegration/CleantalkScriptsIntegrator.php	Implements integration discovery, filtering, and hook registration.
cleantalk.php	Switches public execution path to run the new scripts integrator instead of inline FluentForm handling.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

FluentFormScript::additionalChecks() currently returns $this->is_in_uri || (...), but CleantalkScriptsIntegrator::getPluginsToInline() already requires $plugin->is_in_uri to be true. That makes the logged-in/APBCT_FF_JS_SCRIPTS_LOAD branch ineffective (regression vs the previous condition in cleantalk.php where URI match was optional). Consider changing the integrator selection logic to allow (is_in_uri || additional_checks_passed) (and make the default additionalChecks() return false), or move the OR logic into an override of isInUri() so the integrator’s is_in_uri gate still works.

Suggested change

	public function additionalChecks()
	{
	return $this->is_in_uri || (
	function_exists('apbct_is_user_logged_in') &&
	apbct_is_user_logged_in() &&
	(defined('APBCT_FF_JS_SCRIPTS_LOAD') && APBCT_FF_JS_SCRIPTS_LOAD == true)
	);
	public function isInUri()
	{
	return parent::isInUri() || (
	function_exists('apbct_is_user_logged_in') &&
	apbct_is_user_logged_in() &&
	defined('APBCT_FF_JS_SCRIPTS_LOAD') &&
	APBCT_FF_JS_SCRIPTS_LOAD == true
	);
	}
	public function additionalChecks()
	{
	return false;

[Copilot]

getPluginsToInline() requires is_in_uri AND additional_checks_passed, which prevents integrations from supporting “URI match OR alternative condition” (e.g., the FluentForm case that previously allowed loading for logged-in users via APBCT_FF_JS_SCRIPTS_LOAD). If additionalChecks() is meant to extend/override the URI gating, update the condition to use ( $plugin->is_in_uri || $plugin->additional_checks_passed ) and adjust the base ScriptIntegrationPlugin::additionalChecks() default accordingly (e.g., return false) so scripts don’t load everywhere by default.

[Copilot]

getIntegrations() wraps creation of all integrations in a single try/catch; if one integration throws (e.g., missing dependency), it drops the entire list. To keep other integrations working, instantiate each integration in its own try/catch (or validate prerequisites before new ...()), and only skip the failing one; optionally log/trace the exception for debugging.

Suggested change

	try {
	$integrations = [
	new GiveWPScript(),
	new FluentFormScript(),
	];
	} catch (\Exception $e) {
	$integrations = [];
	}
	$integrations = [];
	try {
	$integrations[] = new GiveWPScript();
	} catch (\Throwable $e) {
	}
	try {
	$integrations[] = new FluentFormScript();
	} catch (\Throwable $e) {
	}

[Copilot]

The constructor throws a generic \Exception with a non-specific message when required properties aren’t set. Using a more specific exception type (e.g., \InvalidArgumentException) and including the concrete class name and the missing/invalid fields in the message would make integration failures much easier to diagnose (especially since getIntegrations() currently swallows exceptions).

Suggested change

	if (
	!isset($this->plugin_file) ||
	!is_string($this->plugin_file) ||
	!isset($this->uri_chunk) ||
	!is_string($this->uri_chunk) ||
	!isset($this->hook_name) ||
	!is_string($this->hook_name)
	) {
	throw new \Exception('Plugin file, URI chunk and hook name must be set');
	}
	$invalid_fields = array();
	if (!isset($this->plugin_file) || !is_string($this->plugin_file)) {
	$invalid_fields[] = 'plugin_file';
	}
	if (!isset($this->uri_chunk) || !is_string($this->uri_chunk)) {
	$invalid_fields[] = 'uri_chunk';
	}
	if (!isset($this->hook_name) || !is_string($this->hook_name)) {
	$invalid_fields[] = 'hook_name';
	}
	if (!empty($invalid_fields)) {
	throw new \InvalidArgumentException(
	sprintf(
	'Invalid script integration configuration for %s: missing or invalid field(s): %s',
	static::class,
	implode(', ', $invalid_fields)
	)
	);
	}

[Copilot]

This test file defines classes in the global namespace, while other tests in the same area use namespaces (e.g., tests/Antispam/Integrations/TestCleantalkPreprocessComment.php). To avoid global class-name collisions and keep test organization consistent, add an appropriate namespace ...; declaration and update references accordingly.