2.6 fix save() with habtm returning error

[Haititi]

bug found while handling PR 3516.
#3516

$Model->save($data) may return false if the given $data to save does not contain some $Model related data.

[dereuromark]

elseif

[dereuromark]

I wonder if those 4 nested levels of ifs are really necessary..
at least the last two could probably be combined.

[lorenzo]

Yeah looks a bit too much

[dogmatic69]

How about this:

$this->whitelist = $_whitelist;

if (!$success) {
    return $success;
}

if ($count > 0) {
    if (!empty($this->data) && $created) {
        $this->data[$this->alias][$this->primaryKey] = $this->id;
    }

    if ($options['callbacks'] === true || $options['callbacks'] === 'after') {
        $event = new CakeEvent('Model.afterSave', $this, array($created, $options));
        $this->getEventManager()->dispatch($event);
    }
}

if (!empty($this->data)) {
    $success = $this->data;
}

$this->_clearCache();
$this->validationErrors = array();
$this->data = false;

return $success;

[Haititi]

this new commit should hopefully allow better readability.
some checks was not needed as the result was obvious.

[Haititi]

Erf... each time I fix a bug, I find another one...

• Test testSaveAllNestedSaveAll and testSaveManyNestedSaveMany are using some TransactionTestModel and TransactionManyTestModel which define an 'afterSave' callback to do another save.
• The second save succeed and set $this->data = false; before returning the array of data saved.
• Then, the first save finishs its process but fails due to the returning data set to false previously...

I will restore the previous behaviour, but that will imply that user will only be able to retrieve 'true/false' instead of the array of saved data.

[markstory]

Seems reasonable to me.

[Haititi]

when do you think this will be merged ? I was thinking I will wait for the merge and rebase my branch used for #3516 to resolve the remaining problems...