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

scout:import broken by this change! #45

Closed
eminos opened this issue Feb 2, 2023 · 5 comments
Closed

scout:import broken by this change! #45

eminos opened this issue Feb 2, 2023 · 5 comments

Comments

@eminos
Copy link

eminos commented Feb 2, 2023

0234cd8

@jasonbosco
Copy link
Member

Sorry about the breakage and all the time-wasted :(

I've now rolled back the 5.2.0 release, and set 5.1.0 as the latest one.

CC: @arayiksmbatyan @karakhanyans

@karakhanyans
Copy link
Collaborator

Hi @eminos

We are currently investigating the problem, could you please give us more details about this problem, as we are not able to reproduce in our end.

We need your Laravel version, your PHP version.

Thanks.

@ricardo-lobo
Copy link

The problem is the usage of getDirty instead of getChanges

See https://github.com/laravel/framework/blob/78eb4dabcc03e189620c16f436358d41d31ae11f/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php#L1126

After a model is updated, getDirty will always return empty and it will not be updated to Typesense.

Using latest Laravel version and PHP 8.1

@eminos
Copy link
Author

eminos commented Feb 3, 2023

The problem is the usage of getDirty instead of getChanges

How would that help when artisan scout:import?
getChanges would, AFAIK, also return empty and thus not letting the typesense document import happen.

The issue seems to be that artisan scout:import is using that update method in TypesenseEngine.php and the PR "Reduce unnecessary API calls" doesn't address that.

@ricardo-lobo
Copy link

@eminos You are probably right. I haven't tested this to be sure. I noticed this yesterday because I have a test that checks if a product updated with certain fields is synced to Typesense.

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

5 participants