Skip to content

fix: login_required import and comment author assignment (#140, #118) #186

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

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

Conversation

07SUJITH
Copy link
Contributor

Fix Issues #140 and #118

🔗 PR: Fix login_required import and comment author bug

Summary

This pull request resolves two major bugs and introduces a security enhancement in the Django Girls extended tutorial:

Changes

🔧 1. Import @login_required (Fixes #140)

Issue:
Tutorial omitted the import for login_required, resulting in:

NameError: name 'login_required' is not defined

Fix:
Added the following line to relevant sections of the tutorial and example code:

from django.contrib.auth.decorators import login_required

🔧 2. Fix Comment Author Assignment (Fixes #118)

Issue:
Using author = models.CharField(...) led to:

ValueError: Cannot assign "'username'": "Comment.author" must be a "User" instance.

Fix:
Changed author field to a ForeignKey to the User model. Also removed the author field from the form.

Before:

author = models.CharField(max_length=200)

After:

author = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)

🔒 3. Add Security Checks for Moderation

Problem:
Any logged-in user could approve or delete any comment.

Fix:
Added permission checks:

  • ✅ Only the post author can approve comments.

  • ✅ Only the comment author or post author can delete comments.


…#140, DjangoGirls#118)

- Added missing import instruction for `login_required` to avoid NameError (DjangoGirls#140)
- Updated Comment model instructions to use ForeignKey for `author` instead of CharField (DjangoGirls#118)
- Reflected changes across all 5 language versions of the tutorial
- Ensures consistency with Django best practices and prevents comment submission errors
@das-g
Copy link
Member

das-g commented Jun 11, 2025

Thanks for stepping forward with possible fixes, @07SUJITH!

I'm not sure the comment authorship changes go in the right direction. See my comment #118 (comment).

As the fix for #140 is surely much less controversial, would you be willing to submit that as a separate pull request, that can probably be merged much sooner? That'd be very helpful!

Problem:
Any logged-in user could approve or delete any comment.

I'm not sure that's a problem per-se. It depends on the assumptions and expectations on how user accounts are to be used.

Software for a small independent blog (other than platforms that can host multiple blogs) usually assumes that that there are few logged in users, that all of them trust each other and that all of them are allowed to author and publish blog posts and to approve or decline comments to any blog post of the single blog of the instance. (That's AFAIK also how self-hosted WordPress acts by default, or at least used to in the olden days. A blogging platform for multiple blogs — each potentially by a different author or group of authors — have to work different of course.)

Traditionally, commenting was possible without authentication and comments could indicate any "comment author" name (and thus also lie about it, of course.) Whether that was ever a good idea is debatable (as full anonymous commenting is of course prone to abuse, of which spam isn't even the worst), which is why most websites nowadays either require login or include comment functionality by a third party. I'm unsure what makes sense implementing in the tutorial extension.

@07SUJITH
Copy link
Contributor Author

Really appreciate the detailed feedback, @das-g — thanks a lot!

You're absolutely right — whether comment approval by any logged-in user is considered a problem really depends on the assumptions about how the blog is intended to be used. For small, single-author or trusted-team blogs, the current behavior is perfectly reasonable, and I appreciate the historical context you shared (especially regarding WordPress defaults).

I've gone ahead and submitted a new pull request that addresses issue #140 separately, as it's a clear-cut fix and should be easy to merge.

Regarding the comment authorship and moderation logic, your perspective makes a lot of sense. As for the comment moderation logic,your perspective helped me see it more clearly. Maybe it's something better framed as an optional improvement or discussion point for learners who are thinking about extending the project into a multi-blog system later on (like Medium, Blogger.), rather than a required change to the core tutorial extension
Thanks again — your input has been incredibly helpful!

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.

@login_required is not defined issue on comment submit
2 participants