feat: move grants to admin dashboard#100
Merged
lorenzocorallo merged 7 commits intomainfrom Apr 12, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRemoves the Telegram "grants" command and unregisters it from management; changes grant log payload to use an absolute Changes
Sequence Diagram(s)sequenceDiagram
participant WS as WebSocket Client
participant Storage as MessageUserStorage
participant Logger as TgLogger Module
participant CB as Socket Callback
WS->>WS: receive "logGrantCreate"(targetId, adminId, since, until, reason)
WS->>Storage: getStoredUser(targetId)
alt target missing
Storage-->>WS: null
WS->>CB: callback("target not found")
else target found
Storage-->>WS: targetUser
WS->>Storage: getStoredUser(adminId)
alt admin missing
Storage-->>WS: null
WS->>CB: callback("admin not found")
else admin found
Storage-->>WS: adminUser
WS->>Logger: grants({action:"CREATE", target:targetUser, by:adminUser, since, until, reason})
Logger-->>WS: result (ok / falsy)
alt logger falsy
WS->>CB: callback("logger failed")
else logger ok
WS->>CB: callback(null)
end
end
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
lorenzocorallo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/websocket.ts`:
- Line 127: The log message in logGrantInterrupt is incorrectly labeled "grant
create log OK"; update the logger.debug call inside the logGrantInterrupt
function to use a correct success label (e.g., "grant interrupt log OK" or
similar) so successful interrupt handling is properly classified in logs; locate
the logger.debug invocation in logGrantInterrupt and change only the message
string to the accurate label.
- Around line 90-109: The websocket handler for "logGrantCreate" can throw
(e.g., in modules.get("tgLogger").grants or other awaits) and currently may
never call the ack cb; wrap the handler body (or at least the await
modules.get("tgLogger").grants call) in a try/catch and ensure cb(...) is always
invoked — on success call cb(null) and on any caught error call
cb(errorOrMessage) and log the error; apply the same change to the
"logGrantRevoke" handler so both handlers always acknowledge the websocket
request even when awaited calls throw (refer to this.io.on("logGrantCreate") /
this.io.on("logGrantRevoke"), MessageUserStorage.getInstance(), and
modules.get("tgLogger").grants).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1346289f-3074-46d7-93a8-846a7c12f5fd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.jsonsrc/modules/tg-logger/index.tssrc/modules/tg-logger/types.tssrc/modules/websocket.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/modules/websocket.ts (2)
91-93: Fetch users concurrently and reuse the storage instance.Both handlers do two sequential lookups and call
MessageUserStorage.getInstance()twice. Use one instance +Promise.allto reduce latency and duplication.Refactor sketch
- const target = await MessageUserStorage.getInstance().getStoredUser(userId) - const admin = await MessageUserStorage.getInstance().getStoredUser(adminId) + const storage = MessageUserStorage.getInstance() + const [target, admin] = await Promise.all([ + storage.getStoredUser(userId), + storage.getStoredUser(adminId), + ])Also applies to: 122-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/websocket.ts` around lines 91 - 93, Replace the two sequential getStoredUser calls and duplicate getInstance() calls by calling MessageUserStorage.getInstance() once and fetching both users concurrently with Promise.all; specifically, obtain a single storage instance and then do Promise.all([storage.getStoredUser(userId), storage.getStoredUser(adminId)]) and check the results for falsy before proceeding. Apply the same change to the other handler that currently repeats MessageUserStorage.getInstance() and sequential lookups (the block around the second pair of getStoredUser calls).
99-109: Don’t swallow grant logger errors silently.The
.catch(() => null)path hides the original failure reason, which hurts debugging. Log the caught error before returningnull.Refactor sketch
- .catch(() => null) + .catch((error) => { + logger.error({ error }, "[WS] grant create log ERROR -- tgLogger.grants failed") + return null + }) ... - .catch(() => null) + .catch((error) => { + logger.error({ error }, "[WS] grant interrupt log ERROR -- tgLogger.grants failed") + return null + })Also applies to: 130-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/websocket.ts` around lines 99 - 109, The current call to modules.get("tgLogger").grants(...).catch(() => null) swallows errors; change the catch to capture the error (e.g. .catch((err) => { /* log err */; return null; })) and log the error before returning null — use the existing app logger if available (e.g. processLogger.error or modules.get("processLogger").error), or console.error as a fallback. Apply the same change to the second grants call in the file (the other catch handling the grant response) so neither failure path hides the original error details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/websocket.ts`:
- Line 113: Replace the misspelled error string "Cannot send te log" with
"Cannot send the log" wherever it's passed to callback invocations in
src/modules/websocket.ts (the cb(...) calls that return the error message);
ensure both occurrences (the cb("Cannot send te log") usages) are updated to the
corrected string so callbacks return the proper message.
---
Nitpick comments:
In `@src/modules/websocket.ts`:
- Around line 91-93: Replace the two sequential getStoredUser calls and
duplicate getInstance() calls by calling MessageUserStorage.getInstance() once
and fetching both users concurrently with Promise.all; specifically, obtain a
single storage instance and then do Promise.all([storage.getStoredUser(userId),
storage.getStoredUser(adminId)]) and check the results for falsy before
proceeding. Apply the same change to the other handler that currently repeats
MessageUserStorage.getInstance() and sequential lookups (the block around the
second pair of getStoredUser calls).
- Around line 99-109: The current call to
modules.get("tgLogger").grants(...).catch(() => null) swallows errors; change
the catch to capture the error (e.g. .catch((err) => { /* log err */; return
null; })) and log the error before returning null — use the existing app logger
if available (e.g. processLogger.error or modules.get("processLogger").error),
or console.error as a fallback. Apply the same change to the second grants call
in the file (the other catch handling the grant response) so neither failure
path hides the original error details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0dbf128c-730e-4b6f-a484-9ecbbcc5c691
📒 Files selected for processing (1)
src/modules/websocket.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/modules/websocket.ts (2)
112-112:⚠️ Potential issue | 🟡 MinorFix typo in callback error messages.
"Cannot send te log"should be"Cannot send the log".Suggested fix
- cb("Cannot send te log") + cb("Cannot send the log") ... - cb("Cannot send te log") + cb("Cannot send the log")Also applies to: 139-139
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/websocket.ts` at line 112, Fix the typographical error in the callback error message: replace the string "Cannot send te log" with "Cannot send the log" wherever it's invoked (e.g., the cb(...) calls shown in websocket.ts, including the other occurrence around the later cb call at line ~139). Update both instances so the callback error message is spelled correctly and maintains the exact same phrasing in both locations.
90-118:⚠️ Potential issue | 🟠 MajorAlways guarantee websocket ack with a top-level
try/catchin both grant handlers.Right now only
grants(...).catch(() => null)is guarded; unexpected throws in other awaited steps can still skipcb(...), leaving the ack hanging.Suggested fix
this.io.on("logGrantCreate", async ({ userId, adminId, validSince, validUntil, reason }, cb) => { - const target = await MessageUserStorage.getInstance().getStoredUser(userId) - const admin = await MessageUserStorage.getInstance().getStoredUser(adminId) - if (!target || !admin) { - logger.error("[WS] grant create log ERROR -- cannot retrieve users") - cb("Cannot retrieve the users") - return - } - - const res = await this.getModule("tgLogger") - .grants({ - action: "CREATE", - target: target, - by: admin, - since: validSince, - until: validUntil, - reason, - }) - .catch(() => null) - - if (!res) { - logger.error("[WS] grant create log ERROR -- cannot send log") - cb("Cannot send te log") - return - } - - logger.debug("[WS] grant create log OK") - cb(null) + try { + const storage = MessageUserStorage.getInstance() + const [target, admin] = await Promise.all([storage.getStoredUser(userId), storage.getStoredUser(adminId)]) + if (!target || !admin) { + logger.error("[WS] grant create log ERROR -- cannot retrieve users") + cb("Cannot retrieve the users") + return + } + + await this.getModule("tgLogger").grants({ + action: "CREATE", + target, + by: admin, + since: validSince, + until: validUntil, + reason, + }) + + logger.debug("[WS] grant create log OK") + cb(null) + } catch (error) { + logger.error({ error }, "[WS] grant create log ERROR") + cb("Cannot send the log") + } })Apply the same structure to
logGrantInterrupt.Also applies to: 120-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/websocket.ts` around lines 90 - 118, Wrap the websocket handler logic for this.io.on("logGrantCreate", async (...) => { ... }) in a top-level try/catch so any unexpected throw still sends an ack; move the await MessageUserStorage.getInstance().getStoredUser(...) calls, the await this.getModule("tgLogger").grants(...), and the logging into the try block and call cb(errorMessage) from the catch (or cb(null) on success) to guarantee the ack is always called. Do the same change for the this.io.on("logGrantInterrupt", ...) handler so both handlers always call cb(...) regardless of where an exception occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/modules/websocket.ts`:
- Line 112: Fix the typographical error in the callback error message: replace
the string "Cannot send te log" with "Cannot send the log" wherever it's invoked
(e.g., the cb(...) calls shown in websocket.ts, including the other occurrence
around the later cb call at line ~139). Update both instances so the callback
error message is spelled correctly and maintains the exact same phrasing in both
locations.
- Around line 90-118: Wrap the websocket handler logic for
this.io.on("logGrantCreate", async (...) => { ... }) in a top-level try/catch so
any unexpected throw still sends an ack; move the await
MessageUserStorage.getInstance().getStoredUser(...) calls, the await
this.getModule("tgLogger").grants(...), and the logging into the try block and
call cb(errorMessage) from the catch (or cb(null) on success) to guarantee the
ack is always called. Do the same change for the this.io.on("logGrantInterrupt",
...) handler so both handlers always call cb(...) regardless of where an
exception occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06f9271a-8e42-4937-86d6-38ade47ccfac
📒 Files selected for processing (1)
src/modules/websocket.ts
toto04
approved these changes
Apr 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Grants can now be created through the admin dashboard with a easier UX.