Skip to content

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

Merged
kjac merged 2 commits intov14/devfrom
v14/feature/examine-powered-entity-search
Apr 3, 2024
Merged

Examine powered entity search for documents, media and members#15972
kjac merged 2 commits intov14/devfrom
v14/feature/examine-powered-entity-search

Conversation

@kjac
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@kjac kjac deleted the v14/feature/examine-powered-entity-search branch April 3, 2024 16:49
@bielu
Copy link
Copy Markdown
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
Copy Markdown
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.

3 participants