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

ISSUE-247: Fixes Search API twig extension #251

Merged
merged 3 commits into from
Jan 24, 2023
Merged

ISSUE-247: Fixes Search API twig extension #251

merged 3 commits into from
Jan 24, 2023

Conversation

DiegoPino
Copy link
Member

See #247

  • Better validating the Result type of data
  • Changed a limit
    Won't go crazy with Strawberry Flavor results

Needs probably more testing/more exception catching

- Checks the Type of the Solr item before trying to load it
- Deals with one possible exception (more to come)
- Raises the Facet number for now to 100. This can be an extra setting, per facet field i guess?
@DiegoPino DiegoPino requested a review from aksm November 16, 2022 22:46
@DiegoPino DiegoPino changed the base branch from main to 1.1.0 November 16, 2022 22:46
@DiegoPino DiegoPino added bug Something isn't working enhancement New feature or request Twig Extension Make those flowers reach the sun labels Nov 16, 2022
@DiegoPino DiegoPino added this to the 1.1.0 milestone Nov 16, 2022
Strange thing i learned late night
If i do entity loading/field extraction the current Open Drupal SESSION gets crazy and if the extension is being used on e.g a Display template the ADO is shown as anonymous user!
QueryInterface::PROCESSING_FULL changes everything triggering internally
Access Control and then affecting the normal flow.
This makes things also faster. We only return the Solr fields and the Entity ID
e.g entity:node/6795:en which allows us then, on the template IF needed to load if based on a pattern.

if ($result_object) {
foreach ($resultItem->getFields() as $field) {
$return['results'][$itemid]['fields'][$field->getFieldIdentifier()]
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like previously the key for the returned results was the node id, but it's now the $itemid, e.g. entity:node/15:en. Is that right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, because we can't load the Entity to get the real ID. We could split it upfront to get the entity but that said having a way to letting the user know (via the ID) what entity was fetched and the language might be good (means entity:node/15:en seems a way of doing that)

Copy link
Contributor

@aksm aksm left a comment

Choose a reason for hiding this comment

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

In testing on local, user session is preserved and permissions for unpublished ADOs are respected.

@aksm aksm merged commit 84c7bda into 1.1.0 Jan 24, 2023
@DiegoPino
Copy link
Member Author

Great!

@DiegoPino DiegoPino deleted the ISSUE-247 branch January 24, 2023 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request Twig Extension Make those flowers reach the sun
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants