[10.x] Fix custom scout keys not being utilized when deleting from queue

[stevebauman]

Related #656

Sending this in for tests to run and ensure everything passes with the recent changes of the above PR. 👍

[driesvints]

Thanks @stevebauman. I'll use this PR as a reference once 9.x is updated and I merge it into master.

[driesvints]

@stevebauman @mmachatschek I've merged 9.x into master now but unsure if everything is now okay. If you have the time, can you maybe check if everything looks in order? Looks like this PR is also conflicting now.

Here's my merge commit: 0337e4c

[stevebauman]

Hey @driesvints, thanks for the ping! I'll review in a couple hours today and comment back 👍

[stevebauman]

@driesvints Okay I've completed reviewing and have updated my PR with master.

I have removed the getUnqualifiedScoutKeyName() inside the Searchable trait. We no longer need it, because in v9, getScoutKeyName() was returning the fully qualified key name for the Eloquent model (i,e. {table}.id):

scout/src/Searchable.php

Lines 388 to 391 in 8489fac

	public function getScoutKeyName()
	{
	return $this->getQualifiedKeyName();
	}

In v10, this was adjusted to return only the key name instead (i.e. id):

scout/src/Searchable.php

Lines 390 to 393 in 0337e4c

	public function getScoutKeyName()
	{
	return $this->getKeyName();
	}

This should not cause any side-effects for those upgrading to v10. getScoutKeyName() was only used in one location in v9:

scout/src/Searchable.php

Lines 247 to 249 in 8489fac

	return $query->{$whereIn}(
	$this->getScoutKeyName(), $ids
	);

However, if a developer has overrode getScoutKeyName() on their Searchable model with a hard-coded string matching the previous format for some reason (the fully qualified key naming structure), such as:

// app/Models/Post.php

public function getScoutKeyName()
{
    return 'posts.id';
}

Then they must update their formatting to return only the DB column itself, excluding the DB table to maintain compatibility with the v10 update:

public function getScoutKeyName()
{
-    return 'posts.id';
+    return 'id';
}

Let me know if there's anything else I can help with! 🙏

[driesvints]

This looks good @stevebauman. Thank you for your work on this!

[taylorotwell]

Hi @stevebauman - I see this is sent to master branch. Can you elaborate on any breaking changes or notes we would need to make in the upgrade guide about this? Basically any relevant notes to what this changes would be helpful to document in this thread.

[stevebauman]

Hey @taylorotwell, I can definitely do that, but I'm leaving my home for the weekend. I'll be able to write something up on Monday! 👍

[taylorotwell]

OK - mark as ready for review when that is done. Thanks!

[stevebauman]

Okay I've done a run though. There are two breaking changes here:

1. Removal of the Searchable::getUnqualifiedScoutKeyName() method.

This method was introduced in v9 in a PR I wrote (#656) to prevent logic duplication of retrieving the key name from the fully qualified key name that was being returned from the getScoutKeyName() method.

2. Change of the Searchable::getScoutKeyName() return value.

The getScoutKeyName() method now returns the unqualified key name from the model by default, instead of the fully qualified name that includes the model's database table. As mentioned above, developers who have overridden the method will need to update their definition to return the unqualified key name to maintain compatibility with v10:

public function getScoutKeyName()
{
-    return 'posts.id';
+    return 'id';
}

This is because this method is now being used in the built-in search engine implementations (such as to store the models primary key in the indexed record), where it was not being used previously. Ex:

MeiliSearch Before:

scout/src/Engines/MeiliSearchEngine.php

Line 69 in 2bc384e

[$model->getKeyName() => $model->getScoutKey()],

MeiliSearch After:

scout/src/Engines/MeiliSearchEngine.php

Line 69 in 0337e4c

[$model->getScoutKeyName() => $model->getScoutKey()],

Developers that have not overridden the getScoutKeyName() method, or have not utilized the getUnqualifiedScoutKeyName() method, will not have to adjust anything.

Let me know if you have any further questions or would like me to tackle additional related tasks!