Skip to content

Comments

fix: remove redundant type assertion (S4325)#3628

Merged
kasya merged 3 commits intoOWASP:mainfrom
anukalp2804:fix/sonar-s4325-remove-redundant-assertion
Feb 3, 2026
Merged

fix: remove redundant type assertion (S4325)#3628
kasya merged 3 commits intoOWASP:mainfrom
anukalp2804:fix/sonar-s4325-remove-redundant-assertion

Conversation

@anukalp2804
Copy link
Contributor

Proposed change

Resolves #3554

This PR fixes a Sonar-reported code smell (typescript:S4325).

The type assertion (session as ExtendedSession) was redundant and did not change the effective type of the expression when optional chaining was already used. The assertion has been removed to improve readability and align with TypeScript and Sonar best practices.

No functional changes.

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

Summary by CodeRabbit

  • Refactor
    • Centralized retrieval of the signed-in user and simplified admin verification logic, removing an internal type dependency and improving code clarity and maintainability; no user-facing changes.

Walkthrough

Replaced runtime type assertion of session with direct optional-chained access session?.user?.login in the mentorship program edit page; removed dependency on ExtendedSession. No other logic or control-flow changes.

Changes

Cohort / File(s) Summary
Type assertion removal
frontend/src/app/my/mentorship/programs/[programKey]/edit/page.tsx
Replaced (session as ExtendedSession)?.user?.login with session?.user?.login in admin comparisons; removed ExtendedSession dependency/import.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: remove redundant type assertion (S4325)' is specific and clearly related to the main change: removing a redundant type assertion to fix a Sonar code smell.
Description check ✅ Passed The description is fully related to the changeset, detailing the removal of the redundant type assertion and its motivation, linking to issue #3554.
Linked Issues check ✅ Passed The PR directly addresses issue #3554 by removing the redundant type assertion (session as ExtendedSession) from the comparison, exactly as required.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the Sonar issue S4325; the refactoring only affects the admin login comparison logic without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@anukalp2804
Copy link
Contributor Author

Hi @arkid15r @kasya , this PR addresses the assigned Sonar issue (typescript:S4325).
Please let me know if any changes are needed. Thanks!

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@frontend/src/app/my/mentorship/programs/`[programKey]/edit/page.tsx:
- Around line 56-59: The access to session?.user?.login in the isAdmin check is
unsafe because the base next-auth Session type doesn't include login; either
reintroduce the cast (session as ExtendedSession)?.user?.login in the isAdmin
expression or add a module augmentation to extend next-auth's Session type to
include user.login so TypeScript recognizes it; update the isAdmin line
(referencing isAdmin, session, ExtendedSession, and data.getProgram.admins)
accordingly to restore type safety.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 29, 2026
@anukalp2804
Copy link
Contributor Author

Thanks for the review! I’ve updated the code to safely narrow the session.user type before accessing login.
Please let me know if anything else needs adjustment.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 3, 2026

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anukalp2804 thanks for working on this!
I updated your PR - you did not run make check-test locally and it is required. Please make sure to run it next time.

Image

@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.67%. Comparing base (71f63bc) to head (64e8977).
⚠️ Report is 36 commits behind head on main.

Files with missing lines Patch % Lines
.../my/mentorship/programs/[programKey]/edit/page.tsx 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3628      +/-   ##
==========================================
+ Coverage   85.55%   87.67%   +2.12%     
==========================================
  Files         463      462       -1     
  Lines       14303    14324      +21     
  Branches     1904     1915      +11     
==========================================
+ Hits        12237    12559     +322     
+ Misses       1687     1346     -341     
- Partials      379      419      +40     
Flag Coverage Δ
backend 87.45% <ø> (+2.91%) ⬆️
frontend 88.29% <50.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../my/mentorship/programs/[programKey]/edit/page.tsx 66.66% <50.00%> (-1.08%) ⬇️

... and 23 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71f63bc...64e8977. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kasya kasya enabled auto-merge February 3, 2026 04:06
@kasya kasya added this pull request to the merge queue Feb 3, 2026
Merged via the queue into OWASP:main with commit d51396a Feb 3, 2026
34 of 36 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Feb 7, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Sonar issue: Remove redundant type assertion (typescript:S4325)

2 participants