Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Search: re-add support for 'lucky' patterntype#64293

Merged
jtibshirani merged 2 commits into
mainfrom
jtibs/lucky
Aug 6, 2024
Merged

Search: re-add support for 'lucky' patterntype#64293
jtibshirani merged 2 commits into
mainfrom
jtibs/lucky

Conversation

@jtibshirani

@jtibshirani jtibshirani commented Aug 6, 2024

Copy link
Copy Markdown
Contributor

In #64215, we removed support for smart search. We continued to allow the smart search mode, but just execute with precise behavior. However, we started to throw an error for patterntype:lucky (which is the old way of specifying smart search). It turns out that lucky made its way into some search links and settings, causing those searches to fail.

This PR restores support for lucky, and remaps it to standard. This preserves the semantics as much as possible (since smart search attempts a 'standard' search as its first rule).

Closes SRCH-840

Test plan

New unit test case. Tested manually with patterntype:lucky and checked semantics match `standard.

@cla-bot cla-bot Bot added the cla-signed label Aug 6, 2024
@github-actions github-actions Bot added team/product-platform team/search-platform Issues owned by the search platform team labels Aug 6, 2024
@jtibshirani

Copy link
Copy Markdown
Contributor Author

Note: we will show patterntype: lucky directly in the search bar, with a red squiggly, so users know they are doing something non-recommended.
Screenshot 2024-08-06 at 12 37 31 PM

@jtibshirani jtibshirani requested a review from a team August 6, 2024 09:43

@stefanhengl stefanhengl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We also removed "Lucky" from the list of allowed values of search.defaultPatternType. I guess we want to keep it that way?

@jtibshirani

Copy link
Copy Markdown
Contributor Author

We also removed "Lucky" from the list of allowed values of search.defaultPatternType. I guess we want to keep it that way?

I believe we will still honor existing settings with search.defaultPatternType: lucky though, which is good. So this seems fine to me.

Is there anywhere else we disallowed patterntype:lucky, like in some of the work you did to add patterntype to the DB?

@jtibshirani jtibshirani merged commit f19af57 into main Aug 6, 2024
@jtibshirani jtibshirani deleted the jtibs/lucky branch August 6, 2024 11:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/product-platform team/search-platform Issues owned by the search platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants