AchievementProgress needs keyType set to string

Summary

This package is using a UUID for the primary key on the AchievementProgress model but the keyType is not getting properly set to string on the model.

This is causing an odd bug when eager loading achievement.details on a model that is related to AchivementProgress thru the id field.

Example

For instance, with the following models:


class Reward extends Model
{
    ...

    public function achievement(): BelongsTo
    {
        return $this->belongsTo(AchievementProgress::class);
    }

    ...
}

class Player extends Model
{
    use Achiever;

    ...

    public function rewards(): HasMany
    {
        return $this->hasMany(Reward::class);
    }

    ...
}

When running this query:

$player->rewards()->with('achievement.details')->get();

will cause the following queries to be run to eager load the AchievementProgress models:

SELECT * FROM achievement_progress WHERE achievement_progress.id IN (0, 0, ...);

For most database engines, this would return 0 rows, but for MySQL, it actually returns all rows due to this issue.

Fortunately, Eloquent actually trims the results to the correct record causing the bug to go unnoticed for small datasets.

However, for large datasets it causes out of memory exceptions which is what led us to discovering this bug.

Solutions

Quick Fix - Set Key Type

I believe setting keyType to a string on the model could possibly fix the SQL query so it properly includes the UUIDs:

SELECT * FROM achievement_progress WHERE achievement_progress.id IN ('783232ef-c7a0-49bb-ad62-73757a77f98f', ...);

This will ensure only the correct rows are returned.

Better Fix - Upgrade Laravel Minimum

I think a better fix would be to bump the minimum Laravel version to at least Laravel 9 and then refactor the AchievementProgress model to use the new HasUuid trait that was introduced in that version for this purpose. This would ensure ongoing compatibility with the core framework.

All support for Laravel 6 was dropped by the framework on September 6th 2022. It seems that packages should follow similar support timelines as the core framework. See https://laravel.com/docs/9.x/releases#support-policy

I'm happy to submit a PR that would raise the minimum to 9 (or 10) and adopt HasUuid if the maintainers are likely to accept this change.

assada / laravel-achievements