Skip to content

refactor: simplify keyset init and extract duplicated filter helper#720

Open
kamilwronka wants to merge 1 commit intodevelopfrom
claude/small-refactor-april-09
Open

refactor: simplify keyset init and extract duplicated filter helper#720
kamilwronka wants to merge 1 commit intodevelopfrom
claude/small-refactor-april-09

Conversation

@kamilwronka
Copy link
Copy Markdown
Contributor

@kamilwronka kamilwronka commented Apr 9, 2026

Summary

  • verify-jwt.ts: Replace mutable let + two if blocks with a single const ternary chain for keyset initialization. This makes the intent clearer and prevents the second if from silently overwriting the first assignment.
  • activities-query.service.ts: Extract the duplicated StringNullableFilter construction (used by both suggestWorlds and suggestClanNames) into a private buildNonEmptyNullableFilter() helper method.

Both changes are behavior-preserving and localized to the same module.

Residual risks

  • None. Both refactors preserve existing logic exactly.
  • Pre-existing test failure in activities.controller.spec.ts (cannot resolve @lootlog/nest-shared) is unrelated — same failure occurs on develop.

Test plan

  • Lint passes on both changed files
  • api-helpers package builds successfully
  • Activity app tests pass (3/3 tests, the 1 failing suite is a pre-existing issue on develop)
  • Format check passes on changed files

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor
    • Code quality improvements and efficiency enhancements for internal services to reduce redundancy and improve maintainability.

- verify-jwt.ts: replace mutable `let` + two `if` blocks with a const
  ternary chain, making the keyset assignment clearer and preventing
  accidental overwrites
- activities-query.service.ts: extract duplicated
  StringNullableFilter construction into buildNonEmptyNullableFilter()
  private helper, used by suggestWorlds and suggestClanNames

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

Two files underwent refactoring: one extracts duplicated Prisma filter-building logic into a private helper method for "suggestWorlds" and "suggestClanNames", and the other simplifies JWT keyset initialization by replacing mutable variable assignments with a const expression.

Changes

Cohort / File(s) Summary
Filter Construction Extraction
apps/activity/src/activities/services/activities-query.service.ts
Introduced private helper buildNonEmptyNullableFilter() to consolidate duplicated inline StringNullableFilter construction logic for null exclusion and conditional contains/insensitive mode assignment.
Keyset Initialization Simplification
packages/api-helpers/src/lib/auth/utils/verify-jwt.ts
Replaced mutable let keyset with a const keyset expression using conditional selection between local and remote JWK sets, reducing lines and eliminating intermediate state mutations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A helper born from duplication's pain,
Filters unified, no more refrain,
And keyset flows where const reigns supreme,
Cleaner code—a rabbit's dream! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes both main changes in the pull request: simplifying keyset initialization and extracting a duplicated filter helper.
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
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/small-refactor-april-09

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.

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
packages/api-helpers/src/lib/auth/utils/verify-jwt.ts (1)

11-15: Replace chained ternary with if / else if for keyset initialization.

Line 11 to Line 15 currently use a nested ternary, which hurts readability and conflicts with project style.

Proposed refactor (behavior-preserving)
-  const keyset = jwks
-    ? createLocalJWKSet(jwks)
-    : jwksUri
-      ? createRemoteJWKSet(new URL(jwksUri))
-      : undefined;
+  let keyset;
+  if (jwks) {
+    keyset = createLocalJWKSet(jwks);
+  } else if (jwksUri) {
+    keyset = createRemoteJWKSet(new URL(jwksUri));
+  }

As per coding guidelines, "Avoid nested (chained) ternary expressions — use early returns or if/else if instead".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/api-helpers/src/lib/auth/utils/verify-jwt.ts` around lines 11 - 15,
Replace the nested ternary that initializes keyset with an explicit if/else if
block: declare keyset (let keyset = undefined), then if (jwks) set keyset =
createLocalJWKSet(jwks), else if (jwksUri) set keyset = createRemoteJWKSet(new
URL(jwksUri)); keep the same behavior when neither is present (keyset stays
undefined) and update references in verify-jwt.ts accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/api-helpers/src/lib/auth/utils/verify-jwt.ts`:
- Around line 11-15: Replace the nested ternary that initializes keyset with an
explicit if/else if block: declare keyset (let keyset = undefined), then if
(jwks) set keyset = createLocalJWKSet(jwks), else if (jwksUri) set keyset =
createRemoteJWKSet(new URL(jwksUri)); keep the same behavior when neither is
present (keyset stays undefined) and update references in verify-jwt.ts
accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 834a8fac-a61b-4307-a84b-c417926af9cb

📥 Commits

Reviewing files that changed from the base of the PR and between b32c122 and 30493a1.

📒 Files selected for processing (2)
  • apps/activity/src/activities/services/activities-query.service.ts
  • packages/api-helpers/src/lib/auth/utils/verify-jwt.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant