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
15 changes: 14 additions & 1 deletion backend/apps/github/api/internal/nodes/issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import strawberry_django
from django.db.models import Prefetch

from apps.common.utils import normalize_limit
from apps.github.api.internal.nodes.pull_request import PullRequestNode
from apps.github.api.internal.nodes.user import UserNode
from apps.github.models.issue import Issue
Expand All @@ -18,6 +19,8 @@
to_attr="merged_pull_requests",
)

MAX_LIMIT = 1000


@strawberry_django.type(
Issue,
Expand All @@ -35,7 +38,17 @@ class IssueNode(strawberry.relay.Node):

assignees: list[UserNode] = strawberry_django.field()
author: UserNode | None = strawberry_django.field()
pull_requests: list[PullRequestNode] = strawberry_django.field()

@strawberry_django.field(prefetch_related=["pull_requests"])
def pull_requests(self, limit: int = 4, offset: int = 0) -> list[PullRequestNode]:
"""Return pull requests linked to this issue."""
if (normalized_limit := normalize_limit(limit, MAX_LIMIT)) is None:
return []

offset = max(0, offset)
return list(
self.pull_requests.all().order_by("-created_at")[offset : offset + normalized_limit]
)

@strawberry_django.field(select_related=["repository__organization", "repository"])
def organization_name(self, root: Issue) -> str | None:
Expand Down
6 changes: 4 additions & 2 deletions backend/apps/mentorship/api/internal/nodes/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,17 +173,19 @@ def task_assigned_at(self, issue_number: int) -> datetime | None:
)

@strawberry.field
def recent_pull_requests(self, limit: int = 5) -> list[PullRequestNode]:
def recent_pull_requests(self, limit: int = 4, offset: int = 0) -> list[PullRequestNode]:
"""Return recent pull requests linked to issues in this module."""
if (normalized_limit := normalize_limit(limit, MAX_LIMIT)) is None:
return []

offset = max(0, offset)

issue_ids = self.issues.values_list("id", flat=True)
return list(
PullRequest.objects.filter(related_issues__id__in=issue_ids)
.select_related("author")
.distinct()
.order_by("-created_at")[:normalized_limit]
.order_by("-created_at")[offset : offset + normalized_limit]
)


