-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: Don't try to create a non-encrypted discussion if the parent room is encrypted #38279
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: develop
Are you sure you want to change the base?
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 |
|
WalkthroughThis PR adds support for creating encrypted discussions with an initial message by detecting when the parent room is encrypted, propagating that status to the CreateDiscussion component, and disabling the encryption toggle accordingly. It includes API field exposure, component logic enhancements, test coverage, and integration updates across multiple action handlers. Changes
Sequence DiagramsequenceDiagram
actor User
participant CreateDiscussion
participant RoomAutoComplete
participant API
participant Form
User->>CreateDiscussion: Open create discussion modal
CreateDiscussion->>CreateDiscussion: Initialize with encryptedParentRoom prop
User->>RoomAutoComplete: Select parent room
RoomAutoComplete->>API: Fetch room details (with encrypted field)
API-->>RoomAutoComplete: Return room with encrypted status
RoomAutoComplete->>CreateDiscussion: onParentRoomChange callback
CreateDiscussion->>CreateDiscussion: Detect if room is encrypted
alt Encrypted Parent Room
CreateDiscussion->>Form: Set encrypted toggle disabled
CreateDiscussion->>Form: Sync encrypted value with parent status
end
CreateDiscussion->>RoomAutoComplete: Display encryption icon if needed
User->>CreateDiscussion: Submit form
CreateDiscussion->>API: Create discussion with encrypted + message
API-->>User: Discussion created successfully
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38279 +/- ##
===========================================
+ Coverage 70.74% 70.83% +0.09%
===========================================
Files 3142 3144 +2
Lines 108929 109338 +409
Branches 19631 19621 -10
===========================================
+ Hits 77064 77455 +391
- Misses 29865 29885 +20
+ Partials 2000 1998 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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.
No issues found across 8 files
abhinavkrin
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.
The endpoint rooms.createDiscussion does allow creating unencrypted discussions for encrypted rooms. It accepts a field encrypted which if not provided defaults to the parent room encryption status.
So, if the endpoint allows it, IMO, we should not prevent it from being done in the UI.
Currently the UI is always creating an encrypted discussion, just to be sure, lets confirm with product what is the expected behavior, since FE and BE have discordant behaviors |




Proposed changes (including videos or screenshots)
Issue(s)
SUP-955
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.