chore: thread replies not loading with an invalid page size config

Merged
AB
3mo ago
+10-4

Proposed changes (including videos or screenshots)

The thread panel and the thread list read their pagination page size from the threadMessagesSize / threadsListSize client config (a URL search param or a rc-config-* localStorage entry). Both were doing parseInt(...) straight on the raw value, and getConfig only falls back to its default when the key is absent — never when it is present but unparseable. A value like ?threadMessagesSize=not-a-number therefore produced NaN, which was sent to GET /v1/chat.getThreadMessages as count=NaN. The endpoint schema declares count as a number, so the request was rejected and the thread panel opened with no replies.

Adds a getNumericConfig helper next to getConfig that parses the value and falls back to the provided default when the result is not a finite number, and uses it at both call sites so an unparseable config value can no longer reach the REST pagination params.

  • apps/meteor/client/lib/utils/getConfig.ts — new getNumericConfig(key, defaultValue)
  • apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts — threadMessagesSize, default 50
  • apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadsList.ts — threadsListSize, default 10

Unit tests were added for both hooks, asserting the count that actually reaches the endpoint for absent, valid, non-numeric and empty config values, plus the loadMessageAround path. Verified they fail against the previous parseInt behaviour.

Issue(s)

CORE-2498

Steps to test or reproduce

  1. Open a channel containing a thread with one or more replies.
  2. Open the thread with a valid page size, e.g. /channel/<name>/thread/<thread-id>?threadMessagesSize=20, and confirm the replies are displayed.
  3. Reload the same thread with an invalid value: ?threadMessagesSize=not-a-number.
  4. Expected: the replies still load, and the chat.getThreadMessages request in the network tab carries count=50 instead of count=NaN.
  5. Repeat with ?threadsListSize=not-a-number on the Threads contextual bar: the thread list loads and chat.getThreadsList carries count=10.

Further comments

The same parseInt(getConfig(...)) pattern exists in four other list hooks (useDiscussionsList, useFilesList, useHistoryMessageList, useTeamsChannelList) and is vulnerable in the same way. They are left untouched here to keep the diff scoped to the reported issue; getNumericConfig is a one-line swap for each if we want them covered too.

CORE-2498

Summary by CodeRabbit

  • Bug Fixes

    • Fixed thread replies and thread lists failing to load when page-size settings contain invalid or empty values.
    • Added safe fallback page sizes to prevent invalid pagination requests.
  • Tests

    • Added coverage for default, configured, invalid, and empty page-size settings for thread messages and thread lists.