Add Candidate Claim Management Dashboard#5006
Open
rudransh-shrivastava wants to merge 37 commits into
Open
Conversation
rudransh-shrivastava
added
the
gsoc2026:rudransh-shrivastava
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a full-stack candidate claim management dashboard. Backend mutations now return the affected claim node in ChangesCandidate Claim Management Dashboard
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.py (1)
528-555:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSet up return value for
objects.create()to match other success tests.The test verifies
result.claim is not None, but unliketest_update_claim_success,test_discard_claim_success, etc., it doesn't set upmock_claim_model.objects.create.return_value. This means when the mutation callsobjects.create(), it returns a default MagicMock, not a configured claim. The assertion passes because the MagicMock is not None, but doesn't verify the returned claim is the one that was created. If the mutation code is later changed to not return the claim, this test would still pass.Align this test with the pattern used in other success tests by setting up the return value and checking object identity.
🔧 Proposed fix to set up return value and check object identity
`@patch`("apps.owasp.api.internal.mutations.board_candidate_claim.BoardOfDirectors") `@patch`("apps.owasp.api.internal.mutations.board_candidate_claim.BoardCandidateClaim") def test_create_claim_success(self, mock_claim_model, mock_board_model): mock_claim_model.Status = BoardCandidateClaim.Status user = MagicMock() user.is_authenticated = True mock_github_user = MagicMock() user.github_user = mock_github_user info = _make_info(user) input_data = self._make_input_data() mock_board = MagicMock() mock_candidate = MagicMock() mock_board.get_candidate.return_value = mock_candidate mock_board_model.objects.get.return_value = mock_board + + claim = MagicMock() + mock_claim_model.objects.create.return_value = claim mutation = BoardCandidateClaimMutations() result = mutation.create_board_candidate_claim(info, input_data) mock_board_model.objects.get.assert_called_once_with(year=2025) mock_board.get_candidate.assert_called_once_with(login=mock_github_user.login) mock_claim_model.objects.create.assert_called_once_with( board=mock_board, candidate=mock_candidate, description=input_data.description, name=input_data.name, ) assert result.ok assert result.code == "SUCCESS" - assert result.claim is not None + assert result.claim is claim🤖 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/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.py` around lines 528 - 555, In the test_create_claim_success method, set up the return value for mock_claim_model.objects.create before calling the mutation to ensure it returns a configured mock claim object. Create a mock claim object, assign it to mock_claim_model.objects.create.return_value, then verify that result.claim is the same object (using identity comparison, not just truthiness check). This aligns the test with the pattern used in other success tests like test_update_claim_success and test_discard_claim_success, ensuring the test would fail if the mutation code is later changed to not return the created claim.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@backend/apps/owasp/api/internal/nodes/board_candidate_claim.py`:
- Around line 32-35: The has_evidence resolver method executes a database query
per claim by calling root.evidences.filter(is_removed=False).exists() each time
it's resolved, causing N+1 query problems when fetching multiple claims. Instead
of querying inside the field resolver, pre-annotate the has_evidence value on
the queryset at the point where claims are fetched (e.g., using Django's
annotate with a Count or Exists expression on the evidences relation filtered by
is_removed=False), then read that pre-computed annotated value directly from the
root object in the has_evidence method instead of executing the filter and
exists call.
In `@e2e/helpers/mockClaimAuth.ts`:
- Around line 15-35: The route mock handler for GraphQL requests currently has a
catch-all fallback that returns success for any operation that is not
SyncDjangoSession, which can mask missing or broken mocks. Instead of the else
block returning success with mockData for all unknown operations, add explicit
handling for each expected operationName and fail fast on unhandled operations
by using route.abort() or throwing an error when an unknown operationName is
encountered. This ensures that any test requesting an unmocked GraphQL operation
will fail rather than silently succeed with default data.
In `@e2e/pages/BoardCandidateClaimDetails.spec.ts`:
- Around line 23-25: The assertion for `Web security experience` on line 24 uses
lowercase casing that does not match the fixture and actual UI output which uses
title case. Update the getByText assertion from `Web security experience` to
`Web Security Experience` to match the expected fixture rendering and ensure the
test passes against the actual page content.
In
`@frontend/src/app/board/`[year]/candidates/[login]/claims/[claimKey]/edit/page.tsx:
- Around line 23-27: The useQuery hook call for GetBoardCandidateClaimDocument
currently only skips the query when claimKey is missing, but does not verify
user ownership before fetching. Update the skip condition to include an
ownership check that compares session?.user?.login with the login parameter, so
the query is skipped if either claimKey is missing or the current user is not
authorized to view this claim.
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/[claimKey]/page.tsx:
- Around line 83-87: The handleAddEvidence function routes to a non-existent
destination which results in a 404 error, breaking the primary call-to-action
for adding evidence. Locate the UI button or element that triggers
handleAddEvidence and apply a conditional to either hide it completely or
disable it (using the disabled prop) until the destination route is properly
implemented. This same approach should also be applied to the button or element
triggering handleEvidenceClick as noted in the comment, ensuring both navigation
paths are protected from routing to undefined destinations.
- Around line 33-37: The useQuery call for GetClaimAndEvidencesDocument
currently skips execution only when claimKey is missing, but it should also
verify session ownership before making the backend request. Modify the skip
condition in the useQuery hook to include additional checks for session
ownership validation (session?.user?.login === login) and syncing status
(!!isSyncing) alongside the existing !claimKey check, so the query only executes
when all conditions are satisfied. Apply the same optimization pattern to any
other similar query calls as indicated in the comment (also at lines 70-74).
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/create/page.tsx:
- Around line 28-33: The useQuery call for GetBoardCandidateDocument is
executing unconditionally and errors are being masked by the isCandidate check,
making network failures indistinguishable from access-denied cases.
Additionally, the query runs before the ownership verification check that occurs
later in the component. Refactor the code to conditionally execute the
GetBoardCandidateDocument query only after confirming route ownership (defer the
useQuery or gate it with a condition based on the ownership check), and add
explicit error handling to capture and display query errors separately from the
access control logic.
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/page.tsx:
- Around line 88-93: The swap operation in this state setter does not validate
that the key actually exists in the prev array before attempting to swap. When
indexOf returns -1 because the key is not found, the down-direction swap will
still execute with invalid indices (swapping position -1 with position 0). Add a
guard check immediately after calculating idx to verify that idx is not -1, and
return prev unchanged if the key is not found in the array.
- Around line 100-129: The reorderClaims mutation call currently shows a success
toast without checking if the mutation actually succeeded at the business logic
level. Modify the code to check the response data from the reorderClaims
mutation for a success indicator (such as an ok field) before displaying the
success toast. If the mutation response indicates failure, either skip the toast
or show an error toast instead to properly reflect the actual operation result.
- Around line 37-40: Add an ownership verification check to the skip condition
of the useQuery call for GetBoardCandidateAndClaimsDocument to gate the claim
fetching on confirmed viewer ownership of the login parameter using the
available session object. Additionally, modify the reorder mutation handling to
check the ok field in the mutation response before conditionally displaying the
success toast instead of showing it unconditionally. Finally, add a guard
condition to check that indexOf does not return -1 before performing array index
operations in the reorder swap logic for down-direction moves to prevent
undefined behavior when a key is missing from the array.
In `@frontend/src/app/board/`[year]/candidates/page.tsx:
- Line 95: Remove the useDjangoSession hook call from the CandidateCard
component and instead call it once at the page level where candidates are
rendered. Pass the session (or the derived isOwnProfile boolean value) as a prop
to each CandidateCard instance. Update the CandidateCard component to accept
this session or isOwnProfile prop and remove the hook invocation, replacing the
hook-based session usage with the prop value. This ensures the hook's side
effects run only once per page instead of once per candidate card.
In `@frontend/src/components/ClaimActions.tsx`:
- Around line 185-193: The textarea input with placeholder="Reason for
withdrawal..." in the withdraw confirmation section lacks an accessible label
for assistive technologies. Add either a proper label element associated with
the textarea (using htmlFor/id) or add an aria-label attribute to the textarea
element to provide a descriptive label that screen readers can announce.
In `@frontend/src/components/ClaimForm.tsx`:
- Around line 89-92: The form inputs are only displaying errors from local
validators but not from the backendErrors state that gets populated when the
extractGraphQLErrors call is executed. Update all form input components to merge
and display both local validation errors and backend errors from the
backendErrors state. This means modifying the error prop or className binding
for each input field to check both the local validators' errors and the
corresponding backendErrors values, ensuring users see inline validation
feedback for both client-side and server-side validation failures. Apply this
fix to all form inputs referenced in the diff and at the additional lines
mentioned (110-110, 122-122).
- Line 121: The onChange handler for the description field at line 121 calls
handleInputChange to update the value but does not mark the field as touched,
causing inconsistent validation feedback compared to the name field. Update the
onChange callback for the description input to also invoke a function that marks
the description field as touched (similar to how the name field is handled),
ensuring both value updates and touched state are properly synchronized when the
user modifies the description.
In `@frontend/src/components/forms/shared/formValidationUtils.ts`:
- Around line 35-41: The validateFileExtension function normalizes the extracted
file extension to lowercase with a leading dot, but does not normalize the
allowedExtensions array before comparison. This causes valid files to be
rejected if callers pass extensions in different formats like 'PDF' (uppercase)
or 'pdf' (without dot). Normalize each element in the allowedExtensions array to
lowercase and ensure each has a leading dot prefix before checking if the ext
value is included in the normalized array.
---
Outside diff comments:
In
`@backend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.py`:
- Around line 528-555: In the test_create_claim_success method, set up the
return value for mock_claim_model.objects.create before calling the mutation to
ensure it returns a configured mock claim object. Create a mock claim object,
assign it to mock_claim_model.objects.create.return_value, then verify that
result.claim is the same object (using identity comparison, not just truthiness
check). This aligns the test with the pattern used in other success tests like
test_update_claim_success and test_discard_claim_success, ensuring the test
would fail if the mutation code is later changed to not return the created
claim.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 902c050e-dc42-4614-a490-a026ce318471
⛔ Files ignored due to path filters (5)
frontend/src/types/__generated__/boardQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/claimMutations.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/claimQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/evidenceQueries.generated.tsis excluded by!**/__generated__/**frontend/src/types/__generated__/graphql.tsis excluded by!**/__generated__/**
📒 Files selected for processing (27)
backend/apps/owasp/api/internal/mutations/board_candidate_claim.pybackend/apps/owasp/api/internal/nodes/board_candidate_claim.pybackend/tests/unit/apps/owasp/api/internal/mutations/board_candidate_claim_test.pybackend/tests/unit/apps/owasp/api/internal/nodes/board_candidate_claim_test.pye2e/helpers/mockClaimAuth.tse2e/pages/BoardCandidateClaimDetails.spec.tse2e/pages/BoardCandidateClaims.spec.tsfrontend/__tests__/mockData/mockClaimData.tsfrontend/__tests__/unit/pages/BoardCandidatesPage.test.tsxfrontend/__tests__/unit/pages/CandidateClaimsPage.test.tsxfrontend/__tests__/unit/pages/ClaimDetailsPage.test.tsxfrontend/__tests__/unit/pages/CreateClaimPage.test.tsxfrontend/__tests__/unit/pages/EditClaimPage.test.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/page.tsxfrontend/src/app/board/[year]/candidates/page.tsxfrontend/src/components/ClaimActions.tsxfrontend/src/components/ClaimForm.tsxfrontend/src/components/DropdownActions.tsxfrontend/src/components/forms/shared/formValidationUtils.tsfrontend/src/server/mutations/claimMutations.tsfrontend/src/server/queries/boardQueries.tsfrontend/src/server/queries/claimQueries.tsfrontend/src/server/queries/evidenceQueries.tsfrontend/src/types/claim.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
19 issues found across 32 files
Confidence score: 2/5
- In
frontend/src/components/ClaimForm.tsx, backend validation errors are stored but not surfaced (andvalidateNameis called withoutbackendErrors.name), so users can hit server-side uniqueness failures like “name already exists” without seeing actionable feedback; this is the highest user-impact risk and should be fixed by wiring backend errors into field validation/rendering before merge. - In
frontend/src/app/board/[year]/candidates/[login]/claims/page.tsx, save/reorder flows can mislead users:handleSavecan report success when mutationokis false, and reorder logic can swap withnext[-1]whenindexOfreturns-1, risking corrupted ordering state; guard missing keys and gate success messaging onok/messagebefore merging. - In
frontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx, query/network failures are treated as authorization failures, so real backend outages or GraphQL errors appear asAccess Deniedand mask root cause for users/support; split auth failures from transport/query errors before merge. - In
frontend/__tests__/mockData/mockClaimData.ts, mock claims/evidences are missing requiredidfields, reducing test realism and potentially hiding runtime/keying bugs, andbackend/apps/owasp/api/internal/nodes/board_candidate_claim.pyintroduces N+1 query behavior that may degrade list performance; align mocks with GraphQL types and add queryset annotation/prefetch forhas_evidenceto de-risk correctness and scalability.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/server/queries/evidenceQueries.ts">
<violation number="1" location="frontend/src/server/queries/evidenceQueries.ts:3">
P2: Duplicates existing `GET_CLAIM_AND_EVIDENCES` from `claimQueries.ts`. The evidence sub-query is identical — same fields, same `boardCandidateClaimEvidences` resolver. Two queries fetching the same data creates a maintenance risk: schema or field changes must be synced across both files.</violation>
</file>
<file name="frontend/src/components/DropdownActions.tsx">
<violation number="1" location="frontend/src/components/DropdownActions.tsx:29">
P2: Focus is lost after selecting a menu item — the trigger button should be refocused. Add `triggerButtonRef.current?.focus()` after closing the dropdown.</violation>
<violation number="2" location="frontend/src/components/DropdownActions.tsx:105">
P2: Menu container has `tabIndex={0}`, creating an extra Tab stop. Use `tabIndex={-1}` — focus is managed via roving tabindex on menuitems.</violation>
</file>
<file name="frontend/__tests__/mockData/mockClaimData.ts">
<violation number="1" location="frontend/__tests__/mockData/mockClaimData.ts:1">
P2: Mock claim objects missing `id` field that is part of the GraphQL query response. The generated type `GetBoardCandidateClaimsQuery` includes `id: string` for every claim. If components use `claim.id` (e.g., React key, navigation), tests won't reflect real behavior.</violation>
<violation number="2" location="frontend/__tests__/mockData/mockClaimData.ts:72">
P2: mockSingleClaim missing required `id` field. The GraphQL query response includes `id: string` for the claim — without it, tests using this mock won't accurately represent real API data.</violation>
<violation number="3" location="frontend/__tests__/mockData/mockClaimData.ts:81">
P2: mockEvidences missing required `id` field. The GraphQL query response includes `id: string` for every evidence item. Tests referencing evidence IDs or using them as keys won't match real behavior.</violation>
</file>
<file name="backend/apps/owasp/api/internal/nodes/board_candidate_claim.py">
<violation number="1" location="backend/apps/owasp/api/internal/nodes/board_candidate_claim.py:33">
P2: This resolver executes a DB query per claim node. On list queries (e.g., `boardCandidateClaims`), this introduces N+1 database hits. Consider annotating `has_evidence` on the queryset (e.g., via `strawberry_django.field(annotate={...})`) and reading the pre-computed value here instead of issuing a per-node `exists()` call.</violation>
</file>
<file name="frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsx">
<violation number="1" location="frontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsx:25">
P2: The query executes when `claimKey` is present, before verifying `session?.user?.login === login`. This allows unauthorized sessions to trigger GraphQL requests for claims belonging to other users. Add ownership verification to the `skip` condition: `skip: !claimKey || session?.user?.login !== login`.</violation>
</file>
<file name="frontend/src/app/board/[year]/candidates/page.tsx">
<violation number="1" location="frontend/src/app/board/[year]/candidates/page.tsx:95">
P2: Calling `useDjangoSession()` inside `CandidateCard` creates N hook instances when rendering many candidates. This hook contains session sync logic with side effects (mutations, toasts, sign-out calls) that should only execute once. Consider lifting it to the parent page and passing `isOwnProfile` as a prop.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| useEffect(() => { | ||
| const handleClickOutside = (event: MouseEvent) => { | ||
| if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) { | ||
| setDropdownOpen(false) |
Contributor
There was a problem hiding this comment.
P2: Focus is lost after selecting a menu item — the trigger button should be refocused. Add triggerButtonRef.current?.focus() after closing the dropdown.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/DropdownActions.tsx, line 29:
<comment>Focus is lost after selecting a menu item — the trigger button should be refocused. Add `triggerButtonRef.current?.focus()` after closing the dropdown.</comment>
<file context>
@@ -0,0 +1,141 @@
+ useEffect(() => {
+ const handleClickOutside = (event: MouseEvent) => {
+ if (dropdownRef.current && !dropdownRef.current.contains(event.target as Node)) {
+ setDropdownOpen(false)
+ setFocusIndex(-1)
+ }
</file context>
Contributor
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 (2)
frontend/src/components/forms/shared/formValidationUtils.ts (1)
35-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix extension comparison to handle dot-prefix inconsistency.
The current logic extracts
extwithout a leading dot (Line 39:file.name.split('.').pop()?.toLowerCase()), but compares it directly againstallowedExtensionsitems (Line 40), which may include a leading dot (e.g.,['.pdf', '.png']). This mismatch causes valid files to be rejected whenallowedExtensionsuses the common dot-prefixed convention.🐛 Proposed fix to normalize both sides
export const validateFileExtension = ( file: File, allowedExtensions: string[] ): string | undefined => { const ext = file.name.split('.').pop()?.toLowerCase() - if (!allowedExtensions.some((e) => e.toLowerCase() === ext)) { + const normalizedAllowed = allowedExtensions.map((e) => + e.toLowerCase().replace(/^\./, '') + ) + if (!ext || !normalizedAllowed.includes(ext)) { return `File extension .${ext} is not supported. Allowed: ${allowedExtensions.join(', ')}` } return undefined }🤖 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/src/components/forms/shared/formValidationUtils.ts` around lines 35 - 44, The validateFileExtension function extracts the file extension without a leading dot (ext variable from split and pop operations), but compares it against allowedExtensions items which may include leading dots (e.g., ['.pdf', '.png']). Normalize both sides of the comparison by stripping any leading dot from each allowedExtensions item before comparing it with ext, or add a dot prefix to ext before comparison. This ensures the logic correctly validates files regardless of whether allowedExtensions uses dot-prefixed or non-prefixed conventions.frontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx (1)
38-50: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winDistinguish query errors from access denial for clearer UX.
The current flow shows a dismissible error toast via
handleAppError(line 40), then continues to line 50 where query errors (which leaveisCandidatefalse) display the same "Access Denied" message as legitimate permission failures. Users experiencing network issues will see the generic access message after the toast dismisses, making it unclear whether they lack permission or encountered a transient error.💡 Suggested improvement
Return early with a specific error state when the query fails:
useEffect(() => { if (candidateQueryError) { handleAppError(candidateQueryError) } }, [candidateQueryError]) + if (candidateQueryError) { + return ( + <AccessDeniedDisplay + title="Error Loading Form" + message="Unable to load candidate data. Please try again." + /> + ) + } + if (isSyncing || isCandidateLoading) { return <LoadingSpinner /> }🤖 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/src/app/board/`[year]/candidates/[login]/claims/create/page.tsx around lines 38 - 50, The current implementation shows a dismissible error toast when candidateQueryError exists via handleAppError, but then continues to check the isCandidate condition which returns the same generic "Access Denied" message, making it unclear whether the error is due to a network failure or actual permission denial. After the useEffect that calls handleAppError with candidateQueryError, add an early return before the isCandidate check to display a specific error state when candidateQueryError is present, ensuring users see the appropriate error context rather than the generic access denial message.
♻️ Duplicate comments (2)
frontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsx (1)
34-34:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSkip condition allows query execution before ownership is verified.
The skip logic
(session && session?.user?.login !== login)evaluates tofalsewhensessionisnullorundefined, allowing the candidate query to execute before authentication completes. This creates unnecessary backend load and potential information disclosure.🔒 Recommended fix
+ const isOwner = !!session?.user?.login && session.user.login === login + const { data: candidateGraphQLData, loading: isCandidateLoading, error: candidateQueryError, } = useQuery(GetBoardCandidateDocument, { - skip: !login || !year || (session && session?.user?.login !== login), + skip: !login || !year || !isOwner, variables: { login: login, year: Number.parseInt(year) }, })🤖 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/src/app/board/`[year]/candidates/[login]/claims/create/page.tsx at line 34, The skip condition in the candidate query uses incorrect logic that allows query execution when the session is null or undefined. The condition (session && session?.user?.login !== login) evaluates to false when session is falsy, causing skip to become false and allowing the query to run before authentication is verified. Fix the skip condition to explicitly check that session exists AND that the user's login matches the requested login parameter. The skip condition should prevent query execution whenever login, year, or session are missing, or when the authenticated user's login does not match the requested login parameter.frontend/src/app/board/[year]/candidates/[login]/claims/page.tsx (1)
38-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSkip condition still allows unauthorized query execution.
The ownership check in the skip condition is incomplete. When
sessionisnullorundefined(duringisSyncing), the expression(session && session?.user?.login !== login)evaluates tofalse, allowing the query to execute before authentication is verified.🔒 Recommended fix
+ const isOwner = !!session?.user?.login && session.user.login === login const { data: graphQLData, loading: isLoading, error: graphQLRequestError, } = useQuery(GetBoardCandidateAndClaimsDocument, { - skip: !login || !year || (session && session?.user?.login !== login), + skip: !login || !year || !isOwner, variables: { login, year: Number.parseInt(year) }, })🤖 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/src/app/board/`[year]/candidates/[login]/claims/page.tsx at line 38, The skip condition for the query does not properly handle the case when session is null or undefined. The expression (session && session?.user?.login !== login) evaluates to false when session is missing, which allows the query to execute without proper authentication verification. Update the skip condition to explicitly check that session exists before attempting to access session.user.login, ensuring the query is skipped whenever session is null/undefined OR when the authenticated user's login does not match the requested login parameter.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@backend/settings/graphql.py`:
- Around line 53-56: In the __init__ method, the kwargs.pop("execution_context")
call on line 55 lacks a default argument and will raise a KeyError if
"execution_context" is not present in the incoming kwargs. Modify the pop call
to include a default value as the second argument, such as None, to gracefully
handle cases where "execution_context" is not provided in kwargs.
---
Outside diff comments:
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/create/page.tsx:
- Around line 38-50: The current implementation shows a dismissible error toast
when candidateQueryError exists via handleAppError, but then continues to check
the isCandidate condition which returns the same generic "Access Denied"
message, making it unclear whether the error is due to a network failure or
actual permission denial. After the useEffect that calls handleAppError with
candidateQueryError, add an early return before the isCandidate check to display
a specific error state when candidateQueryError is present, ensuring users see
the appropriate error context rather than the generic access denial message.
In `@frontend/src/components/forms/shared/formValidationUtils.ts`:
- Around line 35-44: The validateFileExtension function extracts the file
extension without a leading dot (ext variable from split and pop operations),
but compares it against allowedExtensions items which may include leading dots
(e.g., ['.pdf', '.png']). Normalize both sides of the comparison by stripping
any leading dot from each allowedExtensions item before comparing it with ext,
or add a dot prefix to ext before comparison. This ensures the logic correctly
validates files regardless of whether allowedExtensions uses dot-prefixed or
non-prefixed conventions.
---
Duplicate comments:
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/create/page.tsx:
- Line 34: The skip condition in the candidate query uses incorrect logic that
allows query execution when the session is null or undefined. The condition
(session && session?.user?.login !== login) evaluates to false when session is
falsy, causing skip to become false and allowing the query to run before
authentication is verified. Fix the skip condition to explicitly check that
session exists AND that the user's login matches the requested login parameter.
The skip condition should prevent query execution whenever login, year, or
session are missing, or when the authenticated user's login does not match the
requested login parameter.
In `@frontend/src/app/board/`[year]/candidates/[login]/claims/page.tsx:
- Line 38: The skip condition for the query does not properly handle the case
when session is null or undefined. The expression (session &&
session?.user?.login !== login) evaluates to false when session is missing,
which allows the query to execute without proper authentication verification.
Update the skip condition to explicitly check that session exists before
attempting to access session.user.login, ensuring the query is skipped whenever
session is null/undefined OR when the authenticated user's login does not match
the requested login parameter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a810207d-2072-4c56-b2d5-abbc215a184b
📒 Files selected for processing (15)
backend/settings/graphql.pye2e/helpers/mockClaimAuth.tse2e/pages/BoardCandidateClaimDetails.spec.tse2e/pages/BoardCandidateClaims.spec.tsfrontend/__tests__/mockData/mockClaimData.tsfrontend/__tests__/unit/pages/CandidateClaimsPage.test.tsxfrontend/__tests__/unit/pages/ClaimDetailsPage.test.tsxfrontend/__tests__/unit/pages/EditClaimPage.test.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/edit/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/[claimKey]/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/create/page.tsxfrontend/src/app/board/[year]/candidates/[login]/claims/page.tsxfrontend/src/components/ClaimActions.tsxfrontend/src/components/ClaimForm.tsxfrontend/src/components/forms/shared/formValidationUtils.ts
Contributor
There was a problem hiding this comment.
2 issues found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/DropdownActions.tsx">
<violation number="1" location="frontend/src/components/DropdownActions.tsx:29">
P2: Focus is lost after selecting a menu item — the trigger button should be refocused. Add `triggerButtonRef.current?.focus()` after closing the dropdown.</violation>
</file>
<file name="frontend/src/app/board/[year]/candidates/page.tsx">
<violation number="1" location="frontend/src/app/board/[year]/candidates/page.tsx:95">
P2: Calling `useDjangoSession()` inside `CandidateCard` creates N hook instances when rendering many candidates. This hook contains session sync logic with side effects (mutations, toasts, sign-out calls) that should only execute once. Consider lifting it to the parent page and passing `isOwnProfile` as a prop.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/DropdownActions.tsx">
<violation number="1" location="frontend/src/components/DropdownActions.tsx:29">
P2: Focus is lost after selecting a menu item — the trigger button should be refocused. Add `triggerButtonRef.current?.focus()` after closing the dropdown.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
rudransh-shrivastava
force-pushed
the
feature/bod-claim-dash-cherry-claim
branch
from
235a174 to
ee4005b
Compare
rudransh-shrivastava
left a comment
rudransh-shrivastava
left a comment
Collaborator
Author
There was a problem hiding this comment.
Some comments:
- Sonar Qube issues are created by GraphQL generated types.
- All redirects to evidence pages are currently 404 as it will be added in next PR.
Collaborator
Author
There was a problem hiding this comment.
This component is similar to EntityActions but this one is generic. I will update EntityActions to use this component and rename it to something more specific to mentorship portal.
I can't reuse EntityActions as it's highly specific to mentorship portal.
Contributor
|
You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment |
arkid15r
reviewed
Jun 26, 2026
| @strawberry_django.field | ||
| def has_evidence(self, root: BoardCandidateClaim) -> bool: | ||
| """Resolve whether the claim has any evidence.""" | ||
| if hasattr(root, "evidence_exists"): |
Collaborator
There was a problem hiding this comment.
Is there a case for evidence_exists to be absent?
Collaborator
Author
There was a problem hiding this comment.
Yes, I only set it in board_candidate_claims query, single node board_candidate_claim does not set it.
I will add it to single node query as well, but also keep the hasattr check just in case.
Collaborator
Author
There was a problem hiding this comment.
updated both queries to annotate.
4 tasks
Contributor
There was a problem hiding this comment.
3 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/DropdownActions.tsx">
<violation number="1" location="frontend/src/components/DropdownActions.tsx:29">
P2: Focus is lost after selecting a menu item — the trigger button should be refocused. Add `triggerButtonRef.current?.focus()` after closing the dropdown.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Contributor
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Resolves #5004
This PR adds a dashboard for managing claims. The redirects to evidence pages return a 404 as this is the first (1/2) PR, the second PR should fill those gaps.
Checklist
make check-testlocally: all warnings addressed, tests passed