Skip to content

Conversation

@dakota
Copy link
Contributor

@dakota dakota commented Jul 5, 2019

Doing this in the beforeDelete breaks in cases where the sequencing is a unique index.

@ADmad
Copy link
Owner

ADmad commented Jul 5, 2019

Tests failing. Simply changing the callback method won't cut it. All fields required to calculate the new orders might not be available in provided entity in which case the _getOldValues() method re-fetches entity with all required fields.

So the $this->_getOldValues($entity) call needs to be always done in beforeDelete(). You can store it's return value in a property and then use it to make the $this->_sync() call in afterDelete().

Also please add a test case using table with unique index for the sequence field 🙂.

@dakota
Copy link
Contributor Author

dakota commented Jul 8, 2019

👍

@dakota
Copy link
Contributor Author

dakota commented Jul 8, 2019

@ADmad Should be all good now!

* records that were after it in the set.
*
* @param \Cake\Event\Event $event The beforeDelete event that was fired.
* @param \Cake\Event\Event $event The afterDelete event that was fired.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be undone.

* records that were after it in the set.
*
* @param \Cake\Event\Event $event The afterDelete event that was fired.
* @param \Cake\ORM\Entity $entity The entity that is going to be saved.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entity that was deleted.

@dakota
Copy link
Contributor Author

dakota commented Jul 9, 2019

@ADmad Fixed. Can't believe I missed that!

@ADmad ADmad merged commit c8f89bd into ADmad:master Jul 9, 2019
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

Successfully merging this pull request may close these issues.

2 participants