Skip to content

wip#2362

Merged
simo6529 merged 1 commit intomainfrom
reset-unread-080526-1
May 8, 2026
Merged

wip#2362
simo6529 merged 1 commit intomainfrom
reset-unread-080526-1

Conversation

@simo6529
Copy link
Copy Markdown
Collaborator

@simo6529 simo6529 commented May 8, 2026

Summary by CodeRabbit

  • Tests

    • Added tests covering “no active proxy” scenarios: verifies read-marking uses the connected profile’s auth when roles match, and that it skips/send-none when roles disagree (no API call when queued-off).
  • Refactor

    • Authentication selection for marking wave notifications as read now accounts for connected profiles and normalizes role handling to determine whether to use connected-profile auth or a proxy-derived identity.

Signed-off-by: Simo <simo@6529.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: feafaace-2832-4466-a9de-10a608f0e33c

📥 Commits

Reviewing files that changed from the base of the PR and between 5dcdc91 and 99224ce.

📒 Files selected for processing (4)
  • __tests__/hooks/useMarkWaveNotificationsRead.test.tsx
  • hooks/useMarkWaveNotificationsRead.helpers.ts
  • hooks/useMarkWaveNotificationsRead.identity.ts
  • hooks/useMarkWaveNotificationsRead.ts

📝 Walkthrough

Walkthrough

This PR extends the wave notification marking system to use connectedProfileId from authenticated user state, integrating it through JWT identity normalization logic and marker state hooks to enable per-profile authentication when no active proxy exists.

Changes

Connected Profile Identity Support for Wave Notifications

Layer / File(s) Summary
Data Contracts
hooks/useMarkWaveNotificationsRead.identity.ts
WaveReadIdentityConfig interface adds connectedProfileId: string | null property.
JWT Role Normalization
hooks/useMarkWaveNotificationsRead.identity.ts
New normalizeWaveReadJwtRole function computes normalized JWT role using profile and proxy IDs; decodeWaveReadJwtIdentity refactored to use this normalization instead of direct role-truthy check, with expanded memoization dependencies on activeProfileProxyId and connectedProfileId.
Identity State Hook
hooks/useMarkWaveNotificationsRead.identity.ts
useWaveReadIdentityState destructures and forwards connectedProfileId to JWT decoding with updated memo dependencies.
Marker State Configuration
hooks/useMarkWaveNotificationsRead.helpers.ts
useWaveNotificationsReadMarkerState helper forwards connectedProfileId through to useWaveReadIdentityState.
Top-Level Auth Integration
hooks/useMarkWaveNotificationsRead.ts
useWaveNotificationsReadMarkerState hook extracts connectedProfileId from useAuth().connectedProfile and passes it to marker state config alongside active proxy IDs.
Tests / Fixtures
__tests__/hooks/useMarkWaveNotificationsRead.test.tsx
Test helper updated to accept optional connectedProfileId; mock useAuth returns connectedProfile object; two new tests validate identity selection: (1) connected-profile JWT role with null proxyRoleIdentityKey marks wave read via JWT endpoint; (2) different JWT role derives non-null proxyRoleIdentityKey, resulting in skipped request when queueIfBlocked: false.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • ragnep
  • prxt6529

Poem

🐰 I hop through auth with a curious twitch,
connectedProfileId snug in my kit,
I normalize roles and memoize tight,
No proxy? I still mark the wave right.
✨ Tests snug and small — the rabbit's delight.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'wip' is vague and generic, using a non-descriptive placeholder term that does not convey meaningful information about the actual changeset. Replace with a descriptive title that summarizes the main change, such as 'Add connectedProfileId support to wave notifications read marker state' or similar.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 reset-unread-080526-1

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

@simo6529
Copy link
Copy Markdown
Collaborator Author

simo6529 commented May 8, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 8, 2026

✅ Actions performed

Full review triggered.

@simo6529 simo6529 merged commit 8c62a5f into main May 8, 2026
3 checks passed
@simo6529 simo6529 deleted the reset-unread-080526-1 branch May 8, 2026 14:53
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.

2 participants