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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Examine powered entity search for documents, media and members #15972

Merged
merged 2 commits into from Apr 3, 2024

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Apr 2, 2024

Prerequisites

  • I have added steps to test this contribution in the description below

Description

This is the follow-up for #15951 - this time with Examine powering the entity search for documents, media and members. At the same time, this PR ensures that the entity search respects any user start node limitations for the current user.

Testing this PR

The results from the search endpoints in V14 should match the results in V13. In other words, upgrade a database from V13 to V14 and compare the search results.

Also test that user start nodes are being respected. Thus the searches should yield no document or media results "outside" of the current user start nodes.

Don't forget to rebuild the Examine indexes in both versions before comparing 馃槄

Copy link
Member

@bergmania bergmania left a comment

Choose a reason for hiding this comment

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

LGTM tests out good 馃挭

@bergmania
Copy link
Member

Testing the user start nodes are a bit cumbersome, as this is not exposed anywhere. Is this a missing feature somewhere?

In V13, it seems like we use examine search with ignored user start nodes, when searching with a datatype key and that datatype is configured to ignore them

@kjac
Copy link
Contributor Author

kjac commented Apr 3, 2024

Hep. I believe bypassing the user start nodes is only relevant for documet and media trees at this point.

image

We can amend the search endpoints later on if necessary, but for the time being they've been explicitly coded to respect user start nodes.

@kjac kjac merged commit 75b1d47 into v14/dev Apr 3, 2024
16 checks passed
@kjac kjac deleted the v14/feature/examine-powered-entity-search branch April 3, 2024 16:49
@bielu
Copy link
Contributor

bielu commented Apr 3, 2024

Just noticed this pr, I have aquestion:
Is it a goal to adding more examine tied abstraction, instead of hiding examine implementation?
IExamineEntitySearch imho should be just an implementation for IEntitySearch, so it can be easily swapped for any search provider.... 馃

@kjac
Copy link
Contributor Author

kjac commented Apr 4, 2024

Hi @bielu,

It's not a goal 馃槃 we're certainly still looking towards creating abstraction layers around search to make search providers easier to swap. To that end, IExamineEntitySearch is a bad name I agree. I'll see if I can come up with a better one.

Thanks for chipping in 馃憤

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

3 participants