Merge the last updates from the main branch#3136
Merge the last updates from the main branch#3136arkid15r merged 18 commits intoOWASP:feature/e2e-backendfrom
Conversation
…SP#3090) Bumps [strawberry-graphql-django](https://github.com/strawberry-graphql/strawberry-django) from 0.71.0 to 0.72.0. - [Release notes](https://github.com/strawberry-graphql/strawberry-django/releases) - [Changelog](https://github.com/strawberry-graphql/strawberry-django/blob/main/CHANGELOG.md) - [Commits](strawberry-graphql/strawberry-django@v0.71.0...v0.72.0) --- updated-dependencies: - dependency-name: strawberry-graphql-django dependency-version: 0.72.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add keyboard navigation support to various components * Refactor ActionButton to utilize onKeyDown prop for keyboard navigation support * Update code * Add stopPropagation() * Make check update --------- Co-authored-by: Kate <kate@kgthreads.com>
…test class (OWASP#3036) * remove unused function parameter from save method in test class * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
Bumps [pymdown-extensions](https://github.com/facelessuser/pymdown-extensions) from 10.19.1 to 10.20. - [Release notes](https://github.com/facelessuser/pymdown-extensions/releases) - [Commits](facelessuser/pymdown-extensions@10.19.1...10.20) --- updated-dependencies: - dependency-name: pymdown-extensions dependency-version: '10.20' dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix race condition in About page and update unit tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
* Remove unnecessary zero-fraction literals * refactoring barchat tests : use dataset instead of getAttributes --------- Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
…WASP#3106) * fix(ui): unify badge shapes by replacing icon background with css circle * Update code after make-check --------- Co-authored-by: Kate <kate@kgthreads.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
* fix: incorrect Error syntax * fix: unhandled promise rejection in ModuleForm * improve error message in ModuleForm * fix: fix syntax error in ModuleForm * fix: improve error messages in route.ts --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com>
* Removed nested ternary * fixed coderabbit review * fixed soanrqube warning * fixed warning * Added check * Fixed review * Fixed sonarqube warning
* Prefer tag over ARIA role * updated aria * Run check command --------- Co-authored-by: anurag2787 <anurag2787@gmail.com>
* Refactor snapshot API to remove duplicated ordering description Signed-off-by: Tejas Tiwari <tt160705@outlook.com> * Update code --------- Signed-off-by: Tejas Tiwari <tt160705@outlook.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
…s null (OWASP#3084) * fix(board): render error state instead of infinite loader when data is null * Address make-check --------- Co-authored-by: Kate <kate@kgthreads.com>
Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
github-actions
bot
added
docs
Summary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR refactors environment setup documentation, extracts backend constants, adds extensive keyboard accessibility to frontend components, updates test mocks for component changes, refactors state management in frontend pages, and bumps dependencies across the stack. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
|
PR validation failed: No linked issue and no valid closing issue reference in PR description |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
backend/apps/api/rest/v0/snapshot.py (1)
26-27: Good refactoring, but enhance the description text for API consumers.Extracting the constant follows the DRY principle and improves maintainability. However, the description "Ordering field" is quite generic and doesn't explain how to use the parameter.
💡 Suggested improvement for the description
-ORDERING_FIELD_DESCRIPTION = "Ordering field" +ORDERING_FIELD_DESCRIPTION = "Field to order results by. Prefix with '-' for descending order."This provides clearer guidance to API consumers about how to use the ordering parameter.
frontend/__tests__/unit/components/ProjectsDashboardDropDown.test.tsx (1)
546-554: Consider adding a test to verify the semantic HTML structure.While the current test validates data attributes, it doesn't verify that sections render as
fieldsetelements or that titles render aslegendelements. Adding this check would ensure the mock stays aligned with the component's accessibility improvements.🔎 Suggested test case
Add this test case within the "DOM structure / classNames / styles" describe block:
+ it('renders sections using semantic fieldset and legend elements', () => { + render(<ProjectsDashboardDropDown {...defaultProps} onAction={mockOnAction} />) + + const sections = screen.getAllByTestId('dropdown-section') + expect(sections).toHaveLength(2) + + // Verify semantic HTML structure + expect(sections[0].tagName.toLowerCase()).toBe('fieldset') + expect(sections[1].tagName.toLowerCase()).toBe('fieldset') + + // Verify legend elements for section titles + const statusLegend = sections[0].querySelector('legend') + const priorityLegend = sections[1].querySelector('legend') + + expect(statusLegend).toBeInTheDocument() + expect(statusLegend).toHaveTextContent('Status') + expect(priorityLegend).toBeInTheDocument() + expect(priorityLegend).toHaveTextContent('Priority') + })frontend/src/components/SortBy.tsx (1)
3-3: Consider relying on native button keyboard behavior.Native
<button>elements already respond to Enter and Space keys via theonClickhandler (line 72). Adding a separateonKeyDownhandler that callspreventDefault()and then manually invokes the same toggle logic duplicates this built-in behavior and may interfere with assistive technologies expecting standard button semantics.Additionally, the type
React.KeyboardEventcould be more specific:React.KeyboardEvent<HTMLButtonElement>.🔎 Suggested simplification
Remove the
handleKeyDownfunction andonKeyDownbinding—the existingonClickhandler already provides full keyboard support:-import type React from 'react' import { FaArrowDownWideShort, FaArrowUpWideShort } from 'react-icons/fa6' import type { SortByProps } from 'types/sortBy' const SortBy = ({ ... }: SortByProps) => { if (!sortOptions || sortOptions.length === 0) return null - - const handleKeyDown = (e: React.KeyboardEvent) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - onOrderChange(selectedOrder === 'asc' ? 'desc' : 'asc') - } - } return ( ... <button type="button" onClick={() => onOrderChange(selectedOrder === 'asc' ? 'desc' : 'asc')} - onKeyDown={handleKeyDown} className="..."If retaining the
onKeyDownhandler for consistency with other components in the PR, consider making the type more specific:- const handleKeyDown = (e: React.KeyboardEvent) => { + const handleKeyDown = (e: React.KeyboardEvent<HTMLButtonElement>) => {Also applies to: 16-21, 73-73
frontend/src/components/Search.tsx (1)
69-74: Consider relying on native button keyboard behavior.The explicit
onKeyDownhandler is redundant for HTML<button>elements, which natively triggeronClickevents for both Enter and Space keys. The current implementation works correctly but adds unnecessary complexity.💡 Simplification: rely on native button behavior
The
onClickhandler on the button (line 98) already provides full keyboard accessibility. You can remove thehandleKeyDownfunction and theonKeyDownprop:- const handleKeyDown = (e: React.KeyboardEvent) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault() - handleClearSearch() - } - } - return ( <div className="w-full max-w-md p-4"> <div className="relative"> {isLoaded ? ( <> ... {searchQuery && ( <button type="button" className="..." onClick={handleClearSearch} - onKeyDown={handleKeyDown} aria-label="Clear search" > <FaTimes className="h-4 w-4" aria-hidden="true" /> </button> )} </>If this explicit handling is part of a consistent pattern across your codebase or addresses specific edge cases, feel free to keep it as-is.
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx (2)
39-62: Improved readability by extracting deadline logic.The refactor from inline ternaries to explicit if-else statements with computed
statusTextandcolorvariables makes the deadline formatting logic much clearer and easier to maintain.Optional: Consider const for single-assignment variables
Since
statusTextandcolorare each assigned exactly once, you could optionally useconstwith conditional expressions:- let statusText: string - if (isOverdue) { - statusText = '(overdue)' - } else if (daysLeft === 0) { - statusText = '(today)' - } else { - statusText = `(${daysLeft} days left)` - } + const statusText: string = + isOverdue ? '(overdue)' + : daysLeft === 0 ? '(today)' + : `(${daysLeft} days left)`However, the current if-else approach is arguably more readable, so this is purely a stylistic choice.
125-139: Helper function reduces PR status duplication.The
getPRStatushelper effectively centralizes the logic for determining PR badge styling and text, eliminating repetitive inline conditionals.Optional: Consider Tailwind classes for consistency
The function returns a hex
backgroundColorfor inline styles (line 391), while the rest of the component uses Tailwind utility classes. For consistency, you could refactor to return a class name:- const getPRStatus = (pr: Exclude<typeof issue.pullRequests, undefined>[0]) => { - let backgroundColor: string - let label: string - if (pr.state === 'closed' && pr.mergedAt) { - backgroundColor = '#8657E5' - label = 'Merged' - } else if (pr.state === 'closed') { - backgroundColor = '#DA3633' - label = 'Closed' - } else { - backgroundColor = '#238636' - label = 'Open' - } - return { backgroundColor, label } + const getPRStatus = (pr: Exclude<typeof issue.pullRequests, undefined>[0]) => { + let badgeClass: string + let label: string + if (pr.state === 'closed' && pr.mergedAt) { + badgeClass = 'bg-[#8657E5]' + label = 'Merged' + } else if (pr.state === 'closed') { + badgeClass = 'bg-[#DA3633]' + label = 'Closed' + } else { + badgeClass = 'bg-[#238636]' + label = 'Open' + } + return { badgeClass, label } }Then update the usage (lines 389-391):
- <span - className="inline-flex items-center rounded-lg px-2 py-0.5 text-xs font-medium text-white" - style={{ backgroundColor }} - > + <span className={`inline-flex items-center rounded-lg px-2 py-0.5 text-xs font-medium text-white ${badgeClass}`}>This aligns with the pattern used for
issueStatusClass(lines 112-123).frontend/src/components/skeletons/Card.tsx (1)
19-25: Remove redundantaria-liveattribute.The
role="status"attribute already impliesaria-live="polite"by default, making the explicitaria-live="polite"declaration redundant.🔎 Proposed simplification
<div role="status" - aria-live="polite" aria-busy="true" aria-label="Loading" className="flex w-full justify-center" >frontend/src/components/MultiSearch.tsx (1)
269-269: Good refactor using optional chaining.This is cleaner than the explicit null check and maintains the same null-safety.
frontend/src/app/settings/api-keys/page.tsx (2)
23-28: Consider displaying specific error details.The ErrorState component shows a generic error message. For better debugging and user experience, consider accepting an
errorparameter and displaying the actual error message.💡 Suggested enhancement
-const ErrorState = () => ( +interface ErrorStateProps { + error?: Error | ApolloError +} + +const ErrorState = ({ error }: ErrorStateProps) => ( <div className="rounded-md bg-red-50 p-4 text-red-700 dark:bg-red-900/20 dark:text-red-400"> - Error loading API keys + <p className="font-semibold">Error loading API keys</p> + {error?.message && <p className="mt-1 text-sm">{error.message}</p>} </div> )Then update the getContentComponents call at line 85:
- error: () => <ErrorState />, + error: () => <ErrorState error={error} />,And update getContentComponents signature at line 81 to accept the error parameter.
81-89: Consider memoizing content components to avoid recreation on each render.The
getContentComponentsfunction creates new function instances on every render. While functionally correct, memoizing this could provide a minor performance improvement.💡 Optional optimization
+const contentComponents = useMemo( + () => getContentComponents(data, setKeyToRevoke), + [data, setKeyToRevoke] +) -const contentComponents = getContentComponents(data, setKeyToRevoke)Or inline the logic to avoid the helper function entirely.
backend/apps/owasp/models/common.py (1)
241-245: LGTM! More idiomatic regex pattern.The change from
[\s\S]*?to.*?withre.DOTALLis functionally equivalent and more idiomatic. Both patterns match any character including newlines non-greedily, but.*?withre.DOTALLis the standard Python approach.CONTRIBUTING.md (1)
253-262: Updated Algolia onboarding reflects current UX.The updated instructions accurately reflect Algolia's current signup flow (automatic app creation) and clarify API key permission requirements. The note about
addObjectpermission is helpful.Note: Line 254 has a minor grammar issue flagged by static analysis - "sign up" should be hyphenated as "sign-up" when used as a modifier ("sign-up process").
Optional grammar fix
- - During the sign up process, you may be asked to import data. You can skip this step. + - During the sign-up process, you may be asked to import data. You can skip this step.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
backend/poetry.lockis excluded by!**/*.lockdocs/poetry.lockis excluded by!**/*.lockfrontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
CONTRIBUTING.mdbackend/apps/api/rest/v0/snapshot.pybackend/apps/owasp/models/common.pybackend/pyproject.tomlbackend/tests/apps/common/management/commands/add_project_custom_tags_test.pydocs/pyproject.tomlfrontend/__tests__/unit/components/BarChart.test.tsxfrontend/__tests__/unit/components/CardDetailsPage.test.tsxfrontend/__tests__/unit/components/GeneralCompliantComponent.test.tsxfrontend/__tests__/unit/components/Pagination.test.tsxfrontend/__tests__/unit/components/ProjectsDashboardDropDown.test.tsxfrontend/__tests__/unit/pages/About.test.tsxfrontend/__tests__/unit/pages/ApiKeysPage.test.tsxfrontend/__tests__/unit/pages/Home.test.tsxfrontend/package.jsonfrontend/src/app/about/page.tsxfrontend/src/app/api/auth/[...nextauth]/route.tsfrontend/src/app/board/[year]/candidates/page.tsxfrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsxfrontend/src/app/settings/api-keys/page.tsxfrontend/src/components/ActionButton.tsxfrontend/src/components/Card.tsxfrontend/src/components/GeneralCompliantComponent.tsxfrontend/src/components/ModuleCard.tsxfrontend/src/components/ModuleForm.tsxfrontend/src/components/MultiSearch.tsxfrontend/src/components/Pagination.tsxfrontend/src/components/Release.tsxfrontend/src/components/RepositoryCard.tsxfrontend/src/components/Search.tsxfrontend/src/components/ShowMoreButton.tsxfrontend/src/components/SortBy.tsxfrontend/src/components/ToggleableList.tsxfrontend/src/components/skeletons/Card.tsxfrontend/src/components/skeletons/UserCard.tsxfrontend/src/types/button.ts
🧰 Additional context used
🧠 Learnings (21)
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.
Applied to files:
frontend/src/components/ShowMoreButton.tsxfrontend/src/components/ActionButton.tsxfrontend/src/components/SortBy.tsxfrontend/src/components/GeneralCompliantComponent.tsxfrontend/__tests__/unit/components/GeneralCompliantComponent.test.tsxfrontend/src/components/Card.tsx
📚 Learning: 2025-12-29T22:01:53.874Z
Learnt from: ShashaankS
Repo: OWASP/Nest PR: 3086
File: frontend/src/components/SortBy.tsx:45-45
Timestamp: 2025-12-29T22:01:53.874Z
Learning: In frontend/src/components/SortBy.tsx, the SelectItem uses hover:bg-transparent intentionally for better visual appeal and theme consistency. This design choice prioritizes a cleaner aesthetic over hover background feedback.
Applied to files:
frontend/src/components/ShowMoreButton.tsxfrontend/src/components/SortBy.tsx
📚 Learning: 2025-11-17T17:30:42.139Z
Learnt from: anurag2787
Repo: OWASP/Nest PR: 2671
File: frontend/__tests__/unit/components/MultiSearch.test.tsx:169-171
Timestamp: 2025-11-17T17:30:42.139Z
Learning: In the OWASP/Nest frontend tests (PR #2671 context), wrapper functions like `expectChaptersCountEqualsThree` that simply call another helper with a fixed parameter (e.g., `expectChaptersCountEquals(3)`) are intentionally used to avoid arrow function callbacks in `waitFor` calls. This pattern prevents adding nesting depth that would trigger rule typescript:S2004. Example: `await waitFor(expectChaptersCountEqualsThree)` avoids the extra nesting from `await waitFor(() => expectChaptersCountEquals(3))`.
Applied to files:
frontend/__tests__/unit/components/Pagination.test.tsx
📚 Learning: 2025-07-12T17:12:25.807Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/ProgramDetails.test.tsx:35-0
Timestamp: 2025-07-12T17:12:25.807Z
Learning: In React applications, button elements should always have an explicit type attribute (type="button", type="submit", or type="reset") to prevent unintended form submission behavior, even when the button appears to be outside of a form context. The default type is "submit" which can cause unexpected behavior.
Applied to files:
frontend/src/components/ActionButton.tsxfrontend/src/types/button.tsfrontend/src/components/ToggleableList.tsxfrontend/src/components/Card.tsx
📚 Learning: 2025-08-10T11:08:47.258Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/api/auth/[...nextauth]/route.ts:13-25
Timestamp: 2025-08-10T11:08:47.258Z
Learning: In the OWASP Nest codebase (frontend/src/app/api/auth/[...nextauth]/route.ts), input validation and string trimming for authentication-related queries like `isProjectLeader` and `isMentor` are handled in the backend rather than the frontend. The backend is responsible for sanitizing and validating input parameters.
Applied to files:
frontend/src/app/api/auth/[...nextauth]/route.ts
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/__tests__/unit/components/CardDetailsPage.test.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/__tests__/unit/components/CardDetailsPage.test.tsx
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/unit/components/CardDetailsPage.test.tsxfrontend/__tests__/unit/pages/Home.test.tsxfrontend/__tests__/unit/pages/ApiKeysPage.test.tsx
📚 Learning: 2025-07-09T08:37:10.241Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1718
File: frontend/src/app/projects/dashboard/metrics/page.tsx:96-96
Timestamp: 2025-07-09T08:37:10.241Z
Learning: In the OWASP Nest project's MetricsPage component (frontend/src/app/projects/dashboard/metrics/page.tsx), the sorting dropdown intentionally uses selectionMode="multiple" to allow users to select multiple sorting criteria simultaneously. This enables complex sorting scenarios where users can sort by multiple fields in sequence.
Applied to files:
frontend/src/components/SortBy.tsx
📚 Learning: 2025-09-21T17:04:48.154Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2178
File: frontend/src/app/snapshots/[id]/page.tsx:0-0
Timestamp: 2025-09-21T17:04:48.154Z
Learning: User rudransh-shrivastava confirmed that suggested type safety improvements during Apollo Client migration were no longer relevant, reinforcing their preference to keep migration PRs focused on core migration changes rather than additional improvements.
Applied to files:
frontend/src/app/about/page.tsx
📚 Learning: 2025-07-12T17:14:28.536Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/[programKey]/edit/page.tsx:90-128
Timestamp: 2025-07-12T17:14:28.536Z
Learning: Both ProgramForm (programCard.tsx) and ModuleForm (mainmoduleCard.tsx) components already implement HTML validation using the `required` attribute on form fields. The browser's native validation prevents form submission and displays error messages when required fields are empty, eliminating the need for additional JavaScript validation before GraphQL mutations.
Applied to files:
frontend/src/components/ModuleForm.tsx
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/apps/owasp/models/common.pybackend/apps/api/rest/v0/snapshot.pybackend/tests/apps/common/management/commands/add_project_custom_tags_test.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/apps/owasp/models/common.pybackend/apps/api/rest/v0/snapshot.pybackend/tests/apps/common/management/commands/add_project_custom_tags_test.py
📚 Learning: 2026-01-01T18:57:05.007Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/video.py:189-215
Timestamp: 2026-01-01T18:57:05.007Z
Learning: In the OWASP backend area, maintain the established pattern: when dealing with sponsors, include all entries from Sponsor.objects.all() (including NOT_SPONSOR) and perform in-memory sorting using the same criteria/pattern used by the GraphQL sponsor query implemented in backend/apps/owasp/api/internal/queries/sponsor.py. Apply this behavior consistently to files in backend/apps/owasp (not just video.py), and ensure code paths that render sponsor lists follow this in-code sorting approach rather than pre-filtering NOT_SPONSOR entries before sorting.
Applied to files:
backend/apps/owasp/models/common.py
📚 Learning: 2025-04-30T13:41:20.846Z
Learnt from: codic-yeeshu
Repo: OWASP/Nest PR: 1444
File: frontend/src/components/NavDropDown.tsx:0-0
Timestamp: 2025-04-30T13:41:20.846Z
Learning: When implementing dropdown menus or similar interactive components, always include proper accessibility features: ARIA attributes (aria-expanded, aria-haspopup, aria-controls), keyboard navigation support (Enter, Space, Escape keys), and mechanisms to close dropdowns when clicking outside.
Applied to files:
frontend/src/components/ToggleableList.tsx
📚 Learning: 2025-06-21T12:21:32.372Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/GradientRadialChart.tsx:67-116
Timestamp: 2025-06-21T12:21:32.372Z
Learning: The react-apexcharts Chart component does not support ARIA attributes like aria-label and role as direct props. To add accessibility attributes to ApexCharts in React, wrap the Chart component in a container div with the appropriate ARIA attributes.
Applied to files:
frontend/src/components/skeletons/UserCard.tsx
📚 Learning: 2025-11-17T16:47:05.578Z
Learnt from: anurag2787
Repo: OWASP/Nest PR: 2671
File: frontend/__tests__/unit/components/MultiSearch.test.tsx:427-427
Timestamp: 2025-11-17T16:47:05.578Z
Learning: In the frontend test files for the OWASP/Nest repository, `expect(true).toBe(true)` no-op assertions may be intentionally added as workarounds when ESLint rule jest/expect-expect doesn't detect expectations inside helper functions or waitFor callbacks. These can be resolved by configuring the ESLint rule's assertFunctionNames option to recognize custom assertion helpers instead of flagging them as redundant.
Applied to files:
frontend/__tests__/unit/components/GeneralCompliantComponent.test.tsx
📚 Learning: 2025-07-03T03:08:03.290Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1703
File: frontend/src/components/BarChart.tsx:33-46
Timestamp: 2025-07-03T03:08:03.290Z
Learning: In the OWASP Nest project's BarChart component (frontend/src/components/BarChart.tsx), the days and requirements arrays are guaranteed to always have the same length in their use cases, so input validation for array length matching is not needed.
Applied to files:
frontend/__tests__/unit/components/BarChart.test.tsx
📚 Learning: 2025-07-13T07:31:06.511Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/components/ModuleCard.tsx:53-55
Timestamp: 2025-07-13T07:31:06.511Z
Learning: In Next.js 13+ app router, useRouter from 'next/navigation' does not provide asPath or query properties. Use useParams to extract route parameters and usePathname to get the current pathname instead.
Applied to files:
frontend/src/components/ModuleCard.tsx
📚 Learning: 2025-07-13T11:29:25.245Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/page.tsx:59-61
Timestamp: 2025-07-13T11:29:25.245Z
Learning: In Next.js 13+ app router, components with the 'use client' directive run entirely on the client side and don't require window object existence checks or SSR hydration considerations. Direct access to window.location and other browser APIs is safe in client components.
Applied to files:
frontend/src/components/ModuleCard.tsx
📚 Learning: 2025-12-18T05:39:42.678Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:40-40
Timestamp: 2025-12-18T05:39:42.678Z
Learning: In Django management commands, prefer using self.stdout.write(...) over print(...) for user-facing stdout output. This aligns with Django conventions and improves testability. When emitting messages, consider using self.stdout.write and, for styled messages, use self.style.SUCCESS/ERROR as appropriate to maintain consistent command output formatting. Apply this guideline to all Python files within any project's management/commands directory.
Applied to files:
backend/tests/apps/common/management/commands/add_project_custom_tags_test.py
🧬 Code graph analysis (7)
frontend/src/components/ActionButton.tsx (2)
frontend/src/types/link.ts (1)
Link(1-7)frontend/src/types/button.ts (1)
Button(5-11)
frontend/src/app/about/page.tsx (2)
frontend/src/types/__generated__/projectQueries.generated.ts (2)
GetProjectMetadataDocument(37-37)GetTopContributorsDocument(38-38)frontend/src/app/global-error.tsx (1)
handleAppError(67-91)
frontend/src/app/board/[year]/candidates/page.tsx (1)
frontend/src/types/__generated__/boardQueries.generated.ts (1)
GetBoardCandidatesDocument(33-33)
frontend/src/app/settings/api-keys/page.tsx (2)
frontend/src/types/apiKey.ts (1)
ApiKey(9-15)frontend/src/types/button.ts (1)
Button(5-11)
frontend/src/components/GeneralCompliantComponent.tsx (1)
frontend/src/wrappers/IconWrapper.tsx (1)
IconWrapper(13-13)
frontend/__tests__/unit/pages/About.test.tsx (2)
frontend/src/types/__generated__/projectQueries.generated.ts (2)
GetProjectMetadataDocument(37-37)GetTopContributorsDocument(38-38)frontend/__tests__/unit/data/mockProjectData.ts (1)
mockProjectData(1-26)
frontend/src/components/MultiSearch.tsx (5)
frontend/src/types/chapter.ts (1)
Chapter(4-21)frontend/src/types/project.ts (1)
Project(20-49)frontend/src/types/user.ts (1)
User(11-35)frontend/src/types/event.ts (1)
Event(1-10)frontend/src/types/organization.ts (1)
Organization(1-17)
🪛 LanguageTool
CONTRIBUTING.md
[grammar] ~254-~254: Use a hyphen to join words.
Context: ...u when you sign up. - During the sign up process, you may be asked to import d...
(QB_NEW_EN_HYPHEN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Run frontend e2e tests
- GitHub Check: Run frontend unit tests
- GitHub Check: Run fuzz tests
- GitHub Check: Run backend tests
- GitHub Check: CodeQL (python)
- GitHub Check: CodeQL (javascript-typescript)
- GitHub Check: Run frontend checks
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (56)
backend/tests/apps/common/management/commands/add_project_custom_tags_test.py (1)
99-99: LGTM! Appropriate use of NOSONAR comment for interface compliance.The
update_fieldsparameter is required to match Django'sModel.save()signature, even though this test mock doesn't use it. The NOSONAR comment clearly documents that the unused parameter is intentional.frontend/src/components/ModuleForm.tsx (1)
371-378: Good improvement to error handling.The catch block now logs detailed error information to the console and gracefully clears suggestions, replacing the previous silent failure. This provides better debugging visibility while maintaining a good user experience.
frontend/src/app/api/auth/[...nextauth]/route.ts (1)
21-24: Excellent error handling enhancement.Both catch blocks now throw descriptive errors with the original error attached as
cause, preserving the error chain while adding helpful context. This improves debugging and monitoring capabilities during authentication flows.Also applies to: 38-41
frontend/__tests__/unit/components/BarChart.test.tsx (1)
360-374: LGTM: Improved consistency with integer literals.The change from
100.0to100aligns with the integer literal style used throughout the test suite. The values are functionally equivalent in JavaScript/TypeScript, and the test correctly validates decimal value handling in thedaysarray (99.5).frontend/__tests__/unit/pages/Home.test.tsx (1)
46-54: LGTM! Semantic improvement to test mock.The update from
<div role="dialog">to native<dialog open>improves the semantic correctness of the Modal mock while maintaining compatibility with existing test queries that usegetByRole('dialog').frontend/__tests__/unit/pages/ApiKeysPage.test.tsx (1)
25-25: LGTM! Semantic improvement to test mock.The update to render a native
<dialog open>element instead of a div improves the semantic correctness of the @heroui/modal mock while maintaining full compatibility with existing test assertions.backend/apps/api/rest/v0/snapshot.py (1)
119-119: LGTM: Consistent application of the constant.The constant is used consistently across all six endpoints that support ordering, successfully eliminating code duplication.
Also applies to: 165-165, 190-190, 217-217, 242-242, 267-267
frontend/src/app/board/[year]/candidates/page.tsx (2)
91-97: LGTM! Improved loading state management.The refactor to use Apollo's
loadingstate directly fromuseQueryeliminates manual state management and potential race conditions. This is a cleaner, more idiomatic approach for Apollo Client.
680-682: LGTM! Consistent use of query loading state.The loading guard now correctly uses the
loadingflag from the Apollo query, maintaining consistency with the refactored approach.frontend/__tests__/unit/pages/About.test.tsx (1)
467-478: LGTM! Improved test mock routing.The enhanced mock implementation now properly differentiates between
GetProjectMetadataDocument,GetTopContributorsDocument, andGetLeaderDataDocumentqueries. This improves test isolation and aligns with the refactored data flow in the About page where these queries are now handled separately.frontend/src/app/about/page.tsx (4)
63-82: LGTM! Well-structured query separation.Separating the project metadata and top contributors queries with explicit
loadinganderrorstate extraction enables proper handling of each query's lifecycle independently. This is more maintainable and follows Apollo Client best practices.
86-88: LGTM! Prevents race conditions.Deriving data directly from query responses eliminates the race condition that can occur with manual state management via
useStateanduseEffect. The inline comment accurately explains the benefit.
90-100: LGTM! Clear error handling.Separate
useEffectblocks for each query's error handling make the error flow explicit and easy to debug. Each effect properly depends only on its respective error object.
102-102: LGTM! Comprehensive loading state.Combining all three loading flags ensures the UI waits for all required data before rendering, preventing partial or inconsistent state display.
frontend/__tests__/unit/components/ProjectsDashboardDropDown.test.tsx (1)
77-84: LGTM! Semantic HTML improves accessibility.The migration from
divwith ARIA attributes tofieldset/legendprovides native semantic grouping that screen readers understand without additional markup. This is a best practice for accessible component design.frontend/src/components/SortBy.tsx (2)
34-34: LGTM! Improved keyboard navigation feedback.Adding
focus:underlineprovides clear visual feedback when the Select trigger receives keyboard focus, enhancing accessibility for keyboard users.
52-52: LGTM! Cosmetic class reordering.The reordering of class tokens in the
SelectItembase classes doesn't affect functionality or styling. Based on learnings, thehover:bg-transparentstyling is intentional for visual consistency.frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx (3)
112-123: Good extraction of issue status logic.Centralizing the badge class and label computation into
issueStatusClassandissueStatusLabelvariables removes inline conditionals from the JSX and makes the status logic more maintainable.
141-151: Helper standardizes button tooltip text.The
getAssignButtonTitlefunction centralizes the logic for determining the Assign button's tooltip, ensuring consistent messaging across different states.
166-168: Effective use of extracted helpers.The refactored code successfully uses the new helper functions and computed variables to render status badges and tooltips, resulting in cleaner, more maintainable JSX.
Also applies to: 386-396, 450-450
frontend/src/components/skeletons/UserCard.tsx (1)
13-15: Excellent accessibility enhancement for loading states!The addition of
aria-live="polite",aria-busy="true", andaria-label="Loading"properly announces the loading state to screen readers without interrupting the user. This follows accessibility best practices for skeleton loaders.frontend/src/components/Release.tsx (2)
25-37: Well-implemented keyboard navigation!The keyboard handler correctly supports both Enter and Space keys, includes
preventDefault()to avoid unwanted scrolling, and guards against navigation with incomplete data. This enhancement makes the repository link accessible to keyboard users.
92-93: LGTM!The handlers are properly wired to the button element, complementing the existing
disabledattribute for robust accessibility.frontend/src/components/ShowMoreButton.tsx (2)
19-19: Excellent focus styling for keyboard accessibility!The added focus styles (
focus:outline-2,focus:outline-offset-2,focus:outline-blue-500) provide clear visual feedback for keyboard navigation, and thehover:underlineimproves the button's affordance.
27-27: Good catch fixing the aria-hidden attribute!The correction from the invalid
area-hiddentoaria-hiddenensures the icon is properly hidden from assistive technologies.frontend/src/components/ModuleCard.tsx (3)
4-4: LGTM!The React type import is correctly added to support the keyboard event typing.
30-35: Proper keyboard accessibility implementation!The keyboard handler follows the same reliable pattern seen across other components in this PR, supporting Enter and Space keys with proper
preventDefault()to avoid unwanted scrolling.
49-50: Great accessibility enhancements!The keyboard handler and focus styles work together to provide a complete keyboard-accessible experience for the toggle control.
frontend/__tests__/unit/components/CardDetailsPage.test.tsx (1)
164-173: Excellent test mock update for semantic HTML!Updating the mock to render an actual
<button>element whenclickableis true (instead of a<div>with a button role) reflects modern accessibility best practices. This ensures tests validate components using proper semantic HTML.frontend/src/components/Card.tsx (1)
144-152: LGTM!The keyboard event handler is properly wired through to the ActionButton component, enabling keyboard accessibility for the button actions.
frontend/src/components/ToggleableList.tsx (2)
28-33: LGTM!The keyboard event handler correctly implements accessibility best practices by responding to both Enter and Space keys, with proper
preventDefault()to avoid unintended default behavior.
51-51: LGTM!The keyboard handler is properly integrated with the existing disabled state logic, maintaining consistent behavior across mouse and keyboard interactions.
frontend/src/types/button.ts (2)
1-1: LGTM!The React type import is correctly added to support the new keyboard event handler type.
9-9: LGTM!The
onkeydownproperty is properly typed as optional, enabling keyboard event handling while maintaining backward compatibility with existing Button implementations.frontend/src/components/RepositoryCard.tsx (2)
47-52: LGTM!The keyboard event handler is properly implemented, following accessibility best practices by supporting both Enter and Space keys with appropriate
preventDefault()to avoid unintended scrolling behavior.
60-61: LGTM!The keyboard handler is correctly integrated, and the enhanced focus styling (outline with offset) provides clear visual feedback for keyboard navigation users.
frontend/src/components/MultiSearch.tsx (3)
119-122: LGTM: Clean keyboard navigation handling.The Enter key correctly retrieves both the suggestion and its indexName from the highlighted position in the suggestions array.
196-206: Excellent keyboard accessibility enhancement.This handler enables direct button interaction via Tab + Enter/Space, complementing the existing arrow-key navigation pattern. The
stopPropagation()correctly prevents conflicts between the two keyboard interaction modes.
277-279: LGTM: Comprehensive keyboard and focus support.The
onKeyDownhandler enables proper keyboard activation (Enter/Space) on suggestion buttons, and the focus styling provides clear visual feedback for keyboard navigation users.frontend/src/components/ActionButton.tsx (3)
6-20: LGTM: Well-typed keyboard event support.The
onKeyDownprop is correctly typed to handle both anchor and button elements, properly reflecting the dual rendering paths of this component.
34-34: LGTM: Keyboard support for Link.Next.js Link components correctly forward event handlers to the underlying anchor element.
42-49: No action needed. The@heroui/buttonv2.2.29 component fully supports theonKeyDownprop with type signatureonKeyDown: (e: KeyboardEvent) => void. The implementation is correct and the prop will not be silently ignored.frontend/src/app/settings/api-keys/page.tsx (3)
8-8: LGTM: React import is appropriate for React 19.The explicit React import alongside useState is correct for this React version and ensures proper JSX typing.
154-156: LGTM: Content type derivation is correct.The content type and components are properly derived and ready for rendering, assuming the issues in
getContentType()are addressed.
289-289: LGTM: Content rendering pattern is clean and maintainable.The dynamic dispatch to content components based on state is a good abstraction that improves code organization.
CONTRIBUTING.md (3)
226-236: LGTM! Improved environment setup instructions.The updated instructions using
cpcommands from template files (.env.example) are clearer and more reliable than manually creating and populating.envfiles. This reduces the chance of setup errors.
240-240: Good addition: restart reminder for env changes.Adding a note about restarting the application to apply
.envchanges helps prevent common developer confusion.
267-274: Clearer run instructions with concurrency note.The revised instructions clarify that
make runshould be executed from the project root and explain the need to keep it running in parallel with other commands. This prevents common setup confusion.docs/pyproject.toml (1)
17-17: No action required. The pymdown-extensions version constraint ^10.20.0 is valid and current—10.20 is the latest release (Dec 31, 2025).frontend/package.json (1)
76-76: devDependency versions verified as current and secure.All specified versions are current (matching npm's latest releases) and free from known direct security vulnerabilities:
- @swc/core: ^1.15.8 ✓
- @typescript-eslint/eslint-plugin: ^8.51.0 ✓
- @typescript-eslint/parser: ^8.51.0 ✓
- eslint-plugin-jest: ^29.12.0 ✓
- typescript-eslint: ^8.51.0 ✓
backend/pyproject.toml (1)
50-50: strawberry-graphql-django ^0.72.0 is valid with no compatibility concerns.Version 0.72.0 exists and is stable. While this version includes breaking changes to direct StrawberryField/StrawberryArgument construction and the @strawberry.field type_ argument, the codebase uses only the modern, compatible pattern: decorator-based fields with type hints in method signatures. No breaking change patterns are present in the codebase.
frontend/__tests__/unit/components/GeneralCompliantComponent.test.tsx (4)
4-4: LGTM: Import simplified correctly.The removal of FaAward aligns with the component simplification to a single-icon design.
37-45: Test logic is correct.The test correctly validates the background and text color classes for the compliant state. The use of
firstElementChildworks with the current mock structure, though it's somewhat implementation-dependent.
47-55: Test logic is correct.The test correctly validates the background and text color classes for the non-compliant state.
57-61: LGTM: Icon count test updated correctly.The test correctly validates the simplified single-icon design.
frontend/src/components/GeneralCompliantComponent.tsx (1)
19-22: The color contrast ratios are compliant with WCAG AA standards. The green badge achieves 5.20:1 and the red badge achieves 4.24:1. Since the icon is sized at h-7 w-7 (28×28px), both colors meet the WCAG AA requirement of 3:1 for large text and graphics. No changes needed.
| const ellipsisContainers = document.querySelectorAll('div.flex.h-10.w-10') | ||
| const ellipses = Array.from(ellipsisContainers).filter((el) => | ||
| el.querySelector('svg[aria-hidden="true"]') | ||
| ) | ||
| expect(ellipses).toHaveLength(2) |
There was a problem hiding this comment.
Replace CSS class selector with semantic query or add test ID.
The test now queries by CSS classes (div.flex.h-10.w-10) instead of using accessible labels. This approach is brittle because:
- Tight coupling to styling: Any CSS refactor breaks the test
- False positives: Other elements with identical classes will match
- Violates testing best practices: Testing Library recommends querying by accessibility features, not implementation details
The fact that this change was necessary suggests the component's accessibility implementation regressed (the aria-label="More pages" on a non-interactive <div> is not exposed to assistive technologies).
Recommended solutions:
-
Preferred: Add
aria-hidden="true"to the ellipsis container inPagination.tsxand query bydata-testid:<div data-testid="pagination-ellipsis" aria-hidden="true" ...>
Then test with:
const ellipses = screen.getAllByTestId('pagination-ellipsis') expect(ellipses).toHaveLength(2)
-
Alternative: If the ellipsis should remain accessible, give the container a proper role and query by that role.
🤖 Prompt for AI Agents
In frontend/__tests__/unit/components/Pagination.test.tsx around lines 89-93 the
test is selecting ellipsis elements by CSS classes (div.flex.h-10.w-10), which
couples the test to styling; replace this with an accessibility-friendly
selector by updating the Pagination component to mark non-interactive ellipsis
containers with data-testid="pagination-ellipsis" and aria-hidden="true" (or, if
the ellipsis should be accessible, give it an appropriate role/aria-label) then
change the test to query with screen.getAllByTestId('pagination-ellipsis') and
assert the length is 2.
| <Button | ||
| variant="light" | ||
| size="sm" | ||
| onPress={() => onRevoke(key)} | ||
| className="text-red-600 hover:bg-red-50 dark:hover:bg-red-900/20" | ||
| > | ||
| <FaTrash /> | ||
| </Button> | ||
| </td> |
There was a problem hiding this comment.
Add aria-label to icon-only revoke button for screen reader accessibility.
The revoke button contains only an icon without a text label or aria-label attribute. Screen reader users won't be able to identify the button's purpose, making it an accessibility barrier.
🔎 Proposed fix
<Button
variant="light"
size="sm"
onPress={() => onRevoke(key)}
className="text-red-600 hover:bg-red-50 dark:hover:bg-red-900/20"
+ aria-label={`Revoke API key ${key.name}`}
>
<FaTrash />
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button | |
| variant="light" | |
| size="sm" | |
| onPress={() => onRevoke(key)} | |
| className="text-red-600 hover:bg-red-50 dark:hover:bg-red-900/20" | |
| > | |
| <FaTrash /> | |
| </Button> | |
| </td> | |
| <Button | |
| variant="light" | |
| size="sm" | |
| onPress={() => onRevoke(key)} | |
| className="text-red-600 hover:bg-red-50 dark:hover:bg-red-900/20" | |
| aria-label={`Revoke API key ${key.name}`} | |
| > | |
| <FaTrash /> | |
| </Button> | |
| </td> |
🤖 Prompt for AI Agents
In frontend/src/app/settings/api-keys/page.tsx around lines 63 to 71, the revoke
Button is icon-only and lacks an aria-label; add a descriptive aria-label (e.g.,
"Revoke API key" or "Revoke API key {key.name or key.id}") to the Button
component so screen readers can announce its purpose, or alternatively include a
visually-hidden text element inside the button; ensure the label is specific
enough to distinguish which key will be revoked.
| const getContentType = (): ContentType => { | ||
| if (error) { | ||
| return 'error' | ||
| } else if (loading) { | ||
| return 'loading' | ||
| } else if (data?.apiKeys?.length === 0) { | ||
| return 'empty' | ||
| } else { | ||
| return 'table' | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix loading state to avoid hiding data during refetch, and improve empty state check.
Two issues with the content type determination logic:
-
Loading state during refetch (line 145): When
loadingis true during a refetch (with existingdata), the table is replaced withLoadingSpinner. This creates a poor UX as users lose sight of their existing API keys. Typically, you should keep displaying existing data during refetch. -
Empty state check (line 147-148): The condition
data?.apiKeys?.length === 0only handles empty arrays. IfapiKeysisundefined(possible witherrorPolicy: 'all'), the check fails and an empty table renders instead ofEmptyState.
🔎 Proposed fix
const getContentType = (): ContentType => {
if (error) {
return 'error'
- } else if (loading) {
- return 'loading'
- } else if (data?.apiKeys?.length === 0) {
+ } else if (!data?.apiKeys || data.apiKeys.length === 0) {
return 'empty'
} else {
return 'table'
}
}This fix:
- Removes the loading check, allowing existing data to remain visible during refetch (the early return at line 236-238 already handles initial loading)
- Checks for both
undefinedand empty array for the empty state
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getContentType = (): ContentType => { | |
| if (error) { | |
| return 'error' | |
| } else if (loading) { | |
| return 'loading' | |
| } else if (data?.apiKeys?.length === 0) { | |
| return 'empty' | |
| } else { | |
| return 'table' | |
| } | |
| } | |
| const getContentType = (): ContentType => { | |
| if (error) { | |
| return 'error' | |
| } else if (!data?.apiKeys || data.apiKeys.length === 0) { | |
| return 'empty' | |
| } else { | |
| return 'table' | |
| } | |
| } |
🤖 Prompt for AI Agents
In frontend/src/app/settings/api-keys/page.tsx around lines 142 to 152, the
getContentType logic incorrectly hides existing data during a refetch and only
treats an empty array as empty; remove the loading branch so existing data
remains visible during refetch (initial loading is already handled earlier), and
change the empty-state check to treat apiKeys === undefined or apiKeys.length
=== 0 as empty so undefined (from errorPolicy: 'all') shows the EmptyState
instead of an empty table.
| <div | ||
| className={clsx( | ||
| 'group relative flex h-14 w-14 cursor-default items-center justify-center rounded-full shadow-md transition-all duration-300 ease-in-out hover:scale-105 hover:shadow-lg', | ||
| { | ||
| 'bg-green-400/80 text-green-900/90': compliant, | ||
| 'bg-red-400/80 text-red-900/90': !compliant, | ||
| } | ||
| )} | ||
| > | ||
| <IconWrapper icon={icon} className="h-7 w-7" /> | ||
| </div> |
There was a problem hiding this comment.
Add semantic meaning for accessibility.
The badge div relies entirely on the Tooltip for conveying meaning. Users who don't interact with tooltips (screen reader users, keyboard-only users not hovering) won't understand the badge's purpose or status.
Consider adding:
- A
role="status"orrole="img"attribute - An
aria-labelthat describes the compliance state (e.g.,aria-label={compliant ? 'Compliant' : 'Not compliant'})
🔎 Proposed fix for accessibility
<div
className={clsx(
'group relative flex h-14 w-14 cursor-default items-center justify-center rounded-full shadow-md transition-all duration-300 ease-in-out hover:scale-105 hover:shadow-lg',
{
'bg-green-400/80 text-green-900/90': compliant,
'bg-red-400/80 text-red-900/90': !compliant,
}
)}
+ role="status"
+ aria-label={compliant ? 'Compliant' : 'Not compliant'}
>🤖 Prompt for AI Agents
In frontend/src/components/GeneralCompliantComponent.tsx around lines 16 to 26,
the badge div only conveys meaning via the Tooltip which is inaccessible to some
users; add semantic accessibility attributes by setting an appropriate role
(e.g., role="status" or role="img") and include an aria-label that reflects the
compliance state (for example aria-label={compliant ? 'Compliant' : 'Not
compliant'}), ensuring the label updates with the compliant prop so assistive
technologies can read the status.
| <div | ||
| className="flex h-10 w-10 items-center justify-center text-gray-600 dark:text-gray-400" | ||
| role="presentation" | ||
| aria-label="More pages" | ||
| > | ||
| <FaEllipsis className="h-5 w-5" /> | ||
| </span> | ||
| <FaEllipsis className="h-5 w-5" aria-hidden="true" /> | ||
| </div> |
There was a problem hiding this comment.
Fix accessibility regression: aria-label on plain <div> is not exposed.
The aria-label="More pages" on line 76 is not exposed to assistive technologies because it's on a plain <div> without a semantic role. This breaks accessibility and forces tests to query by CSS classes instead of accessible labels.
The ellipsis is decorative and should be hidden from screen readers:
🔎 Proposed fix
- <div
+ <div
+ aria-hidden="true"
className="flex h-10 w-10 items-center justify-center text-gray-600 dark:text-gray-400"
- aria-label="More pages"
>
- <FaEllipsis className="h-5 w-5" aria-hidden="true" />
+ <FaEllipsis className="h-5 w-5" />
</div>Why this fix:
- Moves
aria-hidden="true"to the container to properly hide the entire element from assistive technologies - Removes redundant
aria-labelthat wasn't being announced - Removes redundant
aria-hiddenfrom the icon (inherited from parent) - Allows tests to use a semantic approach (e.g.,
data-testid) instead of CSS class selectors
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| className="flex h-10 w-10 items-center justify-center text-gray-600 dark:text-gray-400" | |
| role="presentation" | |
| aria-label="More pages" | |
| > | |
| <FaEllipsis className="h-5 w-5" /> | |
| </span> | |
| <FaEllipsis className="h-5 w-5" aria-hidden="true" /> | |
| </div> | |
| <div | |
| aria-hidden="true" | |
| className="flex h-10 w-10 items-center justify-center text-gray-600 dark:text-gray-400" | |
| > | |
| <FaEllipsis className="h-5 w-5" /> | |
| </div> |
🤖 Prompt for AI Agents
In frontend/src/components/Pagination.tsx around lines 74 to 79, the container
has aria-label on a plain <div> which is not exposed to assistive tech; remove
the unusable aria-label, add aria-hidden="true" on the container to hide the
decorative ellipsis from screen readers, remove any redundant aria-hidden from
the inner FaEllipsis icon (it will inherit), and add a data-testid (e.g.,
data-testid="pagination-ellipsis") to the container so tests can target it
semantically instead of relying on CSS classes.
|
@arkid15r The pr linked issue action is the only one failing here. |
12 participants
Proposed change
Resolves #(put the issue number here)
Add the PR description here.
Checklist
make check-testlocally and all tests passed