Expand Down
33 changes: 32 additions & 1 deletion backend/tests/apps/github/api/internal/nodes/issue_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""Test cases for IssueNode."""

from unittest.mock import Mock
from unittest.mock import Mock, patch

from apps.github.api.internal.nodes.issue import IssueNode
from tests.apps.common.graphql_node_base_test import GraphQLNodeBaseTest
Expand Down Expand Up @@ -86,6 +86,37 @@ def test_repository_name_without_repository(self):
result = field.base_resolver.wrapped_func(None, mock_issue)
assert result is None

def test_pull_requests_resolver(self):
"""Test pull_requests field resolver with pagination."""
mock_issue = Mock()
mock_qs = Mock()

mock_issue.pull_requests.all.return_value = mock_qs
mock_qs.order_by.return_value = ["pr1", "pr2", "pr3", "pr4", "pr5"]

field = self._get_field_by_name("pull_requests", IssueNode)
result = field.base_resolver.wrapped_func(mock_issue)
assert result == ["pr1", "pr2", "pr3", "pr4"]
result = field.base_resolver.wrapped_func(mock_issue, limit=2, offset=2)
assert result == ["pr3", "pr4"]

@patch("apps.github.api.internal.nodes.issue.normalize_limit")
def test_pull_requests_resolver_security(self, mock_normalize_limit):
"""Test pull_requests field security check."""
mock_issue = Mock()

mock_normalize_limit.return_value = 10
field = self._get_field_by_name("pull_requests", IssueNode)

mock_issue.pull_requests.all.return_value.order_by.return_value = []

field.base_resolver.wrapped_func(mock_issue, limit=10)
mock_normalize_limit.assert_called()

mock_normalize_limit.return_value = None
result = field.base_resolver.wrapped_func(mock_issue, limit=999999)
assert result == []

def test_labels(self):
"""Test labels field returns list of label names."""
mock_issue = Mock()
Expand Down
25 changes: 25 additions & 0 deletions frontend/__tests__/unit/components/CardDetailsPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,31 @@ describe('CardDetailsPage', () => {
expect(screen.getByTestId('metrics-score-circle')).toBeInTheDocument()
})

it('renders Show More button when onLoadMorePullRequests is provided', () => {
render(
<CardDetailsPage
{...defaultProps}
type="module"
pullRequests={mockPullRequests as unknown as PullRequest[]}
onLoadMorePullRequests={jest.fn()}
/>
)
expect(screen.getByRole('button', { name: /Show more/i })).toBeInTheDocument()
})

it('renders Show Less button when onResetPullRequests is provided', () => {
render(
<CardDetailsPage
{...defaultProps}
type="module"
pullRequests={mockPullRequests as unknown as PullRequest[]}
onResetPullRequests={jest.fn()}
onLoadMorePullRequests={undefined}
/>
)
expect(screen.getByRole('button', { name: /Show less/i })).toBeInTheDocument()
})

it('calls scrollToAnchor when MetricsScoreCircle is clicked', () => {
const { scrollToAnchor } = jest.requireMock('utils/scrollToAnchor')

Expand Down
3 changes: 3 additions & 0 deletions frontend/__tests__/unit/pages/ModuleDetailsPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ jest.mock('components/CardDetailsPage', () => (props) => (
<div data-testid="details-card">
<div>{props.title}</div>
<div>{props.summary}</div>
{props.onLoadMorePullRequests && (
<button onClick={props.onLoadMorePullRequests}>Show more</button>
)}
</div>
))

Expand Down
65 changes: 63 additions & 2 deletions frontend/__tests__/unit/pages/ModuleIssueDetailsPage.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useQuery } from '@apollo/client/react'
import { render, screen, fireEvent, waitFor, within } from '@testing-library/react'
import { render, screen, fireEvent, waitFor, within, act } from '@testing-library/react'
import { useIssueMutations } from 'hooks/useIssueMutations'
import { useParams } from 'next/navigation'
import ModuleIssueDetailsPage from 'app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page'
Expand Down Expand Up @@ -402,6 +402,62 @@ describe('ModuleIssueDetailsPage', () => {
const unassignButton = screen.getByRole('button', { name: /Unassign @user1/i })
expect(unassignButton).toBeDisabled()
})
it('calls fetchMore when clicking Show More button', async () => {
const fetchMoreMock = jest.fn().mockResolvedValue({
data: {
getModule: {
issueByNumber: {
pullRequests: [
{
id: 'pr-new',
title: 'New PR',
url: 'http://example.com',
state: 'open',
mergedAt: null,
createdAt: new Date().toISOString(),
author: { login: 'user-new', avatarUrl: '' },
},
],
},
},
},
})

mockUseQuery.mockReturnValue({
data: {
getModule: {
issueByNumber: {
...mockIssueData.getModule.issueByNumber,
pullRequests: Array.from({ length: 4 }, (_, i) => ({
...mockIssueData.getModule.issueByNumber.pullRequests[0],
id: `pr-${i}`,
})),
},
},
},
loading: false,
error: undefined,
fetchMore: fetchMoreMock,
})

render(<ModuleIssueDetailsPage />)

const showMoreButton = screen.getByRole('button', { name: /Show more/i })
expect(showMoreButton).toBeInTheDocument()

await act(async () => {
fireEvent.click(showMoreButton)
})

expect(fetchMoreMock).toHaveBeenCalledWith(
expect.objectContaining({
variables: expect.objectContaining({
offset: 4,
limit: 4,
}),
})
)
})

it('renders "(today)" for deadline that is exactly today', () => {
const today = new Date()
Expand Down Expand Up @@ -455,7 +511,12 @@ describe('ModuleIssueDetailsPage', () => {
},
},
}
mockUseQuery.mockReturnValue({ data: manyPRsData, loading: false, error: undefined })
mockUseQuery.mockReturnValue({
data: manyPRsData,
loading: false,
error: undefined,
fetchMore: jest.fn().mockResolvedValue({ data: manyPRsData }),
})
render(<ModuleIssueDetailsPage />)

// Initially only 4 PRs should be visible
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
import { useQuery } from '@apollo/client/react'
import capitalize from 'lodash/capitalize'
import { useParams } from 'next/navigation'
import { useEffect } from 'react'
import { useState, useEffect } from 'react'
import { ErrorDisplay, handleAppError } from 'app/global-error'
import { GetProgramAdminsAndModulesDocument } from 'types/__generated__/moduleQueries.generated'
import { Module } from 'types/mentorship'
import type { PullRequest } from 'types/pullRequest'
import { formatDate } from 'utils/dateFormatter'
import DetailsCard from 'components/CardDetailsPage'
Expand All @@ -14,20 +15,35 @@ import { getSimpleDuration } from 'components/ModuleCard'

const ModuleDetailsPage = () => {
const { programKey, moduleKey } = useParams<{ programKey: string; moduleKey: string }>()
const [hasMorePRs, setHasMorePRs] = useState(true)
const [visibleCount, setVisibleCount] = useState(4)
const [isFetchingMore, setIsFetchingMore] = useState(false)
const limit = 4

const {
data,
error,
loading: isLoading,
fetchMore,
} = useQuery(GetProgramAdminsAndModulesDocument, {
fetchPolicy: 'cache-and-network',
variables: {
programKey,
moduleKey,
limit,
offset: 0,
},
})

const programModule = data?.getModule
useEffect(() => {
const prCount = data?.getModule?.recentPullRequests?.length
if (prCount == null) return
if (prCount <= limit) {
setHasMorePRs(prCount >= limit)
}
}, [data, limit])

const programModule = data?.getModule as Module
const admins = data?.getProgram?.admins

useEffect(() => {
Expand Down Expand Up @@ -74,11 +90,58 @@ const ModuleDetailsPage = () => {
details={moduleDetails}
domains={programModule.domains ?? undefined}
mentors={programModule.mentors}
pullRequests={(programModule.recentPullRequests as unknown as PullRequest[]) ?? []}
isFetchingMore={isFetchingMore}
pullRequests={((programModule.recentPullRequests as unknown as PullRequest[]) || []).slice(
0,
visibleCount
)}
summary={programModule.description}
tags={programModule.tags ?? undefined}
title={programModule.name}
type="module"
onLoadMorePullRequests={
hasMorePRs || (programModule.recentPullRequests || []).length > visibleCount
? () => {
if (isFetchingMore) return
const currentLength = programModule.recentPullRequests?.length || 0
if (hasMorePRs && currentLength < visibleCount + limit) {
setIsFetchingMore(true)
fetchMore({
variables: {
programKey,
moduleKey,
offset: currentLength,
limit,
},
updateQuery: (prevResult, { fetchMoreResult }) => {
if (!fetchMoreResult) return prevResult
const newPRs = fetchMoreResult.getModule?.recentPullRequests || []
if (newPRs.length < limit) setHasMorePRs(false)
if (newPRs.length === 0) return prevResult
return {
...prevResult,
getModule: {
...prevResult.getModule!,
recentPullRequests: [
...(prevResult.getModule?.recentPullRequests || []),
...newPRs,
],
},
}
},
})
.catch((error) => handleAppError(error))
.finally(() => setIsFetchingMore(false))
}
setVisibleCount((prev) => prev + limit)
}
: undefined
}
onResetPullRequests={
visibleCount > limit && (programModule.recentPullRequests || []).length > limit
? () => setVisibleCount(limit)
: undefined
}
/>
)
}
Expand Down
Loading