Skip to content

Kalnoy\Nestedset\QueryBuilder::moveNode throws exception A non-numeric value encountered if the model uses a global scope to add additional attributes #513

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

Open
nagmat84 opened this issue May 30, 2021 · 2 comments

Comments

@nagmat84
Copy link

Maybe this issue is not an issue of this library but an error in my code, but I am unable to track down the root cause. It simply manifests as the exception "A non-numeric value encountered" inside Kalnoy\Nestedset\QueryBuilder::moveNode at line 556 $height = $rgt - $lft + 1.

I explain what I try to achieve and maybe someone either spots the bug in my code (I guess so) or it is really an actual bug in the library.

A have two Eloquent models Album and Photo. An album can contain further sub-albums and uses the trait Kalnoy\Nestedset\NodeTrait. Photos belong to albums. A photo has an attribute taken_at which stores the timestamp when the photo has been taken.

For each album, I want to have two "transient" or "virtual" attributes max_taken_at and min_taken_at which keeps the smallest and largest value of taken_at for all photos within the album and its sub-albums recursively. The aggregate functions for relations Album:query()->withMin('photos', 'taken_at')->withMax('photos','taken_at') which are provided by Eloquent don't do what I want, because the only consider photos which are direct children of the album, but not aggregate recursively over photos of sub-albums down the tree.

I exploited Eloquent's scopes to modify the default query of an album and include the desired attributes into the query like this:

class Album extends Model {
  protected static function booted() {
    parent::booted();
    // Normally "scopes" are used to restrict the result of the query
    // to a particular subset through adding additional WHERE-clauses
    // to the default query.
    // However, "scopes" can be used to manipulate the query in any way.
    // Here we add to additional "virtual" columns to the query.
    static::addGlobalScope('calc_minmax_taken_at', function (Builder $builder) {
      $builder->addSelect([
        'max_taken_at' => Photo::query()
          ->leftJoin(...)
          ->select('taken_at')
          ->where('a._lft', '>=', DB::raw('_lft'))
          ->where('a._rgt', '<=', DB::raw('_rgt'))
          ->whereNotNull('taken_at')
          ->orderBy('taken_at', 'desc')
          ->limit(1),
        'min_taken_at' => Photo::query()
          ->leftJoin('albums as a', 'a.id', '=', 'album_id')
          ->select('taken_at')
          ->where('a._lft', '>=', DB::raw('_lft'))
          ->where('a._rgt', '<=', DB::raw('_rgt'))
          ->whereNotNull('taken_at')
          ->orderBy('taken_at', 'asc')
          ->limit(1),
      ]);
    });
  }
}

This way I get two more attributes in the array attributes of an Album object when an album is hydrated from the database. The result is correct and as expected.

However, I get an exception when I try to move an album to a different parent album and save it.

$albumA = Album::query()->find(42);
$albumB = Album::query()->find(4711);
// up to here everything is fine, the "virtual" attributes `min_taken_at` and `max_taken_at` are properly hydrated from DB and part of the album model
$albumA->parent_id = $albumB->id;
$albumA->save(); // this throws an exception

throws the exception "A non-numeric value encountered" inside Kalnoy\Nestedset\QueryBuilder::moveNode at line 556 $height = $rgt - $lft + 1.

@nagmat84
Copy link
Author

Addendum: After adding some var_dump I can know confirm that the variable $rgt happens to equal the title of album A (each album has an attribute title) and $lft equals the ID of album A.

@nagmat84
Copy link
Author

I believe I found an solution and it is indeed a bug. QueryBuilder::moveNode($key, $position), line 544 assumes that

list($lft, $rgt) = $this->model->newNestedSetQuery()->getPlainNodeData($key, true);

only returns an array with exactly two values in that order. However, this is incorrect, if the base model has defined scopes. The query in QueryBuilder::getNodeData($id, $required), line 34 and 38,

$query = $this->toBase();
// ...
$data = $query->first([
  $this->model->getLftName(),
  $this->model->getRgtName()
]);

returns more than the queried to columns, if the base model uses a global scope which in turn uses addSelect.

The fix is easy and only requires patching NodeTrait. Instead of calling $this->newQuery (lines 252, 341, 678 and 690)
$this->newQueryWithoutScopes() should be invoked. See PR #514.

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

1 participant