Skip to content
Open
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
65 changes: 58 additions & 7 deletions web/templates/memes.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
{% block title %}
Educational Memes
{% endblock title %}

{% block content %}
<div class="container mx-auto mt-8 mb-16 px-4">
<div class="flex justify-between items-center mb-6">
Expand All @@ -12,22 +13,71 @@ <h1 class="text-2xl font-bold">Educational Memes</h1>
<i class="fas fa-plus mr-2"></i> Add Meme
</a>
</div>
<!-- Subject filter -->
<div class="mb-6">

<!-- Dropdown filters -->
<form method="get" class="mb-6 flex flex-wrap gap-4 items-center">
<div>
<label class="font-semibold mr-2">Subject:</label>
<select name="subject" class="px-3 py-1 rounded-md border border-gray-300 dark:border-gray-600">
<option value="">All Subjects</option>
{% for subject in subjects %}
<option value="{{ subject.slug }}" {% if subject.slug == selected_subject %}selected{% endif %}>
{{ subject.name }}
</option>
{% endfor %}
</select>
</div>
<div>
<label class="font-semibold mr-2">User:</label>
<select name="user" class="px-3 py-1 rounded-md border border-gray-300 dark:border-gray-600">
<option value="">All Users</option>
{% for user in users %}
<option value="{{ user.username }}" {% if user.username == selected_user %}selected{% endif %}>
{{ user.username }}
</option>
{% endfor %}
</select>
</div>
<button type="submit"
class="bg-teal-600 hover:bg-teal-700 text-white px-4 py-1 rounded-md">
Filter
</button>
</form>
Comment on lines +17 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add for/id attributes to dropdowns for better accessibility

Both <label> elements are not associated with their <select> because neither element has for/id pairs. Screen-reader users will not have their focus moved correctly. Add id attributes to the selects and point for on the labels to them.

-<label class="font-semibold mr-2">Subject:</label>
-<select name="subject" class="px-3 py-1 rounded-md border border-gray-300 dark:border-gray-600">
+<label for="subject-filter" class="font-semibold mr-2">Subject:</label>
+<select id="subject-filter" name="subject" class="px-3 py-1 rounded-md border border-gray-300 dark:border-gray-600">
@@
-<label class="font-semibold mr-2">User:</label>
-<select name="user" class="px-3 py-1 rounded-md border border-gray-300 dark:border-gray-600">
+<label for="user-filter" class="font-semibold mr-2">User:</label>
+<select id="user-filter" name="user" class="px-3 py-1 rounded-md border border-gray-300 dark:border-gray-600">
🤖 Prompt for AI Agents
In web/templates/memes.html between lines 17 and 45, the <label> elements for
the subject and user dropdowns lack corresponding for attributes linked to the
select elements' ids, which reduces accessibility. Add unique id attributes to
each <select> element (e.g., id="subject" and id="user") and update the
corresponding <label> elements to include for attributes matching these ids to
ensure proper association and improved screen-reader navigation.


<!-- Subject filter as buttons -->
<div class="mb-4">
<div class="flex flex-wrap items-center gap-2">
<span class="font-semibold">Filter by subject:</span>
<a href="{% url 'meme_list' %}"
<a href="{% url 'meme_list' %}{% if selected_user %}?user={{ selected_user }}{% endif %}"
class="px-3 py-1 rounded-full {% if not selected_subject %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
All
</a>
{% for subject in subjects %}
<a href="{% url 'meme_list' %}?subject={{ subject.slug }}"
<a href="{% url 'meme_list' %}?subject={{ subject.slug }}{% if selected_user %}&user={{ selected_user }}{% endif %}"
class="px-3 py-1 rounded-full {% if selected_subject == subject.slug %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
Comment on lines +51 to 57
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

URL parameters should be URL-encoded

selected_user and subject.slug may legally contain characters that need percent-encoding (space, @, +, etc.). Wrap the variables with the urlencode filter to guarantee a valid query string.

-?subject={{ subject.slug }}{% if selected_user %}&user={{ selected_user }}{% endif %}
+?subject={{ subject.slug|urlencode }}{% if selected_user %}&user={{ selected_user|urlencode }}{% endif %}

Same applies to the “All” button above.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<a href="{% url 'meme_list' %}{% if selected_user %}?user={{ selected_user }}{% endif %}"
class="px-3 py-1 rounded-full {% if not selected_subject %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
All
</a>
{% for subject in subjects %}
<a href="{% url 'meme_list' %}?subject={{ subject.slug }}"
<a href="{% url 'meme_list' %}?subject={{ subject.slug }}{% if selected_user %}&user={{ selected_user }}{% endif %}"
class="px-3 py-1 rounded-full {% if selected_subject == subject.slug %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
<a href="{% url 'meme_list' %}{% if selected_user %}?user={{ selected_user|urlencode }}{% endif %}"
class="px-3 py-1 rounded-full {% if not selected_subject %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
All
</a>
{% for subject in subjects %}
<a href="{% url 'meme_list' %}?subject={{ subject.slug|urlencode }}{% if selected_user %}&user={{ selected_user|urlencode }}{% endif %}"
class="px-3 py-1 rounded-full {% if selected_subject == subject.slug %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
🤖 Prompt for AI Agents
In web/templates/memes.html around lines 51 to 57, the URL parameters
selected_user and subject.slug are used directly in query strings without URL
encoding, which can cause invalid URLs if they contain special characters. Fix
this by applying the Django urlencode filter to selected_user and subject.slug
variables in the href attributes to ensure they are properly percent-encoded in
the URLs.

{{ subject.name }}
</a>
{% endfor %}
</div>
</div>

<!-- User filter as buttons -->
<div class="mb-6">
<div class="flex flex-wrap items-center gap-2">
<span class="font-semibold">Filter by user:</span>
<a href="{% url 'meme_list' %}{% if selected_subject %}?subject={{ selected_subject }}{% endif %}"
class="px-3 py-1 rounded-full {% if not selected_user %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
All
</a>
{% for user in users %}
<a href="{% url 'meme_list' %}?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}user={{ user.username }}"
class="px-3 py-1 rounded-full {% if selected_user == user.username %}bg-teal-600 text-white{% else %}bg-gray-200 dark:bg-gray-700 hover:bg-gray-300 dark:hover:bg-gray-600 text-gray-800 dark:text-gray-200{% endif %}">
{{ user.username }}
</a>
{% endfor %}
</div>
</div>
Comment on lines +17 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This change introduces two different ways to filter memes: a form with dropdowns and a "Filter" button, and separate sections of button-style links for subjects and users.

Having both can make the UI feel redundant and increases the complexity of the template logic.

Consider simplifying the UI by using only one of these filtering mechanisms. The dropdown form is more scalable if the number of subjects or users grows large, as it avoids creating a very long list of buttons on the page.

Comment on lines +64 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Duplicate string-building logic – extract a helper to reduce repetition

The user-filter buttons re-implement the same query-string construction logic already used for the subject filters and pagination. This duplication is error-prone. Consider computing a base_query in the view (e.g. request.GET.urlencode minus the param to be replaced) or adding a custom template filter (add_query_param) so the template only expresses intent:

<a href="{% add_query_param request 'user' user.id %}">
🤖 Prompt for AI Agents
In web/templates/memes.html around lines 64 to 79, the query string construction
for user filter buttons duplicates logic already used elsewhere, making it
error-prone. To fix this, refactor by moving the base query string computation
to the view (e.g., using request.GET.urlencode with the relevant parameter
removed) or implement a custom template filter like add_query_param that adds or
replaces query parameters. Then update the template to use this helper,
simplifying the href attributes to express only the parameter changes without
manual string concatenation.


<!-- Meme grid -->
<div class="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 lg:grid-cols-4 gap-6">
{% for meme in memes %}
Expand All @@ -53,12 +103,13 @@ <h3 class="text-lg font-bold text-gray-900 dark:text-white">{{ meme.title }}</h3
</div>
{% endfor %}
</div>

<!-- Pagination controls -->
{% if memes.paginator.num_pages > 1 %}
<div class="mt-8 flex justify-center">
<nav class="inline-flex rounded-md shadow-sm">
{% if memes.has_previous %}
<a href="?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}page={{ memes.previous_page_number }}"
<a href="?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}{% if selected_user %}user={{ selected_user }}&{% endif %}page={{ memes.previous_page_number }}"
class="px-4 py-2 text-sm font-medium text-gray-700 dark:text-gray-200 bg-white dark:bg-gray-800 border border-gray-300 dark:border-gray-600 rounded-l-md hover:bg-gray-50 dark:hover:bg-gray-700">
Previous
</a>
Expand All @@ -67,14 +118,14 @@ <h3 class="text-lg font-bold text-gray-900 dark:text-white">{{ meme.title }}</h3
{% if memes.number == num %}
<span class="px-4 py-2 text-sm font-medium text-white bg-teal-600 border border-teal-600">{{ num }}</span>
{% else %}
<a href="?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}page={{ num }}"
<a href="?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}{% if selected_user %}user={{ selected_user }}&{% endif %}page={{ num }}"
class="px-4 py-2 text-sm font-medium text-gray-700 dark:text-gray-200 bg-white dark:bg-gray-800 border border-gray-300 dark:border-gray-600 hover:bg-gray-50 dark:hover:bg-gray-700">
{{ num }}
</a>
{% endif %}
{% endfor %}
{% if memes.has_next %}
<a href="?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}page={{ memes.next_page_number }}"
<a href="?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}{% if selected_user %}user={{ selected_user }}&{% endif %}page={{ memes.next_page_number }}"
class="px-4 py-2 text-sm font-medium text-gray-700 dark:text-gray-200 bg-white dark:bg-gray-800 border border-gray-300 dark:border-gray-600 rounded-r-md hover:bg-gray-50 dark:hover:bg-gray-700">
Next
</a>
Expand Down
30 changes: 21 additions & 9 deletions web/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4493,19 +4493,37 @@ def whiteboard(request):
def graphing_calculator(request):
return render(request, "graphing_calculator.html")


def meme_list(request):
memes = Meme.objects.all().order_by("-created_at")

subjects = Subject.objects.filter(memes__isnull=False).distinct()
users = User.objects.filter(memes__isnull=False).distinct() # Get users who have posted memes

# Filter by subject if provided
subject_filter = request.GET.get("subject")
if subject_filter:
memes = memes.filter(subject__slug=subject_filter)

# Filter by user if provided
user_filter = request.GET.get("user")
if user_filter:
memes = memes.filter(user__username=user_filter)

paginator = Paginator(memes, 12) # Show 12 memes per page
page_number = request.GET.get("page", 1)
page_obj = paginator.get_page(page_number)

return render(request, "memes.html", {"memes": page_obj, "subjects": subjects, "selected_subject": subject_filter})
return render(
request,
"memes.html",
{
"memes": page_obj,
"subjects": subjects,
"users": users,
"selected_subject": subject_filter,
"selected_user": user_filter,
},
)


def meme_detail(request: HttpRequest, slug: str) -> HttpResponse:
Expand Down Expand Up @@ -4581,21 +4599,17 @@ def team_goal_detail(request, goal_id):
"""View and manage a specific team goal."""
goal = get_object_or_404(TeamGoal.objects.prefetch_related("members__user"), id=goal_id)

# Check if user has access to this goal
if not (goal.creator == request.user or goal.members.filter(user=request.user).exists()):
messages.error(request, "You do not have access to this team goal.")
return redirect("team_goals")

# Get existing team members to exclude from invitation
existing_members = goal.members.values_list("user_id", flat=True)

# Handle inviting new members
if request.method == "POST":
form = TeamInviteForm(request.POST)
if form.is_valid():
# Check for existing invites using the validated User object
if TeamInvite.objects.filter(
goal__id=goal.id, recipient=form.cleaned_data["recipient"] # Changed to use User object
goal__id=goal.id, recipient=form.cleaned_data["recipient"]
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add missing trailing comma for consistency.

Static analysis correctly identified a missing trailing comma. This should be added for consistent code formatting.

-                goal__id=goal.id, recipient=form.cleaned_data["recipient"] 
+                goal__id=goal.id, recipient=form.cleaned_data["recipient"], 
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
goal__id=goal.id, recipient=form.cleaned_data["recipient"]
goal__id=goal.id, recipient=form.cleaned_data["recipient"],
🧰 Tools
🪛 Ruff (0.12.2)

4474-4474: Trailing comma missing

Add trailing comma

(COM812)

🤖 Prompt for AI Agents
In web/views.py at line 4474, add a trailing comma after the last argument in
the function call or dictionary to maintain consistent code formatting and
comply with static analysis recommendations.

).exists():
messages.warning(request, "An invite for this user is already pending.")
return redirect("team_goal_detail", goal_id=goal.id)
Expand All @@ -4609,8 +4623,6 @@ def team_goal_detail(request, goal_id):

else:
form = TeamInviteForm()

# Get users that can be invited (exclude existing members and the creator)
available_users = User.objects.exclude(id__in=list(existing_members) + [goal.creator.id]).values(
"id", "username", "email"
)
Expand Down