regression(ABAC): Display correct values on user info #37748
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughThe PR changes ABAC attributes from a string array to structured objects ({ key, values[] }) across the UserInfo story and components, and updates rendering to iterate attribute.values (showing values) instead of attribute keys. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-11-27T17:56:26.050ZApplied to files:
📚 Learning: 2025-10-30T19:30:46.541ZApplied to files:
📚 Learning: 2025-10-27T14:38:46.994ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
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. Comment |
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 16:35", "12/10 17:30 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 17 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 10 Dec 2025 17:30:01 GMT |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/client/components/UserInfo/UserInfoABACAttributes.tsx (1)
1-8: Rendering logic correctly uses attribute values; refine keys to avoid collisionsUsing
IAbacAttributeDefinition[]and iterating overattribute.valuesso thatUserInfoABACAttributereceives the value string is aligned with the goal of showing attribute values instead of names.One small improvement: the current
key={${attribute.key}-${value}-${index}}uses only the outer index, so if an attribute has duplicate values, those siblings will share the same key. To avoid potential key collisions in the list, include the inner index as well:- {abacAttributes.map((attribute, index) => - attribute.values.map((value) => ( - <Margins inline={2} blockEnd={4} key={`${attribute.key}-${value}-${index}`}> + {abacAttributes.map((attribute, attributeIndex) => + attribute.values.map((value, valueIndex) => ( + <Margins + inline={2} + blockEnd={4} + key={`${attribute.key}-${value}-${attributeIndex}-${valueIndex}`} > <UserInfoABACAttribute attribute={value} /> </Margins> )), )}This keeps the implementation concise while ensuring stable, unique keys even if values repeat.
Also applies to: 14-20
📜 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.
⛔ Files ignored due to path filters (1)
-
apps/meteor/client/components/UserInfo/__snapshots__/UserInfo.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
-
apps/meteor/client/components/UserInfo/UserInfo.stories.tsx(1 hunks) -
apps/meteor/client/components/UserInfo/UserInfo.tsx(1 hunks) -
apps/meteor/client/components/UserInfo/UserInfoABACAttributes.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/components/UserInfo/UserInfoABACAttributes.tsxapps/meteor/client/components/UserInfo/UserInfo.tsxapps/meteor/client/components/UserInfo/UserInfo.stories.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.
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-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/client/components/UserInfo/UserInfoABACAttributes.tsxapps/meteor/client/components/UserInfo/UserInfo.tsxapps/meteor/client/components/UserInfo/UserInfo.stories.tsx
📚 Learning: 2025-10-30T19:30:46.541Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37244
File: apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.spec.tsx:125-146
Timestamp: 2025-10-30T19:30:46.541Z
Learning: In the AdminABACRoomAttributesForm component (apps/meteor/client/views/admin/ABAC/AdminABACRoomAttributesForm.tsx), the first attribute value field is mandatory and does not have a Remove button. Only additional values beyond the first have Remove buttons. This means trashButtons[0] corresponds to the second value's Remove button, not the first value's.
Applied to files:
apps/meteor/client/components/UserInfo/UserInfoABACAttributes.tsxapps/meteor/client/components/UserInfo/UserInfo.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/components/UserInfo/UserInfoABACAttributes.tsxapps/meteor/client/components/UserInfo/UserInfo.stories.tsx
🧬 Code graph analysis (1)
apps/meteor/client/components/UserInfo/UserInfoABACAttributes.tsx (1)
packages/core-typings/src/IAbacAttribute.ts (1)
IAbacAttributeDefinition(3-14)
🔇 Additional comments (2)
apps/meteor/client/components/UserInfo/UserInfo.tsx (1)
192-197: ABAC attributes wiring now correctly passes full definitionsForwarding
abacAttributesdirectly intoUserInfoABACAttributes(behind the existing null/length guard) aligns the parent with the new prop type and ensures the child can access attribute values instead of just keys. This matches the regression fix goal.apps/meteor/client/components/UserInfo/UserInfo.stories.tsx (1)
45-54: Story data shape matches IAbacAttributeDefinition and new renderingThe
WithABACAttributesstory now uses{ key, values }[], which is consistent withIAbacAttributeDefinitionand the updated UserInfo/UserInfoABACAttributes props. This will accurately exercise the “render values instead of keys” behavior.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/abac #37748 +/- ##
=============================================
- Coverage 54.50% 54.49% -0.01%
=============================================
Files 2633 2633
Lines 50105 50105
Branches 11224 11224
=============================================
- Hits 27308 27304 -4
- Misses 20607 20613 +6
+ Partials 2190 2188 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
MartinSchoeler
force-pushed
the
fix/render-correct-data
branch
from
f4c2d24 to
051b3ae
Compare
Proposed changes (including videos or screenshots)
Instead of showing attribute names, show the values from all attributes that the user has
Issue(s)
ABAC-91
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.