Skip to content

feat: Add repo name and service id to django admin repo search #591

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

Open
calvin-codecov wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: Add repo name and service id to django admin repo search #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e8f261
feat: Add repo name and service id to django admin repo search
calvin-codecov Dec 2, 2025
01d5856
fix: Add service_id to search fields instead of custom
calvin-codecov Dec 3, 2025
ac9554d
feat: Add GIN index for name field
calvin-codecov Dec 4, 2025
4db8f39
feat: Add select_related for author in repos list
calvin-codecov Dec 4, 2025
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
13 changes: 10 additions & 3 deletions apps/codecov-api/core/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ class Meta:
class RepositoryAdmin(AdminMixin, admin.ModelAdmin):
inlines = [RepositoryTokenInline]
list_display = ("name", "service_id", "author")
search_fields = ("author__username__exact",)
search_fields = (
"name",
"author__username__exact",
"service_id__exact",
)
show_full_result_count = False
autocomplete_fields = ("bot",)
form = RepositoryAdminForm
Expand Down Expand Up @@ -172,21 +176,24 @@ def get_search_results(
search_term: str,
) -> tuple[QuerySet[Repository], bool]:
"""
Search for repositories by name or repoid.
Search for repositories by name, service_id, author username, or repoid.
https://docs.djangoproject.com/en/5.2/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_search_results
"""
# Default search is by author username (defined in `search_fields`)
# Default search is by name, author username, and service_id (defined in `search_fields`)
queryset, may_have_duplicates = super().get_search_results(
request,
queryset,
search_term,
)
# Also search by repoid if the search term is numeric
try:
search_term_as_int = int(search_term)
except ValueError:

This comment was marked as outdated.

Sign in to view

pass
else:
queryset |= self.model.objects.filter(repoid=search_term_as_int)
# avoid N+1 queries for foreign key author
queryset = queryset.select_related("author")
Comment on lines +188 to +196
Copy link

@sentry sentry bot Dec 4, 2025

Choose a reason for hiding this comment

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

Bug: Applying select_related("author") after union() for a queryset added without prior optimization causes N+1 queries for the author relationship.
Severity: CRITICAL | Confidence: High

🔍 Detailed Analysis

The code applies select_related("author") after performing a union() operation on two querysets. One of the querysets, self.model.objects.filter(repoid=search_term_as_int), is added without select_related(). This violates Django best practices, which require select_related() to be applied before union(). This will cause N+1 queries when accessing the author relationship for repositories matched by repoid in the Django admin, leading to significant performance degradation.

💡 Suggested Fix

Apply select_related("author") to the new queryset self.model.objects.filter(repoid=search_term_as_int) before the union() operation. The final select_related call can remain or be removed.

🤖 Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: apps/codecov-api/core/admin.py#L188-L196

Potential issue: The code applies `select_related("author")` after performing a
`union()` operation on two querysets. One of the querysets,
`self.model.objects.filter(repoid=search_term_as_int)`, is added without
`select_related()`. This violates Django best practices, which require
`select_related()` to be applied before `union()`. This will cause N+1 queries when
accessing the `author` relationship for repositories matched by `repoid` in the Django
admin, leading to significant performance degradation.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 5359759

return queryset, may_have_duplicates

def has_add_permission(self, _, obj=None):
Expand Down
34 changes: 34 additions & 0 deletions libs/shared/shared/django_apps/core/migrations/0077_repository_repos_name_trgm_idx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Generated by Django 4.2.25 on 2025-12-04 01:26

import django.contrib.postgres.indexes
import django.db.models.functions.text
from django.db import migrations

from shared.django_apps.migration_utils import RiskyAddIndex


class Migration(migrations.Migration):
"""
BEGIN;
--
-- Create index repos_name_trgm_idx on OpClass(Upper(F(name)), name=gin_trgm_ops) on model repository
--
CREATE INDEX "repos_name_trgm_idx" ON "repos" USING gin ((UPPER("name")) gin_trgm_ops);
COMMIT;
"""

dependencies = [
("core", "0076_increment_version"),
]

operations = [
RiskyAddIndex(
model_name="repository",
index=django.contrib.postgres.indexes.GinIndex(
django.contrib.postgres.indexes.OpClass(
django.db.models.functions.text.Upper("name"), name="gin_trgm_ops"
),
name="repos_name_trgm_idx",
),
),
]
3 changes: 3 additions & 0 deletions libs/shared/shared/django_apps/core/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ class Meta:
fields=["service_id", "author"],
name="repos_service_id_author",
),
GinIndex(
OpClass(Upper("name"), name="gin_trgm_ops"), name="repos_name_trgm_idx"
),
]
constraints = [
models.UniqueConstraint(fields=["author", "name"], name="repos_slug"),
Expand Down
Loading