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

Give different result from Resultset::delete() depending on success #12828

Merged

Conversation

1RV34
Copy link
Contributor

@1RV34 1RV34 commented Apr 28, 2017

Hello!

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

Small description of change: When any Model::delete() fails the result bubbles to cause ResultSet::delete() to return false as well.

Thanks

@Jurigag
Copy link
Contributor

Jurigag commented Apr 28, 2017

Update changelog also and add some simple test.

@sergeyklay sergeyklay added this to the 3.2.0 milestone Apr 30, 2017
@sergeyklay sergeyklay self-assigned this Apr 30, 2017
Copy link
Contributor

@Jurigag Jurigag left a comment

Choose a reason for hiding this comment

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

Update changelog, add test

@1RV34
Copy link
Contributor Author

1RV34 commented May 1, 2017

I've added a message to the changelog but not sure if it's according to the standards that those messages should be.

Secondly, I'm not completely sure how to properly create a test for this. Basically if any of the Model::delete() calls that are made fails and causes a transaction rollback to happen the result should be false just like the Model::delete() result. Could I get some help with this?

CHANGELOG.md Outdated
@@ -22,6 +22,7 @@
- Fixed `Phalcon\Mvc\Micro:handle` to correctly handle `before` handlers [#10931](https://github.com/phalcon/cphalcon/pull/10931)
- Fixed `Phalcon\Mvc\Micro:handle` to correctly handle `afterBinding` handlers
- Added way to disable setters in `Phalcon\Mvc\Model::assign` by using `Phalcon\Mvc\Model::setup` or ini option
- Made `Resultset::delete()` return `false` when any of the `Model::delete` calls fails [PR #12828](https://github.com/phalcon/cphalcon/pull/12828) fixing [issue #11133](https://github.com/phalcon/cphalcon/issues/11133)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for link ot pr, just only add issue like other changes aboves. Also without "issue"

@Jurigag
Copy link
Contributor

Jurigag commented May 1, 2017

About test - just return some random false in some of model::delete, then check if result returned by resultset->delete is false.

@sergeyklay
Copy link
Contributor

Just updated CHANGELOG

@sergeyklay sergeyklay merged commit 3959cb2 into phalcon:3.2.x May 13, 2017
@sergeyklay
Copy link
Contributor

Thank you

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.

3 participants