Add has_password arg to validate-counts command

ariskataoka commented 3 weeks ago

Description

To maintain consistent results between wp search health validate-counts and validate-contents, this PR adds the has_password and set it as null in the validate_index_posts_count,.

This modification will result in all posts being retrieved by default for the validate-counts, aligning its behavior with the validate-contents when the ES protected content feature isn't enabled.

Changelog Description

Fixed

Align results from search health validate-counts and validate-contents commands. The previous validate-counts behavior would count indexable posts, excluding protected ones. This change modifies the validate-counts behavior, as it will count all the indexable posts.

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 in Codespaces (or has an appropriate fallback).
[ ] This change works and has been tested on a 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

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 29.54%. Comparing base (0968d6b) to head (cc44f6e). Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
search/includes/classes/class-health.php	0.00%	7 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #5828 +/- ## ========================================== Coverage 29.53% 29.54% - Complexity 4780 4783 +3 ========================================== Files 282 282 Lines 20602 20597 -5 ========================================== Hits 6085 6085 + Misses 14517 14512 -5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ariskataoka commented 3 weeks ago

Build cov failures: https://github.com/Automattic/vip-go-mu-plugins/pull/5828/checks?check_run_id=29462553215 I've invested some time trying to add the unit tests for the method I changed, but it seems to be a bit tricky without refactoring the code.

sonarcloud[bot] commented 2 weeks ago

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

rebeccahum commented 2 weeks ago

@ariskataoka Hi! Thank you for this! How do I test it on dev-env? What I attempted so far: 1) Enabled protected_content 2) Published a post with a password and saw that when I ran wp vip-search health validate-counts, the post count was accurate

sathyapulse commented 2 weeks ago

@ariskataoka Hi! Thank you for this! How do I test it on dev-env? What I attempted so far:

Enabled protected_content

Published a post with a password and saw that when I ran wp vip-search health validate-counts, the post count was accurate

@rebeccahum Can you test without enabling the protected content feature?

rebeccahum commented 2 weeks ago

Gotcha, oops, I did the opposite!

So, in my testing, I have 1 normal post and 1 password protected post. The only thing with this approach is that it will always return with wp vip-search health validate-counts a missing one (which I'm thinking is to be expected):

Validating post count

❌  inconsistencies found when counting entity: post, type: post, index_version: 1 - (DB: 2, ES: 1, Diff: -1)

Which makes sense to me.

But if we do wp vip-search health validate-contents, it can't automatically fix it due to not having protected_content enabled:

Validating posts 1 - 9...✘ (attempted to reconcile)
Warning: Inconsistencies fixed:
id,type,issue
7,post,missing_from_index

Which might end up confusing the user further... One idea is to only count the protected posts if a specific flag is passed into the command. For example, wp vip-search health validate-counts --count-protected

rebeccahum commented 2 weeks ago

After much thinking, if we want to count protected posts, the protected_content feature should be activated. The validation commands are working as expected, so I think we can close this as a wontdo.

rebeccahum commented 2 weeks ago

Closing in favour of #5837

Automattic / vip-go-mu-plugins