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

Added default forum user permissions #414

Merged
merged 4 commits into from
Dec 5, 2023

Conversation

joshuamonroe
Copy link
Contributor

To address issue #412, I've added the necessary django-machina setting to define default user permissions on the forum.

@joshuamonroe joshuamonroe added this to the 2023/Sprint 3 milestone Dec 5, 2023
@joshuamonroe joshuamonroe self-assigned this Dec 5, 2023
@joshuamonroe joshuamonroe linked an issue Dec 5, 2023 that may be closed by this pull request
Copy link
Contributor

@bburgess19 bburgess19 left a comment

Choose a reason for hiding this comment

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

Review

The permissions are reasonable, and the documentation is a great inclusion. LGTM.

Proof of functionality

forum_viewable_authorized.mov

[email protected] is unauthenticated

Comment on lines +360 to +381

# This setting define which permissions should be granted to all authenticated
# users. Note that the permissions specified in this list are granted only if a
# given forum does not have any permissions set for a given authenticated user.
#
# In the future, it may become desirable to take a more fine-grain approach to
# user permissions on the chigame forum, more information on how to implement
# such an approach and a full list of permissions can be found at this link.
# https://django-machina.readthedocs.io/en/latest/forum_permissions.html
#
# https://django-machina.readthedocs.io/en/latest/settings.html#machina-default-authenticated-user-forum-permissions
MACHINA_DEFAULT_AUTHENTICATED_USER_FORUM_PERMISSIONS = [
"can_see_forum",
"can_read_forum",
"can_start_new_topics",
"can_reply_to_topics",
"can_edit_own_posts",
"can_post_without_approval",
"can_create_polls",
"can_vote_in_polls",
"can_download_file",
]
Copy link
Contributor

Choose a reason for hiding this comment

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

These permissions look correct, and thank you for the documentation

Copy link
Contributor

@majorsylvie majorsylvie left a comment

Choose a reason for hiding this comment

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

I can successfully sign in as a non-admin user and see the posts!
Then when I'm completely signed out, I cannot see any posts in the forum!

A good next issue to come from this would be a landing page of some sorts that recognizes if the user is not signed in and tells them they need to sign in to view the forum + a link to sign in.

This way someone who's not signed in will know that "yes, things actually have been posted to the forum, I just can't see them". Instead of "this forum looks empty, I have no idea if this is because there are no posts or if I can't see them.

Thank you for your documention!! Particularly for future implementation. This is phenomenal to put in code as well as github issues.

Thank you for your work!

@majorsylvie majorsylvie merged commit 49c8778 into dev Dec 5, 2023
2 checks passed
@majorsylvie majorsylvie deleted the forums/default-forum-permissions branch December 5, 2023 23:58
@joshuamonroe
Copy link
Contributor Author

joshuamonroe commented Dec 8, 2023

Closed without significant revision. Thanks to @bburgess19 for reviewing the PR and for the video documenting the result.

@majorsylvie We already had an issue open to create a landing page, although it was recently closed due to inactivity and lack of a clear purpose. However, I've gone ahead and reopened it and clarified that it should cater to unauthenticated users, referencing your comment here.

@majorsylvie
Copy link
Contributor

Issue Score: Excellent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add default forum permissions setting
3 participants