Announcement channel type implementation#182
Merged
Merged
Conversation
FirepitProject ID: Tip Preview deployments create instant URLs for every branch and commit |
Contributor
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe PR introduces channel types (text, voice, announcement) across the application stack. Channels now store a ChangesChannel Type Feature
Sequence DiagramsequenceDiagram
participant User
participant ChatUI as Chat Page
participant PermAPI as Permissions API
participant CompUI as Composer UI
participant Toast
User->>ChatUI: Select announcement channel
ChatUI->>PermAPI: GET /api/servers/:id/permissions<br/>(channelId=announce-123)
PermAPI-->>ChatUI: { canSend: false, canRead: true, ... }
ChatUI->>ChatUI: Set canSendMessages = false<br/>selectedChannelData.type = "announcement"
ChatUI->>CompUI: composer = {<br/> disabled: true,<br/> readOnly: true,<br/> readOnlyMessage: "Only moderators..."<br/>}
CompUI-->>User: Render read-only status box<br/>Disable emoji picker
User->>ChatUI: Attempt to type (blocked)
ChatUI-->>User: Composer input disabled,<br/>Announcement icon visible
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Owner
Author
This was referenced Apr 19, 2026
Merged
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/__tests__/api-routes/server-permissions.test.ts (1)
151-173:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing assertions for the new
canRead/canSendfields.
mockGetChannelAccessForUseris set up to returncanRead: true, canSend: false, but theexpectat line 167 doesn't verify those values make it into the response. This is the core behavior added by this PR.✅ Suggested assertion additions
await expect(response.json()).resolves.toMatchObject({ readMessages: true, manageMessages: true, sendMessages: false, + canRead: true, + canSend: false, });The same gap exists in the "base server permissions" test (lines 96–101) — consider adding
canRead: true, canSend: truethere as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/api-routes/server-permissions.test.ts` around lines 151 - 173, Add assertions to verify the new canRead/canSend fields are propagated into the API response: update the test using mockGetChannelAccessForUser (the case returning canRead: true, canSend: false) to assert the GET handler's response JSON includes canRead: true and canSend: false, and likewise update the "base server permissions" test (where mockGetChannelAccessForUser should return canRead: true, canSend: true) to assert those fields are present and true; locate the tests referencing mockGetChannelAccessForUser and the GET(request, { params: ... }) call to add these expectations.src/app/api/channels/route.ts (2)
423-424:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCompute
nextCursorfrom the raw page, not the permission-filtered result.If some channels in the fetched page are filtered out by
readMessages,channels.lengthcan drop belowlimiteven when more documents exist after this page. Clients will stop paginating early and miss accessible channels farther down the dataset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/channels/route.ts` around lines 423 - 424, The pagination cursor is being computed from the permission-filtered array (channels) which can be shorter than the fetched page after readMessages filtering; change the code to compute nextCursor from the raw fetched page (e.g., rawChannels or fetchedPage) instead: keep using readMessages-filtered channels for the response, but derive nextCursor using the raw page's length and its last item (e.g., lastRaw.$id) compared against limit so clients continue paginating when more documents exist beyond the current fetched page.
426-436:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDon't mark this authenticated channel list as
publiccacheable.This response varies by session and per-channel permissions, so a shared cache can serve one member's filtered channel list to another user. This needs a private/no-store policy, or a cache key that varies per authenticated user.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/channels/route.ts` around lines 426 - 436, The response for the authenticated channel list is currently marked "public" which allows shared caches to leak one user's filtered channels to others; update the Cache-Control header in the compressedResponse call (the block that returns { channels, nextCursor } and sets headers) to a private or no-store policy for authenticated responses (e.g., replace "public, s-maxage=60, stale-while-revalidate=300" with "private, s-maxage=60, stale-while-revalidate=300" or "no-store") so per-session/per-permission data is not served from a shared cache. Ensure you change the header in the same compressedResponse call that sets headers for the channels route.
🤖 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/__tests__/category-settings-panel.test.tsx`:
- Around line 196-279: Update the "creates channels from channel setup" test in
category-settings-panel.test.tsx to choose the "announcement" channel type in
the create flow (e.g., change the form control that sets type to "announcement")
and then assert that the POST request body includes type: "announcement" by
inspecting the fetch mock call (parse JSON from expect.objectContaining... or by
JSON.parse((global.fetch as vi.Mock).mock.calls[n][1].body)); additionally, when
the update PATCH request is made later in this flow, assert its parsed request
body.type is also "announcement". Target symbols: the test "creates channels
from channel setup", the CategorySettingsPanel render, the create button click,
and the global.fetch mock calls to validate the request bodies for POST and
PATCH.
In `@src/app/admin/server-management.tsx`:
- Around line 63-77: The component currently only calls loadServers when isAdmin
is true, preventing moderators from ever seeing servers or selecting one; update
the server-loading logic and backend action so moderators are supported
end-to-end: change the useEffect condition that calls loadServers() to check
(isAdmin || isModerator) and ensure the backend/API action listServersAction
(the admin-only logic) is extended or a new listServersForModerator variant is
implemented and wired so that when (isModerator && !isAdmin) the UI can call the
moderator-capable listing and populate selectedServerId, enabling
loadChannels(selectedServerId) to work for moderators as well.
- Around line 277-283: The Button elements that trigger actions (e.g., the
"Create Server" button using onClick={handleCreateServer} and controlled by
isCreatingServer and serverName) are missing an explicit type and currently rely
on the default implicit submit behavior; update those Button components to
include type="button" to prevent accidental form submissions (also apply the
same change to the other action Buttons referenced around the later panel, e.g.,
the buttons in the 442-454 region).
In `@src/app/api/channels/route.ts`:
- Around line 68-75: The handler currently calls .trim() on body.serverId,
body.name and body.topic without type checks—if request.json() yields
non-strings the app will throw a 500; update the request processing in
src/app/api/channels/route.ts so you first validate that body is an object and
that body.serverId, body.name and body.topic (when present) are strings,
returning a 400 Bad Request for invalid types, and only then call .trim() on
those string fields (refer to the variables body, serverId, name, topic and the
route's request.json() parsing to locate the code to change).
In `@src/app/api/servers/`[serverId]/permissions/route.ts:
- Around line 121-132: You’re duplicating heavy permission work by calling
getChannelAccessForUser after already computing getServerPermissionsForUser and
effectivePerms; instead, derive canRead/canSend from effectivePerms plus one
lightweight channel fetch: export and use hasAccessToCategory and
normalizeChannelType from src/lib/server-channel-access.ts (or refactor
getChannelAccessForUser to accept the precomputed ServerAccess) to compute
channel-type-specific rules, and ensure canSend is computed taking announcement
channels and manageChannels into account (don’t rely on spread sendMessages
alone) so the handler returns correct canRead/canSend without re-running
getServerPermissionsForUser/getEffectivePermissions.
In `@src/lib/server-channel-access.ts`:
- Around line 16-29: Duplicate CHANNEL_TYPES and normalizeChannelType should be
moved into a single shared module and all local copies removed; create a shared
export for CHANNEL_TYPES and normalizeChannelType, ensure the
normalizeChannelType return type is the precise union "text" | "voice" |
"announcement" (not Channel["type"] or allowing undefined), then replace the
duplicated definitions in each file by importing the shared symbols
(CHANNEL_TYPES and normalizeChannelType) and update call sites to use the
imported helper.
---
Outside diff comments:
In `@src/__tests__/api-routes/server-permissions.test.ts`:
- Around line 151-173: Add assertions to verify the new canRead/canSend fields
are propagated into the API response: update the test using
mockGetChannelAccessForUser (the case returning canRead: true, canSend: false)
to assert the GET handler's response JSON includes canRead: true and canSend:
false, and likewise update the "base server permissions" test (where
mockGetChannelAccessForUser should return canRead: true, canSend: true) to
assert those fields are present and true; locate the tests referencing
mockGetChannelAccessForUser and the GET(request, { params: ... }) call to add
these expectations.
In `@src/app/api/channels/route.ts`:
- Around line 423-424: The pagination cursor is being computed from the
permission-filtered array (channels) which can be shorter than the fetched page
after readMessages filtering; change the code to compute nextCursor from the raw
fetched page (e.g., rawChannels or fetchedPage) instead: keep using
readMessages-filtered channels for the response, but derive nextCursor using the
raw page's length and its last item (e.g., lastRaw.$id) compared against limit
so clients continue paginating when more documents exist beyond the current
fetched page.
- Around line 426-436: The response for the authenticated channel list is
currently marked "public" which allows shared caches to leak one user's filtered
channels to others; update the Cache-Control header in the compressedResponse
call (the block that returns { channels, nextCursor } and sets headers) to a
private or no-store policy for authenticated responses (e.g., replace "public,
s-maxage=60, stale-while-revalidate=300" with "private, s-maxage=60,
stale-while-revalidate=300" or "no-store") so per-session/per-permission data is
not served from a shared cache. Ensure you change the header in the same
compressedResponse call that sets headers for the channels route.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f07b7940-cceb-4b0f-a560-1d39577c7c5d
📒 Files selected for processing (19)
scripts/setup-appwrite.tssrc/__tests__/admin-server-management.test.tsxsrc/__tests__/api-routes/channels.test.tssrc/__tests__/api-routes/server-permissions.test.tssrc/__tests__/category-settings-panel.test.tsxsrc/__tests__/chat-page.test.tsxsrc/app/admin/server-actions.tssrc/app/admin/server-management.tsxsrc/app/api/channels/[channelId]/route.tssrc/app/api/channels/route.tssrc/app/api/servers/[serverId]/permissions/route.tssrc/app/chat/hooks/useChannels.tssrc/app/chat/page.tsxsrc/components/category-settings-panel.tsxsrc/components/chat-surface-panel.tsxsrc/components/emoji-picker.tsxsrc/lib/appwrite-servers.tssrc/lib/server-channel-access.tssrc/lib/types.ts
Owner
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved and changes approved. |
Owner
Author
Merge activity
|
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
1 participant
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.
This pull request introduces several improvements and new tests for channel and server management, especially around channel creation and permissions. It expands test coverage for API routes, admin UI, and category management, and makes minor enhancements to test utilities and mocks.
Channel and Server Management Tests:
/api/channelsPOST endpoint, including authentication, permissions, and channel creation scenarios.Admin and Category UI Tests:
CategorySettingsPaneltests to cover channel creation from setup, updating channel types, and refined button selection logic for more robust testing. [1] [2]Test Utility and Mock Enhancements:
Database Schema Setup:
type,topic) and an index fortypein thechannelscollection.These changes collectively improve test coverage, reliability, and the flexibility of channel management features in the application.