Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
48f0ad2
Scaffold field for restricting commenting capabilities
amyevans May 16, 2023
8e0a27f
Call API to update write capability setting
amyevans May 16, 2023
18d7341
Use CONST values for capability options
amyevans May 16, 2023
4db2308
Add spanish translations
amyevans May 16, 2023
ab2075d
Hide composer if user is not allowed to post
amyevans May 16, 2023
45ca540
Restrict editing of the field to admins
amyevans May 16, 2023
3766b5a
Show the setting as view-only for non-admins
amyevans May 17, 2023
94d450a
Merge branch 'main' into amy-admins-only
amyevans May 17, 2023
eaa3b93
Remove task related check
amyevans May 17, 2023
daf53e3
Fix prettier style
amyevans May 17, 2023
5cebc09
Address PR feedback
amyevans May 17, 2023
c9d1631
Additional improvements per PR reviews
amyevans May 17, 2023
33f9ce2
Merge branch 'main' into amy-admins-only
amyevans May 18, 2023
c146e0f
Refactor to push to page (WIP)
amyevans May 18, 2023
ca68968
Continue refactor: update page to reference correct values
amyevans May 18, 2023
d6324d2
Remove label from options list
amyevans May 18, 2023
2a27962
Improve comment and JS Doc
amyevans May 18, 2023
a789f27
Prevent crash due to undefined translation
amyevans May 18, 2023
2a245b8
Merge branch 'main' into amy-admins-only
amyevans May 19, 2023
2596a29
Fix bug where default option isn't highlighted
amyevans May 19, 2023
326340a
Map over CONST values for better stability
amyevans May 19, 2023
6d71a73
Pass policies as prop
amyevans May 19, 2023
aec2afb
Implement additional code review feedback
amyevans May 23, 2023
d039b46
Merge branch 'main' into amy-admins-only
amyevans May 23, 2023
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
5 changes: 5 additions & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,11 @@ const CONST = {
DAILY: 'daily',
ALWAYS: 'always',
},
// Options for which room members can post
WRITE_CAPABILITIES: {
Comment thread
amyevans marked this conversation as resolved.
ALL: 'all',
ADMINS: 'admins',
},
VISIBILITY: {
PUBLIC: 'public',
PUBLIC_ANNOUNCE: 'public_announce',
Expand Down
2 changes: 2 additions & 0 deletions src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,11 @@ export default {
REPORT_SETTINGS: 'r/:reportID/settings',
REPORT_SETTINGS_ROOM_NAME: 'r/:reportID/settings/room-name',
REPORT_SETTINGS_NOTIFICATION_PREFERENCES: 'r/:reportID/settings/notification-preferences',
REPORT_SETTINGS_WRITE_CAPABILITY: 'r/:reportID/settings/who-can-post',
getReportSettingsRoute: (reportID) => `r/${reportID}/settings`,
getReportSettingsRoomNameRoute: (reportID) => `r/${reportID}/settings/room-name`,
getReportSettingsNotificationPreferencesRoute: (reportID) => `r/${reportID}/settings/notification-preferences`,
getReportSettingsWriteCapabilityRoute: (reportID) => `r/${reportID}/settings/who-can-post`,
TRANSITION_FROM_OLD_DOT: 'transition',
VALIDATE_LOGIN: 'v/:accountID/:validateCode',
GET_ASSISTANCE: 'get-assistance/:taskID',
Expand Down
7 changes: 7 additions & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,13 @@ export default {
`This workspace chat is no longer active because ${displayName} is no longer a member of the ${policyName} workspace.`,
[CONST.REPORT.ARCHIVE_REASON.POLICY_DELETED]: ({policyName}) => `This workspace chat is no longer active because ${policyName} is no longer an active workspace.`,
},
writeCapabilityPage: {
label: 'Who can post',
writeCapability: {
all: 'All members',
admins: 'Admins only',
},
},
sidebarScreen: {
fabAction: 'New chat',
newChat: 'New chat',
Expand Down
7 changes: 7 additions & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,13 @@ export default {
`Este chat de espacio de trabajo esta desactivado porque ${displayName} ha dejado de ser miembro del espacio de trabajo ${policyName}.`,
[CONST.REPORT.ARCHIVE_REASON.POLICY_DELETED]: ({policyName}) => `Este chat de espacio de trabajo esta desactivado porque el espacio de trabajo ${policyName} se ha eliminado.`,
},
writeCapabilityPage: {
label: 'Quién puede postear',
writeCapability: {
all: 'Todos los miembros',
admins: 'Solo administradores',
},
},
sidebarScreen: {
fabAction: 'Nuevo chat',
newChat: 'Nuevo chat',
Expand Down
7 changes: 7 additions & 0 deletions src/libs/Navigation/AppNavigator/ModalStackNavigators.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,13 @@ const ReportSettingsModalStackNavigator = createModalStackNavigator([
},
name: 'Report_Settings_Notification_Preferences',
},
{
getComponent: () => {
const WriteCapabilityPage = require('../../../pages/settings/Report/WriteCapabilityPage').default;
return WriteCapabilityPage;
},
name: 'Report_Settings_Write_Capability',
},
]);

const TaskModalStackNavigator = createModalStackNavigator([
Expand Down
3 changes: 3 additions & 0 deletions src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,9 @@ export default {
Report_Settings_Notification_Preferences: {
path: ROUTES.REPORT_SETTINGS_NOTIFICATION_PREFERENCES,
},
Report_Settings_Write_Capability: {
path: ROUTES.REPORT_SETTINGS_WRITE_CAPABILITY,
},
},
},
NewGroup: {
Expand Down
21 changes: 21 additions & 0 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,26 @@ function getPolicyName(report) {
return policy.name || report.oldPolicyName || Localize.translateLocal('workspace.common.unavailable');
}

/**
* Checks if the current user is allowed to comment on the given report.
* @param {Object} report
* @param {String} [report.writeCapability]
* @returns {Boolean}
*/
function isAllowedToComment(report) {
// Default to allowing all users to post
const capability = lodashGet(report, 'writeCapability', CONST.REPORT.WRITE_CAPABILITIES.ALL) || CONST.REPORT.WRITE_CAPABILITIES.ALL;

if (capability === CONST.REPORT.WRITE_CAPABILITIES.ALL) {
return true;
}

// If we've made it here, commenting on this report is restricted.
// If the user is an admin, allow them to post.
const policy = allPolicies[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`];
return lodashGet(policy, 'role', '') === CONST.POLICY.ROLE.ADMIN;
}

/**
* Checks if the current user is the admin of the policy given the policy expense chat.
* @param {Object} report
Expand Down Expand Up @@ -2303,5 +2323,6 @@ export {
shouldReportShowSubscript,
isReportDataReady,
isSettled,
isAllowedToComment,
getMoneyRequestAction,
};
30 changes: 30 additions & 0 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,35 @@ function updateNotificationPreferenceAndNavigate(reportID, previousValue, newVal
Navigation.drawerGoBack(ROUTES.getReportSettingsRoute(reportID));
}

/**
* @param {Object} report
* @param {String} newValue
*/
function updateWriteCapabilityAndNavigate(report, newValue) {
if (report.writeCapability === newValue) {
Navigation.drawerGoBack(ROUTES.getReportSettingsRoute(report.reportID));
return;
}

const optimisticData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`,
value: {writeCapability: newValue},
},
];
const failureData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`,
value: {writeCapability: report.writeCapability},
},
];
API.write('UpdateReportWriteCapability', {reportID: report.reportID, writeCapability: newValue}, {optimisticData, failureData});
// Return to the report settings page since this field utilizes push-to-page
Navigation.drawerGoBack(ROUTES.getReportSettingsRoute(report.reportID));
}

/**
* Navigates to the 1:1 report with Concierge
*/
Expand Down Expand Up @@ -1639,6 +1668,7 @@ export {
addComment,
addAttachment,
reconnect,
updateWriteCapabilityAndNavigate,
updateNotificationPreferenceAndNavigate,
subscribeToReportTypingEvents,
unsubscribeFromReportChannel,
Expand Down
1 change: 1 addition & 0 deletions src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ class ReportScreen extends React.Component {
report={this.props.report}
isComposerFullSize={this.props.isComposerFullSize}
onSubmitComment={this.onSubmitComment}
policies={this.props.policies}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is necessary. Correct me if I'm not seeing this properly, but I think the only reason that policies is passed here is so that a policy can be found and passed to ReportUtils.isAllowedToComment(). If that's the case, ReportUtils is already connected to allPolicies and isAllowedToComment() can do it's own logic to find the linked workspace.

A very good reason not to pass around policies like this is that if any property on any policy changes, it will cause all <ReportFooter>s to re-render. Probably not a big deal in this case since it looks like it's only rendered once, but something to think about for the future.

/>
</>
)}
Expand Down
3 changes: 2 additions & 1 deletion src/pages/home/report/ReportFooter.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ class ReportFooter extends React.Component {

render() {
const isArchivedRoom = ReportUtils.isArchivedRoom(this.props.report);
const hideComposer = isArchivedRoom || !_.isEmpty(this.props.errors);
const isAllowedToComment = ReportUtils.isAllowedToComment(this.props.report);
const hideComposer = isArchivedRoom || !_.isEmpty(this.props.errors) || !isAllowedToComment;

return (
<>
Expand Down
3 changes: 3 additions & 0 deletions src/pages/reportPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,7 @@ export default PropTypes.shape({

/** The status of the current report */
statusNum: PropTypes.oneOf(_.values(CONST.REPORT.STATUS)),

/** Which user role is capable of posting messages on the report */
writeCapability: PropTypes.oneOf(_.values(CONST.REPORT.WRITE_CAPABILITIES)),
Comment thread
Beamanator marked this conversation as resolved.
});
26 changes: 26 additions & 0 deletions src/pages/settings/Report/ReportSettingsPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ class ReportSettingsPage extends Component {
const linkedWorkspace = _.find(this.props.policies, (policy) => policy && policy.id === this.props.report.policyID);
const shouldDisableRename = this.shouldDisableRename(linkedWorkspace) || ReportUtils.isThread(this.props.report);
const notificationPreference = this.props.translate(`notificationPreferencesPage.notificationPreferences.${this.props.report.notificationPreference}`);
const writeCapability = this.props.report.writeCapability || CONST.REPORT.WRITE_CAPABILITIES.ALL;
const writeCapabilityText = this.props.translate(`writeCapabilityPage.writeCapability.${writeCapability}`);
const shouldAllowWriteCapabilityEditing = lodashGet(linkedWorkspace, 'role', '') === CONST.POLICY.ROLE.ADMIN;

return (
<ScreenWrapper>
Expand Down Expand Up @@ -131,6 +134,29 @@ class ReportSettingsPage extends Component {
)}
</OfflineWithFeedback>
)}
{shouldAllowWriteCapabilityEditing ? (
<MenuItemWithTopDescription
shouldShowRightIcon
title={writeCapabilityText}
description={this.props.translate('writeCapabilityPage.label')}
onPress={() => Navigation.navigate(ROUTES.getReportSettingsWriteCapabilityRoute(this.props.report.reportID))}
/>
) : (
<View style={[styles.ph5, styles.pv3]}>
<Text
style={[styles.textLabelSupporting, styles.lh16, styles.mb1]}
numberOfLines={1}
>
{this.props.translate('writeCapabilityPage.label')}
</Text>
<Text
numberOfLines={1}
style={[styles.optionAlternateText, styles.pre]}
>
{writeCapabilityText}
</Text>
</View>
)}
<View style={[styles.ph5]}>
{Boolean(linkedWorkspace) && (
<View style={[styles.pv3]}>
Expand Down
67 changes: 67 additions & 0 deletions src/pages/settings/Report/WriteCapabilityPage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import React from 'react';
import _ from 'underscore';
import CONST from '../../../CONST';
import ScreenWrapper from '../../../components/ScreenWrapper';
import HeaderWithCloseButton from '../../../components/HeaderWithCloseButton';
import withLocalize, {withLocalizePropTypes} from '../../../components/withLocalize';
import styles from '../../../styles/styles';
import OptionsList from '../../../components/OptionsList';
import Navigation from '../../../libs/Navigation/Navigation';
import compose from '../../../libs/compose';
import withReportOrNotFound from '../../home/report/withReportOrNotFound';
import reportPropTypes from '../../reportPropTypes';
import ROUTES from '../../../ROUTES';
import * as Report from '../../../libs/actions/Report';
import * as Expensicons from '../../../components/Icon/Expensicons';
import themeColors from '../../../styles/themes/default';

const propTypes = {
...withLocalizePropTypes,

/** The report for which we are setting write capability */
report: reportPropTypes.isRequired,
};
const greenCheckmark = {src: Expensicons.Checkmark, color: themeColors.success};

const WriteCapabilityPage = (props) => {
const writeCapabilityOptions = _.map(CONST.REPORT.WRITE_CAPABILITIES, (value) => ({
value,
text: props.translate(`writeCapabilityPage.writeCapability.${value}`),
keyForList: value,

// Include the green checkmark icon to indicate the currently selected value
customIcon: value === (props.report.writeCapability || CONST.REPORT.WRITE_CAPABILITIES.ALL) ? greenCheckmark : null,

// This property will make the currently selected value have bold text
boldStyle: value === (props.report.writeCapability || CONST.REPORT.WRITE_CAPABILITIES.ALL),
}));

return (
<ScreenWrapper includeSafeAreaPaddingBottom={false}>
<HeaderWithCloseButton
title={props.translate('writeCapabilityPage.label')}
shouldShowBackButton
onBackButtonPress={() => Navigation.navigate(ROUTES.getReportSettingsRoute(props.report.reportID))}
onCloseButtonPress={() => Navigation.dismissModal(true)}
/>
<OptionsList
sections={[{data: writeCapabilityOptions}]}
onSelectRow={(option) => Report.updateWriteCapabilityAndNavigate(props.report, option.value)}
hideSectionHeaders
optionHoveredStyle={{
...styles.hoveredComponentBG,
...styles.mhn5,
...styles.ph5,
}}
shouldHaveOptionSeparator
shouldDisableRowInnerPadding
contentContainerStyles={[styles.ph5]}
/>
</ScreenWrapper>
);
};

WriteCapabilityPage.displayName = 'WriteCapabilityPage';
WriteCapabilityPage.propTypes = propTypes;

export default compose(withLocalize, withReportOrNotFound)(WriteCapabilityPage);