josegonzalez/cakephp-version

Finder should be moved in Table

dereuromark opened this issue · 3 comments

The SQL finder part of the VersionTrait (and therefore inside Entity scope) should be moved into table and then called via ->versions() to keep things DRY
Some would like to rather pull that from the table then from the entity

    public function versions($reset = false)
    {
        if ($reset === false && $this->has('_versions')) {
            return $this->get('_versions');
        }

        $table = TableRegistry::get($this->source());

        # --------------- START (move to table)
        $primaryKey = (array)$table->primaryKey();

        $query = $table->find('versions');
        $pkValue = $this->extract($primaryKey);
        $conditions = [];
        foreach ($pkValue as $key => $value) {
            $field = current($query->aliasField($key));
            $conditions[$field] = $value;
        }
        $entities = $query->where($conditions)->all();

        if (empty($entities)) {
            return new Collection();
        }
       # --------------- END

        $entity = $entities->first();
        $this->set('_versions', $entity->get('_versions'));

        return $this->get('_versions');
    }

Pull requests welcome :)

I did this so far: master...dereuromark:master
Is this worth to PR?
Looks cleaner to me to have the details in the table and only use it from the entity.
Also the "cache" inside the entity now works for empty collections.

Fine by me to PR.