Skip to content

Upd. Integrations. Tutor LMS updated, registration_errors hook refactored. - #848

Open
alexandergull wants to merge 2 commits into
devfrom
tutor_lms_tutros_reg.ag
Open

Upd. Integrations. Tutor LMS updated, registration_errors hook refactored.#848
alexandergull wants to merge 2 commits into
devfrom
tutor_lms_tutros_reg.ag

Conversation

@alexandergull

Copy link
Copy Markdown
Member

No description provided.

@alexandergull
alexandergull requested a review from Copilot August 4, 2026 12:39
@alexandergull alexandergull changed the title tutor_lms_tutros_reg.ag Upd. Integrations. Tutor LMS updated, registration_errors hook refactored. Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the WordPress registration anti-spam flow by extracting the ct_registration_errors() logic into a dedicated WPRegistrationErrors class and adds a focused PHPUnit test suite to cover key registration, skip, allow/deny, and credential-handling paths (including Tutor LMS-specific cases).

Changes:

  • Replaced the large procedural body of ct_registration_errors() with a call to WPRegistrationErrors::handle().
  • Added Cleantalk\Antispam\IntegrationsByClass\WPRegistrationErrors implementing the registration error handling logic.
  • Added a new PHPUnit test set under tests/Antispam/ to validate skip rules, credential detection, reg-flag behavior, allow/deny actions, and handler behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
inc/cleantalk-public-integrations.php Switches ct_registration_errors() to delegate to the new handler class.
lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php Introduces the new class encapsulating registration error handling behavior.
tests/Antispam/WPRegistrationErrorsTestBase.php Adds shared test setup/helpers for the new handler tests.
tests/Antispam/WPRegistrationErrorsSkipTest.php Adds tests for skip-condition logic in doSkipRequest().
tests/Antispam/WPRegistrationErrorsRegFlagTest.php Adds tests for reg-flag forcing and message building for non-registration modes.
tests/Antispam/WPRegistrationErrorsHandleTest.php Adds tests for handle() behavior (skip early return, missing ct_result, deny path).
tests/Antispam/WPRegistrationErrorsDenyTest.php Adds tests for deny-side behavior (WP_Error enrichment, Facebook payload cleanup).
tests/Antispam/WPRegistrationErrorsCredentialsTest.php Adds tests for BuddyPress/Facebook credential inference.
tests/Antispam/WPRegistrationErrorsAllowTest.php Adds a test for allow-side behavior (request id cookie/global).

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

Comment thread lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php
Comment thread lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php
Comment thread lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php
Comment thread lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php
Comment thread tests/Antispam/WPRegistrationErrorsDenyTest.php
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.93458% with 105 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.08%. Comparing base (a86e723) to head (410d5f5).
⚠️ Report is 9 commits behind head on dev.

Files with missing lines Patch % Lines
...ispam/IntegrationsByClass/WPRegistrationErrors.php 51.41% 103 Missing ⚠️
inc/cleantalk-public-integrations.php 0.00% 2 Missing ⚠️

❌ Your patch check has failed because the patch coverage (50.93%) 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              @@
##                dev     #848      +/-   ##
============================================
+ Coverage     27.63%   28.08%   +0.44%     
- Complexity     6061     6175     +114     
============================================
  Files           282      284       +2     
  Lines         25191    25326     +135     
============================================
+ Hits           6962     7112     +150     
+ Misses        18229    18214      -15     

☔ View full report in Codecov by Harness.
📢 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 AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php:182

  • isUserEnabled() returns true when apbct_is_user_enable() is false, which makes the method name misleading (and it’s public, so the mismatch can confuse external callers). Consider renaming it to reflect its semantics (e.g., isUserDisabled()/isLoggedIn()) or inverting the return and updating call sites accordingly.
    /**
     * @return bool
     */
    public function isUserEnabled()
    {
        return apbct_is_user_enable() === false;
    }

lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php:456

  • The onDenyActions() docblock says it can return void|null, but the method always returns $errors (which may be a WP_Error or any other value passed in). Updating the docblock makes the contract clearer for callers and static analysis.
    /**
     * @param CleantalkResponse $ct_result_object
     * @param $errors
     * @return mixed|void|null
     */

lib/Cleantalk/Antispam/IntegrationsByClass/WPRegistrationErrors.php:60

  • The credential-detection order differs from the previous implementation (BuddyPress runs before Facebook). If both BuddyPress fields and FB_userdata are present, BuddyPress will fill $sanitized_user_login/$user_email first, causing probablyFacebookCredentials() to return false and skipping Facebook-specific deny handling (e.g., clearing FB_userdata). To preserve the prior precedence and keep Facebook flow detection reliable, run Facebook detection before BuddyPress (or make probablyFacebookCredentials independent of whether the args were already filled).
        $this->bp_object = $this->defineBuddyPressObject();
        //BUDDYPRESS
        $this->is_buddypress = $this->probablyBuddyPressCredentials($sanitized_user_login, $user_email);
        //FACEBOOK
        $this->is_facebook = $this->probablyFacebookCredentials($sanitized_user_login, $user_email);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants