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

Ability to use allow_lazy_user with class methods #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Ability to use allow_lazy_user with class methods #50

wants to merge 1 commit into from

Conversation

reeteshranjan
Copy link

The allow_lazy_user decorator is not designed to be used with class methods that prevents one from using it with class based views. We have added a new decorator allow_lazy_user_method that accepts the instance/self as required and propagates it through the wrap logic. We have used this version in our project successfully where we needed lazy signup to work with a django rest framework view class.

The ```allow_lazy_user``` decorator is not designed to be used with class methods that prevents one from using it with class based views. We have added a new decorator ```allow_lazy_user_method``` that accepts the instance/self as required and propagates it through the wrap logic. We have used this version in our project successfully where we needed lazy signup to work with a django rest framework view class.
@danfairs
Copy link
Owner

Thanks for your PR! Two comments, really:

  • Could you refactor the PR to avoid all the duplicated code? Most of that new code is the same as allow_lazy_user. Pull that out into a shared function.
  • It'd be awesome to have a test, too! Verifying that your method decorator correctly invoked the new shared function would probably be enough, as that existing code is tested elsewhere.

Thanks!

@danfairs
Copy link
Owner

(Oh - and sorry for the delay - all my GH notifications are going astray at the moment, and I'm trying to figure out why...)

@reeteshranjan
Copy link
Author

I would soon come back with the refactored version and tests. Thanks for your response!

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