backend/apps/mentorship/management/commands/link_issues_by_label.py (1)

43-61: Consider optimizing the linked issues query.

The command logic is sound, but there's a potential N+1 query issue where module.linked_issues.values_list("id", flat=True) executes a database query for each module.

Consider prefetching the linked issues to avoid repeated queries:

 modules = (
     Module.objects.exclude(linked_issue_labels__isnull=True)
     .exclude(linked_issue_labels=[])
+    .prefetch_related("linked_issues")
     .iterator()
 )

Alternatively, you could batch the processing by collecting all module updates and applying them together, though the current approach with individual transactions provides better error isolation.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

backend/apps/mentorship/Makefile (1)

1-2: Optional: add minimal conventional targets to satisfy checkmake

Static analysis flagged missing all, clean, and test phony targets.
If this Makefile is intentionally scoped to a single command you can ignore the warning, otherwise consider stubs to silence CI:

.PHONY: all clean test
all: ## default noop
clean: ## noop
test: ## noop

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.

Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.

backend/apps/mentorship/management/commands/link_issues_by_label.py (3)

30-30: Drop select_related('repository'); limit fields to reduce memory

You only use issue.id and issue.repository_id, so select_related isn't needed. Limit columns to reduce memory.

-        issues_query = Issue.objects.select_related("repository").prefetch_related("labels")
+        issues_query = Issue.objects.only("id", "repository_id").prefetch_related("labels")

---

68-73: Don’t overwrite manual links by default; add --append and --dry-run

set(...) replaces all existing links. Provide safer defaults and options:

• --append: add only the newly matched issues.
• --dry-run: compute and report without writing.

Apply within handle():

-            with transaction.atomic():
-                module.issues.set(matched_issue_ids)
-
-            num_linked = len(matched_issue_ids)
+            dry_run = options.get("dry_run", False)
+            with transaction.atomic():
+                if options.get("append"):
+                    existing_ids = set(module.issues.values_list("id", flat=True))
+                    to_add = matched_issue_ids - existing_ids
+                    if not dry_run and to_add:
+                        module.issues.add(*to_add)
+                    num_linked = len(to_add)
+                else:
+                    if not dry_run:
+                        module.issues.set(matched_issue_ids)
+                    num_linked = len(matched_issue_ids)

And define arguments:

def add_arguments(self, parser):
    parser.add_argument(
        "--append", action="store_true",
        help="Add new matches without removing existing links."
    )
    parser.add_argument(
        "--dry-run", action="store_true",
        help="Compute matches and print changes without writing."
    )

Would you like me to open a follow-up PR with these changes?

---

71-73: Clarify and correct “total links created” accounting

Currently this sums the size of each matched set, not the number of newly-created M2M links. If you add --append, increment by len(to_add). If replacing, you can report “set N links” instead of “created”.

Example:

total_links_created += num_linked  # where num_linked reflects added links in append mode

And tweak the summary message to “set” vs “added” depending on mode.

Also applies to: 83-88

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: backend/apps/mentorship/graphql/mutations/module.py:28-29
Timestamp: 2025-07-16T13:49:58.648Z
Learning: In the OWASP Nest mentorship system, mentors can be created with only github_user initially (without nest_user) when assigned to modules. This allows mentors to be assigned before they've signed into the system. When these users eventually sign in, the nest_user is automatically linked to the existing github_user, creating a complete mentor profile. This design provides flexibility in mentor assignment workflows.

Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/src/components/ModuleForm.tsx:112-134
Timestamp: 2025-07-14T16:18:07.287Z
Learning: In the OWASP Nest mentorship system, date validation for modules (ensuring start date precedes end date) is handled on the backend in the module GraphQL mutations via the `_validate_module_dates` helper function in backend/apps/mentorship/graphql/mutations/module.py, which prevents invalid date ranges from being stored in the database.

backend/apps/mentorship/management/commands/link_issues_by_label.py (2)

52-57: Add distinct() to prevent duplicate modules from M2M joins.

