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

Check methods before calling #262

Merged
merged 7 commits into from
Jun 5, 2023
Merged

Check methods before calling #262

merged 7 commits into from
Jun 5, 2023

Conversation

joshuabaker
Copy link
Contributor

@joshuabaker joshuabaker commented Jun 1, 2023

Fixes #257.

Key aim here is to prevent the Calling unknown method errors that keep surfacing. It’s assumed that this is a result of other plugins clashing in some manner.

The solution is guarding the behavior method calls with method_exists.

janhenckens and others added 6 commits October 27, 2022 21:08
Signed-off-by: Jan Henckens <jan@statik.be>
Signed-off-by: Jan Henckens <jan@statik.be>
Signed-off-by: Jan Henckens <jan@statik.be>
@joshuabaker joshuabaker changed the title Fixes #257 Check methods before calling Jun 1, 2023
@janhenckens janhenckens self-requested a review June 3, 2023 10:17
@janhenckens
Copy link
Member

Hey @joshuabaker, thanks for the PR. That's probably the only thing we can do in Scout to prevent crashes.

Did you intentionally make this for the Craft 3 branch? I'll release it for both version but just checking.
Thanks!

@joshuabaker
Copy link
Contributor Author

Thanks, @janhenckens.

I actually just assumed the Craft 4 version was a rewrite. Feel free to just cheery pick/copy and paste my changes, and discard this PR if that’s easier.

@janhenckens janhenckens changed the base branch from craft3 to develop June 4, 2023 08:58
@janhenckens
Copy link
Member

Wish it was a rewrite but haven't been able to spend much, if any, time on the plugin.
I'll make sure it get's merged for 3 & 4!

@janhenckens janhenckens merged commit 2f66dab into studioespresso:develop Jun 5, 2023
0 of 4 checks passed
@janhenckens
Copy link
Member

@joshuabaker This is out for Craft 4 in 3.1.1. The Craft 3 branch is bit diverged so still looking at merging it there.

@joshuabaker
Copy link
Contributor Author

Hey @janhenckens. Is this in the Craft 3 branch yet?

@janhenckens
Copy link
Member

Not yet.

I have a beta version out for Craft 3 that moves the initial "Should we index this element" check to a queue job. If you're good with that I can merge it there and make a new release right away.

@joshuabaker
Copy link
Contributor Author

Oh, you’ve merged features in this PR. I didn’t realise that. 😅

There’s a $element->searchable(); call in the middle of the IndexElement. Guessing that might suffer the same issue raised in #257, right?

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

Successfully merging this pull request may close these issues.

None yet

3 participants