Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

Laravel Scout: Issue with newQueryForRestoration($id) (Add Array Support) #65

Closed
reminisc3 opened this issue Sep 10, 2018 · 2 comments
Closed

Comments

@reminisc3
Copy link

Greetings,

Firstly, excellent package and nice work being done here!

The issue:

I am trying to get this to play nice with Laravel Scout and am encountering an issue when using queues with Scout. In the HasBinaryUuid trait, the following function is implemented in your package:

public function newQueryForRestoration($id)
{
		return $this->newQueryWithoutScopes()->whereKey(base64_decode($id));
}

When performing operations on models with the Searchable trait, it seems to be passing an array to this function and causing an exception. It seems to be a one element array based on my testing as such:

array(1) { [0]=> string(36) "2d6a1fc0-b4a8-11e8-8375-ac220bbf9530" } (example)

The error: base64_decode() expects parameter 1 to be string, array given

The (very quick) workaround for me:

public function newQueryForRestoration($id)
{
		if ( is_array($id) ) {
			$id = $id[0];
		}
	  return $this->newQueryWithoutScopes()->whereKey(base64_decode($id));
}

This is the official source of this function from Laravel:

public function newQueryForRestoration($ids)
{
	return is_array($ids)
			? $this->newQueryWithoutScopes()->whereIn($this->getQualifiedKeyName(), $ids)
			: $this->newQueryWithoutScopes()->whereKey($ids);
}

https://github.com/laravel/framework/blob/5.6/src/Illuminate/Database/Eloquent/Model.php#L972

Does it make sense to make your function override support arrays as well?

Thanks

@vpratfr
Copy link
Collaborator

vpratfr commented Sep 11, 2018

Hi

I think it should indeed work with arrays too.

So instead of picking the first element of the array it should use a whereKeyIn call like the original method.

Could you submit a proposal with corresponding tests?

isocroft added a commit to isocroft/laravel-binary-uuid that referenced this issue Sep 26, 2018
@freekmurze
Copy link
Member

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants