chore: add new icons to abac rooms #37424

MartinSchoeler · 2025-11-06T20:39:04Z

Proposed changes (including videos or screenshots)

ABAC-71

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

Refactor
- Removed the ABAC header tag from room headers; ABAC status is now indicated by room icons (team-shield or hash-shield).
Tests
- Deleted component tests for the removed header tag.
- Added tests for room icon selection logic, including ABAC scenarios.
Chores
- Updated icon dependency version.
- Removed related localization entries.

dionisio-bot · 2025-11-06T20:39:08Z

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

changeset-bot · 2025-11-06T20:39:08Z

⚠️ No Changeset found

Latest commit: e9d47f9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2025-11-06T20:39:29Z

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

yarn.lock is excluded by !**/yarn.lock, !**/*.lock

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Removed the ABACHeaderTag component and its tests and i18n keys, added ABAC-aware icon selection to room icon logic (hook and private room type), updated an icons dependency, and removed ABACHeaderTag usage from room header views.

Changes

Cohort / File(s)	Change Summary
ABACHeaderTag removal `apps/meteor/client/components/ABAC/ABACHeaderTag.tsx`, `apps/meteor/client/components/ABAC/ABACHeaderTag.spec.tsx`	Deleted the ABACHeaderTag component and its test file (including snapshots and jest-axe accessibility tests).
Room header usage removal `apps/meteor/client/views/room/Header/RoomHeader.tsx`, `apps/meteor/client/views/room/HeaderV2/RoomHeader.tsx`	Removed import and JSX usage of `ABACHeaderTag`.
Icon selection logic `apps/meteor/client/hooks/useRoomIcon.tsx`, `apps/meteor/client/lib/rooms/roomTypes/private.ts`	Added early ABAC checks: if `room.abacAttributes` exists return `team-shield` when `room.teamMain` is true, otherwise `hash-shield`. This short-circuits prior icon branches.
Icon selection tests `apps/meteor/client/hooks/useRoomIcon.spec.tsx`	Added tests exercising icon selection across room variants including ABAC cases.
Dependency `apps/meteor/package.json`	Bumped `@rocket.chat/icons` from `~0.44.0` to `~0.45.0`.
Localization `packages/i18n/src/locales/en.i18n.json`, `packages/i18n/src/locales/nb.i18n.json`	Removed `ABAC_header_tag_title` and `ABAC_header_tag` entries (en); removed `ABAC_header_tag_title` (nb).

Sequence Diagram(s)

sequenceDiagram
    participant RoomComp as Room Component
    participant Hook as useRoomIcon
    participant IconLogic as Icon Selection

    RoomComp->>Hook: useRoomIcon(room)
    Hook->>IconLogic: evaluate room.abacAttributes?
    rect rgb(230,245,230)
    alt abacAttributes present
        IconLogic->>IconLogic: check room.teamMain
        alt teamMain true
            IconLogic-->>Hook: "team-shield"
        else teamMain false
            IconLogic-->>Hook: "hash-shield"
        end
    else no abacAttributes
        Note right of IconLogic: proceed with existing checks (federated, prid, team, default)
        IconLogic-->>Hook: other icon id
    end
    end
    Hook-->>RoomComp: icon identifier

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Review TypeScript suppression around room.abacAttributes access in useRoomIcon.
Confirm all imports/usages of the removed ABACHeaderTag are cleared.
Validate the new useRoomIcon.spec.tsx covers ABAC branches and edge cases.

Possibly related PRs

PR #37126: Removes the ABACHeaderTag component, its tests, translations, and header usage — closely related to this change.

Suggested reviewers

tassoevan
dougfabris
cardoso

Poem

🐇 I hopped through code to swap the tag for shield,
ABAC now whispers where the icons yield,
Team or hash — a tiny, tidy sign,
Tests follow paths where guarded branches shine,
A rabbit nods: the header's trimmed and healed. ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'chore: add new icons to abac rooms' accurately reflects the main changes: adding ABAC icons to the icon selection logic and removing the text-based ABACHeaderTag component.
Linked Issues check	✅ Passed	The PR addresses the core requirements from ABAC-71: icons are added to useRoomIcon and private room types for ABAC channels/teams, and the text tag component is removed from room headers.
Out of Scope Changes check	✅ Passed	All changes are scoped to ABAC icon implementation: icon logic additions, component removals, and supporting updates (i18n, icon package version bump) are all directly related to replacing the text tag with icons.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.}

codecov · 2025-11-06T21:01:08Z

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.03%. Comparing base (b27e9fb) to head (e9d47f9).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #37424      +/-   ##
===========================================
+ Coverage    68.99%   69.03%   +0.04%     
===========================================
  Files         3357     3356       -1     
  Lines       114243   114227      -16     
  Branches     20531    20571      +40     
===========================================
+ Hits         78820    78855      +35     
+ Misses       33339    33287      -52     
- Partials      2084     2085       +1

Flag	Coverage Δ
e2e	`57.48% <0.00%> (-0.03%)`	⬇️
e2e-api	`43.85% <ø> (+1.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

apps/meteor/client/lib/rooms/roomTypes/private.ts (1)
85-91: Consider consolidating duplicate ABAC icon logic.

The ABAC icon selection logic (lines 86-91) duplicates the same logic in apps/meteor/client/hooks/useRoomIcon.tsx (lines 12-17). Consider extracting this into a shared utility function to maintain DRY principles and ensure consistent behavior.

Example refactor:
// In a shared utility file (e.g., utils/abacIcons.ts)
export function getAbacIcon(room: { abacAttributes?: unknown; teamMain?: boolean }): string | null {
  if (!room.abacAttributes) {
    return null;
  }
  return room.teamMain ? 'team-shield' : 'hash-shield';
}
Then use it in both locations:
const abacIcon = getAbacIcon(room);
if (abacIcon) {
  return { name: abacIcon };
}
apps/meteor/client/hooks/useRoomIcon.spec.tsx (1)
1-83: Good test coverage for ABAC icon selection.

The test suite effectively validates icon selection across various room types including ABAC-enabled rooms. The structure using mock rooms and expected results makes it easy to verify behavior.

Consider adding edge case tests for:

ABAC attributes with falsy values (empty object, null)

Direct message rooms with a single user (testing ReactiveUserStatus component return)

Private rooms (type 'p') with ABAC attributes

Example addition:
abacPrivateRoom: createFakeRoom({
  t: 'p',
  name: 'abac-private',
  teamMain: false,
  // @ts-expect-error TODO: Implement ABAC attributes in rooms
  abacAttributes: true,
}),
With expected result:
abacPrivateRoom: { name: 'hash-shield' },

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 18f83d1 and b0173ad.

⛔ Files ignored due to path filters (2)

apps/meteor/client/components/ABAC/__snapshots__/ABACHeaderTag.spec.tsx.snap is excluded by !**/*.snap
yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (12)

