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

Model attributes are empty, in relationship method, over the api. #296

Open
ahmed-osama-saad opened this issue Oct 14, 2024 · 2 comments
Open

Comments

@ahmed-osama-saad
Copy link

I have relationship defined on WorkoutResults model

public function unfinishedExercises()
    {
        $exercises = $this->exercises();
        $id = $this->id;

        return $exercises->whereDoesntHave('exercisesResults', function ($query) use ($id) {
            $query->where('workout_result_id', $id);
        });
    }

In WorkoutResultSchema, I define the relationship field

HasMany::make('unfinished_exercises', 'unfinishedExercises')->type('exercises'),

The issue I'm facing:

  • $this in the unfinishedExercises method is an empty model.
  • Consequently, $this->id is null, so the filtering in the whereDoesntHave clause doesn't work as expected.

After some debugging, I found that the problem only occurs when this relationship is requested over the api.
When using the relationship in tinker, $this is defined correctly, and the filtering works as expected.

If I request this relationship from workout-results/:id?include=unfinished_exercise endpoint, the filtering doesn't happen, I get all exercises.

@lindyhopchris
Copy link
Contributor

lindyhopchris commented Oct 14, 2024

Hi. Yeah I don't actually think this does work in Tinker. It would need to work via Eager Loading in Tinker.

What happens if yo try this in Tinker:

$models = WorkoutResult::query()->with('unfinishedExercises')->get();

Basically, you can't use $this->id in a relationship method, as the relationships are not always loaded from a specific model - otherwise you'd get N+1 problems.

We use eager loading, so that we don't get N+1 problems.

@efmjackhardcastle
Copy link

LaravelJsonAPI package aside, this is bad by design.

    public function unfinishedExercises()
    {
        $exercises = $this->exercises();
        $id = $this->id;

        return $exercises->whereDoesntHave('exercisesResults', function ($query) use ($id) {
            $query->where('workout_result_id', $id);
        });
    }

Should instead be changed to something along these lines:

    // class Exercise
   ... 

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

    public function ExerciseResults(): HasMany
    {
        return $this->hasMany(ExerciseResult::class);
    }
    // class WorkoutResult
    ...
    public function exercises(): HasMany
    {
        return $this->hasMany(Exercise::class);
    }

    public function unfinishedExercises(): HasMany
    {
        return $this->exercises()
            ->whereDoesntHave('exerciseResults');
    }

Without knowing your keys etc can't give a more precise answer.

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

3 participants