Skip to content

Conversation

@deepak0x
Copy link

@deepak0x deepak0x commented Jan 21, 2026

This PR fixes the Directory screen issue where users without the view-outside-room permission experience a continuous loading indicator when trying to access the "Users" filter. The fix adds client-side permission checks to prevent API calls that would result in 400 errors and provides better user feedback.

The changes include:

  • Adding permission check for view-outside-room before allowing access to Users directory
  • Disabling the "Users" filter option in the Directory screen when the user lacks the required permission

Issue(s)

Closes #6877

How to test or reproduce

  1. Log in as a user without the view-outside-room permission (or disable this permission for the user role)
  2. Navigate to the Directory screen
  3. Verify that:
    • The "Users" filter option is disabled/grayed out in the filter menu
    • No continuous loading indicator appears
    • Channels and Teams filters work normally

Screenshots

Before After
Before After

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

This fix implements client-side permission validation to prevent the 400 Bad Request error that was occurring when users without the view-outside-room permission tried to access the Users directory. By checking permissions before making API calls

Summary by CodeRabbit

  • New Features
    • Directory now respects role-based permissions: items users aren't allowed to view appear disabled and show visual disabled styling.
    • Default directory view falls back to channels when users lack permission to view user listings.
  • Bug Fixes
    • Attempting to access restricted directory views now shows an error notification and prevents navigation.

✏️ Tip: You can customize this high-level summary in your review settings.

- Disable Users filter option when user lacks view-outside-room permission
- Show error message when user tries to access Users without permission
- Fallback to channels view if users is default but permission is missing
- Add hasViewOutsideRoomPermission method to check user roles
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

Walkthrough

Directory view now enforces a "view outside room" permission: DirectoryView derives the permission from redux, initializes the default view accordingly, prevents switching to the "users" view when unauthorized (showing an error), and Options receives a prop to visually and functionally disable the "users" option.

Changes

Cohort / File(s) Summary
Permission Gating: Options UI
app/views/DirectoryView/Options.tsx
Added hasViewOutsideRoomPermission prop; compute isDisabled for users; make List.Radio item disabled and its onPress a no-op when disabled; set left icon color to colors.fontDisabled when disabled.
Permission Logic & State: DirectoryView
app/views/DirectoryView/index.tsx
Imported showErrorAlert and reduxStore; added hasViewOutsideRoomPermission() method; compute defaultType and initialize state.type based on permission; guard changeType('users') to call showErrorAlert and abort when permission is absent; pass permission flag to Options.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Options as DirectoryOptions (UI)
    participant DirView as DirectoryView
    participant Perm as Redux/PermissionChecker
    participant Alert as showErrorAlert

    User->>Options: tap "Users" option
    Options->>DirView: request changeType('users')
    DirView->>Perm: hasViewOutsideRoomPermission()
    alt permission granted
        DirView-->>Options: update state to 'users'
        Options-->>User: users view selected
    else permission denied
        DirView->>Alert: showErrorAlert(...)
        DirView-->>Options: no state change (keep previous view)
        Options-->>User: "Users" option remains disabled
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped to the directory, eager and bright,
I checked every badge in the soft morning light.
If the key isn't shown, I politely refrain,
I thump and I wiggle and wink once again.
Safe hops keep the list tidy — I'll wait by the lane.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main change: adding a permission check for view-outside-room in the Directory screen, which directly addresses the core objective.
Linked Issues check ✅ Passed The pull request implements all coding requirements from #6877: client-side permission validation prevents loading loops, disables the Users filter when lacking permissions, prevents failed API calls, and provides appropriate user feedback.
Out of Scope Changes check ✅ Passed All changes directly address the linked issue objective. The modifications to Options.tsx and DirectoryView index.tsx add permission checks and UI updates specifically for preventing the infinite loading state when users lack view-outside-room permission.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@app/views/DirectoryView/index.tsx`:
- Around line 134-136: The sort option is hardcoded to { usersCount: -1 } for
all directory types; update the code that builds the query/options (the object
containing count: 50 and sort) to choose the sort key based on the directory
"type" value: for type === 'users' use a user-appropriate sort (e.g., {
username: 1 } or { name: 1 }), and for type === 'channels' or 'teams' use {
usersCount: -1 }; modify the block that currently sets "sort" so it
conditionally assigns the sort object based on the "type" variable.
🧹 Nitpick comments (2)
app/views/DirectoryView/index.tsx (2)

57-60: DRY violation: Permission check logic is duplicated.

The permission check in the constructor duplicates the logic in hasViewOutsideRoomPermission() method (lines 93-99). Consider reusing the method or extracting a static helper.

♻️ Suggested refactor
 class DirectoryView extends React.Component<IDirectoryViewProps, IDirectoryViewState> {
+	private static checkViewOutsideRoomPermission(
+		viewOutsideRoomPermission?: string[],
+		userRoles?: string[]
+	): boolean {
+		if (!viewOutsideRoomPermission || !userRoles) {
+			return false;
+		}
+		return userRoles.some(role => viewOutsideRoomPermission.includes(role));
+	}
+
 	constructor(props: IDirectoryViewProps) {
 		super(props);
-		const hasPermission = props.viewOutsideRoomPermission && props.user?.roles
-			? props.user.roles.some(role => props.viewOutsideRoomPermission!.includes(role))
-			: false;
+		const hasPermission = DirectoryView.checkViewOutsideRoomPermission(
+			props.viewOutsideRoomPermission,
+			props.user?.roles
+		);
 		const defaultType = props.directoryDefaultView === 'users' && !hasPermission ? 'channels' : props.directoryDefaultView;

Then update hasViewOutsideRoomPermission to use the static method:

 	hasViewOutsideRoomPermission = (): boolean => {
 		const { viewOutsideRoomPermission, user } = this.props;
-		if (!viewOutsideRoomPermission || !user?.roles) {
-			return false;
-		}
-		return user.roles.some(role => viewOutsideRoomPermission.includes(role));
+		return DirectoryView.checkViewOutsideRoomPermission(viewOutsideRoomPermission, user?.roles);
 	};

349-350: Avoid as any type cast for better type safety.

The (state.permissions as any)['view-outside-room'] cast bypasses TypeScript's type checking. Consider extending the permissions type definition in IApplicationState to include this permission key properly.

#!/bin/bash
# Description: Check how permissions are typed in the application state

# Find the IApplicationState interface or permissions type definition
rg -n "permissions.*:" --type ts -C5 | head -80

# Check if there's a permissions reducer or type
ast-grep --pattern 'interface $_ {
  $$$
  permissions$_
  $$$
}'

- Sort users by username (ascending)
- Sort channels/teams by usersCount (descending)
- Prevents sorting users by non-existent usersCount field
- Remove viewOutsideRoomPermission prop, access permission from Redux state
- Use same permission checking logic as usePermissions hook
- Access permission via reduxStore.getState() for class component compatibility
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: directory view shows infinite loader when filtering users or searching

1 participant