feat: Attribute Based Access Control (ABAC)#6856
Conversation
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
WalkthroughPropagates ABAC attributes through types, DB, models, and UI; adds ABAC-aware icons, localized strings, RoomInfoABAC UI, gating for actions/invite flows, and support for per-item Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
diegolmello
temporarily deployed
to
experimental_ios_build
GitHub Actions
Inactive
diegolmello
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
app/lib/methods/helpers/mergeSubscriptionsRooms.ts (1)
17-17: Consider refactoring to reduce complexity.The ESLint complexity warning suppression indicates the
mergefunction has high cyclomatic complexity. While adding the ABAC logic is reasonable, consider refactoring this function into smaller, more focused functions to improve maintainability.For example, you could extract room property merging into separate helper functions grouped by concern (metadata, encryption, team data, ABAC data, etc.).
app/views/RoomInfoView/components/RoomInfoABAC.stories.tsx (1)
33-37: Optional: Consider extracting ThemeProvider if reused.The ThemeProvider helper works well for this story. If this pattern is repeated across multiple story files, consider extracting it to a shared Storybook utility for better maintainability.
app/views/RoomInfoView/components/RoomInfoABAC.tsx (1)
27-27: Consider moving inline styles to the stylesheet.The component uses inline style objects with
gapproperty at lines 27 and 36. For consistency with the rest of the codebase (which uses the importedstylesobject), consider moving these to the styles file. Additionally, verify that thegapproperty is supported in your target React Native version.Apply this diff to use stylesheet-based styles:
In
app/views/RoomInfoView/styles.ts, add:+ abacContainer: { + gap: 16 + }, + abacAttributeContainer: { + gap: 8 + },Then update the component:
- <View style={{ gap: 16 }}> + <View style={styles.abacContainer}> <RoomInfoTagContainer> <RoomInfoTag name={I18n.t('ABAC_managed')} icon={teamMain ? 'team-shield' : 'hash-shield'} /> </RoomInfoTagContainer> <Text style={[styles.abacDescription, { color: colors.fontSecondaryInfo }]}>{I18n.t('ABAC_managed_description')}</Text> <ItemLabel label={I18n.t('ABAC_room_attributes')} /> {abacAttributes.map(attribute => ( - <View key={attribute.key} style={{ gap: 8 }}> + <View key={attribute.key} style={styles.abacAttributeContainer}> <Text style={[styles.abacDescription, { color: colors.fontDefault }]}>{attribute.key}</Text>Also applies to: 36-36
app/containers/MessageActions/index.tsx (1)
445-446: Consider refining the disabledReason logic.The "Reply in DM" action shows the ABAC disabled reason even when the user lacks
hasCreateDirectMessagePermission. This could be confusing if a user without permission sees "Not available in ABAC-managed rooms" when the actual reason is lack of permission.Consider updating the logic to show the ABAC reason only when permission exists:
title: I18n.t('Reply_in_direct_message'), icon: 'arrow-back', onPress: () => handleReplyInDM(message), enabled: permissions.hasCreateDirectMessagePermission && !room.abacAttributes, - disabledReason: room.abacAttributes && I18n.t('ABAC_disabled_action_reason') + disabledReason: permissions.hasCreateDirectMessagePermission && room.abacAttributes ? I18n.t('ABAC_disabled_action_reason') : undefinedThis ensures the ABAC message only appears when permission is granted but ABAC blocks the action.
📜 Review details
Configuration used: Organization 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 (7)
android/app/src/main/assets/fonts/custom.ttfis excluded by!**/*.ttfapp/containers/RoomHeader/__snapshots__/RoomHeader.test.tsx.snapis excluded by!**/*.snapapp/containers/RoomItem/__snapshots__/RoomItem.test.tsx.snapis excluded by!**/*.snapapp/containers/RoomTypeIcon/__snapshots__/RoomTypeIcon.test.tsx.snapis excluded by!**/*.snapapp/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snapapp/views/RoomInfoView/components/__snapshots__/RoomInfoABAC.test.tsx.snapis excluded by!**/*.snapios/custom.ttfis excluded by!**/*.ttf
📒 Files selected for processing (64)
app/containers/ActionSheet/Item.tsx(1 hunks)app/containers/ActionSheet/Provider.tsx(1 hunks)app/containers/CustomIcon/mappedIcons.js(2 hunks)app/containers/MessageActions/index.tsx(2 hunks)app/containers/RoomHeader/RoomHeader.stories.tsx(1 hunks)app/containers/RoomHeader/RoomHeader.tsx(5 hunks)app/containers/RoomHeader/index.tsx(4 hunks)app/containers/RoomItem/IconOrAvatar.tsx(2 hunks)app/containers/RoomItem/RoomItem.stories.tsx(1 hunks)app/containers/RoomItem/RoomItem.tsx(3 hunks)app/containers/RoomItem/TypeIcon.tsx(1 hunks)app/containers/RoomItem/index.tsx(1 hunks)app/containers/RoomItem/interfaces.ts(3 hunks)app/containers/RoomTypeIcon/RoomTypeIcon.stories.tsx(1 hunks)app/containers/RoomTypeIcon/index.tsx(3 hunks)app/containers/message/Message.stories.tsx(2 hunks)app/containers/message/utils.ts(2 hunks)app/definitions/IMessage.ts(1 hunks)app/definitions/IRoom.ts(1 hunks)app/definitions/ISubscription.ts(1 hunks)app/i18n/locales/ar.json(1 hunks)app/i18n/locales/bn-IN.json(1 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/en.json(1 hunks)app/i18n/locales/es.json(1 hunks)app/i18n/locales/fi.json(1 hunks)app/i18n/locales/fr.json(1 hunks)app/i18n/locales/hi-IN.json(1 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/it.json(1 hunks)app/i18n/locales/ja.json(1 hunks)app/i18n/locales/nl.json(1 hunks)app/i18n/locales/nn.json(1 hunks)app/i18n/locales/no.json(2 hunks)app/i18n/locales/pt-BR.json(1 hunks)app/i18n/locales/pt-PT.json(1 hunks)app/i18n/locales/ru.json(1 hunks)app/i18n/locales/sl-SI.json(1 hunks)app/i18n/locales/sv.json(1 hunks)app/i18n/locales/ta-IN.json(1 hunks)app/i18n/locales/te-IN.json(1 hunks)app/i18n/locales/tr.json(1 hunks)app/i18n/locales/zh-CN.json(1 hunks)app/i18n/locales/zh-TW.json(1 hunks)app/lib/database/model/Subscription.js(2 hunks)app/lib/database/model/migrations.js(1 hunks)app/lib/database/schema/app.js(2 hunks)app/lib/methods/helpers/mergeSubscriptionsRooms.ts(2 hunks)app/views/RoomActionsView/index.tsx(1 hunks)app/views/RoomInfoView/Channel.tsx(2 hunks)app/views/RoomInfoView/Direct.tsx(2 hunks)app/views/RoomInfoView/Item.tsx(2 hunks)app/views/RoomInfoView/components/ItemLabel.tsx(1 hunks)app/views/RoomInfoView/components/RoomInfoABAC.stories.tsx(1 hunks)app/views/RoomInfoView/components/RoomInfoABAC.test.tsx(1 hunks)app/views/RoomInfoView/components/RoomInfoABAC.tsx(1 hunks)app/views/RoomInfoView/components/RoomInfoTag.tsx(1 hunks)app/views/RoomInfoView/components/RoomInfoViewTitle.tsx(1 hunks)app/views/RoomInfoView/index.tsx(1 hunks)app/views/RoomInfoView/styles.ts(3 hunks)app/views/RoomMembersView/components/ActionsSection.tsx(2 hunks)app/views/RoomMembersView/index.tsx(1 hunks)app/views/RoomView/index.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (20)
app/lib/database/model/Subscription.js (1)
app/lib/database/utils.ts (1)
sanitizer(19-19)
app/views/RoomMembersView/index.tsx (1)
app/views/RoomMembersView/components/ActionsSection.tsx (1)
ActionsSection(29-112)
app/views/RoomInfoView/components/RoomInfoTag.tsx (3)
app/containers/CustomIcon/index.tsx (2)
TIconsName(21-21)CustomIcon(47-47)app/theme.tsx (1)
useTheme(29-29)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/RoomItem/index.tsx (1)
app/sagas/deepLinking.js (1)
item(71-76)
app/containers/ActionSheet/Item.tsx (1)
app/containers/Toast.tsx (1)
LISTENER(24-24)
app/views/RoomMembersView/components/ActionsSection.tsx (1)
app/definitions/ISubscription.ts (1)
TSubscriptionModel(122-125)
app/containers/RoomHeader/index.tsx (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
app/containers/RoomItem/interfaces.ts (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
app/views/RoomInfoView/Item.tsx (1)
app/views/RoomInfoView/components/ItemLabel.tsx (1)
ItemLabel(11-18)
app/containers/RoomItem/TypeIcon.tsx (1)
app/containers/RoomItem/interfaces.ts (1)
ITypeIconProps(59-71)
app/views/RoomInfoView/components/RoomInfoABAC.tsx (5)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)app/theme.tsx (1)
useTheme(29-29)app/views/RoomInfoView/components/RoomInfoTag.tsx (2)
RoomInfoTagContainer(20-22)RoomInfoTag(9-18)app/lib/constants/colors.ts (1)
colors(280-302)app/views/RoomInfoView/components/ItemLabel.tsx (1)
ItemLabel(11-18)
app/views/RoomInfoView/components/RoomInfoABAC.test.tsx (1)
.rnstorybook/generateSnapshots.tsx (1)
generateSnapshots(10-22)
app/views/RoomInfoView/Channel.tsx (1)
app/views/RoomInfoView/components/RoomInfoABAC.tsx (1)
RoomInfoABAC(11-48)
app/containers/RoomTypeIcon/index.tsx (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
app/views/RoomInfoView/components/RoomInfoABAC.stories.tsx (3)
app/views/RoomInfoView/components/RoomInfoABAC.tsx (1)
RoomInfoABAC(11-48)app/theme.tsx (2)
TSupportedThemes(8-8)ThemeContext(18-18)app/lib/constants/colors.ts (1)
colors(280-302)
app/views/RoomInfoView/index.tsx (1)
app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/MessageActions/index.tsx (4)
app/reducers/permissions.ts (1)
permissions(13-25)app/sagas/deepLinking.js (2)
room(69-69)room(222-222)app/lib/methods/helpers/compareServerVersion.ts (1)
compareServerVersion(10-15)app/sagas/messages.js (1)
message(13-13)
app/views/RoomInfoView/Direct.tsx (1)
app/views/RoomInfoView/components/RoomInfoTag.tsx (2)
RoomInfoTagContainer(20-22)RoomInfoTag(9-18)
app/views/RoomInfoView/components/ItemLabel.tsx (2)
app/theme.tsx (1)
useTheme(29-29)app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/RoomHeader/RoomHeader.tsx (1)
app/definitions/ISubscription.ts (1)
ISubscription(40-120)
🔇 Additional comments (71)
app/i18n/locales/tr.json (1)
11-15: LGTM! ABAC translation keys properly integrated.The five new ABAC-related translation keys are correctly placed in alphabetical order and follow the file's naming conventions. The JSON structure is valid, and the keys align with the PR's ABAC feature requirements (disabled action messaging, ABAC-managed indicators, system messages, and room attributes).
However, as a standard i18n practice, consider having the Turkish translations reviewed by a native speaker to ensure grammatical correctness and natural phrasing, especially for user-facing strings like "ABAC_managed_description."
app/i18n/locales/te-IN.json (1)
15-19: LGTM! ABAC localization entries added correctly.The five new Telugu i18n entries for ABAC features are properly formatted and alphabetically positioned. The JSON syntax is valid, and the keys align with the PR objectives covering disabled actions, managed rooms, system messages, and room attributes.
Note: Translation accuracy should ideally be verified by a native Telugu speaker to ensure linguistic quality and cultural appropriateness.
app/i18n/locales/fi.json (1)
15-19: LGTM! Clean addition of ABAC localization strings.The Finnish translations for ABAC features have been properly added with valid JSON syntax and consistent key naming. The five new keys cover the essential ABAC messaging: disabled action reasons, managed room indicators, descriptions, system messages, and room attributes.
Since translation accuracy is critical for user experience, consider having these strings reviewed by a native Finnish speaker or through your project's established translation review process to ensure linguistic accuracy and natural phrasing.
app/i18n/locales/ru.json (1)
15-19: LGTM! Russian translations are accurate and consistent.The ABAC localization entries are well-translated and follow Russian grammatical conventions. The terminology is consistent with existing translations in the file (using "комнаты" for rooms), and the use of "ABAC'ом" in line 18 correctly applies Russian case endings to the foreign acronym.
app/i18n/locales/it.json (1)
15-19: LGTM!The Italian translations for the ABAC feature keys are grammatically correct and follow the existing localization patterns in the file. The lowercase start in line 18 ("è stato rimosso da ABAC") is appropriate since this fragment is designed to follow a username in system messages (e.g., "username è stato rimosso da ABAC").
app/i18n/locales/hu.json (1)
15-19: LGTM!The five new ABAC-related localization keys are properly formatted and follow the established naming conventions in the file. The key naming is consistent—uppercase prefixes for UI labels and lowercase for system messages.
If Hungarian translation accuracy hasn't been verified by a native speaker, consider having these reviewed to ensure the translations convey the intended meaning correctly.
app/i18n/locales/no.json (2)
15-19: LGTM! ABAC localization keys added correctly.The new ABAC-related localization strings are properly formatted and follow the established JSON structure and naming conventions. All five expected keys from the PR objectives are present with complete Norwegian translations.
Note: Technical correctness verified; linguistic accuracy would require Norwegian language expertise.
962-963: LGTM! Correct JSON formatting and new workspace key.The trailing comma addition on line 962 and the new
Your_workspacekey are properly formatted and consistent with the rest of the file.app/i18n/locales/sv.json (1)
15-19: LGTM! Swedish ABAC localization entries are correctly structured.The added ABAC translation keys are properly formatted, alphabetically ordered, and align with the PR objectives. The translations appear to be appropriate Swedish equivalents for:
- Disabled action messaging for ABAC-managed rooms
- ABAC-managed room labels and descriptions
- System messages for ABAC-based user removal
- Room attributes labels
The JSON structure is valid and the key naming follows the existing conventions in the file.
app/i18n/locales/hi-IN.json (1)
15-19: Verify consistency of ABAC keys across all locale files.The JSON structure and key placement are correct. The five new ABAC-related translation keys align with the PR objectives for Attribute-Based Access Control functionality.
However, please verify that these exact keys exist in all other locale files (e.g., en.json, ar.json, cs.json, de.json, ja.json, etc.) to ensure consistency across the internationalization layer.
Run the following script to verify the presence of these ABAC keys in all locale files:
#!/bin/bash # Description: Check for ABAC translation keys across all locale files echo "Checking for ABAC keys in all locale files..." echo "" # Define the keys to search for keys=( "ABAC_disabled_action_reason" "ABAC_managed" "ABAC_managed_description" "abac_removed_user_from_the_room" "ABAC_room_attributes" ) # Find all locale JSON files locale_files=$(fd -e json . app/i18n/locales) # Check each key in each file for key in "${keys[@]}"; do echo "Key: $key" for file in $locale_files; do if ! grep -q "\"$key\":" "$file"; then echo " ❌ Missing in: $(basename $file)" fi done echo "" done echo "Locale files found:" echo "$locale_files" | wc -lapp/i18n/locales/nn.json (1)
10-14: LGTM! ABAC translations properly structured.The five new ABAC-related translation keys are correctly formatted and follow existing naming conventions:
- UI/action keys (
ABAC_disabled_action_reason,ABAC_managed, etc.) use uppercase prefixes- System message key (
abac_removed_user_from_the_room) uses lowercase, consistent with other system messages likechanged_room_announcementandroom_changed_typeThe keys are properly placed in alphabetical order and the JSON structure is valid.
app/i18n/locales/fr.json (1)
11-15: LGTM! French translations verified as accurate.All five ABAC-related translation entries are grammatically correct and accurately translate the English source strings. The translations read naturally in French and maintain consistent formatting with existing patterns in the file.
app/i18n/locales/sl-SI.json (1)
15-19: LGTM. The JSON structure is correct and the keys are properly ordered alphabetically. The naming convention with lowercaseabac_for the removed user message is consistent across all 30+ supported locales and the English source, indicating this is intentional and not a mistake.For completeness, a native Slovenian speaker should review the translation accuracy of these new strings to ensure they convey the intended meaning.
app/i18n/locales/ta-IN.json (1)
15-19: LGTM! ABAC localization keys properly structured.The five ABAC-related internationalization keys are correctly formatted and inserted in alphabetical order. The JSON structure is valid.
The casing distinction between
ABAC_*(uppercase) andabac_removed_user_from_the_room(lowercase) is intentional—uppercase keys are used for UI labels and descriptions (as seen in RoomInfoABAC component), while lowercase follows the established pattern for system messages (consistent with existing keys likemuted,unmuted,leave,delete).Recommendation: Have a native Tamil speaker review these strings to ensure they accurately convey the ABAC concepts, especially the technical terminology.
app/containers/RoomItem/RoomItem.stories.tsx (1)
74-75: LGTM! Good story coverage for ABAC attributes.The new story entries effectively demonstrate ABAC attributes on private channels, testing both standalone and with the
teamMainflag. This provides good visual documentation of the ABAC feature.app/i18n/locales/bn-IN.json (1)
15-19: LGTM! ABAC localization strings added correctly.The five new ABAC-related translation keys are properly formatted and follow the existing localization patterns. The keys align with the ABAC feature implementation described in the PR objectives.
app/containers/message/Message.stories.tsx (1)
1922-1923: LGTM! System message stories added for ABAC and unsupported types.The new story entries provide good coverage for the ABAC-removed-user system message and the generic unsupported message type. Including both standard and large font variants ensures accessibility testing.
Also applies to: 1965-1966
app/definitions/ISubscription.ts (1)
119-119: Type definition added for ABAC attributes.The
abacAttributesfield is properly added to theISubscriptioninterface. Note that there's a type inconsistency withIServerRoom.abacAttributeswherevaluesis required rather than optional (see comment on IRoom.ts line 236). Consider aligning these definitions.app/lib/methods/helpers/mergeSubscriptionsRooms.ts (1)
68-70: LGTM! ABAC attributes properly propagated from room to subscription.The logic correctly checks for the presence of
abacAttributesand safely falls back to an empty array. This follows the same pattern used for other room properties in this function.app/containers/CustomIcon/mappedIcons.js (1)
109-109: LGTM! ABAC shield icons added correctly.The new
hash-shieldandteam-shieldicon mappings are properly placed in alphabetical order and follow the existing naming convention. These support the ABAC feature's visual indicators for managed rooms.Also applies to: 206-206
app/views/RoomInfoView/components/ItemLabel.tsx (1)
1-18: LGTM! Clean component extraction with proper accessibility support.The
ItemLabelcomponent is well-implemented:
- Proper TypeScript typing
- Theme-aware styling
- Accessibility support with
accessibilityLabel- Test support with
testID- Follows React best practices
This extraction improves code reusability and maintainability across the RoomInfoView components.
app/views/RoomInfoView/Item.tsx (1)
19-19: LGTM! Clean refactor to use ItemLabel component.The extraction of label rendering logic into the ItemLabel component improves code reusability and maintains separation of concerns.
app/i18n/locales/cs.json (1)
15-19: LGTM! ABAC translations added correctly.The new Czech localization keys follow existing naming conventions and provide appropriate translations for ABAC-related UI elements.
app/containers/RoomItem/interfaces.ts (1)
70-70: LGTM! Consistent interface extensions for ABAC support.The abacAttributes property has been appropriately added to all relevant room item interfaces, properly typed using ISubscription's definition to ensure consistency across the codebase.
Also applies to: 128-128, 166-166
app/definitions/IMessage.ts (1)
275-276: LGTM! New ABAC message type added.The 'abac-removed-user-from-room' message type has been appropriately added to the MessageTypesValues union to support ABAC system messages.
app/containers/message/utils.ts (2)
78-79: LGTM! Message author name configuration updated.The ABAC message type has been correctly added to the messagesWithAuthorName array to ensure proper message attribution display.
163-164: LGTM! ABAC message translation handler added.The case handler for 'abac-removed-user-from-room' follows the existing pattern and uses the appropriate translation key.
app/views/RoomInfoView/components/RoomInfoABAC.stories.tsx (3)
1-19: LGTM! Proper Storybook configuration.The story setup follows Storybook best practices with appropriate decorators for ScrollView wrapping.
21-31: LGTM! Comprehensive test scenarios.The Story component tests both simple and complex ABAC attribute configurations, including an edge case with 10 values to verify proper wrapping and layout behavior.
39-53: LGTM! Complete theme coverage.All three theme variants (Light, Dark, Black) are properly tested, enabling thorough visual regression testing of the ABAC component.
app/views/RoomInfoView/components/RoomInfoTag.tsx (2)
9-18: LGTM! Well-structured tag component.The RoomInfoTag component is cleanly implemented with proper theme integration, conditional icon rendering, and test support via testID.
20-22: LGTM! Simple and effective container component.The RoomInfoTagContainer provides a clean wrapper for tag collections, following React composition patterns appropriately.
app/views/RoomInfoView/styles.ts (5)
16-17: LGTM! Gap property added for consistent spacing.The gap property provides clean vertical spacing between item children, improving the layout for ABAC attribute display.
63-65: LGTM! Proper flexbox configuration for tag wrapping.The flexWrap and gap properties enable tags to wrap naturally across multiple lines with consistent spacing, which is essential for displaying multiple ABAC attribute values.
67-73: LGTM! Badge layout updated for icon support.The roleBadge style has been properly restructured to support horizontal icon + text layout with appropriate spacing and alignment.
74-78: LGTM! Badge typography optimized.The role text styling has been appropriately updated with smaller font size and bold weight for improved readability in compact badges.
97-101: LGTM! ABAC description style added.The new abacDescription style provides appropriate typography for ABAC description text with good readability.
app/containers/RoomItem/index.tsx (1)
97-97: LGTM! Clean ABAC attribute propagation.The addition of
abacAttributesprop follows the existing pattern and integrates well with the broader ABAC feature implementation across the codebase.app/views/RoomInfoView/components/RoomInfoABAC.test.tsx (1)
1-4: LGTM! Standard snapshot test setup.The test file correctly uses the
generateSnapshotsutility to create snapshot tests for the RoomInfoABAC stories, following the established pattern in the codebase.app/containers/ActionSheet/Provider.tsx (1)
17-17: LGTM! Type-safe extension for custom disabled messages.The optional
disabledReasonfield enables action items to provide custom explanations when disabled (e.g., "Not available in ABAC-managed rooms"), improving user experience without breaking existing code.app/views/RoomView/index.tsx (1)
538-538: LGTM! Consistent ABAC attribute propagation to header.The
abacAttributesprop is correctly passed to RoomHeader, enabling ABAC-aware rendering in the room header component.app/views/RoomInfoView/components/RoomInfoViewTitle.tsx (1)
66-66: LGTM! Safe ABAC attribute forwarding.The use of optional chaining ensures safe propagation of
abacAttributesto RoomTypeIcon, following the pattern of other optional props.app/views/RoomActionsView/index.tsx (1)
798-798: LGTM! ABAC attributes correctly propagated to icon.The
abacAttributesprop is properly passed to RoomTypeIcon in the room info rendering, enabling ABAC-aware icon selection.app/views/RoomMembersView/index.tsx (1)
409-414: LGTM! ABAC attributes enable proper action gating.The
abacAttributesprop is correctly passed to ActionsSection, enabling it to conditionally disable the "Invite users" action in ABAC-managed rooms as per the PR objectives.app/i18n/locales/pt-BR.json (1)
15-19: LGTM! Complete i18n support for ABAC features.The five new translation keys provide comprehensive Portuguese localization for ABAC-related UI elements including disabled action messaging, ABAC badges, room attributes, and system messages.
app/containers/RoomTypeIcon/RoomTypeIcon.stories.tsx (1)
21-22: LGTM!The new story examples effectively demonstrate the
abacAttributesprop usage with both standalone and combined withteamMainflag.app/i18n/locales/ar.json (1)
11-15: LGTM!The ABAC-related localization entries are properly added with appropriate Arabic translations.
app/containers/RoomHeader/RoomHeader.stories.tsx (1)
58-72: LGTM!The new story examples properly demonstrate RoomHeader rendering with
abacAttributes, both standalone and combined with theteamMainflag.app/views/RoomInfoView/index.tsx (1)
287-287: LGTM!The additional
contentContainerStylewith bottom padding improves the scrollable area layout, likely to accommodate the new ABAC content sections.app/views/RoomInfoView/Channel.tsx (2)
6-6: LGTM!The import of
RoomInfoABACis properly placed alongside other component imports.
19-19: LGTM!The
RoomInfoABACcomponent is correctly integrated into the Channel view, passing the necessaryabacAttributesandteamMainprops from the room object. The component handles the conditional rendering internally whenabacAttributesis empty or undefined.app/lib/database/model/Subscription.js (2)
156-157: LGTM!The
abacAttributesfield is correctly defined using the@jsondecorator with thesanitizerfunction, following the established pattern for other JSON fields in the model. The mapping fromabac_attributes(database) toabacAttributes(JavaScript) is consistent with the codebase conventions.
224-225: LGTM!The
abacAttributesfield is properly included in theasPlain()serialization method, ensuring the field is available when converting the model to a plain object representation.app/containers/ActionSheet/Item.tsx (1)
29-29: LGTM!The enhancement to support custom
disabledReasonmessages is well-implemented. The fallback to the default i18n message maintains backward compatibility while allowing ABAC and other features to provide context-specific disabled messages.app/lib/database/model/migrations.js (1)
334-343: LGTM! Schema version alignment confirmed.The migration to version 28 correctly adds the
abac_attributescolumn to the subscriptions table, following the established migration pattern. The column definition (string type, optional) is appropriate for storing JSON-serialized ABAC attributes. Schema version inapp/lib/database/schema/app.jsis properly set to version 28, confirming alignment with this migration.app/containers/RoomItem/IconOrAvatar.tsx (2)
37-48: LGTM - Prop threading is clean and follows existing patterns.The
abacAttributesprop is correctly passed through to theTypeIconcomponent, maintaining consistency with how other props likesourceTypeare handled.
11-25: The TypeScript interfaces are properly updated withabacAttributes. BothITypeIconPropsandIIconOrAvatarinterfaces correctly define the property asabacAttributes?: ISubscription['abacAttributes'], and all components properly thread this prop through to child components.app/views/RoomInfoView/Direct.tsx (2)
10-10: Good refactoring - using shared components.Introducing
RoomInfoTagandRoomInfoTagContainerpromotes consistency and reusability across the codebase.
19-23: LGTM - Maintained test IDs and improved code structure.The refactoring preserves the existing
testIDformat while using standardized components. This makes the badge rendering consistent with other parts of the app (e.g., RoomInfoABAC).app/lib/database/schema/app.js (1)
4-4: Migration script for version 28 exists and correctly adds theabac_attributescolumn.The migration at line 336-340 of
app/lib/database/model/migrations.jsproperly implements theabac_attributescolumn addition to the subscriptions table with the correct schema definition (type: 'string', isOptional: true), matching the schema definition in app.js.app/i18n/locales/es.json (1)
11-15: Five ABAC translation keys are actively used in the codebase.All five new ABAC-related translation keys have been successfully integrated:
ABAC_disabled_action_reason(MessageActions)ABAC_managed&ABAC_managed_description(RoomInfoABAC)abac_removed_user_from_the_room(message utils)ABAC_room_attributes(RoomInfoABAC)app/containers/RoomItem/RoomItem.tsx (1)
56-58: LGTM! Clean prop threading for ABAC attributes.The abacAttributes prop is correctly destructured and forwarded to TypeIcon in both render paths (expanded and collapsed). The implementation maintains consistency across both branches and aligns with the broader ABAC propagation pattern in the PR.
Also applies to: 116-116, 158-158
app/views/RoomInfoView/components/RoomInfoABAC.tsx (1)
35-44: LGTM! Proper key usage in mapped elements.The component correctly uses
attribute.keyandvalueas keys for the mapped elements, preventing React key warnings and ensuring proper reconciliation.app/containers/RoomTypeIcon/index.tsx (1)
53-55: LGTM! Correct ABAC icon precedence.The conditional logic properly prioritizes ABAC shield icons when
abacAttributesis present. The ternary selection betweenteam-shieldandhash-shieldbased onteamMainis appropriate and maintains consistency with the ABAC visual indicators across the app.app/containers/MessageActions/index.tsx (1)
464-465: LGTM! Consistent ABAC gating for Forward and Get link actions.The disabled logic and messaging for Forward and Get link actions are implemented consistently. These actions are appropriately blocked in ABAC-managed rooms with clear user feedback.
Also applies to: 474-475
app/i18n/locales/de.json (1)
15-19: German translations look reasonable.The ABAC-related German localization entries are placed correctly alphabetically and appear grammatically sound. The translations use appropriate technical terminology (e.g., "attributbasierte zugriffsgesteuerte Räume" for attribute-based access controlled rooms).
app/containers/RoomItem/TypeIcon.tsx (1)
7-7: LGTM! Clean prop forwarding.The abacAttributes prop is correctly added to the component signature and forwarded to RoomTypeIcon. The memoization is preserved, maintaining the performance optimization.
Also applies to: 17-17
app/views/RoomMembersView/components/ActionsSection.tsx (1)
93-93: LGTM! Appropriate ABAC gating for Invite_users.The "Invite_users" action is correctly hidden when
abacAttributesis present. Hiding the action entirely (rather than showing it as disabled) provides a cleaner UX for ABAC-managed rooms where this action is fundamentally not available.app/i18n/locales/ja.json (1)
11-15: Japanese translation verification required for ABAC terminology.The ABAC-related localization entries are correctly positioned alphabetically. However, verification of Japanese translation accuracy should be performed by a native speaker or localization specialist, particularly for technical terminology like "属性ベースのアクセス制御" (attribute-based access control) to ensure consistent and accurate rendering of security concepts.
app/containers/RoomHeader/RoomHeader.tsx (1)
10-10: LGTM! Both RoomTypeIcon render paths correctly updated.The changes properly propagate
abacAttributesthrough both rendering scenarios:
- Thread/discussion view (line 187, inside
renderFunc)- Default view (line 214, main render path)
Type consistency is maintained throughout with proper optional typing, and the optional prop ensures backward compatibility. The
abacAttributesis actively used in RoomTypeIcon's rendering logic to determine icon selection based on ABAC attributes presence.app/i18n/locales/nl.json (1)
11-15: LGTM! Dutch translations for ABAC features are well-integrated.The five new ABAC translation keys are properly formatted, alphabetically sorted, and appear grammatically correct. The translations appropriately cover the ABAC feature set: disabled action messaging, ABAC-managed room indicators, descriptions, system messages for user removal, and room attributes labeling.
|
|
||
| isLastOwner?: boolean; | ||
| federated?: boolean; | ||
| abacAttributes?: { key: string; values: string[] }[]; |
There was a problem hiding this comment.
Type inconsistency: values field differs between IRoom and ISubscription.
The values field is required in IServerRoom.abacAttributes but optional in ISubscription.abacAttributes (line 119 in ISubscription.ts shows values?: string[]). This inconsistency could lead to type mismatches when propagating ABAC attributes from rooms to subscriptions in mergeSubscriptionsRooms.ts (line 69).
Consider aligning the type definitions. If values can genuinely be undefined, make it optional in both interfaces:
- abacAttributes?: { key: string; values: string[] }[];
+ abacAttributes?: { key: string; values?: string[] }[];Or if it should always be present, make it required in both places.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| abacAttributes?: { key: string; values: string[] }[]; | |
| abacAttributes?: { key: string; values?: string[] }[]; |
🤖 Prompt for AI Agents
In app/definitions/IRoom.ts around line 236, the abacAttributes property
declares values as required (values: string[]), while
ISubscription.abacAttributes marks values optional; make these definitions
consistent to avoid type mismatches when merging room -> subscription data (see
mergeSubscriptionsRooms.ts line 69). Decide whether values can be undefined: if
yes, change IRoom.abacAttributes to values?: string[]; if no, change
ISubscription.abacAttributes to values: string[]; then update any call sites
(notably mergeSubscriptionsRooms.ts) to match the chosen contract and adjust
runtime handling (null/undefined checks or assume presence) accordingly.
|
Android Build Available Rocket.Chat Experimental 4.68.0.107889 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTOPSksAaxQ9XXHr7AIDrX2kWI2GW2S5XV04vyKbTlig_SS1BBSWffigcGwdty1sENJe2sBVsm4v0tVMEq4 |
|
iOS Build Available Rocket.Chat Experimental 4.68.0.107890 |
- Added `disabledReason` prop to IListItemButton and IListItemContent interfaces. - Implemented event emission for disabled buttons in ListItem. - Updated ActionsSection to conditionally disable the invite user action based on ABAC attributes.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Error
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/containers/List/ListItem.tsx (1)
229-242: Consider includingdisabledReasonin the accessibility label.The current implementation emits a toast when a disabled item with a
disabledReasonis pressed. However, users with screen readers won't know why the item is disabled until they press it. Consider appending thedisabledReasonto the accessibility label computation (lines 138-159) so assistive technology users can understand the disabled state without needing to press the item first.For example, in the
handleAcessibilityLabeluseMemo, you could add:if (disabled && disabledReason) { label = `${label} ${disabledReason}`; }This would improve the user experience for assistive technology users while maintaining the toast feedback for sighted users.
Based on learnings, for iOS VoiceOver, consider how this disabled-but-pressable pattern interacts with VoiceOver's expectations around interactive elements.
📜 Review details
Configuration used: Organization 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 selected for processing (4)
app/containers/List/ListItem.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomMembersView/components/ActionsSection.tsxapp/views/RoomView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- app/views/RoomActionsView/index.tsx
- app/views/RoomMembersView/components/ActionsSection.tsx
- app/views/RoomView/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-17T15:56:22.578Z
Learnt from: OtavioStasiak
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6499
File: app/containers/ServerItem/index.tsx:34-36
Timestamp: 2025-12-17T15:56:22.578Z
Learning: In the Rocket.Chat React Native codebase, for radio button components on iOS, include the selection state ("Selected"/"Unselected") in the accessibilityLabel instead of using accessibilityState={{ checked: hasCheck }}, because iOS VoiceOver has known issues with accessibilityRole="radio" + accessibilityState that prevent correct state announcement.
Applied to files:
app/containers/List/ListItem.tsx
🧬 Code graph analysis (1)
app/containers/List/ListItem.tsx (1)
app/containers/Toast.tsx (1)
LISTENER(24-24)
⏰ 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (3)
app/containers/List/ListItem.tsx (3)
21-22: LGTM!The imports are correctly structured to support toast message emission for disabled items.
95-95: LGTM!The
disabledReasonproperty is cleanly added to both interfaces as an optional string, maintaining backward compatibility.Also applies to: 219-219
160-168: LGTM!The expanded dependency array correctly includes all values referenced in the
useMemocomputation, ensuring the accessibility label updates properly when any input changes.
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/definitions/IRoom.ts (1)
241-241: Type inconsistency with ISubscription already flagged.A previous review comment identified that the
valuesfield is required here (values: string[]) but optional inISubscription.abacAttributes(values?: string[]). This inconsistency should be resolved to prevent type mismatches when merging room and subscription data.
🧹 Nitpick comments (1)
app/lib/methods/helpers/mergeSubscriptionsRooms.ts (1)
68-70: ABAC attributes merge logic is correct.The conditional merge only sets
abacAttributeswhen the property exists on the room object, which is appropriate for a new feature. The fallback to an empty array is consistent with how other array fields are handled in this file.Optional: Consider simplifying the condition
For consistency with similar fields elsewhere in the file, you could use optional chaining:
-if (room && 'abacAttributes' in room) { - mergedSubscription.abacAttributes = room.abacAttributes || []; -} +if (room?.abacAttributes !== undefined) { + mergedSubscription.abacAttributes = room.abacAttributes || []; +}This is clearer about the intent and matches patterns used for other optional fields.
📜 Review details
Configuration used: Organization 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)
app/containers/RoomItem/__snapshots__/RoomItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (36)
app/containers/RoomItem/RoomItem.stories.tsxapp/containers/RoomItem/RoomItem.tsxapp/containers/RoomItem/index.tsxapp/containers/RoomItem/interfaces.tsapp/definitions/IRoom.tsapp/definitions/ISubscription.tsapp/i18n/locales/ar.jsonapp/i18n/locales/bn-IN.jsonapp/i18n/locales/cs.jsonapp/i18n/locales/de.jsonapp/i18n/locales/en.jsonapp/i18n/locales/es.jsonapp/i18n/locales/fi.jsonapp/i18n/locales/fr.jsonapp/i18n/locales/hi-IN.jsonapp/i18n/locales/hu.jsonapp/i18n/locales/it.jsonapp/i18n/locales/ja.jsonapp/i18n/locales/nl.jsonapp/i18n/locales/nn.jsonapp/i18n/locales/no.jsonapp/i18n/locales/pt-BR.jsonapp/i18n/locales/pt-PT.jsonapp/i18n/locales/ru.jsonapp/i18n/locales/sl-SI.jsonapp/i18n/locales/sv.jsonapp/i18n/locales/ta-IN.jsonapp/i18n/locales/te-IN.jsonapp/i18n/locales/tr.jsonapp/i18n/locales/zh-CN.jsonapp/i18n/locales/zh-TW.jsonapp/lib/database/model/Subscription.jsapp/lib/database/model/migrations.jsapp/lib/database/schema/app.jsapp/lib/methods/helpers/mergeSubscriptionsRooms.tsapp/views/RoomView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (21)
- app/i18n/locales/bn-IN.json
- app/containers/RoomItem/interfaces.ts
- app/i18n/locales/ja.json
- app/i18n/locales/it.json
- app/i18n/locales/zh-CN.json
- app/i18n/locales/tr.json
- app/i18n/locales/en.json
- app/lib/database/schema/app.js
- app/i18n/locales/ru.json
- app/i18n/locales/hi-IN.json
- app/i18n/locales/zh-TW.json
- app/i18n/locales/sv.json
- app/i18n/locales/ar.json
- app/i18n/locales/te-IN.json
- app/i18n/locales/hu.json
- app/i18n/locales/pt-PT.json
- app/i18n/locales/es.json
- app/i18n/locales/sl-SI.json
- app/i18n/locales/ta-IN.json
- app/i18n/locales/nn.json
- app/containers/RoomItem/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/lib/database/model/Subscription.js (1)
app/lib/database/utils.ts (1)
sanitizer(19-19)
⏰ 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (15)
app/i18n/locales/fi.json (1)
15-19: LGTM! ABAC localization strings added correctly.The new ABAC-related translation keys are properly structured and follow the existing naming conventions in the file. The key naming is consistent: uppercase for UI labels (
ABAC_*) and lowercase for system messages (abac_removed_user_from_the_room), matching the pattern used elsewhere in the locale file.However, since I cannot verify the accuracy of the Finnish translations or the appropriateness of technical terminology, it would be ideal to have these translations reviewed by a native Finnish speaker or someone familiar with Finnish technical terminology to ensure quality and consistency with Finnish UI conventions.
app/i18n/locales/cs.json (1)
15-19: LGTM! ABAC localization keys added correctly.The five new Czech localization keys for ABAC functionality are properly structured and align with the PR objectives. The JSON syntax is valid, keys follow the existing naming conventions, and translations include appropriate Czech diacritics.
app/i18n/locales/fr.json (1)
11-15: LGTM! French translations for ABAC added correctly.The five new ABAC-related translation keys are well-formed, properly placed in alphabetical order, and the French translations appear accurate and consistent with the existing localization style.
app/i18n/locales/pt-BR.json (1)
15-19: LGTM! ABAC localization strings added correctly.The five new ABAC-related localization keys are properly formatted, alphabetically positioned, and align with the PR objectives for ABAC UI components and messaging. The JSON syntax is valid, and the keys follow existing naming conventions.
If translation accuracy has not been verified by a native Portuguese speaker, consider having these reviewed to ensure they convey the intended meaning correctly.
app/i18n/locales/nl.json (1)
11-15: JSON structure looks good; verify Dutch translations with a native speaker.The ABAC localization entries are properly formatted and correctly integrated into the file. The keys align with the PR objectives for ABAC features (disabled actions, badges, system messages, and room attributes).
However, since these are user-facing strings for a security feature, the accuracy of the Dutch translations should be verified by a native Dutch speaker or localization expert. This is particularly important for the longer description on line 13, which explains attribute-based access control to users.
Consider having a native Dutch speaker or professional localization service review these translations to ensure they are natural, grammatically correct, and accurately convey the ABAC concepts to Dutch-speaking users.
app/i18n/locales/de.json (1)
15-19: LGTM! German ABAC translations added correctly.The five new ABAC-related translation entries are properly formatted, maintain alphabetical ordering, and appear grammatically correct. The translations appropriately convey the ABAC feature concepts in German.
If your project has a translation review process or native German speakers, consider having them verify the phrasing aligns with your localization standards—particularly the description on Line 17, which uses compound technical terms like "attributbasierte zugriffsgesteuerte Räume."
app/i18n/locales/no.json (2)
15-19: LGTM! ABAC localization strings added correctly.The five new ABAC-related localization keys are properly formatted and complete. The key naming follows the established pattern: uppercase
ABAC_prefix for UI strings and lowercase for the system message (abac_removed_user_from_the_room), which is consistent with other system message keys in this file.
972-973: LGTM! Workspace localization added.The new
Your_workspacekey is properly added with appropriate Norwegian translation. The trailing comma on line 972 is valid JSON and follows best practices for easier future modifications.app/containers/RoomItem/RoomItem.stories.tsx (1)
74-75: LGTM! Story examples for ABAC attributes.The new story instances appropriately demonstrate the
abacAttributesprop with sample data, following the established pattern in the file.app/containers/RoomItem/RoomItem.tsx (1)
59-59: LGTM! Clean prop propagation.The
abacAttributesprop is correctly received and consistently forwarded toTypeIconin both display modes (expanded and condensed).Also applies to: 120-120, 171-171
app/views/RoomView/index.tsx (1)
547-547: LGTM!The abacAttributes prop is correctly passed from the subscription to RoomHeader. Since abacAttributes is optional in the ISubscription interface, passing it without null-checking is acceptable.
app/definitions/ISubscription.ts (1)
119-119: LGTM!The
abacAttributestype definition is clear and appropriate. Making it optional ensures backward compatibility with existing subscriptions.app/lib/database/model/migrations.js (1)
341-341: LGTM!The migration correctly adds the
abac_attributescolumn as optional, ensuring backward compatibility with existing databases.app/lib/database/model/Subscription.js (2)
156-156: LGTM!The
abacAttributesfield is correctly defined with the@jsondecorator and sanitizer, following the same pattern as other JSON-serialized fields in this model.
231-231: LGTM!The
abacAttributesfield is properly included in theasPlain()method, ensuring it's part of the plain object representation of the subscription.
diegolmello
force-pushed
the
feat.abac
branch
from
58a11a3 to
dee58ef
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Failure
diegolmello
temporarily deployed
to
experimental_android_build
GitHub Actions
Inactive
diegolmello
temporarily deployed
to
experimental_ios_build
GitHub Actions
Inactive
diegolmello
had a problem deploying
to
official_ios_build
GitHub Actions
Failure
diegolmello
had a problem deploying
to
official_android_build
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/i18n/locales/ta-IN.json (1)
15-19: Recommend professional Tamil translation verification.The five new ABAC translation keys have been added correctly with valid JSON syntax. However, as these user-facing strings will be displayed in production to Tamil-speaking users, consider having them reviewed by a native Tamil speaker or professional translator to ensure:
- Semantic accuracy and naturalness
- Consistency with existing Tamil terminology in the app
- Appropriate formality level for the context
- Correct use of technical terms (e.g., "ABAC", "attributes")
Would you like me to generate a verification script to check consistency of these keys across all locale files in the PR?
app/i18n/locales/zh-TW.json (1)
11-15: Minor spacing inconsistency in Traditional Chinese translation.Line 14 (
"被ABAC刪除") is missing spaces around "ABAC", while other lines consistently use spaces between Chinese characters and the English acronym (e.g.,"無法在 ABAC 管理的房間中使用"). Consider adding spaces for consistency:"被 ABAC 刪除".Suggested fix
- "abac_removed_user_from_the_room": "被ABAC刪除", + "abac_removed_user_from_the_room": "被 ABAC 刪除",app/i18n/locales/hu.json (1)
15-19: Hungarian ABAC translations look good!The five new Hungarian localization strings for ABAC features are well-translated and use proper Hungarian grammar and special characters (á, é, ő, ü). The translations appropriately convey the ABAC concepts for UI labels, descriptions, and system messages.
Optional: Consider alphabetical key ordering.
For maintainability, the keys could be reordered to follow strict alphabetical sorting. Currently,
abac_removed_user_from_the_room(line 18) appears beforeABAC_room_attributes(line 19), but in case-sensitive ASCII order, uppercase keys should precede lowercase keys.Suggested ordering
"ABAC_disabled_action_reason": "Nem érhető el ABAC által kezelt szobákban", "ABAC_managed": "ABAC által kezelt", "ABAC_managed_description": "Csak a megfelelő felhasználók férhetnek hozzá az attribútum alapú hozzáférés-vezérelt szobákhoz. Az attribútumok határozzák meg a szoba hozzáférését.", - "abac_removed_user_from_the_room": "eltávolítva az ABAC által", "ABAC_room_attributes": "Szoba attribútumok", + "abac_removed_user_from_the_room": "eltávolítva az ABAC által",Note: This assumes case-sensitive alphabetical sorting. If the project uses case-insensitive sorting or has a different convention, disregard this suggestion.
📜 Review details
Configuration used: Organization 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)
app/containers/RoomItem/__snapshots__/RoomItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (36)
app/containers/RoomItem/RoomItem.stories.tsxapp/containers/RoomItem/RoomItem.tsxapp/containers/RoomItem/index.tsxapp/containers/RoomItem/interfaces.tsapp/definitions/IRoom.tsapp/definitions/ISubscription.tsapp/i18n/locales/ar.jsonapp/i18n/locales/bn-IN.jsonapp/i18n/locales/cs.jsonapp/i18n/locales/de.jsonapp/i18n/locales/en.jsonapp/i18n/locales/es.jsonapp/i18n/locales/fi.jsonapp/i18n/locales/fr.jsonapp/i18n/locales/hi-IN.jsonapp/i18n/locales/hu.jsonapp/i18n/locales/it.jsonapp/i18n/locales/ja.jsonapp/i18n/locales/nl.jsonapp/i18n/locales/nn.jsonapp/i18n/locales/no.jsonapp/i18n/locales/pt-BR.jsonapp/i18n/locales/pt-PT.jsonapp/i18n/locales/ru.jsonapp/i18n/locales/sl-SI.jsonapp/i18n/locales/sv.jsonapp/i18n/locales/ta-IN.jsonapp/i18n/locales/te-IN.jsonapp/i18n/locales/tr.jsonapp/i18n/locales/zh-CN.jsonapp/i18n/locales/zh-TW.jsonapp/lib/database/model/Subscription.jsapp/lib/database/model/migrations.jsapp/lib/database/schema/app.jsapp/lib/methods/helpers/mergeSubscriptionsRooms.tsapp/views/RoomView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (22)
- app/i18n/locales/sv.json
- app/i18n/locales/fi.json
- app/i18n/locales/tr.json
- app/containers/RoomItem/RoomItem.stories.tsx
- app/i18n/locales/it.json
- app/i18n/locales/ar.json
- app/i18n/locales/ja.json
- app/i18n/locales/nn.json
- app/containers/RoomItem/index.tsx
- app/i18n/locales/zh-CN.json
- app/i18n/locales/de.json
- app/i18n/locales/te-IN.json
- app/views/RoomView/index.tsx
- app/i18n/locales/hi-IN.json
- app/lib/database/model/migrations.js
- app/i18n/locales/cs.json
- app/i18n/locales/fr.json
- app/i18n/locales/pt-PT.json
- app/i18n/locales/es.json
- app/definitions/ISubscription.ts
- app/containers/RoomItem/interfaces.ts
- app/definitions/IRoom.ts
🧰 Additional context used
🧬 Code graph analysis (1)
app/lib/database/model/Subscription.js (1)
app/lib/database/utils.ts (1)
sanitizer(19-19)
⏰ 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). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (13)
app/i18n/locales/pt-BR.json (1)
15-19: LGTM! Portuguese translations are accurate and well-formed.The five new ABAC-related translation keys provide appropriate Portuguese (Brazilian) localization for the feature. The translations are grammatically correct and accurately convey the intended meanings for ABAC-managed room indicators, action restrictions, system messages, and room attributes.
app/i18n/locales/nl.json (1)
11-15: LGTM! Consider verifying translations with a native Dutch speaker.The JSON structure is correct and the five new ABAC-related translation keys are properly integrated. The translations appear reasonable, but the longer description on Line 13 (
ABAC_managed_description) should ideally be verified by a native Dutch speaker to ensure it reads naturally and accurately conveys the intended meaning of attribute-based access control.Minor observation: There's a naming convention inconsistency between uppercase keys (
ABAC_disabled_action_reason,ABAC_managed, etc.) and the lowercase key (abac_removed_user_from_the_room). This may be intentional based on usage context (e.g., system messages vs. UI labels).app/i18n/locales/en.json (1)
15-19: ABAC i18n keys look good.The five new ABAC-related keys are properly ordered alphabetically and the values are clear. The lowercase
abac_removed_user_from_the_roomcorrectly follows the existing naming convention for system message keys (similar toroom_archived,room_avatar_changed, etc.).app/lib/database/schema/app.js (1)
74-74: LGTM! Schema column definition is correct.The new
abac_attributescolumn is properly defined as an optional string field, which is appropriate for storing JSON data in WatermelonDB. The alphabetical placement betweenfederatedandfederationmaintains consistency with the rest of the schema.app/lib/database/model/Subscription.js (2)
156-157: LGTM! Field decorator is correctly configured.The
@json('abac_attributes', sanitizer)decorator properly maps the database column to the model attribute. The use of thesanitizerfunction (pass-through) is consistent with other JSON fields in this model.
231-231: LGTM! Field properly included in serialization.The
abacAttributesfield is correctly added to theasPlain()method, maintaining alphabetical order and ensuring the attribute is included when the model is serialized.app/i18n/locales/sl-SI.json (1)
15-19: LGTM! ABAC translations properly added.The five new ABAC-related translation keys are correctly structured with valid JSON syntax, proper alphabetical placement, and consistent key naming conventions. The mix of uppercase (
ABAC_*) and lowercase (abac_*) follows the existing pattern where system message fragments use lowercase.app/i18n/locales/no.json (2)
15-19: LGTM! ABAC localization strings added correctly.The new ABAC-related localization keys follow the existing naming conventions and JSON structure. The Norwegian translations appear consistent with terminology used elsewhere in the file (e.g., "rom" for room, "attributter" for attributes).
972-973: LGTM! Workspace string added with correct formatting.The trailing comma on line 972 and the new "Your_workspace" entry follow proper JSON structure. The translation "Arbeidsområdet ditt" is consistent with the existing usage of "arbeidsområde" throughout the file for workspace-related strings.
app/containers/RoomItem/RoomItem.tsx (1)
59-59: LGTM! Clean prop propagation for ABAC support.The
abacAttributesprop is correctly added at line 59 and consistently passed toTypeIconin both the expanded (line 120) and collapsed (line 171) display modes. RoomTypeIcon uses the prop to determine the appropriate icon—when ABAC attributes are present, it displaysteam-shieldorhash-shielddepending on whether it's a team main, ensuring the ABAC logic is properly delegated to downstream components.app/lib/methods/helpers/mergeSubscriptionsRooms.ts (1)
68-70: LGTM! Backward-compatible ABAC attribute propagation.The implementation correctly propagates
abacAttributesfrom room to merged subscription using a guard pattern consistent with other optional room properties in this file. The fallback to an empty array is appropriate and ensures consumers receive a defined value.Type definitions in
ISubscriptionandIRoomalready include theabacAttributesproperty, and all consumers (RoomInfoABAC, RoomTypeIcon, MessageActions, etc.) properly handle both undefined and empty array cases.app/i18n/locales/ru.json (1)
15-19: The Russian translations for the ABAC keys are accurate and grammatically correct. They properly convey the English source meanings and use appropriate technical terminology ("атрибутивный контроль доступа" for attribute-based access control). No issues found.app/i18n/locales/bn-IN.json (1)
15-19: LGTM! Bengali ABAC translations added correctly.All five ABAC-related translation keys are properly formatted and alphabetically positioned. Verified that all keys exist in the English locale (en.json) with corresponding Bengali translations in bn-IN.json. These additions enable ABAC feature messaging for Bengali locale users.
diegolmello
had a problem deploying
to
upload_experimental_android
GitHub Actions
Failure
|
Android Build Available Rocket.Chat Experimental 4.68.0.107942 Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNQsweeOdACQBbuyeNhqNorcrg04fhlPfvwRJgoiRRjUhp0J4lDUmPKTr2hmHfxYCpP8h2QecMiZzIVdcE4E |
|
iOS Build Available Rocket.Chat Experimental 4.68.0.107943 |
2 participants
Proposed changes
This PR implements ABAC (Attribute-Based Access Control) support in the app. It adds UI components and functionality to display ABAC-managed room information, including room attributes, visual indicators, and appropriate restrictions for ABAC-managed rooms.
The main changes include:
RoomInfoABACcomponent that displays ABAC-managed room badge and room attributes in the Room Info viewhash-shieldfor channels,team-shieldfor teams`) to visually identify ABAC-managed roomsabacAttributesfield in subscriptionsIssue(s)
https://rocketchat.atlassian.net/browse/ABAC-65
How to test or reproduce
Room Info View (ABAC Display):
System Messages:
Action Restrictions:
Message Actions Restrictions:
Room List Indicators:
Screens affected:
Screenshots
Types of changes
Checklist
Further comments
This PR consolidates multiple ABAC-related features that were developed incrementally. The implementation follows the existing patterns in the codebase:
RoomInfoABAC,RoomInfoTag) that follow the existing RoomInfoView component patternsabacAttributesfield in subscriptionsISubscription,IRoom,IMessage) to include ABAC-related fieldsThe changes are backward compatible - rooms without ABAC attributes will continue to work as before, and the ABAC components will simply not render if no attributes are present.
Summary by CodeRabbit
New Features
UI Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.