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

Invoke with extensions issue with #75

Open
josephlewisnz opened this issue Nov 12, 2024 · 2 comments
Open

Invoke with extensions issue with #75

josephlewisnz opened this issue Nov 12, 2024 · 2 comments

Comments

@josephlewisnz
Copy link
Contributor

josephlewisnz commented Nov 12, 2024

Hi, Not sure if this is a bug or a feature!

But I am running into the issues where I have a data object that is NOT indexing.

Here's the scenario:

I have DocumentObject

And is configured to use the extension

DocumentObject:
  extensions:
    - Wilr\SilverStripe\Algolia\Extensions\AlgoliaObjectExtension

My DocumentObject class also just has it's won canIndexInAlgolia function eg.

    public function canIndexInAlgolia(): bool
    {
        return true ;
    }

The issue is that this class also for some unrelated reason (legacy) has the same "ShowInSearch" field.
My legacy ShowInSearch is flagged here as false.

if ($this->owner->hasField('ShowInSearch')) {

This basically means that due to the way the indexing is done and the "canIndexInAlgolia" is merged, this item can't be indexed

if (!$instance || min($instance->invokeWithExtensions('canIndexInAlgolia')) == false) {

image

Hopefully this makes sense.

Before I make any kind of pull request, I would be interest to know If there is a work around or something I'm missing?

I imagine changin the indexing logic drastically will be a major version change, so maybe an approach is to also include an extension hook before resolving, albiet this seems slightly against the normal way?

$instance = DataObject::get_by_id($item->ClassName, $item->ID);
if (!$instance) {
    $skipped++;

    continue;
}

$canIndexInAlgolia = min($instance->invokeWithExtensions('canIndexInAlgolia')) == false;
$instance->extend('updateCanIndexInAlgolia ', $canIndexInAlgolia );

if (!$canIndexInAlgolia ) {
    $skipped++;

    continue;
}


@matt-in-a-hat
Copy link
Contributor

As a possible work around you could try define a public function getShowInSearch() on your DataObject which returns true.

I've never fully understood the best practices for Silverstripe's use of invokeWithExtensions / extend.

An alternative change could be to make the ShowInSearch field name configurable on AlgoliaObjectExtension, and update canIndexInAlgolia to something like:

$flagField = $this->config()->get('index_flag_field_name');
if ($flagField && $this->owner->hasField($flagField)) {
    return $this->owner->$flagField;
}

return true;

@josephlewisnz
Copy link
Contributor Author

Thanks @matt-in-a-hat , yes I have actually already gone with a custom public function getShowInSearch() for now!

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

No branches or pull requests

2 participants