-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Support read only messages #40010
Support read only messages #40010
Changes from 41 commits
7f4f512
297a231
cd7a95f
26bcce6
de29862
e2ad69d
471a02a
b4aed0f
0c1efc3
d72ceab
287fbe1
7adce5f
c386da0
8243a2b
3896162
af3b05f
09a118c
cd25155
15478c0
c5da50e
6b41cde
84c583b
1b7d32a
990e3cb
8fa54e6
a93647a
23b0ba0
ace67dc
517253c
c98a79f
68bcbd2
ab9b7f2
5eb36c8
a35ea06
cfc1231
a85b757
4a92c91
2f17417
ae9af7f
3512f7c
0c80ad7
9076bc0
62b193a
90a72e9
907167c
38cbb1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -7,6 +7,7 @@ import useTheme from '@hooks/useTheme'; | |||||
import useThemeStyles from '@hooks/useThemeStyles'; | ||||||
import getButtonState from '@libs/getButtonState'; | ||||||
import CONST from '@src/CONST'; | ||||||
import type IconAsset from '@src/types/utils/IconAsset'; | ||||||
import Hoverable from './Hoverable'; | ||||||
import Icon from './Icon'; | ||||||
import * as Expensicons from './Icon/Expensicons'; | ||||||
|
@@ -17,7 +18,13 @@ import Tooltip from './Tooltip'; | |||||
|
||||||
type BannerProps = { | ||||||
/** Text to display in the banner. */ | ||||||
text: string; | ||||||
text?: string; | ||||||
|
||||||
/** Content to display in the banner. */ | ||||||
content?: React.ReactNode; | ||||||
|
||||||
/** The icon asset to display to the left of the text */ | ||||||
icon?: IconAsset | null; | ||||||
|
||||||
/** Should this component render the left-aligned exclamation icon? */ | ||||||
shouldShowIcon?: boolean; | ||||||
|
@@ -41,7 +48,18 @@ type BannerProps = { | |||||
textStyles?: StyleProp<TextStyle>; | ||||||
}; | ||||||
|
||||||
function Banner({text, onClose, onPress, containerStyles, textStyles, shouldRenderHTML = false, shouldShowIcon = false, shouldShowCloseButton = false}: BannerProps) { | ||||||
function Banner({ | ||||||
text, | ||||||
content, | ||||||
icon = Expensicons.Exclamation, | ||||||
onClose, | ||||||
onPress, | ||||||
containerStyles, | ||||||
textStyles, | ||||||
shouldRenderHTML = false, | ||||||
shouldShowIcon = false, | ||||||
shouldShowCloseButton = false, | ||||||
}: BannerProps) { | ||||||
const theme = useTheme(); | ||||||
const styles = useThemeStyles(); | ||||||
const StyleUtils = useStyleUtils(); | ||||||
|
@@ -65,15 +83,17 @@ function Banner({text, onClose, onPress, containerStyles, textStyles, shouldRend | |||||
]} | ||||||
> | ||||||
<View style={[styles.flexRow, styles.flexGrow1, styles.mw100, styles.alignItemsCenter]}> | ||||||
{shouldShowIcon && ( | ||||||
{shouldShowIcon && icon && ( | ||||||
<View style={[styles.mr3]}> | ||||||
<Icon | ||||||
src={Expensicons.Exclamation} | ||||||
src={icon} | ||||||
fill={StyleUtils.getIconFillColor(getButtonState(shouldHighlight))} | ||||||
/> | ||||||
</View> | ||||||
)} | ||||||
{shouldRenderHTML ? ( | ||||||
{content && content} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NAB: Should we use a Boolean here? I feel like it looks a bit more readable that way
Suggested change
|
||||||
|
||||||
{shouldRenderHTML && text ? ( | ||||||
<RenderHTML html={text} /> | ||||||
) : ( | ||||||
<Text | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2763,6 +2763,16 @@ export default { | |
}, | ||
copyReferralLink: 'Copy invite link', | ||
}, | ||
onboardingBottomMessage: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I propose to name this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done - 62b193a. |
||
[CONST.INTRO_CHOICES.MANAGE_TEAM]: { | ||
phrase1: 'Chat with your setup specialist in ', | ||
phrase2: ' for help', | ||
}, | ||
default: { | ||
phrase1: 'Message ', | ||
phrase2: ' for help with setup', | ||
}, | ||
}, | ||
violations: { | ||
allTagLevelsRequired: 'All tags required', | ||
autoReportedRejectedExpense: ({rejectReason, rejectedBy}: ViolationsAutoReportedRejectedExpenseParams) => `${rejectedBy} rejected this expense with the comment "${rejectReason}"`, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3256,6 +3256,16 @@ export default { | |
}, | ||
copyReferralLink: 'Copiar enlace de invitación', | ||
}, | ||
onboardingBottomMessage: { | ||
[CONST.INTRO_CHOICES.MANAGE_TEAM]: { | ||
phrase1: 'Chatea con tu especialista asignado en ', | ||
phrase2: ' para obtener ayuda', | ||
}, | ||
default: { | ||
phrase1: 'Envía un email a ', | ||
phrase2: ' para obtener ayuda con la configuración', | ||
}, | ||
Comment on lines
+3260
to
+3267
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would check the style does not go crazy in Spanish because I think the translations is slightly longer |
||
}, | ||
violations: { | ||
allTagLevelsRequired: 'Todas las etiquetas son obligatorias', | ||
autoReportedRejectedExpense: ({rejectedBy, rejectReason}: ViolationsAutoReportedRejectedExpenseParams) => `${rejectedBy} rechazó la solicitud y comentó "${rejectReason}"`, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1173,10 +1173,25 @@ function isJoinRequestInAdminRoom(report: OnyxEntry<Report>): boolean { | |
return ReportActionsUtils.isActionableJoinRequestPending(report.reportID); | ||
} | ||
|
||
/** | ||
* Checks if the user can write in the provided report | ||
*/ | ||
function canWriteInReport(report: OnyxEntry<Report>): boolean { | ||
if (Array.isArray(report?.permissions) && report?.permissions.length > 0) { | ||
return report?.permissions?.includes(CONST.REPORT.PERMISSIONS.WRITE); | ||
} | ||
|
||
return true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If permissions are not an array or the array length is 0, I think this should return false? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think yes, because it is a new property and I am not sure if it is applied to all kinds of reports. If we return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Web, we do not return the permissions and we dont return it from the getChats method hence in OpenApp (which uses that method) we also do not include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We however return this in the OpenReport using the getStructuredOnyxReportAndLastMessageData So actually the only one I found that does not have the permissions is the selfDM 🤔 although I see the report should be shared with read, write permissions cc @thienlnam There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah this is because the user owns the report and is so is not in the sharedReports table. Maybe we can an owner check and then populate those permissions in the response |
||
} | ||
|
||
/** | ||
* Checks if the current user is allowed to comment on the given report. | ||
*/ | ||
function isAllowedToComment(report: OnyxEntry<Report>): boolean { | ||
if (!canWriteInReport(report)) { | ||
return false; | ||
} | ||
|
||
// Default to allowing all users to post | ||
const capability = report?.writeCapability ?? CONST.REPORT.WRITE_CAPABILITIES.ALL; | ||
|
||
|
@@ -5375,7 +5390,7 @@ function canUserPerformWriteAction(report: OnyxEntry<Report>) { | |
return false; | ||
} | ||
|
||
return !isArchivedRoom(report) && isEmptyObject(reportErrors) && report && isAllowedToComment(report) && !isAnonymousUser; | ||
return !isArchivedRoom(report) && isEmptyObject(reportErrors) && report && isAllowedToComment(report) && !isAnonymousUser && canWriteInReport(report); | ||
} | ||
|
||
/** | ||
|
@@ -6282,7 +6297,6 @@ export { | |
getParsedComment, | ||
getParticipantAccountIDs, | ||
getParticipants, | ||
getPayeeName, | ||
getPendingChatMembers, | ||
getPersonalDetailsForAccountID, | ||
getPolicyDescriptionText, | ||
|
@@ -6317,6 +6331,7 @@ export { | |
getWorkspaceChats, | ||
getWorkspaceIcon, | ||
goBackToDetailsPage, | ||
getPayeeName, | ||
hasActionsWithErrors, | ||
hasAutomatedExpensifyAccountIDs, | ||
hasExpensifyGuidesEmails, | ||
|
@@ -6404,6 +6419,7 @@ export { | |
isValidReport, | ||
isValidReportIDFromPath, | ||
isWaitingForAssigneeToCompleteTask, | ||
canWriteInReport, | ||
navigateToDetailsPage, | ||
navigateToPrivateNotes, | ||
parseReportRouteParams, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -210,6 +210,7 @@ function ReportScreen({ | |
isOptimisticReport: reportProp?.isOptimisticReport, | ||
lastMentionedTime: reportProp?.lastMentionedTime, | ||
avatarUrl: reportProp?.avatarUrl, | ||
permissions: reportProp?.permissions, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When a user signs-in via google after viewing a public room, the |
||
}), | ||
[ | ||
reportProp?.lastReadTime, | ||
|
@@ -249,6 +250,7 @@ function ReportScreen({ | |
reportProp?.isOptimisticReport, | ||
reportProp?.lastMentionedTime, | ||
reportProp?.avatarUrl, | ||
reportProp?.permissions, | ||
], | ||
); | ||
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I propose to name this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done - 9076bc0. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
import React, {useMemo} from 'react'; | ||
import {withOnyx} from 'react-native-onyx'; | ||
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; | ||
import Banner from '@components/Banner'; | ||
import * as Expensicons from '@components/Icon/Expensicons'; | ||
import Text from '@components/Text'; | ||
import TextLink from '@components/TextLink'; | ||
import useLocalize from '@hooks/useLocalize'; | ||
import useThemeStyles from '@hooks/useThemeStyles'; | ||
import * as PolicyUtils from '@libs/PolicyUtils'; | ||
import Navigation from '@navigation/Navigation'; | ||
import * as ReportInstance from '@userActions/Report'; | ||
import type {OnboardingPurposeType} from '@src/CONST'; | ||
import CONST from '@src/CONST'; | ||
import ONYXKEYS from '@src/ONYXKEYS'; | ||
import ROUTES from '@src/ROUTES'; | ||
import type {Policy as PolicyType, Report} from '@src/types/onyx'; | ||
|
||
type OnboardingReportFooterMessageOnyxProps = { | ||
/** Saved onboarding purpose selected by the user */ | ||
choice: OnyxEntry<OnboardingPurposeType>; | ||
|
||
/** Collection of reports */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which reports? All users reports? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the app is looking for an admin chat to show in the banner. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is another way: to the active policyID by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about this approach - 90a72e9? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a problem when the user creates a new account, the active policy does not exist yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test.mp4 |
||
reports: OnyxCollection<Report>; | ||
|
||
/** The list of this user's policies */ | ||
policies: OnyxCollection<PolicyType>; | ||
}; | ||
|
||
type OnboardingReportFooterMessageProps = OnboardingReportFooterMessageOnyxProps; | ||
rezkiy37 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
function OnboardingReportFooterMessage({choice, reports, policies}: OnboardingReportFooterMessageProps) { | ||
const {translate} = useLocalize(); | ||
const styles = useThemeStyles(); | ||
|
||
const adminChatReport = useMemo(() => { | ||
const adminsReports = Object.values(reports ?? {}).filter((report) => report?.chatType === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS); | ||
const activePolicies = Object.values(policies ?? {}).filter((policy) => PolicyUtils.shouldShowPolicy(policy, false)); | ||
|
||
return adminsReports.find((report) => activePolicies.find((policy) => policy?.id === report?.policyID)); | ||
}, [policies, reports]); | ||
|
||
const content = useMemo(() => { | ||
switch (choice) { | ||
case CONST.ONBOARDING_CHOICES.MANAGE_TEAM: | ||
return ( | ||
<> | ||
{translate('onboardingBottomMessage.newDotManageTeam.phrase1')} | ||
<TextLink onPress={() => Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(adminChatReport?.reportID ?? ''))}> | ||
{adminChatReport?.reportName ?? CONST.REPORT.WORKSPACE_CHAT_ROOMS.ADMINS} | ||
</TextLink> | ||
{translate('onboardingBottomMessage.newDotManageTeam.phrase2')} | ||
</> | ||
); | ||
default: | ||
return ( | ||
<> | ||
{translate('onboardingBottomMessage.default.phrase1')} | ||
<TextLink onPress={() => ReportInstance.navigateToConciergeChat()}>{CONST?.CONCIERGE_CHAT_NAME}</TextLink> | ||
{translate('onboardingBottomMessage.default.phrase2')} | ||
</> | ||
); | ||
} | ||
}, [adminChatReport?.reportName, adminChatReport?.reportID, choice, translate]); | ||
|
||
return ( | ||
<Banner | ||
containerStyles={[styles.archivedReportFooter]} | ||
shouldShowIcon | ||
icon={Expensicons.Lightbulb} | ||
content={<Text suppressHighlighting>{content}</Text>} | ||
/> | ||
); | ||
} | ||
|
||
OnboardingReportFooterMessage.displayName = 'OnboardingReportFooterMessage'; | ||
|
||
export default withOnyx<OnboardingReportFooterMessageProps, OnboardingReportFooterMessageOnyxProps>({ | ||
choice: { | ||
key: ONYXKEYS.ONBOARDING_PURPOSE_SELECTED, | ||
}, | ||
reports: { | ||
key: ONYXKEYS.COLLECTION.REPORT, | ||
}, | ||
policies: { | ||
key: ONYXKEYS.COLLECTION.POLICY, | ||
}, | ||
})(OnboardingReportFooterMessage); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,7 @@ import type * as OnyxTypes from '@src/types/onyx'; | |
import type {OriginalMessageActionableMentionWhisper, OriginalMessageActionableTrackedExpenseWhisper, OriginalMessageJoinPolicyChangeLog} from '@src/types/onyx/OriginalMessage'; | ||
import {isEmptyObject} from '@src/types/utils/EmptyObject'; | ||
import AnimatedEmptyStateBackground from './AnimatedEmptyStateBackground'; | ||
import {RestrictedReadOnlyContextMenuActions} from './ContextMenu/ContextMenuActions'; | ||
import MiniReportActionContextMenu from './ContextMenu/MiniReportActionContextMenu'; | ||
import * as ReportActionContextMenu from './ContextMenu/ReportActionContextMenu'; | ||
import {hideContextMenu} from './ContextMenu/ReportActionContextMenu'; | ||
|
@@ -914,6 +915,7 @@ function ReportActionItem({ | |
originalReportID={originalReportID ?? ''} | ||
isArchivedRoom={ReportUtils.isArchivedRoom(report)} | ||
displayAsGroup={displayAsGroup} | ||
disabledActions={!ReportUtils.canWriteInReport(report) ? RestrictedReadOnlyContextMenuActions : []} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coming from #43562, we also missed adding those disabledActions in the context menu. |
||
isVisible={hovered && draftMessage === undefined && !hasErrors} | ||
draftMessage={draftMessage} | ||
isChronosReport={ReportUtils.chatIncludesChronos(originalReport)} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -187,6 +187,8 @@ type Report = OnyxCommon.OnyxValueWithOfflineFeedback< | |
transactionThreadReportID?: string; | ||
|
||
fieldList?: Record<string, PolicyReportField>; | ||
|
||
permissions?: Array<ValueOf<typeof CONST.REPORT.PERMISSIONS>>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be a comment here |
||
}, | ||
PolicyReportField['fieldID'] | ||
>; | ||
|
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.
Why did this become optional? A banner without text sounds weird.
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.
That's because I've added the
content
property for a case when I need to pass a react node, not a simple text.