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

Not able to skip an object from being indexed #24

Closed
nicomengual opened this issue Apr 14, 2022 · 7 comments
Closed

Not able to skip an object from being indexed #24

nicomengual opened this issue Apr 14, 2022 · 7 comments
Labels
good first issue Good for newcomers

Comments

@nicomengual
Copy link

Description

Problems when trying to skip an object so it DOES NOT become part of the index. There's no apparent way to do it using the function toSearchableArray

Steps to reproduce

The problem seems to be the function toSearchableArray

if I want to bypass an object on that function (ie: I don't want it into the index):

1- Returning an empty array results in a malformed JASON error when syncing:

 Typesense\Exceptions\TypesenseClientError 

  Error importing document: Bad JSON: not a properly formed document.
  
  at vendor/typesense/laravel-scout-typesense-driver/src/Typesense.php:165

2- Returning FALSE results in an error of unexpected response type:

  TypeError 

  Typesense\Documents::Typesense\{closure}(): Argument #1 ($document) must be of type array, bool given

  at vendor/typesense/typesense-php/src/Documents.php:139

3- if I use the Scout method 'unsearchable()' function results in an ObjectNotFound exception:

 Typesense\Exceptions\ObjectNotFound 

  Could not find a document with id: 1105

  at vendor/typesense/typesense-php/src/ApiCall.php:347

Expected Behavior

Ideally, if we would like this to work like Algolia does with Laravel Scout, by using the unsearchable method and returning either an empty array or a false flag, we would be able to skip the object from being added to the index.

Actual Behavior

The import is interrupted with one of those exceptions and the collection is being indexed only up to the point of the first failure.

Metadata

Typsense Version: 0.2.22

OS: Ubuntu 22.04

@AbdullahFaqeir AbdullahFaqeir self-assigned this Apr 19, 2022
@AbdullahFaqeir AbdullahFaqeir added the good first issue Good for newcomers label Apr 19, 2022
nicomengual added a commit to nicomengual/laravel-scout-typesense-driver that referenced this issue Apr 20, 2022
This allows the current object to be skipped, just by returning false, empty array or null in the toSearchableArray method of the model.
Ref to issue: typesense#24
@nicomengual
Copy link
Author

@AbdullahFaqeir Here's one possible solution: nicomengual@c95a89a

@MeisamMulla
Copy link

MeisamMulla commented Sep 5, 2022

I was having this issue as well and found this in the documentation that helped:

https://laravel.com/docs/9.x/scout#conditionally-searchable-model-instances

@BrandonJamesBorders
Copy link

I've run into this issue as well. This package does not adequately support what @MeisamMulla linked as the delete record method still fires, and if the record has not been created, as is the case in my application, it will throw an error.

I've submitted a PR to address this issue: #32

@BrandonJamesBorders
Copy link

If you guys want to use this, you can update your composer.json

In require:
"typesense/laravel-scout-typesense-driver": "dev-patch-1"

Add or update your repositories:

"repositories": [
    {
        "type": "vcs",
        "url": "https://github.com/BrandonJamesBorders/laravel-scout-typesense-driver"
    }
]

Then run composer update

@karakhanyans
Copy link
Collaborator

Hi @nicomengual

This solution should work for you: https://laravel.com/docs/9.x/scout#conditionally-searchable-model-instances

Just add following inside your model and return false whenever you don't want to sync that model.

/**
 * Determine if the model should be searchable.
 *
 * @return bool
 */
public function shouldBeSearchable()
{
    return $this->isPublished();
}

Let me know if this solves your problem.

Thanks.

@manavo
Copy link
Contributor

manavo commented Feb 27, 2023

@karakhanyans I think you're right with the shouldBeSearchable solution, so I think this issue can be closed

@karakhanyans
Copy link
Collaborator

Closing this issue due to provided solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants