Add log suppression to VIP_Request_Block

[maxschmeling]

Description

This PR introduces a new method that allows suppression of the logs. In case it's too noisy a new helper can be used in vip-config.php

/* vip-config.php */
// Supress the logs
VIP_Request_Block::should_log( false );

// Resume the logs
VIP_Request_Block::should_log( true );

Changelog Description

VIP_Request_Block Log Output Control

Added an option to suppress logging for requests blocked by VIP_Request_Block:

/* In vip-config.php */

// Supress the logs
VIP_Request_Block::should_log( false );

// Resume the logs
VIP_Request_Block::should_log( true );

Pre-review checklist

Please make sure the items below have been covered before requesting a review:

• This change works and has been tested locally (or has an appropriate fallback).
• This change works and has been tested on a Go sandbox.
• This change has relevant unit tests (if applicable).
• This change uses a rollout method to ease with deployment (if applicable - especially for large scale actions that require writes).
• This change has relevant documentation additions / updates (if applicable).
• I've created a changelog description that aligns with the provided examples.

Pre-deploy checklist

• VIP staff: Ensure any alerts added/updated conform to internal standards (see internal documentation).

Steps to Test

1. Check out the PR
2. Spin up a dev environment
3. Add VIP_Request_Block::should_log( false ); to vip-config.php
4. Add a block for a local request (I used user-agent, but there's probably a simpler option)
5. Make a request, verify it was blocked, then verify no log entry for the block
6. Remove the VIP_Request_Block::should_log( false ); line, repeat step 5, verify the block was logged.

[codecov]

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 29.42%. Comparing base (e52f028) to head (8778670).

Files	Patch %	Lines
lib/class-vip-request-block.php	20.00%	8 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop    #5478      +/-   ##
=============================================
+ Coverage      29.41%   29.42%   +0.01%     
- Complexity      4759     4764       +5     
=============================================
  Files            281      281              
  Lines          20533    20530       -3     
=============================================
+ Hits            6039     6041       +2     
+ Misses         14494    14489       -5     

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

[maxschmeling]

I wanted to add a test for this change, but there currently isn't a test for the error_log call and from what Copilot told me, I'd have to introduce a wrapper for it that can be mocked and then pass that into VIP_Request_Block . I can do that, but wanted to get feedback first.

[sjinks]

Yes, that is correct.

[sjinks]

Maybe smth like

public static function log( string $criteria, string $value ): void {
    // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
    error_log( sprintf( 'VIP Request Block: request was blocked based on "%s" with value of "%s"', $criteria, $value ) );
}

-				// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
-				error_log( sprintf( 'VIP Request Block: request was blocked based on "%s" with value of "%s"', $criteria, $value ) );
+				static::log( $criteria, $value );

Then in the test:

class MyRequestBlock extends VIP_Request_Block {
    static $log_called = false;

    public static function log( string $criteria, string $value ): void {
        self::$log_called = true;
    }
}

and in the test case in setUp() (or wherever is appropriate) you set MyRequestBlock::$log_called = false; and in a test case you do static::assertTrue( MyRequestBlock::$log_called ); or static::assertFalse( MyRequestBlock::$log_called );

This is probably easier than mocking.

[maxschmeling]

@sjinks I've tried a few different things but couldn't get the test to work. I haz no PHP skills.

If you could give me a little help getting that across the finish line, that would be much appreciated. Thanks

[rinatkhaziev]

@yolih for awareness

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud