Fix: Restrict Pin Icon Visibility Based on User Permissions #674
Conversation
SinghaAnirban005
force-pushed
the
tool
branch
from
eb828e3 to
43807a7
Compare
|
Hey @SinghaAnirban005 your code is not working properly for admin user, check this |
|
Check if it's due to alphabet case? |
|
meaning? |
|
lower/upper case difference while comparing |
| @@ -56,6 +60,7 @@ export const useRCAuth = () => { | |||
| setIsTotpModalOpen(false); | |||
| setEmailorUser(null); | |||
| setPassword(null); | |||
| setUserPinPermissions(permissions.update[150]); | |||
There was a problem hiding this comment.
Hey @SinghaAnirban005, I think it should be permissions.update[146] according to the pr - #520
There was a problem hiding this comment.
The 150th index returns the pin permissions correctly
There was a problem hiding this comment.
okay then what is on 146th??
There was a problem hiding this comment.
check and try pr 520
There was a problem hiding this comment.
i am getting this log on 146
There was a problem hiding this comment.
Ohh so might this permissions array got changed later
There was a problem hiding this comment.
Yes exactly
we are setting the permissions for pin messages according to the user roles, so how this lower/upper case difference comparison came, could you please explain? I still don't get it. |
| @@ -67,6 +69,7 @@ export const MessageToolbox = ({ | |||
| setShowDeleteModal(false); | |||
| }; | |||
|
|
|||
| const isAllowedToPin = userRoles.some((role) => pinRoles.has(role)); | |||
There was a problem hiding this comment.
At this point, we are comparing two different roles. There is a possibility that one of the rules is in a different case. (Maybe returned from api in different case)
It's just a possibility, but I wanted to check.
I'm not sure if that's the issue.
The problem might lie in fetching the wrong PIN permission list,
or it could be that the fetch operation somehow fails.
There was a problem hiding this comment.
Okay so you mean, the roles in the entire roles array?
There was a problem hiding this comment.
Suppose in userRole, my role is Admin
In roles array, in pin permission, the role is admin
Now Admin != admin
Hence check fails
There was a problem hiding this comment.
Hey @Spiral-Memory when I was researching for the solution of this issue, I somehow fetched the entire roles array from the permissions response and in that I have seen all the roles were having same case all small
There was a problem hiding this comment.
Ohh ok then,
Try figuring out the issue and let me know
There was a problem hiding this comment.
@devanshkansagra @Spiral-Memory
In my POV , the code seems to work well for all user groups including admin , the reason why it might not have worked is that when pin roles are changed by admin in the panel, changes are instantaneously not updated in the user's permission store . Thus a user needs to logout and re-login in order to see changes in permissions
There was a problem hiding this comment.
rather than doing logout and login again is it possible to view changes on refreshing the page?
There was a problem hiding this comment.
i can try fixing it !!
Brief Title
Restrict Pin Icon Visibility Based on User Permissions
Acceptance Criteria fulfillment
Fixes #672
Video/Screenshots
Case 1: When non-priviledged users are granted permission to pin/unpin messages
Permissions.-.Singha.-.Brave.2024-11-30.23-37-09.mp4
Case 2: When non-priviledged users are not granted permission to pin/unpin messages
Permissions.-.Singha.-.Brave.2024-12-01.00-19-14.mp4
PR Test Details
Note: The PR will be ready for live testing at https://rocketchat.github.io/EmbeddedChat/pulls/pr-674 after approval. Contributors are requested to replace
<pr_number>with the actual PR number.