Skip to content

fix: reset isChannelPrivate when switching to a public room - #1387

Open
rohanesor wants to merge 1 commit into
RocketChat:developfrom
rohanesor:fix/1232-reset-is-channel-private
Open

rohanesor wants to merge 1 commit into
RocketChat:developfrom
rohanesor:fix/1232-reset-is-channel-private

Conversation

@rohanesor

Copy link
Copy Markdown

fix: reset isChannelPrivate when switching to a public room

Fixes #1232

What changed

isChannelPrivate was only ever latched on, never reset, so after visiting a private room every later public room was still treated as private and the groups.* endpoints were used for it.

- if (channelIsPrivate) setIsChannelPrivate(true);      // useFetchChatData.js
+ setIsChannelPrivate(channelIsPrivate);
- if (res.room.t === 'p') setIsChannelPrivate(true);     // ChatHeader.js
+ setIsChannelPrivate(res.room.t === 'p');

Acceptance Criteria fulfillment

  • Message history loads again after visiting a private room and then a public one
  • Every request uses the matching endpoint immediately (see "Known limitation")

How it was verified

Reproduced against a local Rocket.Chat 6.13 with a public room (GENERAL) and a private room, opening the private room first and then switching to the public one without reloading (a reload resets the store and hides the bug).

  • Before: GET /api/v1/groups.messages?roomId=GENERAL → 400, the chat pane shows "No messages", and it never recovers.
  • After: the same switch settles on GET /api/v1/channels.messages?roomId=GENERAL → 200 and the messages render in the chat pane.

Known limitation

This makes the flag converge to the correct value; it does not remove every mis-targeted request. channelInfo is cached in channelStore and only cleared on logout, so the first request right after a room switch can still use the previous room's endpoint and 400 before it settles. Refreshing channelInfo on room change is a larger change and I have kept it out of this PR — happy to follow up if maintainers want it here.

Prior art

A previous attempt, #1234, solved this in the API layer by caching the room type from channelInfo() and resolving the endpoint from it (getRoomRoute / resolveRoomRoute) rather than from a boolean. That approach also removes the transient request described above. It was closed unmerged by its author. This PR deliberately takes the smaller path of fixing the flag that was latching, so the change stays reviewable — but if maintainers prefer the API-layer route resolution, I am happy to implement that direction instead.

Testing

yarn format:check, yarn lint (0 errors; 30 warnings, unchanged from clean develop at the same commit), yarn build, yarn build:storybook — all pass on Node 24 / yarn 3.6.4. The repo's pre-push husky hook (npm run lint) passes.

No unit test added: packages/react has no unit-test runner (no test script, CI runs only format/lint/build) and src/index.test.js currently fails on develop — see #1191. Happy to add the tooling here or in a follow-up.

Video/Screenshots

Before - switching private -> public leaves groups.messages?roomId=GENERAL at 400 and the chat pane shows "No messages":

before: groups.messages 400, chat shows No messages

After - the same switch settles on channels.messages?roomId=GENERAL 200 and the messages render:

after: channels.messages 200, messages render

The two 400 rows still visible in the "after" capture are the transient described under Known limitation above - the final state is correct and the history loads.

The channel-private flag was only ever latched to true and never reset,
so after visiting a private room the store kept reporting every
subsequent public room as private. Downstream callers
(getOlderMessages, getThreadMessages, getChannelMembers) then used the
groups.* endpoints for public rooms, which responds 400 and leaves the
message history empty.

Set the flag to the actual room type instead of only setting it on
private rooms.

Fixes RocketChat#1232
@CLAassistant

CLAassistant commented Oct 15, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@rohanesor

Copy link
Copy Markdown
Author

Opened #1387 for this

This branch has not been deployed

No deployments
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.

BUG: Message history fails to load

2 participants