Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search: Allow no active versions and change initial state to no versions #3963

Closed
wants to merge 14 commits into from

Conversation

rebeccahum
Copy link
Contributor

@rebeccahum rebeccahum commented Dec 8, 2022

Description

This change does the below:

  • Allows for a state of having no active versions in the index version management (before, we would default to 1 no matter what)
  • Adds a deactivate WP-CLI command: wp vip-search index-versions deactivate post 1
  • Defaults to having no index versions on initial state

If there's no active versions but search is enabled, search will fall back to DB.

Changelog Description

Plugin Updated: Enterprise Search

Add deactivate CLI command, allow a state of having no active indexes and set initial state to no versions

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.

Steps to Test

Note: when testing on dev-env, you'll need to run wp vip-search index --setup since the current version uses wp elasticsearch index --setup which doesn't take into account the changes in this PR. see Automattic/vip-container-images#390

  1. Run wp vip-search index-versions deactivate post <index-id> on an active version and use wp vip-search index-versions list post to verify it's no longer marked as active
  2. Run a search query via ?s=<searchTerm> and observe that in Search Dev Tools, there's no query run anymore
  3. Run wp vip-search index-versions delete post <index-id until there's no more indexes left in wp vip-search index-versions list post
  4. Run wp vip-search index --using-versions --setup --indexables=post and expect an error message returned
  5. Runwp vip-search index --setup and verify that a new version has been created per wp vip-search index-versions list post
  6. Repeat step 5 but with the expectation that the is only one index and it's active and different from the one in step 6)
  7. Runwp vip-search index --setup --indexables=post --version=<index id from step 6> and verify that no new indexes have been added and only the index from step 6 was indexed

@rebeccahum rebeccahum added [Status] Needs Review [Component] Search ChangelogTagID: 2373 [Status] Needs Docs Signal to Yoli the docs need to be updated labels Dec 8, 2022
@rebeccahum rebeccahum requested a review from a team as a code owner December 8, 2022 20:55
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #3963 (c855b2a) into develop (9a736d9) will increase coverage by 3.96%.
The diff coverage is 29.36%.

❗ Current head c855b2a differs from pull request most recent head cd0e9f6. Consider uploading reports for the commit cd0e9f6 to get more accurate results

@@              Coverage Diff              @@
##             develop    #3963      +/-   ##
=============================================
+ Coverage      28.52%   32.48%   +3.96%     
+ Complexity      4566     3874     -692     
=============================================
  Files            265      232      -33     
  Lines          20246    17138    -3108     
=============================================
- Hits            5775     5568     -207     
+ Misses         14471    11570    -2901     
Impacted Files Coverage Δ
...ch/includes/classes/commands/class-corecommand.php 0.00% <0.00%> (ø)
...includes/classes/commands/class-versioncommand.php 0.00% <0.00%> (ø)
search/includes/classes/class-versioning.php 86.39% <90.90%> (+0.04%) ⬆️
search/includes/classes/class-search.php 83.92% <100.00%> (+1.47%) ⬆️

... and 111 files with indirect coverage changes

@rebeccahum rebeccahum force-pushed the search_no_active_versions branch 8 times, most recently from 464ac40 to e898170 Compare January 4, 2023 00:15
@rebeccahum rebeccahum force-pushed the search_no_active_versions branch 12 times, most recently from d7225e5 to d5c37d3 Compare January 5, 2023 20:08
rinatkhaziev
rinatkhaziev previously approved these changes Feb 15, 2023
Copy link
Contributor

@rinatkhaziev rinatkhaziev left a comment

Choose a reason for hiding this comment

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

Manually tested in all possible scenarios I could come up with, seems to work great, but would still be nice to figure out the rollout strategy for this, as the change is pretty big.

@sonarcloud
Copy link

sonarcloud bot commented Mar 3, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
3.6% 3.6% Duplication

@sonarcloud
Copy link

sonarcloud bot commented May 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

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

Successfully merging this pull request may close these issues.

None yet

2 participants