fix: make readonly permission reactive in RoomView#6804
fix: make readonly permission reactive in RoomView#6804deepak0x wants to merge 2 commits intoRocketChat:developfrom
Conversation
- Modified isReadOnly to accept postReadOnlyPermission as optional parameter - Updated RoomView to get permission from Redux via mapStateToProps - Added permission change detection in shouldComponentUpdate - Component now updates immediately when post-readonly permission changes
WalkthroughAdds an optional Changes
Sequence DiagramsequenceDiagram
participant Redux as Redux State
participant Component as RoomView (mapStateToProps)
participant Helper as isReadOnly()
Redux->>Component: postReadOnlyPermission (state.permissions['post-readonly'])
Component->>Component: shouldComponentUpdate checks prop change
rect rgb(240,248,255)
Note over Component,Helper: Read-only evaluation (new flow)
Component->>Helper: setReadOnly(..., postReadOnlyPermission)
Helper->>Helper: canPostReadOnly(postReadOnlyPermission?)
alt permission provided
Helper-->>Component: use provided permission → determine RO
else fallback
Helper->>Redux: read permission snapshot
Helper-->>Component: determine RO from snapshot
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/views/RoomView/index.tsx (1)
574-579: PassingpostReadOnlyPermissionintoisReadOnlyis correct; consider avoiding redundantsetStateWiring
postReadOnlyPermissionthrough toisReadOnlyhere is the key to making the composer reactive to live permission changes and aligns with the helper’s new signature.If you want to avoid scheduling redundant updates, you could add a small guard so state is only updated when
readOnlyactually changes:setReadOnly = async () => { const { room } = this.state; - const { user, postReadOnlyPermission } = this.props; - const readOnly = await isReadOnly(room as ISubscription, user.username as string, postReadOnlyPermission); - this.setState({ readOnly }); + const { user, postReadOnlyPermission } = this.props; + const readOnly = await isReadOnly(room as ISubscription, user.username as string, postReadOnlyPermission); + if (readOnly !== this.state.readOnly) { + this.setState({ readOnly }); + } };This isn’t required for correctness but trims unnecessary
setStatecalls.
📜 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 selected for processing (3)
app/lib/methods/helpers/isReadOnly.ts(1 hunks)app/views/RoomView/definitions.ts(1 hunks)app/views/RoomView/index.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/lib/methods/helpers/isReadOnly.ts (2)
app/definitions/ISubscription.ts (1)
ISubscription(40-119)app/lib/methods/helpers/helpers.ts (1)
hasPermission(94-118)
app/views/RoomView/index.tsx (2)
app/lib/methods/helpers/isReadOnly.ts (1)
isReadOnly(17-36)app/definitions/ISubscription.ts (1)
ISubscription(40-119)
🔇 Additional comments (4)
app/views/RoomView/definitions.ts (1)
14-36:postReadOnlyPermissionprop wiring looks consistentThe new optional
postReadOnlyPermission?: string[]aligns with the existing permission props onIRoomViewPropsand with how it’s mapped from Redux inRoomView. No issues here.app/views/RoomView/index.tsx (3)
248-287:shouldComponentUpdatecorrectly reacts topostReadOnlyPermissionchangesIncluding
postReadOnlyPermissionin the props comparison (withdequal) ensures that Redux permission updates will no longer be blocked byshouldComponentUpdate, allowing the component to rerender and recomputereadOnlywhen the permission changes. This matches the PR’s intent.
316-319:componentDidUpdatecomment matches the intended read‑only refresh behaviorThe updated comments clarify that
setReadOnlyis called after every update so the read‑only state is always derived from the latest room and permission data. GivenshouldComponentUpdatealready filters out no‑op state updates, this is acceptable from a behavior standpoint.
1570-1588: Redux mapping forpostReadOnlyPermissionis consistent with other permissionsMapping
postReadOnlyPermission: state.permissions['post-readonly']mirrors how the other permission props are sourced and matches theIRoomViewPropstype. This cleanly closes the loop from Redux → props →isReadOnly.
Add nullish coalescing to handle case where hasPermission returns undefined on error, preventing runtime crash when accessing permission[0]
When an admin grants the "post-readonly" permission while a user is in a read-only room, the composer stays disabled until they navigate away or restart the app. The UI doesn't update immediately.
Issue(s)
Closes #6802
Solution
isReadOnlyto accept the permission as a parameter instead of reading it statically.Changes
isReadOnlyto acceptpostReadOnlyPermissionas an optional parameter.shouldComponentUpdate.Summary by CodeRabbit