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

[16.0][FIX] shopinvader_search_engine: Fix categories into products #1480

Merged

Conversation

lmignon
Copy link
Collaborator

@lmignon lmignon commented Nov 30, 2023

  • Improve perfs?

@lmignon lmignon marked this pull request as draft November 30, 2023 21:19
@lmignon
Copy link
Collaborator Author

lmignon commented Nov 30, 2023

@@ -16,3 +16,25 @@ def _filter_by_index(self):
in rec.se_binding_ids.mapped("index_id").ids
)
return records

def _filter_by_bound_in_same_lang(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please explain why, while working on a particular index, you would need to return records linked to another index (with the same backend and lang) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

when you serialize a product, you also serialize categories linked to the product. In this case, you're working with the product's index not the one from the category. If you add tests to check that the categories field is filled into the product information, you'll see that before this change it was always empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

There was a test, but it was wrong. I've corrected it (+ added one for the lang case).

I have also opened OCA/search-engine#177 to avoid such mistakes in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lmignon ping

@lmignon lmignon marked this pull request as ready for review December 14, 2023 12:34
@lmignon
Copy link
Collaborator Author

lmignon commented Dec 14, 2023

/ocabot merge patch

@shopinvader-git-bot
Copy link

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1480-by-lmignon-bump-patch, awaiting test results.

@shopinvader-git-bot shopinvader-git-bot merged commit 391d328 into shopinvader:16.0 Dec 14, 2023
3 checks passed
@shopinvader-git-bot
Copy link

Congratulations, your PR was merged at 458b51a. Thanks a lot for contributing to shopinvader. ❤️

@lmignon lmignon deleted the 16.0-fix-catagories-compute branch December 14, 2023 18:08
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

4 participants