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/8694r3yfv community enhancement with privacy #85

Merged
merged 19 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/templates/core/community-list.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ <h1>Communities</h1>
{% for community in communities %}
<a href="{% url 'community-detail' community.slug %}" class="list-group-item list-group-item-action">
{{ community.name }}
{% if not community.has_access %}
<span class="private-community" title="Private Community - No Access">🔒</span>
{% endif %}
</a>
{% empty %}
<p>No communities available.</p>
Expand Down
20 changes: 19 additions & 1 deletion core/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def test_reported_detail_post_by_user(client: Client, user: User, post: Post, re
client.force_login(user)
response = client.get(reverse("reported-post", kwargs={"pk": post.pk}))
assert response.status_code == 403
assert "<h1>403 Forbidden</h1>" in response.content.decode()
assert "You don't have access to this community." in response.content.decode()


def test_reported_detail_post_by_anonymous_user(client: Client, post_report: PostReport) -> None:
Expand Down Expand Up @@ -554,3 +554,21 @@ def test_add_non_existing_moderator(client: Client, community: Community, user:

messages = list(response.context["messages"])
assert any("Invalid user or nickname." in message.message for message in messages)


def test_remove_non_existing_moderator(client: Client, community: Community, user: User) -> None:
admin_password = generate_random_password()

admin = User.objects.create_user(email="[email protected]", password=admin_password, nickname="adminnick")

client.force_login(admin)
CommunityMember.objects.create(community=community, user=admin, role=CommunityMember.ADMIN)

url = reverse("community-detail", kwargs={"slug": community.slug})
form_data = {"nickname": user.nickname}

response = client.post(url, {"action": "remove_moderator", **form_data})
assert response.status_code == 200

messages = list(response.context["messages"])
assert any("User is not a moderator of this community." in message.message for message in messages)
32 changes: 31 additions & 1 deletion core/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
from django.db import models
from django.db.models import QuerySet
from django.db.models import Exists, OuterRef, QuerySet
from django.http import Http404, HttpRequest, HttpResponse, HttpResponseRedirect
from django.shortcuts import get_object_or_404, redirect
from django.template.loader import render_to_string
Expand Down Expand Up @@ -135,6 +135,24 @@ class CommunityListView(ListView):
context_object_name = "communities"
paginate_by = 10

def get_queryset(self: "CommunityListView") -> models.QuerySet:
user = self.request.user
return (
super()
.get_queryset()
.prefetch_related("members")
.annotate(has_access=Exists(CommunityMember.objects.filter(community=OuterRef("pk"), user=user)))
)

def get_context_data(self: "CommunityListView", **kwargs: any) -> dict[str, any]:
memorisanka marked this conversation as resolved.
Show resolved Hide resolved
context = super().get_context_data(**kwargs)
communities = context["communities"]

for community in communities:
community.has_access = community.privacy != "30_PRIVATE" or community.has_access

return context


class CommunityCreateView(LoginRequiredMixin, CreateView):
model = Community
Expand Down Expand Up @@ -193,6 +211,7 @@ def post_add_moderator(self: "CommunityDetailView", request: "HttpRequest", *arg
if add_moderator_form.is_valid():
user = add_moderator_form.cleaned_data["nickname"]
self.object.add_moderator(user)
messages.success(request, f"{user.nickname} is now a moderator of this community.")
else:
messages.error(request, "Invalid user or nickname.")
return self.get(request, *args, **kwargs)
Expand All @@ -203,7 +222,13 @@ def post_remove_moderator(self: "CommunityDetailView", request: "HttpRequest", *
remove_moderator_form = RemoveModeratorForm(request.POST)
if remove_moderator_form.is_valid():
user = remove_moderator_form.cleaned_data["nickname"]
if not CommunityMember.objects.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (performance): Consider combining the moderator check with the remove_moderator operation.

The current implementation performs an additional database query to check if the user is a moderator before calling remove_moderator. Consider combining these operations to avoid unnecessary database queries, possibly by handling the case in the remove_moderator method itself.

            try:
                moderator = CommunityMember.objects.get(
                    community=self.object, user=user, role=CommunityMember.MODERATOR
                )
                self.object.remove_moderator(moderator)
            except CommunityMember.DoesNotExist:
                # Handle the case where the user is not a moderator
                pass

community=self.object, user=user, role=CommunityMember.MODERATOR
).exists():
messages.error(request, "User is not a moderator of this community.")
return self.get(request, *args, **kwargs)
self.object.remove_moderator(user)
messages.success(request, f"{user.nickname} was successfully removed from moderators.")
else:
messages.error(request, "Invalid user or nickname.")
return self.get(request, *args, **kwargs)
Expand Down Expand Up @@ -239,6 +264,11 @@ def handle_no_permission(self: "CommunityUpdateView") -> HttpResponse:
messages.error(self.request, "You do not have permission to update this community.")
return redirect("community-detail", slug=self.get_object().slug)

def form_valid(self: "CommunityUpdateView", form: forms.ModelForm) -> HttpResponseRedirect:
response = super().form_valid(form)
messages.success(self.request, "Community updated successfully.")
return response

def get_success_url(self: "CommunityUpdateView") -> str:
return reverse_lazy("community-detail", kwargs={"slug": self.object.slug})

Expand Down
8 changes: 0 additions & 8 deletions reddit/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
# Build paths inside the project like this: BASE_DIR / 'subdir'.
BASE_DIR = Path(__file__).resolve().parent.parent


# Quick-start development settings - unsuitable for production
# See https://docs.djangoproject.com/en/5.0/howto/deployment/checklist/

Expand All @@ -30,13 +29,11 @@

DEBUG = config("DEBUG")


# SECURITY WARNING: don't run with debug turned on in production!


ALLOWED_HOSTS = json.loads(config("ALLOWED_HOSTS"))


# Application definition

INSTALLED_APPS = [
Expand Down Expand Up @@ -86,7 +83,6 @@

WSGI_APPLICATION = "reddit.wsgi.application"


# Database
# https://docs.djangoproject.com/en/5.0/ref/settings/#databases

Expand All @@ -97,7 +93,6 @@
},
}


# Password validation
# https://docs.djangoproject.com/en/5.0/ref/settings/#auth-password-validators

Expand All @@ -116,7 +111,6 @@
},
]


# Internationalization
# https://docs.djangoproject.com/en/5.0/topics/i18n/

Expand All @@ -128,7 +122,6 @@

USE_TZ = True


# Static files (CSS, JavaScript, Images)
# https://docs.djangoproject.com/en/5.0/howto/static-files/

Expand Down Expand Up @@ -168,6 +161,5 @@
LIMIT_WARNINGS = 5
LOGIN_URL = reverse_lazy("login")


REST_FRAMEWORK = {"DEFAULT_PAGINATION_CLASS": "rest_framework.pagination.PageNumberPagination", "PAGE_SIZE": 10}
DEFAULT_AVATAR_URL = "/media/users_avatars/default.png"
13 changes: 13 additions & 0 deletions templates/403.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{% extends 'base.html' %}
{% load static %}

{% block content %}
<div class="container mt-5">
<div class="card bg-dark shadow-sm text-light">
<div class="card-body">
<p class="card-text">You don't have access to this community.</p>
<a href="{% url 'community-list' %}" class="btn btn-primary">Back to Community List</a>
</div>
</div>
</div>
{% endblock %}
Loading