apps/meteor/client/components/ABAC/ABACHeaderTag.spec.tsx (0 hunks)
apps/meteor/client/components/ABAC/ABACHeaderTag.tsx (0 hunks)
apps/meteor/client/hooks/useRoomIcon.spec.tsx (1 hunks)
apps/meteor/client/hooks/useRoomIcon.tsx (1 hunks)
apps/meteor/client/lib/rooms/roomTypes/private.ts (1 hunks)
apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (2 hunks)
apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (2 hunks)
apps/meteor/client/views/room/Header/RoomHeader.tsx (0 hunks)
apps/meteor/client/views/room/HeaderV2/RoomHeader.tsx (0 hunks)
apps/meteor/package.json (2 hunks)
packages/i18n/src/locales/en.i18n.json (0 hunks)
packages/i18n/src/locales/nb.i18n.json (0 hunks)

💤 Files with no reviewable changes (6)

apps/meteor/client/views/room/Header/RoomHeader.tsx
apps/meteor/client/views/room/HeaderV2/RoomHeader.tsx
apps/meteor/client/components/ABAC/ABACHeaderTag.spec.tsx
packages/i18n/src/locales/en.i18n.json
packages/i18n/src/locales/nb.i18n.json
apps/meteor/client/components/ABAC/ABACHeaderTag.tsx

🧰 Additional context used

🧠 Learnings (13)

📓 Common learnings

Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.

📚 Learning: 2025-10-28T16:53:42.761Z

Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

apps/meteor/package.json
apps/meteor/client/lib/rooms/roomTypes/private.ts
apps/meteor/client/hooks/useRoomIcon.tsx

📚 Learning: 2025-09-19T15:15:04.642Z

Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.

Applied to files:

apps/meteor/package.json

📚 Learning: 2025-10-24T17:32:05.348Z

Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37299
File: apps/meteor/ee/server/lib/ldap/Manager.ts:438-454
Timestamp: 2025-10-24T17:32:05.348Z
Learning: In Rocket.Chat, ABAC attributes can only be set on private rooms and teams (type 'p'), not on public rooms (type 'c'). Therefore, when checking for ABAC-protected rooms/teams during LDAP sync or similar operations, it's sufficient to query only private rooms using methods like `findPrivateRoomsByIdsWithAbacAttributes`.

Applied to files:

apps/meteor/client/lib/rooms/roomTypes/private.ts
apps/meteor/client/hooks/useRoomIcon.tsx

📚 Learning: 2025-10-27T14:38:46.994Z

Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37303
File: apps/meteor/tests/end-to-end/api/abac.ts:1125-1137
Timestamp: 2025-10-27T14:38:46.994Z
Learning: In Rocket.Chat ABAC feature, when ABAC is disabled globally (ABAC_Enabled setting is false), room-level ABAC attributes are not evaluated when changing room types. This means converting a private room to public will succeed even if the room has ABAC attributes, as long as the global ABAC setting is disabled.

Applied to files:

apps/meteor/client/lib/rooms/roomTypes/private.ts
apps/meteor/client/hooks/useRoomIcon.tsx

📚 Learning: 2025-09-25T09:59:26.461Z

Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).

Applied to files:

apps/meteor/client/lib/rooms/roomTypes/private.ts
apps/meteor/client/hooks/useRoomIcon.tsx