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
21 changes: 21 additions & 0 deletions web/migrations/0064_alter_goods_options_alter_order_options.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Generated by Django 5.1.15 on 2026-02-23 05:38

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('web', '0063_virtualclassroom_virtualclassroomcustomization_and_more'),
]

operations = [
migrations.AlterModelOptions(
name='goods',
options={'ordering': ['-created_at']},
),
migrations.AlterModelOptions(
name='order',
options={'ordering': ['-created_at']},
),
]
Comment on lines +8 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Suppress RUF012 for migration files via Ruff config rather than editing auto-generated code.

Both dependencies (lines 8–10) and operations (lines 12–21) are flagged by Ruff's RUF012 rule ("Mutable class attribute should be annotated with typing.ClassVar"). This is a well-known false positive for Django migration files — Django's framework intentionally reads these as mutable class-level lists, and annotating them with ClassVar in every generated file is impractical.

The idiomatic fix is a one-time Ruff config change:

⚙️ Suppress RUF012 for all migration files via per-file-ignores

In pyproject.toml (or ruff.toml):

 [tool.ruff.lint.per-file-ignores]
+"*/migrations/*.py" = ["RUF012"]
🧰 Tools
🪛 Ruff (0.15.1)

[warning] 8-10: Mutable default value for class attribute

(RUF012)


[warning] 12-21: Mutable default value for class attribute

(RUF012)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/migrations/0064_alter_goods_options_alter_order_options.py` around lines
8 - 21, Do not modify the autogenerated migration's dependencies or operations
lists; instead suppress RUF012 for migration files via the Ruff config by adding
a per-file-ignores entry that targets your migrations (e.g.,
"web/migrations/*.py" or "*/migrations/*.py") and ignores RUF012; update
pyproject.toml or ruff.toml accordingly so the linter skips the mutable class
attribute warning for the generated symbols dependencies and operations in
migration files.

6 changes: 6 additions & 0 deletions web/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,9 @@ def save(self, *args, **kwargs):
self.slug = slug
super().save(*args, **kwargs)

class Meta:
ordering = ["-created_at"]

def __str__(self):
return f"{self.name} (${self.price})"

Expand Down Expand Up @@ -1636,6 +1639,9 @@ def save(self, *args, **kwargs):
def generate_tracking_number(self):
return f"TRACK-{self.pk}-{int(time.time())}-{uuid.uuid4().hex[:6].upper()}"

class Meta:
ordering = ["-created_at"]

def __str__(self):
return f"Order #{self.id} ({self.user.email})"

Expand Down
128 changes: 107 additions & 21 deletions web/templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -343,16 +343,22 @@
</div>
<!-- Search bar (persistent element) -->
<div class="relative hidden lg:inline-block w-[250px]">
<form action="{% url 'course_search' %}" method="get" class="m-0">
<form action="{% url 'course_search' %}"
method="get"
class="m-0"
id="desktop-search-form">
<input type="text"
name="q"
autocomplete="off"
placeholder="What do you want to learn?"
class="rounded-full w-[250px] bg-white dark:bg-gray-700 text-gray-900 dark:text-gray-100 px-3 py-1.5 focus:outline-none focus:ring-2 focus:ring-teal-300 dark:focus:ring-teal-700" />
class="navbar-search-input rounded-full w-[250px] bg-white dark:bg-gray-700 text-gray-900 dark:text-gray-100 px-3 py-1.5 focus:outline-none focus:ring-2 focus:ring-teal-300 dark:focus:ring-teal-700" />
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 | 🟠 Major

Replace custom CSS classes with data-* attributes for JavaScript hooks.

navbar-search-input (lines 354, 563) and navbar-search-results (lines 360, 569) are custom CSS class names added solely as JS selectors, violating the project guideline. Use data-* attributes instead and update the JS selectors accordingly.

♻️ Proposed fix

In the desktop search input (line 354):

-class="navbar-search-input rounded-full w-[250px] ..."
+data-search-input class="rounded-full w-[250px] ..."

In the desktop results container (line 360):

-<div class="navbar-search-results hidden absolute top-full ...">
+<div data-search-results class="hidden absolute top-full ...">

Apply the same changes at lines 563 and 569 for the mobile equivalents.

In the JavaScript (lines 1155, 1159, 1211):

-const searchInputs = document.querySelectorAll('.navbar-search-input');
+const searchInputs = document.querySelectorAll('[data-search-input]');
 ...
-const resultsContainer = input.closest('.relative').querySelector('.navbar-search-results');
+const resultsContainer = input.closest('.relative').querySelector('[data-search-results]');
 ...
-document.querySelectorAll('.navbar-search-results').forEach(...)
+document.querySelectorAll('[data-search-results]').forEach(...)

As per coding guidelines: "Never use custom CSS classes" (**/*.{html,htm,css}).

Also applies to: 360-360, 563-563, 569-569

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/templates/base.html` at line 354, Replace the JS-only CSS selectors by
adding data attributes to the desktop and mobile search input and results
elements (e.g. change class="navbar-search-input" to data-navbar-search-input
and class="navbar-search-results" to data-navbar-search-results) and then update
the JS that currently queries ".navbar-search-input" and
".navbar-search-results" to use
document.querySelector('[data-navbar-search-input]') /
'[data-navbar-search-results]' (and the same for querySelectorAll or closest
uses) so the styling classes are not used as JS hooks; apply the change
consistently for both desktop and mobile inputs/results and update all JS
references that select these elements.

<button type="submit"
class="absolute right-3 top-2 text-gray-500 dark:text-gray-300">
<i class="fas fa-search"></i>
</button>
</form>
<div class="navbar-search-results hidden absolute top-full left-0 right-0 mt-2 bg-white dark:bg-gray-800 rounded-lg shadow-xl z-[100] max-h-96 overflow-y-auto border border-gray-100 dark:border-gray-700">
</div>
</div>
<!-- Messaging Button -->
<a href="{% url 'inbox' %}"
Expand All @@ -365,15 +371,19 @@
<a href="{% url 'cart_view' %}"
class="relative hover:underline flex items-center p-2 hover:bg-teal-700 rounded-lg">
<i class="fa-solid fa-shopping-cart"></i>
{% if request.user.cart.item_count > 0 or request.session.session_key and request.session.session_key|get_cart_item_count > 0 %}
<span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
{% if request.user.is_authenticated %}
{{ request.user.cart.item_count }}
{% else %}
{{ request.session.session_key|get_cart_item_count }}
{% with item_count=request.user.cart.item_count|default:0 %}
{% with s_count=request.session.session_key|get_cart_item_count|default:0 %}
{% if item_count > 0 or s_count > 0 %}
<span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
{% if request.user.is_authenticated %}
{{ item_count }}
{% else %}
{{ s_count }}
{% endif %}
</span>
{% endif %}
</span>
{% endif %}
{% endwith %}
{% endwith %}
Comment on lines +374 to +386
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Cart badge can render 0 for an authenticated user when only session items exist.

If item_count = 0 and s_count > 0 (e.g., items were added before login), the outer {% if %} fires but the inner branch displays item_count (0). The same issue exists in the mobile equivalent at lines 777–789.

🐛 Proposed fix (desktop; apply the same to the mobile block at lines 777–789)
-{% with item_count=request.user.cart.item_count|default:0 %}
-  {% with s_count=request.session.session_key|get_cart_item_count|default:0 %}
-    {% if item_count > 0 or s_count > 0 %}
-      <span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
-        {% if request.user.is_authenticated %}
-          {{ item_count }}
-        {% else %}
-          {{ s_count }}
-        {% endif %}
-      </span>
-    {% endif %}
-  {% endwith %}
-{% endwith %}
+{% if request.user.is_authenticated %}
+  {% with item_count=request.user.cart.item_count|default:0 %}
+    {% if item_count > 0 %}
+      <span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
+        {{ item_count }}
+      </span>
+    {% endif %}
+  {% endwith %}
+{% else %}
+  {% with s_count=request.session.session_key|get_cart_item_count|default:0 %}
+    {% if s_count > 0 %}
+      <span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
+        {{ s_count }}
+      </span>
+    {% endif %}
+  {% endwith %}
+{% endif %}
📝 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
{% with item_count=request.user.cart.item_count|default:0 %}
{% with s_count=request.session.session_key|get_cart_item_count|default:0 %}
{% if item_count > 0 or s_count > 0 %}
<span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
{% if request.user.is_authenticated %}
{{ item_count }}
{% else %}
{{ s_count }}
{% endif %}
</span>
{% endif %}
</span>
{% endif %}
{% endwith %}
{% endwith %}
{% if request.user.is_authenticated %}
{% with item_count=request.user.cart.item_count|default:0 %}
{% if item_count > 0 %}
<span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
{{ item_count }}
</span>
{% endif %}
{% endwith %}
{% else %}
{% with s_count=request.session.session_key|get_cart_item_count|default:0 %}
{% if s_count > 0 %}
<span class="absolute -top-1 -right-1 bg-orange-500 text-white text-xs rounded-full h-4 w-4 flex items-center justify-center">
{{ s_count }}
</span>
{% endif %}
{% endwith %}
{% endif %}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/templates/base.html` around lines 374 - 386, The badge currently chooses
{{ item_count }} whenever request.user.is_authenticated even if item_count == 0
while s_count > 0; update the span rendering logic so that when
request.user.is_authenticated you display s_count if item_count is zero (or
prefer s_count when >0), otherwise display item_count — i.e. make the inner
branch consider both item_count and s_count (use a nested conditional that shows
s_count when item_count == 0 and s_count > 0); apply the same change to the
mobile block that uses the same variables at the other location.

</a>
<!-- New Notification Button for Invitations -->
<a href="{% url 'user_invitations' %}"
Expand Down Expand Up @@ -542,16 +552,22 @@
<div class="pt-16 px-4 pb-8">
<!-- Search bar -->
<div class="relative mb-6">
<form action="{% url 'course_search' %}" method="get" class="m-0">
<form action="{% url 'course_search' %}"
method="get"
class="m-0"
id="mobile-search-form">
<input type="text"
name="q"
autocomplete="off"
placeholder="What do you want to learn?"
class="w-full rounded-full bg-gray-100 dark:bg-gray-700 text-gray-900 dark:text-gray-100 px-3 py-2 focus:outline-none focus:ring-2 focus:ring-teal-300 dark:focus:ring-teal-700" />
class="navbar-search-input w-full rounded-full bg-gray-100 dark:bg-gray-700 text-gray-900 dark:text-gray-100 px-3 py-2 focus:outline-none focus:ring-2 focus:ring-teal-300 dark:focus:ring-teal-700" />
<button type="submit"
class="absolute right-3 top-2.5 text-gray-500 dark:text-gray-400">
<i class="fas fa-search"></i>
</button>
</form>
<div class="navbar-search-results hidden absolute top-full left-0 right-0 mt-2 bg-white dark:bg-gray-800 rounded-lg shadow-xl z-[100] max-h-96 overflow-y-auto border border-gray-100 dark:border-gray-700">
</div>
</div>
<!-- Mobile Navigation Menu with Accordions -->
<div class="space-y-4">
Expand Down Expand Up @@ -758,15 +774,19 @@
<i class="fa-solid fa-shopping-cart mr-2 text-teal-500"></i>
<span>Cart</span>
</div>
{% if request.user.cart.item_count > 0 or request.session.session_key and request.session.session_key|get_cart_item_count > 0 %}
<span class="bg-orange-500 text-white text-xs rounded-full h-5 w-5 flex items-center justify-center">
{% if request.user.is_authenticated %}
{{ request.user.cart.item_count }}
{% else %}
{{ request.session.session_key|get_cart_item_count }}
{% with item_count=request.user.cart.item_count|default:0 %}
{% with s_count=request.session.session_key|get_cart_item_count|default:0 %}
{% if item_count > 0 or s_count > 0 %}
<span class="bg-orange-500 text-white text-xs rounded-full h-5 w-5 flex items-center justify-center">
{% if request.user.is_authenticated %}
{{ item_count }}
{% else %}
{{ s_count }}
{% endif %}
</span>
{% endif %}
</span>
{% endif %}
{% endwith %}
{% endwith %}
</a>
<!-- User Account Section -->
{% if user.is_authenticated %}
Expand Down Expand Up @@ -1060,7 +1080,8 @@ <h3 class="text-lg font-bold mb-4 text-gray-700 dark:text-gray-200">CONNECT WITH
<a href="{% url 'about' %}"
class="hover:text-teal-600 dark:hover:text-teal-400">About Us</a>
<a href="{% url 'terms' %}#terms"
class="hover:text-teal-600 dark:hover:text-teal-400">Terms & Conditions</a>
class="hover:text-teal-600 dark:hover:text-teal-400">Terms &
Conditions</a>
<a href="{% url 'terms' %}#privacy"
class="hover:text-teal-600 dark:hover:text-teal-400">Privacy Policy</a>
<a href="{% url 'terms' %}#cookies"
Expand Down Expand Up @@ -1116,6 +1137,71 @@ <h3 class="text-lg font-bold mb-4 text-gray-700 dark:text-gray-200">CONNECT WITH
}
});
}

// Global Search Functionality
document.addEventListener('DOMContentLoaded', function() {
const searchInputs = document.querySelectorAll('.navbar-search-input');
let debounceTimer;

searchInputs.forEach(input => {
Comment on lines +1143 to +1146
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Shared debounceTimer across multiple inputs can cause cross-input cancellation.

The single debounceTimer variable is closed over by every input's input event listener. A keystroke in one input will clearTimeout the pending debounce started by the other, preventing the first request from firing. Move the timer inside the forEach closure so each input gets its own:

♻️ Proposed fix
-    const searchInputs = document.querySelectorAll('.navbar-search-input');
-    let debounceTimer;
-
-    searchInputs.forEach(input => {
-        const resultsContainer = input.closest('.relative').querySelector('.navbar-search-results');
-
-        input.addEventListener('input', function() {
-            clearTimeout(debounceTimer);
+    document.querySelectorAll('[data-search-input]').forEach(input => {
+        let debounceTimer;
+        const resultsContainer = input.closest('.relative').querySelector('[data-search-results]');
+
+        input.addEventListener('input', function() {
+            clearTimeout(debounceTimer);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/templates/base.html` around lines 1155 - 1158, The shared debounceTimer
declared outside the loop causes cross-input cancellation; move the timer into
the per-input closure so each `.navbar-search-input` gets its own timer.
Specifically, inside the searchInputs.forEach(input => { ... }) block, declare a
local let debounceTimer (or const createDebounce) and use that timer in the
input event listener instead of the outer debounceTimer so
clearTimeout/setTimeout only affect the current input's pending request.

const resultsContainer = input.closest('.relative').querySelector('.navbar-search-results');

input.addEventListener('input', function() {
clearTimeout(debounceTimer);
const query = this.value.trim();

if (query.length < 2) {
resultsContainer.innerHTML = '';
resultsContainer.classList.add('hidden');
return;
}

debounceTimer = setTimeout(() => {
fetch(`/api/search/?q=${encodeURIComponent(query)}`)
.then(response => response.json())
.then(data => {
if (data.results.length > 0) {
resultsContainer.innerHTML = data.results.map(result => `
<a href="${result.url}" class="flex items-center px-4 py-3 hover:bg-teal-50 dark:hover:bg-teal-900/30 border-b border-gray-100 dark:border-gray-700 last:border-0 transition-colors">
<div class="flex-shrink-0 w-8 h-8 flex items-center justify-center rounded-full bg-teal-100 dark:bg-teal-900 text-teal-600 dark:text-teal-400 mr-3">
<i class="${result.icon}"></i>
</div>
<div>
<div class="text-sm font-semibold text-gray-900 dark:text-gray-100">${result.title}</div>
<div class="text-xs text-gray-500 dark:text-gray-400 capitalize">${result.type}</div>
</div>
</a>
`).join('');
resultsContainer.classList.remove('hidden');
} else {
resultsContainer.innerHTML = `
<div class="px-4 py-3 text-sm text-gray-500 dark:text-gray-400 text-center">
No results found for "${query}"
</div>
`;
Comment on lines +1164 to +1181
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical XSS — never inject unsanitized data into innerHTML.

Four vectors exist in this block:

  1. Line 1191 — direct user input: "${query}" is the raw search query inserted into innerHTML. A user can type <img src=x onerror=alert(1)> and it will execute because innerHTML processes event-handler injections even though <script> tags are blocked.
  2. Lines 1182–1183result.title and result.type from the API injected as raw HTML. A compromised or manipulated DB record causes stored XSS.
  3. Line 1177result.url placed in an href attribute via template literal; a javascript: URI here executes code on click.
  4. Line 1179result.icon placed in a class attribute; an injected " onclick=" payload escapes the attribute.

Use DOM construction with textContent / setAttribute and validate URLs instead of template-literal innerHTML:

🔒 Proposed safe DOM builder
-resultsContainer.innerHTML = data.results.map(result => `
-    <a href="${result.url}" class="flex items-center px-4 py-3 ...">
-        <div class="...">
-            <i class="${result.icon}"></i>
-        </div>
-        <div>
-            <div class="...">${result.title}</div>
-            <div class="...">${result.type}</div>
-        </div>
-    </a>
-`).join('');
+resultsContainer.innerHTML = '';
+data.results.forEach(result => {
+    const safeUrl = (result.url.startsWith('/') || result.url.startsWith('http')) ? result.url : '#';
+    const a = document.createElement('a');
+    a.href = safeUrl;
+    a.className = 'flex items-center px-4 py-3 hover:bg-teal-50 dark:hover:bg-teal-900/30 border-b border-gray-100 dark:border-gray-700 last:border-0 transition-colors';
+    const iconWrapper = document.createElement('div');
+    iconWrapper.className = 'flex-shrink-0 w-8 h-8 flex items-center justify-center rounded-full bg-teal-100 dark:bg-teal-900 text-teal-600 dark:text-teal-400 mr-3';
+    const icon = document.createElement('i');
+    icon.className = String(result.icon);  // class only, no HTML injection
+    iconWrapper.appendChild(icon);
+    const textWrapper = document.createElement('div');
+    const titleDiv = document.createElement('div');
+    titleDiv.className = 'text-sm font-semibold text-gray-900 dark:text-gray-100';
+    titleDiv.textContent = result.title;   // safe: textContent escapes HTML
+    const typeDiv = document.createElement('div');
+    typeDiv.className = 'text-xs text-gray-500 dark:text-gray-400 capitalize';
+    typeDiv.textContent = result.type;     // safe
+    textWrapper.appendChild(titleDiv);
+    textWrapper.appendChild(typeDiv);
+    a.appendChild(iconWrapper);
+    a.appendChild(textWrapper);
+    resultsContainer.appendChild(a);
+});

For the "no results" message:

-resultsContainer.innerHTML = `
-    <div class="...">No results found for "${query}"</div>
-`;
+const msg = document.createElement('div');
+msg.className = 'px-4 py-3 text-sm text-gray-500 dark:text-gray-400 text-center';
+msg.textContent = `No results found for "${query}"`;
+resultsContainer.innerHTML = '';
+resultsContainer.appendChild(msg);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/templates/base.html` around lines 1176 - 1193, The code injects
unsanitized values into innerHTML (resultsContainer.innerHTML) using query,
result.title, result.type, result.url, and result.icon which enables XSS; fix by
replacing the template-literal innerHTML construction with explicit DOM
creation: for each item in data.results, create elements via
document.createElement, set text nodes via textContent for title/type and use
element.setAttribute('href', validatedUrl) after validating result.url only
allows http(s) or safe relative paths, and only apply result.icon to className
after validating/whitelisting allowed icon class tokens (or escape/omit unsafe
characters); for the "no results" branch create a div and set its textContent to
include the query safely rather than interpolating into innerHTML, then append
the constructed nodes to resultsContainer and remove any use of innerHTML to
prevent attribute/event injection.

resultsContainer.classList.remove('hidden');
}
});
Comment on lines +1160 to +1184
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Unhandled fetch failures will silently break the search UI.

There is no .catch() handler and no response.ok guard. A 4xx/5xx response triggers response.json() on an error body (often HTML), throws a SyntaxError, and the rejection propagates unhandled. Even a valid JSON error body would crash at data.results.length.

🛡️ Proposed fix
 fetch(`/api/search/?q=${encodeURIComponent(query)}`)
     .then(response => {
+        if (!response.ok) throw new Error(`Search request failed: ${response.status}`);
         return response.json();
     })
     .then(data => {
-        if (data.results.length > 0) {
+        const results = Array.isArray(data.results) ? data.results : [];
+        if (results.length > 0) {
             // … render results …
         } else {
             // … no results …
         }
     })
+    .catch(() => {
+        resultsContainer.innerHTML = '';
+        resultsContainer.classList.add('hidden');
+    });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/templates/base.html` around lines 1172 - 1196, The fetch block that calls
fetch(`/api/search/?q=${encodeURIComponent(query)}`) lacks error handling and a
response.ok check, so add a guard after the fetch to verify response.ok before
calling response.json() and handle non-OK responses (e.g., show a friendly "No
results" or error message in resultsContainer); also chain a .catch(...) to the
promise to handle network/parse errors and ensure resultsContainer is hidden or
displays an error instead of leaving the UI broken, and when using the parsed
data check that data && Array.isArray(data.results) before accessing
data.results.length (update the anonymous then callbacks around response.json()
and the subsequent data handler accordingly).

}, 300);
});

// Close results on escape
input.addEventListener('keydown', function(e) {
if (e.key === 'Escape') {
resultsContainer.classList.add('hidden');
}
});
});
Comment on lines +1146 to +1194
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Search combobox is missing required ARIA attributes for screen readers.

The live-results pattern requires role="combobox", aria-expanded, aria-autocomplete, and aria-controls on the input, plus role="listbox" on the results container. Without these, assistive technologies cannot announce or navigate the dynamic results.

Minimum additions to the input elements (lines 350–354 and 559–563):

 <input type="text"
        name="q"
        autocomplete="off"
        placeholder="What do you want to learn?"
+       role="combobox"
+       aria-autocomplete="list"
+       aria-expanded="false"
+       aria-controls="desktop-search-results"
        class="..." />

Results container (line 360):

-<div class="navbar-search-results hidden absolute ...">
+<div id="desktop-search-results" role="listbox" class="hidden absolute ...">

Also toggle aria-expanded in the JS when results appear/disappear:

+input.setAttribute('aria-expanded', 'true');
 resultsContainer.classList.remove('hidden');
+input.setAttribute('aria-expanded', 'false');
 resultsContainer.classList.add('hidden');

As per coding guidelines: "Include proper ARIA labels where needed for accessibility" (web/templates/**/*.html).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/templates/base.html` around lines 1158 - 1206, Add the required ARIA
attributes to the search input and results container to implement the
live-results combobox pattern: ensure the input elements (the ones iterated by
searchInputs in the searchInputs.forEach block) include role="combobox",
aria-autocomplete="list", aria-expanded="false" and aria-controls pointing to a
unique id for their corresponding resultsContainer, and give the
resultsContainer element role="listbox" and that same id; in the JS (inside the
input 'input' handler and the Escape key handler) toggle the input's
aria-expanded between "true"/"false" when you show/hide resultsContainer and
update resultsContainer.innerHTML as before so screen readers can navigate the
dynamic list.


// Close results when clicking outside
document.addEventListener('click', function(event) {
if (!event.target.closest('.relative')) {
document.querySelectorAll('.navbar-search-results').forEach(container => {
container.classList.add('hidden');
});
}
});
Comment on lines +1197 to +1203
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Click-outside guard is too broad — closest('.relative') matches unrelated navbar elements.

.relative is used on the cart icon wrapper, user dropdown, language dropdown, and other elements throughout the page. Clicking any of them satisfies the guard, keeping the search results open indefinitely. Scope the check to the actual search containers.

🛡️ Proposed fix
-document.addEventListener('click', function(event) {
-    if (!event.target.closest('.relative')) {
-        document.querySelectorAll('.navbar-search-results').forEach(container => {
-            container.classList.add('hidden');
-        });
-    }
-});
+document.addEventListener('click', function(event) {
+    // Only keep results open when the click is inside a search container
+    const isInsideSearch = event.target.closest('[data-search-input]') ||
+                           event.target.closest('[data-search-results]');
+    if (!isInsideSearch) {
+        document.querySelectorAll('[data-search-results]').forEach(container => {
+            container.classList.add('hidden');
+        });
+    }
+});

(This also aligns with the data-* attribute refactor suggested above.)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/templates/base.html` around lines 1209 - 1215, The click-outside guard in
the document click handler is too broad because it checks
event.target.closest('.relative'), which matches many unrelated navbar elements;
update the guard inside the document.addEventListener('click', ...) handler to
only treat clicks inside the actual search component(s) as in-scope (e.g., use a
specific selector like closest('.navbar-search') or a dedicated data attribute
such as closest('[data-navbar-search]') or closest('.navbar-search-results')),
and leave the existing logic that hides elements matching
'.navbar-search-results' unchanged so only clicks outside the real search
container hide the results.

});
</script>
{% block extra_js %}
{% endblock extra_js %}
Expand Down
1 change: 1 addition & 0 deletions web/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
# Course Management
path("courses/create/", views.create_course, name="create_course"),
path("courses/search/", views.course_search, name="course_search"),
path("api/search/", views.global_search_api, name="global_search_api"),
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's verify the urls.py structure and confirm the i18n_patterns context
cat -n web/urls.py | head -120 | tail -40

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 3229


🏁 Script executed:

# Search for how the search API is being called in the frontend
rg -n "api/search" --type html --type js -C 3

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 538


🏁 Script executed:

# Check for Django URL template tag usage in base.html
rg -n "url.*global_search" --type html

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 69


🏁 Script executed:

# Examine the i18n_patterns configuration mentioned in the review
rg -n "prefix_default_language|i18n_patterns" web/urls.py -C 2

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 576


🏁 Script executed:

# Check if there are other API endpoints to see how they handle i18n URLs
rg -n "path.*api/" web/urls.py -A 1

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 346


🏁 Script executed:

# Look at the broader context of how JavaScript calls other Django URLs in the templates
rg -n "fetch.*url|ajax.*url|\{% url" web/templates/base.html -C 2 | head -50

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 4492


🏁 Script executed:

# Check if map-data API is also hardcoded or properly handled with template tags
rg -n "api/map-data|map_data" web/templates/base.html -C 2

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 69


🏁 Script executed:

# Verify the full context of the search implementation to understand severity
cat -n web/templates/base.html | sed -n '1110,1135p'

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 1895


Use Django template tag to resolve the search API URL in JavaScript to support i18n URL prefixes.

The endpoint is registered inside i18n_patterns with prefix_default_language=True (line 629), meaning its effective URL is /en/api/search/, /fr/api/search/, etc., not /api/search/. The current hardcoded fetch call on line 1121 of web/templates/base.html (fetch('/api/search/?q=...')) will silently fail with a 404 in any non-default language context.

Resolve the URL dynamically using Django's template tag. For example, wrap the search input in a data attribute:

Suggested approach
<input type="text" data-search-url="{% url 'global_search_api' %}" ...>

Then reference it in JavaScript:

const searchUrl = input.getAttribute('data-search-url');
fetch(`${searchUrl}?q=${encodeURIComponent(query)}`)

This matches the existing pattern used throughout the template for all other route references (e.g., {% url 'index' %}, {% url 'course_search' %}).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/urls.py` at line 105, The fetch to the search endpoint is hardcoded and
will 404 under i18n prefixes; update the template (web/templates/base.html) to
resolve the URL with Django's url tag and expose it to JS (e.g. add a
data-search-url attribute on the search input using {% url 'global_search_api'
%}), then change the JS that currently calls fetch('/api/search/?q=...') to read
that attribute (e.g. input.getAttribute('data-search-url')) and call
fetch(`${searchUrl}?q=${encodeURIComponent(query)}`) so global_search_api is
correctly resolved for all language prefixes.

path("courses/<slug:slug>/", views.course_detail, name="course_detail"),
path("courses/<slug:course_slug>/enroll/", views.enroll_course, name="enroll_course"),
path("courses/<slug:slug>/add-session/", views.add_session, name="add_session"),
Expand Down
35 changes: 35 additions & 0 deletions web/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8839,3 +8839,38 @@ def leave_session_waiting_room(request, course_slug):
messages.info(request, "You are not in the session waiting room for this course.")

return redirect("course_detail", slug=course_slug)


def global_search_api(request):
"""API endpoint for global search of subjects and courses."""
query = request.GET.get("q", "").strip()
if not query or len(query) < 2:
return JsonResponse({"results": []})

Comment on lines +8844 to +8849
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine web/views.py
fd "views.py" --path "*/web/*"

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 302


🏁 Script executed:

# Get the file size to understand how to read it efficiently
wc -l web/views.py

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 104


🏁 Script executed:

# Read the specific section mentioned in the review (lines 8849-8854 and surrounding context)
# Let's read a broader range to see the full function implementation
sed -n '8845,8885p' web/views.py | cat -n

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 1477


🏁 Script executed:

# Check if there are any lines exceeding 120 characters in the function
sed -n '8849,8881p' web/views.py | awk 'length > 120 {print NR+8848 ": " length " chars: " $0}'

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 69


🏁 Script executed:

# Check imports at the top of web/views.py to see what's available
head -50 web/views.py | cat -n

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 2025


🏁 Script executed:

# Search for `@require_GET` usage in the file to see the pattern
rg "@require_GET" web/views.py -B 2 -A 2

Repository: alphaonelabs/alphaonelabs-education-website

Length of output: 380


Add a GET-only decorator and type hints for the new API.

Line 8849 should include type hints and the @require_GET decorator to satisfy the repository's typing requirements and establish the GET-only contract for this API endpoint.

Suggested change
+@require_GET
-def global_search_api(request):
+def global_search_api(request: HttpRequest) -> JsonResponse:
     """API endpoint for global search of subjects and courses."""
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/views.py` around lines 8849 - 8854, Add the `@require_GET` decorator to the
global_search_api view and add type hints: annotate the parameter as request:
HttpRequest and the return as -> JsonResponse in the global_search_api function
signature; also ensure require_GET is imported from django.views.decorators.http
and HttpRequest/JsonResponse are imported from django.http if not already
present so the endpoint is explicitly GET-only and satisfies typing
requirements.

results = []

# Search Subjects
subjects = Subject.objects.filter(name__icontains=query)[:5]
for subject in subjects:
results.append(
{
"type": "subject",
"title": subject.name,
"url": reverse("course_search") + f"?subject={subject.slug}",
"icon": "fas fa-tag",
}
)

# Search Courses
courses = Course.objects.filter(Q(title__icontains=query) | Q(tags__icontains=query), status="published")[:5]
for course in courses:
Comment on lines +8864 to +8866
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Wrap the course filter to stay within the 120‑char limit.
Line 8870 exceeds the configured maximum line length.

✅ Suggested change
-    courses = Course.objects.filter(Q(title__icontains=query) | Q(tags__icontains=query), status="published")[:5]
+    courses = Course.objects.filter(
+        Q(title__icontains=query) | Q(tags__icontains=query),
+        status="published",
+    )[:5]

As per coding guidelines: Maximum Python line length is 120 characters.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/views.py` around lines 8869 - 8871, The Course queryset line exceeds the
120-char limit; refactor the call to Course.objects.filter(...) by breaking the
arguments across multiple lines using parentheses so each line is under 120
chars — e.g., place the Q(title__icontains=query) | Q(tags__icontains=query)
expression and the status="published" kwarg on separate indented lines inside
the filter invocation (referencing Course.objects.filter and Q) and keep the
slice [:5] on its own line or together with the closing parenthesis to maintain
readability.

results.append(
{
"type": "course",
"title": course.title,
"url": reverse("course_detail", kwargs={"slug": course.slug}),
"icon": "fas fa-book",
}
)

return JsonResponse({"results": results})