Skip to content
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

[Fix] Pin icon should not be visible to users without pin permission (revised) #549

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions packages/react/src/components/ChatHeader/ChatHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,29 +155,44 @@ const ChatHeader = ({
setShowSearch(false);
}, [setShowMentions, setShowSearch]);

const authenticatedUserRoles = useUserStore((state) => state.roles);
const setCanPinMsg = useUserStore((state) => state.setCanPinMsg);

useEffect(() => {
const getMessageLimit = async () => {
const messageLimitObj = await RCInstance.getMessageLimit();
setMessageLimit(messageLimitObj?.value);
};

const setMessageAllowed = async () => {
const setMsgAndPinAllowed = async (ro) => {
const permissionRes = await RCInstance.permissionInfo();
const channelRolesRes = await RCInstance.getChannelRoles(
isChannelPrivate
);

if (permissionRes.success && channelRolesRes.success) {
const pinMsgRoles = permissionRes.update[146]?.roles || [];
const postMsgRoles = permissionRes.update[140]?.roles || [];

const userRoles = channelRolesRes.roles
const channelMemberRoles = channelRolesRes.roles
.filter((chRole) => chRole.u?._id === authenticatedUserId)
.flatMap((chRole) => chRole.roles);

const canSendMsg =
userRoles.length > 0 &&
postMsgRoles.some((role) => userRoles.includes(role));
setCanSendMsg(canSendMsg);
const concatenatedRoles = channelMemberRoles.concat(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use shorthand here, destructure and combine. I have raised a PR feat: Addition of chatinfo to display status, have a look here..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Spiral-Memory, can you share that pr number?

Copy link
Contributor

@devanshkansagra devanshkansagra Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Spiral-Memory, if we destructure and combine, the users are unable to login

authenticatedUserRoles
);

if (ro) {
const canSendMsg =
channelMemberRoles.length > 0 &&
postMsgRoles.some((role) => channelMemberRoles.includes(role));
setCanSendMsg(canSendMsg);
}

const canPinMsg =
concatenatedRoles.length > 0 &&
pinMsgRoles.some((role) => concatenatedRoles.includes(role));
setCanPinMsg(canPinMsg);
}
};

Expand All @@ -186,7 +201,7 @@ const ChatHeader = ({
if (res.success) {
setChannelInfo(res.room);
if (res.room.t === 'p') setIsChannelPrivate(true);
if (res.room.ro) setMessageAllowed();
setMsgAndPinAllowed(res.room.ro);
} else if (
'errorType' in res &&
res.errorType === 'error-room-not-found'
Expand Down Expand Up @@ -221,7 +236,9 @@ const ChatHeader = ({
toastPosition,
isChannelPrivate,
setCanSendMsg,
setCanPinMsg,
authenticatedUserId,
authenticatedUserRoles,
setMessageLimit,
]);

Expand Down
1 change: 1 addition & 0 deletions packages/react/src/components/Message/Message.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ const Message = ({
{!message.t && showToolbox ? (
<MessageToolbox
message={message}
RCInstance={RCInstance}
Copy link
Collaborator

@Spiral-Memory Spiral-Memory Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this added ? Any specific reason?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Spiral-Memory the adding of RCInstance as a prop was unnecessary, after removing it I have seen no change

isEditing={editMessage._id === message._id}
authenticatedUserId={authenticatedUserId}
handleOpenThread={handleOpenThread}
Expand Down
8 changes: 6 additions & 2 deletions packages/react/src/components/Message/MessageToolbox.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useState, useEffect, useRef } from 'react';
import { css } from '@emotion/react';
import Popup from 'reactjs-popup';
import useComponentOverrides from '../../theme/useComponentOverrides';
Expand All @@ -11,6 +11,7 @@ import { Icon } from '../Icon';
import { Button } from '../Button';
import { parseEmoji } from '../../lib/emoji';
import { Tooltip } from '../Tooltip';
import { useUserStore } from '../../store';

const MessageToolboxWrapperCss = css`
display: none;
Expand Down Expand Up @@ -48,6 +49,7 @@ const popupStyle = {
export const MessageToolbox = ({
className = '',
message,
RCInstance,
style = {},
isThreadMessage = false,
authenticatedUserId,
Expand Down Expand Up @@ -79,6 +81,8 @@ export const MessageToolbox = ({
setShowDeleteModal(true);
};

const canPinMsg = useUserStore((state) => state.canPinMsg);

return (
<>
<Box css={MessageToolboxWrapperCss}>
Expand Down Expand Up @@ -141,7 +145,7 @@ export const MessageToolbox = ({
}}
/>
</Popup>
{!isThreadMessage && (
{!isThreadMessage && canPinMsg && (
<Tooltip text={message.pinned ? 'Unpin' : 'Pin'} position="top">
<ActionButton
ghost
Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/store/userStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ const useUserStore = create((set) => ({
})),
isUserAuthenticated: false,
canSendMsg: true,
canPinMsg: true,
setIsUserAuthenticated: (isUserAuthenticated) =>
set(() => ({ isUserAuthenticated })),
setCanSendMsg: (canSendMsg) => set(() => ({ canSendMsg })),
setCanPinMsg: (canPinMsg) => set(() => ({ canPinMsg })),
password: null,
setPassword: (password) => set(() => ({ password })),
emailoruser: null,
Expand Down
Loading