-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(ABAC): Show system message when a member is removed #6760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat.abac
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request adds support for a new system message type Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ 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 |
There was a problem hiding this 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
🧹 Nitpick comments (5)
app/i18n/locales/cs.json (1)
15-15: Optional wording tweak for naturalness in Czech.Current: “byl odstraněn pomocí ABAC” can read like “removed using ABAC”. Consider “byl odstraněn systémem ABAC” or “byl odebrán systémem ABAC” for agentive “by”.
app/i18n/locales/ru.json (1)
15-15: Russian phrasing: avoid apostrophe and make gender‑neutral.Suggest “удалён(а) политикой ABAC” for naturalness and neutrality.
- "abac_removed_user_from_the_room": "был удалён ABAC'ом", + "abac_removed_user_from_the_room": "удалён(а) политикой ABAC",app/i18n/locales/ja.json (1)
11-11: Optional JP style tweak.Current is fine. If you prefer the common formal style: “ABACにより削除されました”.
- "abac_removed_user_from_the_room": "ABACによって削除されました", + "abac_removed_user_from_the_room": "ABACにより削除されました",app/i18n/locales/fr.json (1)
11-11: French gender agreement.Consider “supprimé(e)” to cover neutral agreement after the username.
- "abac_removed_user_from_the_room": "a été supprimé par ABAC", + "abac_removed_user_from_the_room": "a été supprimé(e) par ABAC",app/i18n/locales/pt-BR.json (1)
15-15: Portuguese gender neutrality (optional).If you want to avoid gender bias after the username, consider “removido(a)”.
- "abac_removed_user_from_the_room": "foi removido pelo ABAC", + "abac_removed_user_from_the_room": "foi removido(a) pelo ABAC",
📜 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)
app/containers/message/__snapshots__/Message.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (28)
app/containers/message/Message.stories.tsx(2 hunks)app/containers/message/utils.ts(2 hunks)app/definitions/IMessage.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(1 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)
⏰ 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). (1)
- GitHub Check: ESLint and Test / run-eslint-and-test
🔇 Additional comments (24)
app/i18n/locales/fi.json (1)
15-15: ABAC removal string added — LGTM.Translation reads correctly and matches system-message style.
app/i18n/locales/ar.json (1)
11-11: ABAC removal string added — LGTM.Wording fits “was removed by ABAC”; no punctuation, as expected.
app/i18n/locales/it.json (1)
15-15: ABAC removal string added — LGTM.Consistent with other system messages.
app/i18n/locales/es.json (1)
11-11: ABAC removal string added — LGTM.Matches expected pattern; no terminal punctuation.
app/i18n/locales/nn.json (1)
10-10: ABAC removal string added — LGTM.Natural phrasing; consistent style.
app/i18n/locales/sl-SI.json (1)
15-15: ABAC removal string added — LGTM.Reads naturally; consistent with system-message pattern.
app/i18n/locales/tr.json (1)
11-11: LGTM (TR).Translation is clear and idiomatic.
app/i18n/locales/hi-IN.json (1)
15-15: LGTM (HI).Natural and consistent with system message style.
app/i18n/locales/hu.json (1)
15-15: LGTM (HU).Clear and idiomatic.
app/i18n/locales/te-IN.json (1)
15-15: Translation shape LGTM.Reads correctly after a username; formatting is consistent.
app/i18n/locales/nl.json (1)
11-11: Dutch entry looks good.Correct post-username wording; consistent with other system messages.
app/i18n/locales/ta-IN.json (1)
15-15: Tamil entry OK.Works grammatically after a username; no JSON issues spotted.
app/i18n/locales/zh-CN.json (1)
11-11: Simplified Chinese entry OK.Natural after username; style matches other info messages.
app/i18n/locales/zh-TW.json (1)
11-11: Traditional Chinese entry OK.Reads correctly appended to a username; formatting consistent.
app/i18n/locales/no.json (1)
15-15: Norwegian entry looks good.Post-username wording is correct; consistent with existing system message tone.
app/i18n/locales/sv.json (1)
15-15: ABAC system message key verified — all wiring is consistent.Type-to-key mapping confirmed:
'abac-removed-user-from-room'routes correctly to'abac_removed_user_from_the_room'ingetInfoMessage, the type is defined inMessageTypesValues, the Swedish translation exists, and both Storybook variants cover the new message type.app/i18n/locales/de.json (1)
15-15: LGTM! Translation added correctly.The German translation for the new ABAC removal system message has been added in the correct alphabetical position and follows the existing format.
app/i18n/locales/bn-IN.json (1)
15-15: LGTM! Bengali translation added correctly.The translation key is properly positioned and follows the established i18n pattern.
app/containers/message/Message.stories.tsx (2)
1965-1966: LGTM! Story entries added for new system message types.Both the new
abac-removed-user-from-roomsystem message and theunsupportedfallback example have been added correctly. These story entries will help document and visualize the new message type in Storybook.
2008-2009: LGTM! Large font variants added consistently.The large font story variants mirror the standard stories correctly, ensuring the new message types render properly across different font scales.
app/i18n/locales/pt-PT.json (1)
11-11: LGTM! Portuguese translation added correctly.The translation is properly positioned alphabetically and follows the established i18n structure.
app/containers/message/utils.ts (2)
78-79: LGTM! Message type correctly added to author name list.Adding
'abac-removed-user-from-room'to themessagesWithAuthorNamearray is consistent with other system messages that display the author's name in the UI, even when the author name isn't interpolated into the message text itself.
163-164: LGTM! Translation handler added correctly.The new case returns the appropriate translation key without dynamic interpolation, which is consistent with other simple system messages like
'room_archived'and'room_unarchived'. The implementation follows the established pattern.app/definitions/IMessage.ts (1)
253-254: LGTM! Message type added to union correctly.The new
'abac-removed-user-from-room'literal type has been added to theMessageTypesValuesunion, ensuring type safety throughout the codebase. The placement at the end of the union is appropriate.
| "A11y_in_app_notification": "New message from {{name}}: {{message}}", | ||
| "A11y_incoming_call_dismiss": "Dismiss", | ||
| "A11y_incoming_call_swipe_down_to_view_options": "Swipe down to view options", | ||
| "abac_removed_user_from_the_room": "was removed by ABAC", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
ABAC removal base string added — LGTM.
English baseline looks good and matches web-style phrasing.
Run to verify coverage and wiring:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Ensure every locale defines abac_removed_user_from_the_room"
missing=0
for f in $(fd -t f . app/i18n/locales -e json | sort); do
if ! jq -e 'has("abac_removed_user_from_the_room")' "$f" >/dev/null; then
echo "Missing in: $f"
missing=1
fi
done
test $missing -eq 0 && echo "All locales have the key."
echo "2) Sanity-check spacing/casing (no leading/trailing spaces; lower-case start)"
issues=0
while IFS= read -r f; do
val=$(jq -r '."abac_removed_user_from_the_room" // empty' "$f")
[[ -z "$val" ]] && continue
if [[ "$val" =~ ^[[:space:]]|[[:space:]]$ ]]; then
echo "Trim spaces in: $f => '$val'"
issues=1
fi
done < <(fd -t f . app/i18n/locales -e json | sort)
test $issues -eq 0 && echo "No leading/trailing spaces found."
echo "3) Check type mapping and message utils"
rg -n "abac-removed-user-from-room|abac_removed_user_from_the_room" app/definitions/IMessage.ts app/containers/message/utils.ts app/containers/message/Message.stories.tsxLength of output: 1783
ABAC removal string is incomplete — missing from 41+ locale files.
The key abac_removed_user_from_the_room is only present in en.json but is missing from all other locales (af, az, bas-CM, be-BY, bg, bn-BD, bs, ca, cy, da, de-AT, el, eo, et, eu, fa, gl, he, hr, id, ka-GE, kg, km, ko, ku, ln, lo, lt, lv, mn, ms-MY, pl, ro, si, sk-SK, sq, sr, th-TH, ug, uk, vi-VN, zh-HK, and others).
Add the key and appropriate translations to all locale files under app/i18n/locales/.
🤖 Prompt for AI Agents
In app/i18n/locales/en.json around line 15, the key
"abac_removed_user_from_the_room" is present but missing across 41+ other locale
files; add this key to every file under app/i18n/locales/ (e.g., af, az, bas-CM,
be-BY, bg, bn-BD, bs, ca, cy, da, de-AT, el, eo, et, eu, fa, gl, he, hr, id,
ka-GE, kg, km, ko, ku, ln, lo, lt, lv, mn, ms-MY, pl, ro, si, sk-SK, sq, sr,
th-TH, ug, uk, vi-VN, zh-HK, etc.) with appropriate translations matching the
English meaning "was removed by ABAC"; ensure each locale file uses its existing
JSON formatting, place the new key alongside similar room/user message keys, and
run a JSON validation/lint pass to confirm no syntax errors.
OtavioStasiak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Proposed changes
abac-removed-user-from-roomsystem messageunsupported system messageIssue(s)
https://rocketchat.atlassian.net/browse/ABAC-66
How to test or reproduce
Screenshots
Types of changes
Checklist
Further comments
Summary by CodeRabbit