Add deadline status badges and deadline filters to mentorship issues list#3695
Add deadline status badges and deadline filters to mentorship issues list#3695anurag2787 wants to merge 7 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
WalkthroughAdds task deadline support end-to-end: backend IssueNode.task_deadline resolver and ModuleNode context preloading; GraphQL selection extended with taskDeadline; frontend issues list and IssuesTable updated to display and filter deadlines with UTC-based deadline arithmetic. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi I’m currently working on it i need a little more time and will make it ready for review soon |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx (1)
48-52:⚠️ Potential issue | 🟡 MinorGrammar issue: "1 days left" should be "1 day left".
When
daysLeft === 1, the text displays "1 days left" which is grammatically incorrect.📝 Proposed fix for singular/plural handling
} else if (daysLeft === 0) { statusText = '(today)' } else { - statusText = `(${daysLeft} days left)` + statusText = `(${daysLeft} ${daysLeft === 1 ? 'day' : 'days'} left)` }
🧹 Nitpick comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx (1)
54-54: Potential timezone mismatch between calculation and display.The days-left calculation uses UTC normalization, but
displayDateusestoLocaleDateString()which converts to the user's local timezone. When deadlines are stored as end-of-day UTC (23:59:59.999 as seen in line 195), users in timezones significantly ahead of UTC (e.g., UTC+12) may see the next calendar day displayed, while the days-left calculation correctly reflects the intended deadline date.Consider using a UTC-based display format for consistency:
♻️ Option to display date in UTC
- const displayDate = deadlineDate.toLocaleDateString() + const displayDate = deadlineDate.toLocaleDateString(undefined, { timeZone: 'UTC' })
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsx (1)
42-51:⚠️ Potential issue | 🟠 MajorDeadline filtering is not implemented on this page.
The deadline column is displayed and deadline data is fetched, but the page only exposes a label filter. The GraphQL
GetModuleIssuesquery has no deadline parameter, and there is no UI or state management for deadline-based filtering. To meet the deadline state filtering objectives, add a deadline filter dropdown alongside the label filter and extend the GraphQL query to support filtering by deadline state (Overdue, Due Soon, No Deadline).
🤖 Fix all issues with AI agents
In `@backend/apps/github/api/internal/nodes/issue.py`:
- Around line 66-91: The task_deadline resolver in Issue (task_deadline) issues
a Task DB query per issue causing N+1; similarly ModuleNode.task_deadline
repeats this pattern—fix by batching: implement a dataloader keyed by
(module.id, issue.number) or, at the parent/module resolver, run a single Task
query for the module to build a mapping of issue number → latest deadline and
attach it to info.context (e.g., info.context.task_deadlines_by_issue); then
update Issue.task_deadline to read from that preloaded map (falling back to a
dataloader if not present) so no per-issue DB queries occur.
In `@backend/apps/mentorship/api/internal/nodes/module.py`:
- Around line 78-85: The issues resolver sets info.context.current_module
without restoring it, causing cross-request leakage; update the issues method
(the strawberry.field resolver named issues) to save the previous value of
info.context.current_module, set it to self, and then restore the saved value in
a finally block so the context is always reverted; apply the same pattern to the
other resolver around lines 116-121 that also sets info.context.current_module
(save prior, set to self, and restore in finally) to prevent module bleed
between resolvers.
In `@frontend/src/components/IssuesTable.tsx`:
- Around line 77-109: getDeadlineStatus currently constructs local-midnight Date
objects (new Date(...)) which misclassifies UTC timestamps across time
zones/DST; update it to compute day boundaries in UTC by using
getUTCFullYear/getUTCMonth/getUTCDate and Date.UTC to build nowStartUTC and
deadlineStartUTC, then compute diffDays from (deadlineStartUTC.getTime() -
nowStartUTC.getTime())/86400000 (or equivalent using the UTC millisecond values)
and keep the existing branch logic; target the getDeadlineStatus function and
replace the local-midnight math with UTC-based day boundary calculations.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@frontend/src/app/my/mentorship/programs/`[programKey]/modules/[moduleKey]/issues/page.tsx:
- Around line 71-87: The deadline filter is currently applied after pagination
which hides matches on other pages; update the logic that builds and paginates
the issues so filtering happens before pagination and issuesCount is derived
from the filtered set. Specifically, in the component that computes issues from
moduleData (the block creating issues from moduleData?.issues), apply
getDeadlineCategory-based filtering using selectedDeadline and DEADLINE_ALL to
produce a filteredIssues array first, use filteredIssues to compute
issuesCount/pagination, and only then slice for the current page; alternatively,
if you prefer server-side filtering, wire the deadline param into the server
fetch that returns moduleData so the backend returns filtered items and a
correct count.
- Around line 15-40: getDeadlineCategory currently builds local-midnight Date
objects which causes off-by-one classifications for UTC-stored deadlines; change
it to use UTC day boundaries by constructing the comparison dates using UTC
getters (getUTCFullYear, getUTCMonth, getUTCDate) or Date.UTC for now and
deadlineDate so diffDays is calculated on UTC midnights, and apply the same
UTC-based fix to getDeadlineStatus in IssuesTable.tsx so both functions classify
deadlines consistently with formatDeadline.
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsx
Show resolved
Hide resolved
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/mentorship/api/internal/nodes/module.py (1)
134-145:⚠️ Potential issue | 🟠 Major
issue_by_numberdoesn't populate deadline context, causingtaskDeadlineto return null.
IssueNode.task_deadlinereturnsNoneifinfo.context.task_deadlines_by_issueis missing. Theissues()method populates this context map, butissue_by_number()does not. Any query usingissue_by_numberto accesstaskDeadlinewill return null even when deadline data exists.Consider either:
- Populating the context maps in
issue_by_number(), or- Adding a fallback query in
IssueNode.task_deadline()(similar to the pattern already used inModuleNode.task_deadline())🔧 Option 1: Populate context maps in issue_by_number
@@ - info.context.current_module = self - - return ( + info.context.current_module = self + info.context.task_deadlines_by_issue = { + number: ( + Task.objects.filter( + module=self, issue__number=number, deadline_at__isnull=False + ) + .order_by("-assigned_at") + .values_list("deadline_at", flat=True) + .first() + ) + } + info.context.task_assigned_at_by_issue = { + number: ( + Task.objects.filter( + module=self, issue__number=number, assigned_at__isnull=False + ) + .order_by("-assigned_at") + .values_list("assigned_at", flat=True) + .first() + ) + } + return ( self.issues.select_related("repository", "author") .prefetch_related("assignees", "labels") .filter(number=number) .first() )
🤖 Fix all issues with AI agents
In `@backend/apps/mentorship/api/internal/nodes/module.py`:
- Around line 85-103: The preload currently only queries tasks with deadline_at
and populates both deadline_map and assigned_map, and consumers use "if
mapping:" which treats empty dicts as falsy; fix by running two separate bulk
queries: one Task.objects.filter(module=self, deadline_at__isnull=False)... to
build deadline_map, and a second Task.objects.filter(module=self,
assigned_at__isnull=False)... to build assigned_map (populate
info.context.task_deadlines_by_issue and info.context.task_assigned_at_by_issue
with the resulting dicts or empty dicts if none), and update the checks in
task_deadline() and task_assigned_at() to test "if mapping is not None:" so an
intentionally empty preload (empty dict) does not trigger fallback queries.
In
`@frontend/src/app/my/mentorship/programs/`[programKey]/modules/[moduleKey]/issues/page.tsx:
- Around line 52-61: The code uses a hard cap MAX_ISSUES_FOR_DEADLINE_FILTER
(1000) when isDeadlineFilterActive to fetch a subset and then applies the
deadline filter client-side, which can silently drop matches for modules with
>1000 issues; change behavior so filtering and counting happen server-side
(update the GetModuleIssuesDocument/query to accept a deadline param and return
totalCount), or if server change is not possible iterate pages: in the useQuery
call (GetModuleIssuesDocument) remove the 1000 cap and instead fetch pages until
all issues are retrieved when selectedDeadline !== DEADLINE_ALL (respecting
ITEMS_PER_PAGE and currentPage), and surface a truncated warning if you still
must limit results; update logic that uses selectedDeadline,
isDeadlineFilterActive, MAX_ISSUES_FOR_DEADLINE_FILTER, ITEMS_PER_PAGE and
currentPage accordingly so counts and pagination are accurate.
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/mentorship/api/internal/nodes/module.py (1)
143-152:⚠️ Potential issue | 🟠 MajorMissing bulk preload causes
IssueNode.task_deadlineto always returnNone.Unlike
issues(), this method does not populateinfo.context.task_deadlines_by_issueorinfo.context.task_assigned_at_by_issue. SinceIssueNode.task_deadlinereturnsNonewhen the mapping is absent (see relevant snippet at lines 64-68), issues fetched viaissue_by_numberwill never expose their deadline through that field.🔧 Suggested fix: preload deadline for the single issue
`@strawberry.field` def issue_by_number(self, info: Info, number: int) -> IssueNode | None: """Return a single issue by its GitHub number within this module's linked issues.""" info.context.current_module = self + # Preload deadline/assigned_at for the single issue so IssueNode.task_deadline works + task = ( + Task.objects.filter(module=self, issue__number=number) + .order_by("-assigned_at") + .values("deadline_at", "assigned_at") + .first() + ) + info.context.task_deadlines_by_issue = {number: task["deadline_at"]} if task and task.get("deadline_at") else {} + info.context.task_assigned_at_by_issue = {number: task["assigned_at"]} if task and task.get("assigned_at") else {} + return ( self.issues.select_related("repository", "author") .prefetch_related("assignees", "labels") .filter(number=number) .first() )
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/apps/mentorship/api/internal/nodes/module.py (1)
147-157:⚠️ Potential issue | 🔴 Critical
issue_by_numbernever populates the deadline/assignment context maps —task_deadlinewill always returnNone.
IssueNode.task_deadline(inissue.py) returnsNonewhentask_deadlines_by_issueis not set on the context. Sinceissue_by_numberonly setscurrent_modulebut never buildstask_deadlines_by_issue/task_assigned_at_by_issue, any single-issue query through this resolver will silently lose deadline data.Either replicate the bulk-preload here (scoped to the single issue) or at minimum populate the maps so the
IssueNoderesolver can find its data.🔧 Proposed fix — preload for the single issue
`@strawberry.field` def issue_by_number(self, info: Info, number: int) -> IssueNode | None: """Return a single issue by its GitHub number within this module's linked issues.""" info.context.current_module = self + # Preload deadline / assigned_at for this single issue so IssueNode + # resolvers can find the data on the context. + task = ( + Task.objects.filter( + module=self, issue__number=number, + ) + .order_by("-assigned_at") + .values("deadline_at", "assigned_at") + .first() + ) + info.context.task_deadlines_by_issue = ( + {number: task["deadline_at"]} if task and task["deadline_at"] else {} + ) + info.context.task_assigned_at_by_issue = ( + {number: task["assigned_at"]} if task and task["assigned_at"] else {} + ) + return ( self.issues.select_related("repository", "author") .prefetch_related("assignees", "labels") .filter(number=number) .first() )
🤖 Fix all issues with AI agents
In `@frontend/src/components/IssuesTable.tsx`:
- Around line 216-230: The Deadline cell is missing the responsive "block" class
and uses an unnecessary IIFE; fix by computing const status =
getDeadlineStatus(issue.deadline) immediately before rendering (or inline the
call) and change the <td> for the Deadline (conditioned by showDeadline) to
include "block" alongside the existing "lg:table-cell" classes so it matches the
other cells' "block ... lg:table-cell" pattern and then render status.text and
status.class inside the span.
🧹 Nitpick comments (3)
backend/apps/mentorship/api/internal/nodes/module.py (1)
88-113: Bulk preload always fires two extra queries, even when no downstream resolver needs them.The two
Task.objects.filter(...)queries execute unconditionally on everyissues()call, loading deadline/assignment data for all tasks in the module (not just the paginated slice). This is fine for small modules but could become expensive for large ones.Consider either:
- Scoping the queries to only the issue numbers in the returned page (query issues first, then bulk-load for those numbers).
- Lazy-loading: store a callable on the context and only execute it if
task_deadlineis actually resolved.This is a performance suggestion, not a blocker.
♻️ Option 1 — scope preload to the returned page
+ # Resolve the page first + queryset = self.issues.select_related("repository", "author").prefetch_related( + "assignees", "labels" + ) + if label and label != "all": + queryset = queryset.filter(labels__name=label) + page = list(queryset.order_by("-updated_at")[offset : offset + normalized_limit]) + + issue_numbers = [i.number for i in page] + deadline_rows = ( - Task.objects.filter(module=self, deadline_at__isnull=False) + Task.objects.filter(module=self, deadline_at__isnull=False, issue__number__in=issue_numbers) .order_by("issue__number", "-assigned_at") .values("issue__number", "deadline_at") ) assigned_rows = ( - Task.objects.filter(module=self, assigned_at__isnull=False) + Task.objects.filter(module=self, assigned_at__isnull=False, issue__number__in=issue_numbers) .order_by("issue__number", "-assigned_at") .values("issue__number", "assigned_at") ) # ... build maps ... + + return pagefrontend/src/components/IssuesTable.tsx (2)
79-107: Consider renaming theclassproperty toclassNamefor React consistency.
classis a reserved keyword in JavaScript. While it works as an object property key here, usingclassNamewould be more idiomatic in a React codebase and consistent with how this value is ultimately consumed (as aclassNameattribute on JSX elements).
53-70:getStatusBadgeandgetDeadlineStatushave inconsistent return patterns.
getStatusBadgereturns JSX directly, whilegetDeadlineStatusreturns a data object{ text, class }. Consider aligning them — either both return data objects (preferred, more testable) or both return JSX. This would also eliminate the IIFE in the deadline cell.Also applies to: 79-107
|
1 participant
Proposed change
This PR adds deadline status to the Issues list in the Mentorship module and provides an option to filter issues by their deadline state.
Resolves #3365
Screencast.from.2026-02-01.00-26-01.webm
Checklist
make check-testlocally: all warnings addressed, tests passed