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

fix: an error when an anonymous user fills the form #13

Merged
merged 2 commits into from
Aug 25, 2024

Conversation

arunk
Copy link
Contributor

@arunk arunk commented Aug 25, 2024

SaveDBAction.execute method uses request.user for
form_user but request.user is not an instance of User for AnonymousUsers therefore we need to check for whether the user is logged in or not

SaveDBAction.execute method uses request.user for
form_user but request.user is not an instance of User for AnonymousUsers
therefore we need to check for whether the user is logged in or not
@arunk arunk changed the title Fix an error when an anonymous user fills the form fix: an error when an anonymous user fills the form Aug 25, 2024
@fsbraun
Copy link
Member

fsbraun commented Aug 25, 2024

@arunk Well spotted! Thank you!

Cleaner way of checking for anonymous user

Co-authored-by: Fabian Braun <[email protected]>
@arunk
Copy link
Contributor Author

arunk commented Aug 25, 2024

Do you mean record the key "login_required" in the FormEntry ? or do you mean record that the user was anonymous?

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@20d0f69). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #13   +/-   ##
=======================================
  Coverage        ?   62.79%           
=======================================
  Files           ?       21           
  Lines           ?     1419           
  Branches        ?      219           
=======================================
  Hits            ?      891           
  Misses          ?      487           
  Partials        ?       41           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@arunk
Copy link
Contributor Author

arunk commented Aug 25, 2024

I'm also having trouble figuring out where the Action is supposed to go. I have finally put it in apps.py in the ready method, where I load the actions module and register an object to do the FormAction. I'm not sure if this is the right approach. If it is, then I'll submit another PR, to update the docs saying how to do this. If the FormAction should be in another file, I tried forms.py, actions.py none of them worked. Then let me know and I'll do likewise and update the README.rst and submit a PR

@fsbraun
Copy link
Member

fsbraun commented Aug 25, 2024

Now, this should record any logged-in user and leave the "form_user" entry empty (None) if no user was logged. That seems reasonable to me.

@fsbraun
Copy link
Member

fsbraun commented Aug 25, 2024

I'm also having trouble figuring out where the Action is supposed to go. I have finally put it in apps.py in the ready method, where I load the actions module and register an object to do the FormAction. I'm not sure if this is the right approach. If it is, then I'll submit another PR, to update the docs saying how to do this. If the FormAction should be in another file, I tried forms.py, actions.py none of them worked. Then let me know and I'll do likewise and update the README.rst and submit a PR

Yes, that works. The form action has to be somewhere where it is imported upon django startup. I have often imported them from models.py but that's not so helpful for forms that are not associated with models. An update of the README would be awesome!

@fsbraun fsbraun merged commit 8abab17 into django-cms:main Aug 25, 2024
24 checks passed
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