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

Fix search suggestions #110

Merged
merged 2 commits into from
Oct 30, 2024
Merged

Conversation

rhoerr
Copy link
Contributor

@rhoerr rhoerr commented Oct 29, 2024

Description (*)

Fixes autocomplete search suggestions pulling terms from non-searchable fields.

Cherry-pick of changes from 2.4-develop to release/1.x.

Related Pull Requests

#102
#104

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)

kuafucode and others added 2 commits October 29, 2024 18:26
* added additional check to ensure autocomplete suggests only searchable terms

---------

Co-authored-by: Ryan Sun <[email protected]>
* added additional check to ensure autocomplete suggests only searchable terms

* updated unitest for search suggest fix

* added type casting for isSuggestible

* reverted suggestible flag and fixed getSuggestFields method

* updated suggest contructor params to avoid bic

* updated unit test with new dependency productAttributeCollectionFactory for Suggestions

* add fieldProvider back to Suggestions constructor

---------

Co-authored-by: Ryan Sun <[email protected]>
@rhoerr rhoerr requested a review from a team as a code owner October 29, 2024 22:32
@fballiano
Copy link
Contributor

@rhoerr it seems to me the PR is complete, about the checkpoints, is there a README to write?

@rhoerr
Copy link
Contributor Author

rhoerr commented Oct 30, 2024

@rhoerr it seems to me the PR is complete, about the checkpoints, is there a README to write?

I don't think that applies for this one. This is the relevant module readme: https://github.com/mage-os/mageos-magento2/blob/e31c588dd262be976341fd80dfd675f901e3c605/app/code/Magento/Elasticsearch/README.md

That checkbox probably almost never applies, actually, except in cases where a core module is being added or completely overhauled. For us, any changes of that nature would be in separate modules and repositories to avoid BC issues.

@fballiano
Copy link
Contributor

the I think this is ready to merge

@fballiano
Copy link
Contributor

@rhoerr I'd squash this specific one, seems more appropriated to me

@rhoerr rhoerr merged commit 7fc2e31 into mage-os:release/1.x Oct 30, 2024
6 of 7 checks passed
@rhoerr rhoerr deleted the fix/search-suggestions branch October 30, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants