Skip to content

Feature/improve module management ux #5059

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.

Sign up for GitHub

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

Draft
harsitagarwalla187 wants to merge 3 commits into
base: main
Choose a base branch
from
+88 −21
Draft

Feature/improve module management ux #5059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c46fa14
feat(mentorship): sort module mentors alphabetically and improve modu…
harsitagarwalla187 Jun 26, 2026
12b0464
Merge remote-tracking branch 'upstream/main' into feature/improve-mod…
harsitagarwalla187 Jun 26, 2026
bdf299b
feat(mentorship): improve module UX and optimize mentor queries
harsitagarwalla187 Jun 26, 2026
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
49 changes: 42 additions & 7 deletions backend/apps/mentorship/api/internal/queries/module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
import logging

import strawberry
from django.db.models import Q
from django.db.models import Prefetch, Q

from apps.mentorship.api.internal.graphql_errors import (
AuthenticationRequiredError,
ManagementProgramAccessDeniedError,
)
from apps.mentorship.api.internal.nodes.module import ModuleNode
from apps.mentorship.models import Module, Program
from apps.mentorship.models import Mentor, Module, Program

logger = logging.getLogger(__name__)

Expand All @@ -35,7 +35,14 @@ def get_program_modules(self, info: strawberry.Info, program_key: str) -> list[M
return (
Module.objects.filter(program=program)
.select_related("program", "project")
.prefetch_related("mentors__github_user")
.prefetch_related(
Prefetch(
"mentors",
queryset=Mentor.objects.select_related("github_user").order_by(

@cubic-dev-ai cubic-dev-ai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Use Lower("github_user__login") instead of "github_user__login" for case-insensitive alphabetical sorting. Without it, the sort depends on database collation—PostgreSQL's default is case-sensitive, so "Zed" would sort before "alice". Since GitHub usernames are case-insensitive, this would produce unintuitive ordering.

from django.db.models.functions import Lower

queryset=Mentor.objects.select_related("github_user").order_by(
    Lower("github_user__login")
)

This applies to all five occurrences in this file.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At backend/apps/mentorship/api/internal/queries/module.py, line 41:

<comment>Use `Lower("github_user__login")` instead of `"github_user__login"` for case-insensitive alphabetical sorting. Without it, the sort depends on database collation—PostgreSQL's default is case-sensitive, so `"Zed"` would sort before `"alice"`. Since GitHub usernames are case-insensitive, this would produce unintuitive ordering.

```python
from django.db.models.functions import Lower

queryset=Mentor.objects.select_related("github_user").order_by(
    Lower("github_user__login")
)

This applies to all five occurrences in this file.

@@ -35,7 +35,14 @@ def get_program_modules(self, info: strawberry.Info, program_key: str) -> list[M + .prefetch_related( + Prefetch( + "mentors", + queryset=Mentor.objects.select_related("github_user").order_by( + "github_user__login" + ), ```

"github_user__login"
Comment on lines +41 to +42

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Look for any normalization of GitHub logins before persistence.
rg -n -C2 --type=py 'lower\(|casefold\(|github_user.*login|login.*github_user' backend frontend

# Check whether resolver/tests already cover mixed-case mentor ordering.
rg -n -C2 --type=py 'github_user__login|Lower\("github_user__login"\)|mentors.*order' backend
rg -n -C2 --glob '*test*.py' --glob '*test*.tsx' 'alice|Alice|zed|Zed|mentor.*order|github.*username' backend frontend

Repository: OWASP/Nest

Length of output: 50367

Ensure deterministic, case-insensitive mentor ordering.

order_by("github_user__login") relies on database collation, which may sort mixed-case values unexpectedly (e.g., "Zed" before "alice"). While mutations fetch users using login__iexact, the retrieval of the full mentor list uses raw casing.

Use Lower() to guarantee alphabetical order:

Code fix 
from django.db.models.functions import Lower

# ...
queryset=Mentor.objects.select_related("github_user").order_by(
    Lower("github_user__login")
)
# ...

This applies to all occurrences in backend/apps/mentorship/api/internal/queries/module.py (lines 41-42, 79-80, 97-98, 116-117, 150-151).

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/apps/mentorship/api/internal/queries/module.py` around lines 41 - 42,
The mentor list ordering in the query methods is currently using raw login
values, which can produce nondeterministic mixed-case sorting. Update all Mentor
queryset orderings in the module’s query builders to use a case-insensitive sort
with Lower on github_user__login, and ensure Lower is imported from
django.db.models.functions so the ordering is consistent across every occurrence
in the query class methods.

),
)
)
.order_by("order", "started_at")
)

Expand Down Expand Up @@ -66,7 +73,14 @@ def get_management_program_modules(

return (
modules.select_related("program", "project")
.prefetch_related("mentors__github_user")
.prefetch_related(
Prefetch(
"mentors",
queryset=Mentor.objects.select_related("github_user").order_by(
"github_user__login"
),
)
)
.distinct()
.order_by("order", "started_at")
)
Expand All @@ -77,7 +91,14 @@ def get_project_modules(self, project_key: str) -> list[ModuleNode]:
return (
Module.objects.filter(project__key=project_key)
.select_related("program", "project")
.prefetch_related("mentors__github_user")
.prefetch_related(
Prefetch(
"mentors",
queryset=Mentor.objects.select_related("github_user").order_by(
"github_user__login"
),
)
)
.order_by("order", "started_at")
)

Expand All @@ -89,7 +110,14 @@ def get_module(
try:
module = (
Module.objects.select_related("program", "project")
.prefetch_related("mentors__github_user")
.prefetch_related(
Prefetch(
"mentors",
queryset=Mentor.objects.select_related("github_user").order_by(
"github_user__login"
),
)
)
.get(key=module_key, program__key=program_key)
)
except Module.DoesNotExist:
Expand All @@ -116,7 +144,14 @@ def get_management_module(
try:
module = (
Module.objects.select_related("program", "project")
.prefetch_related("mentors__github_user")
.prefetch_related(
Prefetch(
"mentors",
queryset=Mentor.objects.select_related("github_user").order_by(
"github_user__login"
),
)
)
.get(key=module_key, program__key=program_key)
)
except Module.DoesNotExist:
Expand Down
10 changes: 8 additions & 2 deletions frontend/__tests__/unit/components/ModuleForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,16 @@ describe('ModuleForm', () => {
expect(screen.getByTestId('text-input-module-labels')).toBeInTheDocument()
})

it('renders mentor logins field only when isEdit is true (line 312)', () => {
it('renders mentor logins field when isEdit is false', () => {
renderModuleForm({ isEdit: false })
expect(screen.queryByTestId('text-input-module-mentor-logins')).not.toBeInTheDocument()
expect(screen.getByTestId('text-input-module-mentor-logins')).toBeInTheDocument()
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.

const input = screen.getByTestId('input-module-mentor-logins')
fireEvent.change(input, { target: { value: 'johndoe, Kateryna' } })
expect(mockSetFormData).toHaveBeenCalled()
Comment on lines +332 to +334

@coderabbitai coderabbitai Bot Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert the mentorLogins update, not just any setter call.

toHaveBeenCalled() still passes if the change handler writes the wrong field. Inspect the setter argument and verify it produces mentorLogins: 'johndoe, Kateryna' from the baseline form state.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/__tests__/unit/components/ModuleForm.test.tsx` around lines 332 -
334, The ModuleForm test is only checking that the form setter was called, which
can pass even if the wrong field is updated. In the `ModuleForm.test.tsx`
scenario around the `input-module-mentor-logins` change, assert the argument
passed to `mockSetFormData` and verify it updates `mentorLogins` to `johndoe,
Kateryna` from the existing form state, using the same setter call triggered by
the change handler.

})

it('renders mentor logins field when isEdit is true', () => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
renderModuleForm({ isEdit: true })
expect(screen.getByTestId('text-input-module-mentor-logins')).toBeInTheDocument()
})
Expand Down
29 changes: 29 additions & 0 deletions frontend/src/app/globals.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,3 +482,32 @@ select:disabled,
.chapter-map .leaflet-marker-shadow {
cursor: pointer !important;
}

/* MarkdownWrapper CSS */

.md-wrapper p {
margin: 0 0 16px;
}

.md-wrapper ul {
margin: 16px 0;
padding-left: 24px;
}

.md-wrapper ol {
margin: 16px 0;
padding-left: 24px;
}

.md-wrapper li {
margin-bottom: 8px;
}

.md-wrapper h1,
.md-wrapper h2,
.md-wrapper h3,
.md-wrapper h4,
.md-wrapper h5,
.md-wrapper h6 {
margin: 24px 0 16px;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
2 changes: 1 addition & 1 deletion frontend/src/components/ModuleCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ const ModuleItem = ({
<div className="flex items-center justify-between">
<Link
href={`${pathname}/modules/${module.key}`}
className="text-start font-semibold text-gray-600 hover:underline dark:text-gray-300"
className="mr-2 min-w-0 flex-1 text-start font-semibold text-gray-600 hover:underline dark:text-gray-300"
>
<TruncatedText text={module?.name} />
</Link>
Expand Down
19 changes: 8 additions & 11 deletions frontend/src/components/ModuleForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
setFormData: React.Dispatch<React.SetStateAction<ModuleFormProps['formData']>>
onSubmit: (e: React.FormEvent) => void
loading: boolean
isEdit?: boolean

Check warning on line 42 in frontend/src/components/ModuleForm.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'isEdit' PropType is defined but prop is never used 


See more on https://sonarcloud.io/project/issues?id=OWASP_Nest&issues=AZ8E_nsIRt_76T3rJjfZ&open=AZ8E_nsIRt_76T3rJjfZ&pullRequest=5059
title: string
submitText?: string
minDate?: string
Expand All @@ -60,7 +60,6 @@
onSubmit,
loading,
title,
isEdit,
submitText = 'Save',
minDate,
maxDate,
Expand Down Expand Up @@ -308,16 +307,14 @@
errorMessage={touched.projectId ? errors.projectId : undefined}
/>
</div>
{isEdit && (
<FormTextInput
id="module-mentor-logins"
label="Mentor GitHub Usernames"
placeholder="johndoe, jane-doe"
value={formData.mentorLogins}
onValueChange={(value) => handleInputChange('mentorLogins', value)}
className="w-full min-w-0 lg:col-span-2"
/>
)}
<FormTextInput
id="module-mentor-logins"
label="Mentor GitHub Usernames"
placeholder="johndoe, jane-doe"
value={formData.mentorLogins}
onValueChange={(value) => handleInputChange('mentorLogins', value)}
className="w-full min-w-0 lg:col-span-2"
/>
</div>
</section>

Expand Down