feat(bot-discord): sala empresarial private rooms#450
Open
gvieira18 wants to merge 6 commits into
Open
Conversation
Add /sala-empresarial, converting a /sala-tracked voice room into a private company room by stamping Discord permission overwrites: deny @everyone and allow the selected Empresa Parceira's Partner Role for connect, speak, use_vad, send_messages and read_message_history. The last two lock the room's built-in text chat; VIEW_CHANNEL is left untouched (visible-but-locked) and MENTION_EVERYONE keeps its category inheritance. The decision (guards + role resolution + overwrite plan) lives in a pure, unit-tested ConfigureEmpresarialRoomAction; the command edge only performs the setPermissions I/O. Gating is by Partner Role membership, not room ownership, so any company member can privatize the room. Partners are a flat bot-discord.roles.partners registry (slug => role id); onboarding a new partner is one env + one line. Closes #448
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app-modules/bot-discord/docs/adr/0001-sala-empresarial-role-gated-private-rooms.md`:
- Around line 16-17: Update the ADR statement describing MENTION_EVERYONE to say
the permission remains inherited or unchanged, rather than guaranteeing it is
denied. Keep the existing VIEW_CHANNEL and visible-but-locked behavior
description intact.
In `@app-modules/bot-discord/src/SlashCommands/SalaEmpresarialCommand.php`:
- Around line 86-113: Update applyOverwrites to remove existing configured
partner-role permission overwrites before applying the selected
plan->partnerRoleId permissions. Preserve the everyone overwrite and the
selected partner role, and ensure re-stamping a room replaces the previous
partner access rather than accumulating roles.
- Around line 108-113: In the permission-update flow, reorder the operations so
the $partnerRole setPermissions call and its await complete before the
$everyoneRole denial. Keep both existing permission payloads unchanged, ensuring
a failed partner-role write does not leave the channel globally locked.
- Around line 106-117: Defer the interaction ephemerally at the start of the try
block in SalaEmpresarialCommand before either setPermissions call. Keep both
permission updates and await operations afterward, then complete the deferred
interaction with the existing success response rather than attempting a new
initial reply.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: fb8cda84-cb5e-4fe1-a588-55e94ad60573
📒 Files selected for processing (10)
app-modules/bot-discord/CONTEXT.mdapp-modules/bot-discord/config/bot-discord.phpapp-modules/bot-discord/docs/adr/0001-sala-empresarial-role-gated-private-rooms.mdapp-modules/bot-discord/src/Actions/VoiceChannel/ConfigureEmpresarialRoomAction.phpapp-modules/bot-discord/src/DTO/EmpresarialOverwritePlan.phpapp-modules/bot-discord/src/DTO/EmpresarialRoomDecision.phpapp-modules/bot-discord/src/Enums/EmpresarialRejectionReason.phpapp-modules/bot-discord/src/SlashCommands/SalaEmpresarialCommand.phpapp-modules/bot-discord/tests/Unit/Actions/VoiceChannel/ConfigureEmpresarialRoomActionTest.phpapp-modules/bot-discord/tests/Unit/Enums/EmpresarialRejectionReasonTest.php
Apply the partner-role allow overwrite before the @everyone deny so a failed partner write leaves the room public instead of fully locked (deny-first could strand everyone out of the room). Also reword ADR-0001: the command leaves MENTION_EVERYONE and VIEW_CHANNEL unchanged rather than guaranteeing MENTION_EVERYONE is denied — it keeps whatever the category inherits.
Deny VIEW_CHANNEL to @everyone (and allow it for the Partner Role) so a Sala Empresarial is fully private: non-members no longer see the room or its occupant list, and any non-partner connected at conversion time is force-disconnected by Discord.
Contributor
|
Dúvida legítima: Visto que uma pessoa sem a tag não pode entrar na sala, mas um admin ou alguma outra tag que permita movimentar membros entre as salas consegue colocar alguem sem a tag da partner dentro da sala? Pois sem que comandos admin sobrepoem qualquer checgame. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app-modules/bot-discord/docs/adr/0001-sala-empresarial-role-gated-private-rooms.md (1)
36-40: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftRevoke stale partner-role access when changing companies.
The command adds the newly selected role’s allow overwrite but does not remove a previous partner-role overwrite. Reconfiguring
brdto3pcan therefore leave both companies with access, contradicting “private for the selected company.” Remove stale configured partner-role overwrites or reject reconfiguration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app-modules/bot-discord/docs/adr/0001-sala-empresarial-role-gated-private-rooms.md` around lines 36 - 40, Update the empresarial room reconfiguration behavior described in the ADR so selecting a new company removes any previously configured partner-role overwrites before applying the selected role’s allow overwrite, preserving access for only the current company; alternatively, explicitly reject reconfiguration. Align the stateless/private-room description with the chosen behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app-modules/bot-discord/docs/adr/0001-sala-empresarial-role-gated-private-rooms.md`:
- Around line 14-19: Resolve the conflicting VIEW_CHANNEL behavior across the
ADR, overwrite plan, command implementation, tests, and documentation: choose
whether the command denies it for `@everyone` or leaves it inherited/unchanged,
then make every affected artifact consistently implement and assert that choice,
including room discoverability and occupant visibility.
- Around line 48-51: Update the privacy guarantees in this ADR, including the
statements around channel visibility and non-member disconnection, to explicitly
exclude Discord members with the ADMINISTRATOR permission. Clarify that
administrators can bypass the `@everyone` VIEW_CHANNEL deny and may see or access
empresarial rooms, or revise the design to enforce privacy beyond guild
permission checks.
---
Outside diff comments:
In
`@app-modules/bot-discord/docs/adr/0001-sala-empresarial-role-gated-private-rooms.md`:
- Around line 36-40: Update the empresarial room reconfiguration behavior
described in the ADR so selecting a new company removes any previously
configured partner-role overwrites before applying the selected role’s allow
overwrite, preserving access for only the current company; alternatively,
explicitly reject reconfiguration. Align the stateless/private-room description
with the chosen behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 182c01ab-a371-41ab-8115-9e92ded60ba2
📒 Files selected for processing (4)
app-modules/bot-discord/CONTEXT.mdapp-modules/bot-discord/docs/adr/0001-sala-empresarial-role-gated-private-rooms.mdapp-modules/bot-discord/src/DTO/EmpresarialOverwritePlan.phpapp-modules/bot-discord/tests/Unit/Actions/VoiceChannel/ConfigureEmpresarialRoomActionTest.php
🚧 Files skipped from review as they are similar to previous changes (3)
- app-modules/bot-discord/CONTEXT.md
- app-modules/bot-discord/src/DTO/EmpresarialOverwritePlan.php
- app-modules/bot-discord/tests/Unit/Actions/VoiceChannel/ConfigureEmpresarialRoomActionTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/sala-empresarial empresa:<slug>, converting a/sala-tracked voice room into a private company room by stamping Discord permission overwrites —deny @everyoneandallowthe selected Empresa Parceira's Partner Role forconnect,speak,use_vad,send_messagesandread_message_history(voice + the room's built-in text chat).VIEW_CHANNELstays untouched (visible-but-locked) andMENTION_EVERYONEkeeps category inheritance.ConfigureEmpresarialRoomAction; the command edge only performs thesetPermissionsI/O.not-in-tracked-room/unknown-company/missing-partner-role, all ephemeral).bot-discord.roles.partnersregistry (slug => role id), seeded withbrdand3p; onboarding a new partner is one env + one config line.Notes
send_messageswas added after a live server test showed outsiders could still post in the room's text chat./sala-tracked rooms are eligible andDynamicVoiceTaskauto-deletes them when empty, so overwrites die with the channel. No revert command.Test plan
/salaroom →/sala-empresarial empresa:brdprivatizes it; outsiders can't connect, speak, or post in chat, but the room stays visible./salaroom (permanent channel or no voice) → ephemeral rejection.Closes #448
Screenshots
Ephemeral replies, captured on a test guild (
/sala-empresarial).Company picker — choices built from the
partnersregistry (BRD,3P):Success — room privatized for BRD:
Rejected — caller doesn't hold the selected company's Partner Role (
missing-partner-role):Rejected — caller isn't inside a
/sala-tracked room (not-in-tracked-room):