This addresses previous feedback about duplicate module processing due to the M2M relationship with repositories.

         modules_to_process = (
             Module.objects.prefetch_related("project__repositories")
             .exclude(project__repositories__isnull=True)
             .exclude(labels__isnull=True)
             .exclude(labels=[])
+            .distinct()
         )

---

31-41: Performance issue: Query optimization needed to avoid loading entire database.

The current implementation fetches all issues and labels from the database without filtering, which can cause significant performance issues as the database grows. This addresses the previous review feedback about scaling.

Based on the previous review suggestions, implement the optimized approach using the through table:

-        issues_query = Issue.objects.select_related("repository").prefetch_related("labels")
-
-        for issue in issues_query:
-            if not issue.repository_id:
-                continue
-
-            for label in issue.labels.all():
-                normalized_label = (label.name or "").strip().casefold()
-                key = (issue.repository_id, normalized_label)
-                repo_label_to_issue_ids.setdefault(key, set()).add(issue.id)
+        # First, get modules to determine which repos/labels we care about
+        modules_queryset = (
+            Module.objects.prefetch_related("project__repositories")
+            .exclude(project__repositories__isnull=True)
+            .exclude(labels__isnull=True)
+            .exclude(labels=[])
+            .distinct()
+        )
+        
+        # Gather repos and labels of interest
+        repo_ids = set()
+        wanted_labels = set()
+        for module in modules_queryset:
+            for repo in module.project.repositories.all():
+                repo_ids.add(repo.id)
+            for label_name in (module.labels or []):
+                wanted_labels.add((label_name or "").strip().casefold())
+        
+        if not repo_ids or not wanted_labels:
+            self.stdout.write("No repositories or labels found to process.")
+            return
+            
+        # Query via through table for efficiency
+        Through = Issue.labels.through
+        pairs = (
+            Through.objects
+            .select_related("issue__repository", "label")
+            .filter(
+                issue__repository_id__in=repo_ids,
+                label__name__in=wanted_labels
+            )
+            .values_list("issue_id", "issue__repository_id", "label__name")
+        )
+        
+        # Build the map efficiently
+        for issue_id, repo_id, label_name in pairs:
+            normalized_label = (label_name or "").strip().casefold()
+            if normalized_label in wanted_labels:  # Double-check normalization
+                key = (repo_id, normalized_label)
+                repo_label_to_issue_ids.setdefault(key, set()).add(issue_id)

backend/apps/mentorship/management/commands/link_issues_by_label.py (1)

76-79: Potential performance issue: Consider using prefetch_related for assignees.

The current query fetches issues and then accesses assignees.first() for each issue individually, which could result in N+1 query problems.

                 if matched_issue_ids:
                     issues = Issue.objects.filter(
                         id__in=matched_issue_ids,
                         assignees__isnull=False,
-                    ).distinct()
+                    ).prefetch_related("assignees").distinct()

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

backend/apps/mentorship/Makefile (1)

1-5: Declare these targets as .PHONY and standardize verbosity.

Make can skip targets if a file/dir matches the name. Mark them phony. Also consider consistent verbosity across the two commands.

+.PHONY: sync/module-issues sync/issue-levels
 sync/module-issues:
 	@CMD="python manage.py sync_module_issues -v 2" $(MAKE) exec-backend-command

 sync/issue-levels:
-	@CMD="python manage.py sync_issue_levels" $(MAKE) exec-backend-command
+	@CMD="python manage.py sync_issue_levels -v 2" $(MAKE) exec-backend-command

backend/apps/mentorship/utils.py (1)

4-6: Type annotations and docstring alignment for normalize_name

Minor polish: reflect that casefold is used (more robust than lower) and add type hints for clarity.

Apply this diff to the function:

-def normalize_name(name):
-    """Normalize a string by stripping whitespace and converting to lowercase."""
-    return (name or "").strip().casefold()
+def normalize_name(name: "Optional[str]") -> str:
+    """Normalize a string by stripping whitespace and applying casefold for case-insensitive matching."""
+    return (name or "").strip().casefold()

Add the missing import at the top of the file:

from typing import Optional

backend/apps/github/models/issue.py (1)

57-64: Rename related_name and fix help_text to reflect Issue semantics

The FK lives on Issue, so using related_name="tasks" and mentioning “task” in help_text is misleading. Recommend aligning both to “issue”.

Apply this diff:

-    level = models.ForeignKey(
-        "mentorship.TaskLevel",
-        null=True,
-        blank=True,
-        on_delete=models.SET_NULL,
-        related_name="tasks",
-        help_text="The difficulty level of this task.",
-    )
+    level = models.ForeignKey(
+        "mentorship.TaskLevel",
+        null=True,
+        blank=True,
+        on_delete=models.SET_NULL,
+        related_name="issues",
+        help_text="The difficulty level of this issue.",
+    )

Note: This will require a migration and updating any usages of TaskLevel.tasks (if any) to TaskLevel.issues.

backend/apps/github/admin/issue.py (1)

22-22: Add filter and select_related for level to improve admin UX and performance

• Add level to list_filter to quickly narrow issues by difficulty.
• Use list_select_related to avoid N+1 queries for repository and level in the changelist.

Proposed additions:

# Improve filters
list_filter = (
    "state",
    "is_locked",
    "level",
)

# Avoid N+1 on FK columns shown in list_display
list_select_related = ("repository", "level")

backend/apps/mentorship/models/task.py (1)

28-30: Consider indexing assigned_at since it’s used for ordering/search

With admin ordering by assigned_at and likely frequent sorting/filtering, adding a DB index will help at scale.

Apply this diff:

-    assigned_at = models.DateTimeField(
-        null=True,
-        blank=True,
-        help_text="Timestamp when the task was assigned to the mentee.",
-    )
+    assigned_at = models.DateTimeField(
+        null=True,
+        blank=True,
+        db_index=True,
+        help_text="Timestamp when the task was assigned to the mentee.",
+    )

backend/apps/mentorship/admin/task.py (1)

22-23: Ordering by -assigned_at: consider nulls-last and select_related/search for better admin usability

• Null assigned_at will sort first on some DBs; if you want assigned tasks first, consider nulls_last (requires Django 4.1+ via get_ordering with F expression).
• Add list_select_related to avoid N+1 on issue/assignee/module columns in list_display.
• Restoring simple search_fields will improve discoverability.

Option (if Django 4.1+):

from django.db.models import F

def get_ordering(self, request):
    return [F("assigned_at").desc(nulls_last=True)]

Additional enhancements:

list_select_related = ("issue", "assignee", "module")

search_fields = (
    "issue__title",
    "assignee__login",
    "module__name",
)

backend/apps/mentorship/management/commands/sync_issue_levels.py (2)

1-1: Fix docstring typo: “Tasklevel” → “TaskLevel”.

Minor polish.

-"""A command to sync issue level with Tasklevel."""
+"""A command to sync issue level with TaskLevel."""

---

70-77: Iterate with a cursor to reduce memory footprint on large datasets.

Using QuerySet.iterator() prevents loading all Issues into memory at once.

-        for issue in issues_query:
+        for issue in issues_query.iterator(chunk_size=2000):
             issue_labels_normalized = {normalize_name(label.name) for label in issue.labels.all()}

backend/apps/mentorship/management/commands/sync_module_issues.py (3)

60-67: Use explicit None-check on repository_id and rely on the FK’s numeric id for dict keys.

Guard against pathological cases and keep keys as ints.

-        issues_query = Issue.objects.select_related("repository").prefetch_related("labels")
+        issues_query = Issue.objects.select_related("repository").prefetch_related("labels")
         for issue in issues_query:
-            if not issue.repository_id:
+            if issue.repository_id is None:
                 continue
             for label in issue.labels.all():
                 normalized_label = normalize_name(label.name)
-                key = (issue.repository_id, normalized_label)
+                key = (int(issue.repository_id), normalized_label)
                 repo_label_to_issue_ids.setdefault(key, set()).add(issue.id)

---

24-31: Simplify by removing URL parsing at callsites.

Now that _extract_repo_full_name accepts the repo object, pass the Repository instance directly.

-                        repo_full_name = self._extract_repo_full_name(str(issue.repository.url))
+                        repo_full_name = self._extract_repo_full_name(issue.repository)

Also applies to: 113-129

---

1-2: Polish the command’s help text.

Minor grammar fix.

-"""A command to sync update relation between module and issue and create task."""
+"""Sync module–issue relations and create tasks."""

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

backend/apps/mentorship/management/commands/sync_module_issues.py (1)

23-34: Harden repo full_name extraction; prefer model attribute; fix host-case; support api.github.com.

Current parsing misses api.github.com, is case-sensitive, and depends on stringified URLs. Prefer Repository.full_name when available to avoid URL parsing entirely. This also addresses the CodeQL “incomplete URL sanitization” family of findings.

-    ALLOWED_GITHUB_HOSTS = {"github.com", "www.github.com"}
+    ALLOWED_GITHUB_HOSTS = {"github.com", "www.github.com", "api.github.com"}
@@
-    def _extract_repo_full_name(self, repo_url):
-        parsed = urlparse(repo_url or "")
-        if parsed.netloc in self.ALLOWED_GITHUB_HOSTS:
-            parts = parsed.path.strip("/").split("/")
-            if len(parts) >= self.REPO_PATH_PARTS:
-                return "/".join(parts[: self.REPO_PATH_PARTS])
-            return None
-        return None
+    def _extract_repo_full_name(self, repo_or_url):
+        """Return 'owner/repo' using Repository.full_name if present, else parse URL safely."""
+        full_name = getattr(repo_or_url, "full_name", None)
+        if full_name:
+            return full_name
+
+        parsed = urlparse(str(repo_or_url or ""))
+        host = (parsed.netloc or "").lower()
+        parts = [p for p in (parsed.path or "").strip("/").split("/") if p]
+        if host in self.ALLOWED_GITHUB_HOSTS and len(parts) >= 2:
+            # Handle api.github.com/repos/{owner}/{repo}
+            if host == "api.github.com" and parts[0] == "repos" and len(parts) >= 3:
+                return f"{parts[1]}/{parts[2]}"
+            return f"{parts[0]}/{parts[1]}"
+        return None

backend/apps/mentorship/management/commands/sync_module_issues.py (6)

1-2: Polish the command docstring for clarity.

Reword to read more naturally.

-"""A command to sync update relation between module and issue and create task."""
+"""Sync module–issue links and create/update mentorship tasks."""

---

97-106: Pre-normalize labels once per module.

Minor perf/readability improvement.

-        linked_label_names = module.labels
+        linked_label_names = module.labels
+        normalized_labels = {normalize_name(n) for n in (linked_label_names or [])}
@@
-        for repo in project_repos:
-            for label_name in linked_label_names:
-                normalized_label = normalize_name(label_name)
+        for repo in project_repos:
+            for normalized_label in normalized_labels:
                 key = (repo.id, normalized_label)
                 issues_for_label = repo_label_to_issue_ids.get(key, set())
                 matched_issue_ids.update(issues_for_label)

---

165-169: Log owner/repo for clarity.

Show repository owner/name, not just name.

-                                    f"{issue.repository.name}#{issue.number} "
+                                    f"{getattr(issue.repository, 'full_name', issue.repository.name)}#{issue.number} "

---

200-201: Tidy startup log.

Minor cosmetic tweak.

-        self.stdout.write("starting...")
+        self.stdout.write("Starting...")

---

23-34: Add unit tests for repo name extraction edge cases.

Cover: api.github.com/repos/{owner}/{repo}, mixed-case hosts, trailing slashes, extra path segments, None/empty input, Repository instances with/without full_name.

I can generate pytest tests under backend/apps/mentorship/tests/ for _extract_repo_full_name. Want me to open a follow-up PR?

---

122-169: GitHub API rate limits: minimize calls and cache aggressively.

The proposed refactor reduces calls, but consider short-circuiting when Task is COMPLETED and issue is still closed (no event fetch needed).

I can add guards to skip event lookups for closed issues and memoize assigned_at per (repo_full_name, issue.number, assignee.login) during a single run.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: Rajgupta36
PR: OWASP/Nest#2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.

Learnt from: Rajgupta36
PR: OWASP/Nest#2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.

backend/apps/mentorship/management/commands/sync_module_issues.py (1)

128-133: Make assignee selection deterministic.

first() on an unordered M2M can vary by backend; order by PK (or desired field) to keep stable behavior. Note: honoring the “one assignee per issue” rule matches the documented product decision.

-                    assignee = issue.assignees.first()
+                    assignee = issue.assignees.order_by("id").first()

I’m applying your stored learning that one task is created per issue using only the first assignee.

backend/apps/mentorship/management/commands/sync_module_issues.py (6)

1-1: Polish the command docstring for clarity.

Suggest: “Sync the relation between modules and issues and create tasks.”

---

51-67: Select the most recent matching assignment event explicitly.

PyGithub event ordering isn’t guaranteed; use max(created_at) instead of the last iterated value.

-            last_dt = None
-            for event in gh_issue.get_events():
+            last_dt = None
+            for event in gh_issue.get_events():
                 if (
                     event.event == "assigned"
                     and event.assignee
                     and event.assignee.login == assignee_login
                 ):
-                    last_dt = event.created_at
+                    if last_dt is None or event.created_at > last_dt:
+                        last_dt = event.created_at

---

75-91: Optional: build the map only for labels used by modules to reduce DB work.

Collect normalized labels from target modules first and filter the Issue query by those labels; this can cut rows scanned on large datasets.

---

114-117: Reduce transaction scope; avoid holding DB locks during external API calls.

Wrap only the minimal DB mutations (e.g., module.issues.set and per-task saves) in atomic blocks; perform GitHub lookups outside to lower contention and latency.

Would you like a follow-up patch that narrows the atomic blocks while preserving consistency?

---

118-125: Drop unused prefetch('labels') on issues queryset.

Labels aren’t used in the loop; removing this reduces memory and query work.

-                    .prefetch_related("assignees", "labels")
+                    .prefetch_related("assignees")

---

170-178: Minor: include owner/repo in creation log for clarity.

Using the repo’s full name (owner/repo) improves traceability across organizations.

-                                f"{issue.repository.name}#{issue.number} "
+                                f"{getattr(issue.repository, 'path', issue.repository.name)}#{issue.number} "

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: Rajgupta36
PR: OWASP/Nest#2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.

Learnt from: Rajgupta36
PR: OWASP/Nest#2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.

backend/apps/mentorship/Makefile (1)

1-5: Declare PHONY targets for reliability

Make may skip these if files with the same names appear. Also allow arg pass‑through for convenience.

+.PHONY: mentorship-sync-module-issues mentorship-sync-issue-levels
 mentorship-sync-module-issues:
-	@CMD="python manage.py sync_module_issues" $(MAKE) exec-backend-command
+	@CMD="python manage.py sync_module_issues $(ARGS)" $(MAKE) exec-backend-command
@@
 mentorship-sync-issue-levels:
-	@CMD="python manage.py sync_issue_levels" $(MAKE) exec-backend-command
+	@CMD="python manage.py sync_issue_levels $(ARGS)" $(MAKE) exec-backend-command

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: Rajgupta36
PR: OWASP/Nest#2000
File: backend/apps/mentorship/management/commands/sync_module_issues.py:109-145
Timestamp: 2025-08-17T11:55:55.990Z
Learning: In the OWASP/Nest mentorship system, tasks are designed to be assigned to only one assignee per issue, even if GitHub issues can have multiple assignees. The sync_module_issues command correctly uses issue.assignees.first() to create one task per issue for the first assignee only.