django-rest-auth icon indicating copy to clipboard operation
django-rest-auth copied to clipboard

add option to roll back reg on error

Open dradetsky opened this issue 7 years ago • 9 comments

The PR mentioned in #414.

Notes:

  • Per the django standard txn guidelines, I add an option to wrap just that particular operation rather than just turning off autocommit (autocommit in that op is IMO much worse than anywhere else).

  • I wrap RegisterView.perform_create instead of RegisterView.create because Open Session In View Considered Harmful.

dradetsky avatar Mar 04 '18 18:03 dradetsky

Coverage Status

Coverage decreased (-0.6%) to 96.465% when pulling a6834e6718551786d60886877926bbe08c743e65 on dradetsky:rollback-reg-on-error into a3057b7aa1963da834ffc33edaf188b0e0e4556d on Tivix:master.

coveralls avatar Mar 04 '18 18:03 coveralls

Coverage Status

Coverage decreased (-0.6%) to 96.465% when pulling a6834e6718551786d60886877926bbe08c743e65 on dradetsky:rollback-reg-on-error into a3057b7aa1963da834ffc33edaf188b0e0e4556d on Tivix:master.

coveralls avatar Mar 04 '18 18:03 coveralls

Coverage Status

Coverage decreased (-0.6%) to 96.465% when pulling 4b4d06d0e89f1c517b536d851d46c56acaf48ebc on dradetsky:rollback-reg-on-error into a3057b7aa1963da834ffc33edaf188b0e0e4556d on Tivix:master.

coveralls avatar Mar 04 '18 18:03 coveralls

Coverage Status

Coverage decreased (-0.6%) to 96.465% when pulling a6834e6718551786d60886877926bbe08c743e65 on dradetsky:rollback-reg-on-error into a3057b7aa1963da834ffc33edaf188b0e0e4556d on Tivix:master.

coveralls avatar Mar 04 '18 18:03 coveralls

Coverage Status

Coverage decreased (-0.6%) to 96.465% when pulling a6834e6718551786d60886877926bbe08c743e65 on dradetsky:rollback-reg-on-error into a3057b7aa1963da834ffc33edaf188b0e0e4556d on Tivix:master.

coveralls avatar Mar 04 '18 18:03 coveralls

Coverage Status

Coverage decreased (-0.6%) to 96.465% when pulling a6834e6718551786d60886877926bbe08c743e65 on dradetsky:rollback-reg-on-error into a3057b7aa1963da834ffc33edaf188b0e0e4556d on Tivix:master.

coveralls avatar Mar 04 '18 18:03 coveralls

I tried a bit to write tests, but it's somewhat difficult, possibly due to the fact that I'm not good at django. To write the tests, I have to patch complete_signup to raise, and then add something like@override_settings(REST_AUTH_ROLL_BACK_REGISTER_ON_ERROR=True). The problem is that decoration occurs at compile-time, and @override_settings happens at runtime, so I never decorate the method during the test. I haven't figured out how to get around this yet, and I've spent too long on this for now.

dradetsky avatar Mar 04 '18 19:03 dradetsky

@Tivix @maxim-kht any opinion on this? I was considering using django-rest-auth, but my concern over this is preventing me.

dradetsky avatar Mar 20 '18 18:03 dradetsky

Hi, as a user of rest-auth, thanks for the contribution! This repo is not maintained anymore, so the development moved to dj-rest-auth. (reference: https://github.com/Tivix/django-rest-auth/issues/568) It may be best, if you move this PR there. (and upgrade to using dj_rest_auth)

new repo link: https://github.com/jazzband/dj-rest-auth (I'm not the upkeeper of that repo, it just makes sense for me to help you merge your PR)

Many Thanks, Barney

BarnabasSzabolcs avatar May 30 '20 01:05 BarnabasSzabolcs