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.

Sign up for GitHub

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

Jump to bottom

IBX-7236: Suggestions only work with entire words #37

Merged
merged 6 commits into from
Jan 8, 2024
Merged

IBX-7236: Suggestions only work with entire words #37

merged 6 commits into from
Jan 8, 2024

Conversation

kisztof
Copy link
Contributor

@kisztof kisztof commented Nov 29, 2023 •
edited
Loading

Question Answer
Tickets IBX-7236
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

This PR fixes:

Checklist:

  • Implement tests
  • Coding standards ($ composer fix-cs)

@ciastektk ciastektk requested a review from a team November 29, 2023 10:54
konradoboza
konradoboza approved these changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@konradoboza konradoboza left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Apart from failing CI.

adamwojs
adamwojs requested changes Nov 29, 2023
View reviewed changes
Copy link
Member

@adamwojs adamwojs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Could you please add test case for a fixed issue and add meaningful PR title ?

src/bundle/Controller/SuggestionController.php Outdated Show resolved Hide resolved
@bogusez bogusez self-assigned this Nov 29, 2023
@webhdx
Copy link
Contributor

webhdx commented Nov 29, 2023

@kisztof Can you fix PHPStan errors? You only need to remove some ignored entries.

@webhdx webhdx requested a review from adamwojs November 29, 2023 12:24
@bogusez
Copy link

bogusez commented Nov 29, 2023

@kisztof here you have several examples where search suggestions still does not work correctly and one which works good (I have added screenshot newFolder_correct to give you comparison that some phrases are treated correctly by search engine in terms of suggestions).

Incorrect behaviour:

Screenshot 2023-11-29 at 13 04 18 Screenshot 2023-11-29 at 13 17 36 Screenshot 2023-11-29 at 13 19 10 Screenshot 2023-11-29 at 13 20 32

Correct behaviour:

newFolder_correct

@webhdx
Copy link
Contributor

webhdx commented Nov 29, 2023

I think you should wrap the query with wildcard character on both sides. Righty now, you essentially search for content items where the name is prefixed with the query.

This was referenced Nov 30, 2023
Closed
Closed
Closed
@adamwojs
Copy link
Member

@kisztof Could you please address my comment #37 (review) ?

Could you please add test case for a fixed issue and add meaningful PR title ?

@kisztof kisztof changed the title IBX-7236 search min query IBX-7236: Suggestions only work with entire words Dec 1, 2023
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Dec 1, 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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

This was referenced Dec 1, 2023
Closed
Closed
Closed
mikadamczyk
mikadamczyk requested changes Dec 22, 2023
View reviewed changes
src/lib/View/SearchViewFilter.php Outdated Show resolved Hide resolved
@webhdx webhdx force-pushed the ibx-7236-search-min-query branch from f557dff to b8240d3 Compare January 8, 2024 11:57
@webhdx webhdx requested a review from mikadamczyk January 8, 2024 11:57
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Jan 8, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

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

See analysis details on SonarCloud

bogusez
bogusez approved these changes Jan 8, 2024
View reviewed changes
Copy link

@bogusez bogusez left a comment

Choose a reason for hiding this comment

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

Regression tests passed

@webhdx webhdx merged commit 740d01f into main Jan 8, 2024
9 checks passed
@webhdx webhdx deleted the ibx-7236-search-min-query branch January 8, 2024 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webhdx webhdx webhdx approved these changes

@Steveb-p Steveb-p Steveb-p approved these changes

@Nattfarinn Nattfarinn Nattfarinn approved these changes

@ciastektk ciastektk ciastektk approved these changes

@konradoboza konradoboza konradoboza approved these changes

@bogusez bogusez bogusez approved these changes

@adamwojs adamwojs Awaiting requested review from adamwojs

@mikadamczyk mikadamczyk Awaiting requested review from mikadamczyk

Assignees

@kisztof kisztof

@ViniTou ViniTou

@bogusez bogusez

Labels
QA approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@kisztof @webhdx @bogusez @adamwojs @mikadamczyk @Steveb-p @Nattfarinn @ciastektk @konradoboza @ViniTou