fix: Users without preview permission subscribing to public channel stream #34922
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 |
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #34922 +/- ##
===========================================
+ Coverage 59.13% 59.18% +0.04%
===========================================
Files 2819 2819
Lines 67957 67762 -195
Branches 15138 15075 -63
===========================================
- Hits 40188 40102 -86
+ Misses 24935 24838 -97
+ Partials 2834 2822 -12
Flags with carried forward coverage won't be shown. Click here to find out more. |
| if (!hasPermission('preview-c-room') && room.t === 'c' && !subscription) { | ||
| return; | ||
| } | ||
|
|
||
| if (record.streamActive !== true) { |
There was a problem hiding this comment.
Shouldn't if (record.streamActive !== true) { check come first?
There was a problem hiding this comment.
No, because if stream not active we will have exactly what we are trying to avoid, load the stream.
In this case we have another check who comes first, and it's related to preview the public room with no subscriptions...
| @@ -85,11 +86,31 @@ const ComposerMessage = ({ tmid, onSend, ...props }: ComposerMessageProps): Reac | |||
| useCallback(() => LegacyRoomManager.getOpenedRoomByRid(room._id)?.streamActive ?? false, [room._id]), | |||
| ); | |||
|
|
|||
| if (!publicationReady) { | |||
| if (!publicationReady && hasPreviewPublicRoomPermission) { | |||
There was a problem hiding this comment.
What if I'm part of the room and I don't have this permission? Won't it break the loading composer?
There was a problem hiding this comment.
No, because publicationReady turns into true!
But if we proceed with the new design, the one without composer, we should be fine and remove this permission check!
| const subscribe = () => { | ||
| if (!hasPreviewPublicRoomPermission) { | ||
| // Get room subscription | ||
| LegacyRoomManager.open({ rid: room._id, typeName: room.t }); | ||
| LegacyRoomManager.computation.invalidate(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
useOpenRoom already does LegacyRoomManager.open. Also, why is this fn called subscribe?
There was a problem hiding this comment.
Yes, but a lot of other things too. And it does not do the computation.invalidate too...
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
CORE-855