refactor: remove redundant type assertions in frontend#3258
refactor: remove redundant type assertions in frontend#3258arkid15r merged 7 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRemoved unnecessary TypeScript casts and non‑null assertions across multiple frontend files, replacing explicit assertions with type inference or optional chaining; made two GitHub env vars possibly undefined. No control flow or exported/public signatures changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-07-13T11:34:31.823ZApplied to files:
📚 Learning: 2025-06-20T16:12:59.256ZApplied to files:
🔇 Additional comments (2)
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}
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
Hi @hussainjamal760 !
Thanks for updating these.
Could you also resolve Sonar issue that was introduced in this PR?
|
Hm... it now says there are no issues 😅 @hussainjamal760 there are still checks failing, so please make sure everything is green and ready for review. |
…les to pass make check
|
@kasya thanks for the update. I’ve resolved the issue and have now pushed a new commit with all checks passing. I ran make check-test locally and ensured that all tests and linters pass, 
Please let me know if anything else needs to be addressed. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/CardDetailsPage.tsx (2)
102-104: NewExtendedSessioncast slightly conflicts with PR goal; consider typinguseSessioninstead.
const session = data as ExtendedSession | nullreintroduces a type assertion. If the repo supports it, prefer typing via NextAuth session module augmentation (oruseSession<...>()if available) so the compiler “knows”user.loginwithoutas. Based on learnings,user.loginaccess is the intended pattern forExtendedSession.Possible direction (depends on repo + next-auth typing support)
- const { data } = useSession() - const session = data as ExtendedSession | null + const { data: session } = useSession() // ideally already typed as ExtendedSession | null via module augmentation
128-132: Cleaner module-admin check; consider extracting to a boolean for readability.The
admins?.some((admin) => admin.login === session?.user?.login)is a nice improvement over forcingas string—undefinedsimply won’t match. A small readability win would be extractingconst isModuleAdmin = ...before JSX to reduce nesting.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/CardDetailsPage.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
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.
📚 Learning: 2025-07-13T11:34:31.823Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/mentorship/programs/page.tsx:14-14
Timestamp: 2025-07-13T11:34:31.823Z
Learning: In the Next.js frontend mentorship application, there are two distinct types for authentication-related data: ExtendedSession for useSession hook (containing accessToken and user.login properties) and UserRolesData for useUserRoles hook (containing currentUserRoles.roles array). The correct access pattern for GitHub username is `(session as ExtendedSession)?.user?.login`.
Applied to files:
frontend/src/components/CardDetailsPage.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 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/src/components/CardDetailsPage.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/src/components/CardDetailsPage.tsx
|
@kasya . I’ve resolved the sonar issue that was introduced in this PR : https://sonarcloud.io/project/issues?pullRequest=3258&open=AZugeER74ckZxBvSsqH4&id=OWASP_Nest and have now pushed a new commit |
|
* refactor: remove redundant type assertions in frontend * resolved line-ending/formatting issues across backend and frontend files to pass make check * refactor: remove redundant type assertion for ExtendedSession * type assertion in DetailsCard to satisfy linting and PR feedback. --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
3 participants
Issue fixed : #3225
Summary
This PR cleans up the frontend codebase by removing redundant type assertions (as) and non-null assertions (!). These assertions were unnecessary because the TypeScript compiler already had sufficient contextual information to infer the correct types.
Changes
Removed unnecessary assertions in the following files:
frontend/src/app/members/[memberKey]/page.tsx
frontend/src/components/CardDetailsPage.tsx
frontend/src/components/ChapterMap.tsx
frontend/src/components/MultiSearch.tsx
frontend/src/components/SingleModuleCard.tsx
frontend/src/components/UserMenu.tsx
frontend/src/hooks/useSearchPage.ts
frontend/src/utils/env.server.ts
Technical Details
Used @typescript-eslint/no-unnecessary-type-assertion and @typescript-eslint/no-extra-non-null-assertion rules to identify redundant code.
Ensured that no logical changes were introduced, only type-level cleanup.
Fixed an issue in ApiKeysPage.test.tsx where a required cast was accidentally flagged as redundant.
Verification Results
Backend Tests: Passed (Coverage: 83.83%)
Frontend Tests: Passed (make test-frontend-unit)
Linting: All checks passed locally via make lint-frontend and pre-commit.