Add 'Filter by User' functionality to Edu Memes section#660
Add 'Filter by User' functionality to Edu Memes section#660aavi4012-cmd wants to merge 4 commits intoalphaonelabs:mainfrom
Conversation
|
Warning Rate limit exceeded@A1L13N has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds user-based filtering to the Educational Memes list: view now accepts a Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant B as Browser
participant S as Server
participant DB as Database
U->>B: choose subject/user filter or page
B->>S: GET /memes?subject=...&user=...&page=...
S->>DB: query users (distinct authors)
S->>DB: query memes filtered by subject and user (with pagination)
DB-->>S: users list, filtered memes & page info
S-->>B: render memes.html with memes, users, subjects, selected filters, pagination links
B->>U: display filtered memes and controls
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @aavi4012-cmd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces and fixes the 'Filter by User' functionality within the Educational Memes section. The changes enable users to effectively filter memes by their creators, addressing a previously reported issue and enhancing the overall usability of the meme browsing experience. Both frontend UI components and backend filtering logic have been implemented to support this feature.
Highlights
- User Filtering Implementation: I've added new UI elements to the
memes.htmltemplate, including both a dropdown and a set of buttons, to allow users to filter educational memes by the user who posted them. This provides multiple ways for users to interact with the new filtering functionality. - Backend Logic for User Filtering: In
web/views.py, I've updated thememe_listview to retrieve a distinct list of users who have posted memes. Crucially, I've implemented the logic to filter the meme queryset based on theuserparameter received in the request, ensuring that only memes from the selected user are displayed. - Persistent Filtering Across Navigation: I've modified the existing subject filter links and all pagination links in
memes.htmlto correctly preserve both thesubjectand the newly addeduserfilter parameters. This ensures a consistent user experience where filters remain active when navigating through pages or switching between other filter options.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a 'Filter by User' feature to the memes section, adding both dropdown and button-based UIs for filtering. The backend logic in web/views.py has been updated to handle the new user filter, and the pagination controls in web/templates/memes.html are correctly modified to preserve the filter state.
I've found a couple of critical issues in the view logic that will prevent the feature from working as intended. Additionally, I have a suggestion regarding the UI to improve clarity and reduce code complexity. Please see my detailed comments below.
| <!-- 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> | ||
|
|
||
| <!-- 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 %}"> | ||
| {{ 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> |
There was a problem hiding this comment.
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
web/templates/memes.html (1)
112-129: Factor out common query-string into a variable to simplify pagination linksEvery pagination link manually repeats:
?{% if selected_subject %}subject={{ selected_subject }}&{% endif %}{% if selected_user %}user={{ selected_user }}&{% endif %}Building the query once (e.g.
query_base) in the view or at the top of the template avoids copy-paste and ensures future parameters (search term, sorting, etc.) propagate automatically.<a href="?{{ query_base }}page={{ memes.next_page_number }}">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
web/templates/memes.html(4 hunks)web/views.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: bits-and-atoms
PR: alphaonelabs/alphaonelabs-education-website#0
File: :0-0
Timestamp: 2025-03-18T17:43:54.133Z
Learning: The Meme model in this project has validators for image size (2MB limit) and file extensions (.jpg, .jpeg, .png, .gif), with specific field constraints including an optional caption (blank=True) and a subject that uses SET_NULL on deletion.
Learnt from: bits-and-atoms
PR: alphaonelabs/alphaonelabs-education-website#0
File: :0-0
Timestamp: 2025-03-18T17:43:54.133Z
Learning: The Meme model in this project has validators for image size (2MB limit) and file extensions (.jpg, .jpeg, .png, .gif), with comprehensive test coverage needed for these validators and field constraints.
web/templates/memes.html (1)
Learnt from: bits-and-atoms
PR: alphaonelabs/alphaonelabs-education-website#0
File: :0-0
Timestamp: 2025-03-18T17:43:54.133Z
Learning: The Meme model in this project has validators for image size (2MB limit) and file extensions (.jpg, .jpeg, .png, .gif), with specific field constraints including an optional caption (blank=True) and a subject that uses SET_NULL on deletion.
web/views.py (3)
Learnt from: Inshamhaque
PR: alphaonelabs/alphaonelabs-education-website#0
File: :0-0
Timestamp: 2025-04-15T10:37:09.638Z
Learning: When adding delete functionality for user-owned objects in Django, use a POST-only view with ownership checks, a confirmation prompt, and a redirect with a success message.
Learnt from: Inshamhaque
PR: alphaonelabs/alphaonelabs-education-website#0
File: :0-0
Timestamp: 2025-04-15T10:38:52.032Z
Learning: When adding delete functionality for user-owned objects in Django, use a POST-only view with ownership checks, a confirmation prompt, and a redirect with a success message.
Learnt from: IITI-tushar
PR: #264
File: web/forms.py:1582-1596
Timestamp: 2025-03-31T07:19:49.516Z
Learning: In the AlphaOneLabs education website, the MeetupForm intentionally excludes the 'creator' field from the form fields, following the pattern where the request.user is assigned as the creator in the view after form validation. This is a secure approach to prevent users from manipulating the creator field.
🪛 Ruff (0.12.2)
web/views.py
4474-4474: Trailing comma missing
Add trailing comma
(COM812)
🔇 Additional comments (2)
web/views.py (2)
4369-4372: LGTM! User filtering implementation is correct.The user filtering logic properly extracts the username from GET parameters and applies it to the queryset. This enables the requested "Filter by User" functionality while maintaining compatibility with existing subject filtering.
4378-4388: LGTM! Template context properly supports dual filtering.The updated render call includes all necessary context variables for the enhanced filtering UI:
users: Available users for the dropdownselected_subjectandselected_user: Preserve filter state across page loadsThis supports the combined subject and user filtering functionality as intended.
| <!-- 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> |
There was a problem hiding this comment.
🛠️ 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.
| <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 %}"> |
There was a problem hiding this comment.
🧹 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.
| <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.
| <!-- 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> |
There was a problem hiding this comment.
🧹 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.
| # 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"] |
There was a problem hiding this comment.
🧹 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.
| 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.
Update web/views.py to fix incorrect related_name in meme filter Replaced 'meme__isnull=False' with 'memes__isnull=False' in the user query to align with the correct related_name defined in the Meme model. Fixes potential FieldError and ensures accurate filtering of users who have posted memes. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
A1L13N
left a comment
A1L13N
left a comment
There was a problem hiding this comment.
Please update this PR:
✅ Ensure all review comments are addressed
🔄 Resolve any merge conflicts
🧭 Verify that database migrations are correct and up to date
Once everything is done, please push the updated commits so we can proceed with the review.
2 participants
This pull request fixes an issue where the filter by user functionality was not working as expected. The logic has been corrected to ensure that items are now filtered properly based on the currently selected or logged-in user.
Related issues
Fixes #619
Summary by CodeRabbit