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

Filter out assigned guides from the policy members #16338

Merged
merged 6 commits into from
Mar 29, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
11 changes: 11 additions & 0 deletions src/libs/PolicyUtils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import _ from 'underscore';
import lodashGet from 'lodash/get';
import Str from 'expensify-common/lib/str';
import CONST from '../CONST';
import ONYXKEYS from '../ONYXKEYS';

Expand Down Expand Up @@ -75,10 +76,20 @@ function shouldShowPolicy(policy, isOffline) {
);
}

/**
* @param {string} email
* @returns {boolean}
*/
function isExpensifyTeam(email) {
const emailDomain = Str.extractEmailDomain(email);
return emailDomain === CONST.EXPENSIFY_PARTNER_NAME || emailDomain === CONST.EMAIL.GUIDES_DOMAIN;
}

export {
hasPolicyMemberError,
hasPolicyError,
hasCustomUnitsError,
getPolicyBrickRoadIndicatorStatus,
shouldShowPolicy,
isExpensifyTeam,
};
32 changes: 23 additions & 9 deletions src/pages/workspace/WorkspaceMembersPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import * as ReportUtils from '../../libs/ReportUtils';
import FormHelpMessage from '../../components/FormHelpMessage';
import TextInput from '../../components/TextInput';
import KeyboardDismissingFlatList from '../../components/KeyboardDismissingFlatList';
import withCurrentUserPersonalDetails from '../../components/withCurrentUserPersonalDetails';
import * as PolicyUtils from '../../libs/PolicyUtils';

const propTypes = {
/** The personal details of the person who is logged in */
Expand Down Expand Up @@ -235,11 +237,11 @@ class WorkspaceMembersPage extends React.Component {
}

/**
* @param {String} value
* @param {String} [value = '']
* @param {String} keyword
* @returns {Boolean}
*/
isKeywordMatch(value, keyword) {
isKeywordMatch(value = '', keyword) {
return value.trim().toLowerCase().includes(keyword);
}

Expand Down Expand Up @@ -323,6 +325,19 @@ class WorkspaceMembersPage extends React.Component {
|| this.isKeywordMatch(member.firstName, searchValue)
|| this.isKeywordMatch(member.lastName, searchValue));

// eslint-disable-next-line arrow-body-style
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: We already have other arrow-body-style functions, so I think this is not needed.

Suggested change
// eslint-disable-next-line arrow-body-style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a disable next line not disable for the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're right it's not needed though because the style is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That comment is for when you want to use a curly brace for style reasons but the linter wants you to do it all on one line.

data = _.reject(data, (member) => {
// If this policy is owned by Expensify then show all support (expensify.com or team.expensify.com) emails
if (PolicyUtils.isExpensifyTeam(lodashGet(this.props.policy, 'owner'))) {
return;
}
Comment on lines +329 to +332
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused a "bug" #17252. The root cause was internal, just noting here for reference. If we delete the workspace the owner would be undefined and the PolicyUtils.isExpensifyTeam will crash.


// We don't want to show guides as policy members unless the user is not a guide. Some customers get confused when they
// see random people added to their policy, but guides having access to the policies help set them up.
const isCurrentUserExpensifyTeam = PolicyUtils.isExpensifyTeam(this.props.currentUserPersonalDetails.login);
return !isCurrentUserExpensifyTeam && PolicyUtils.isExpensifyTeam(member.login);
});

_.each(data, (member) => {
if (member.login === this.props.session.email || member.login === this.props.policy.owner || member.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) {
return;
Expand Down Expand Up @@ -414,13 +429,11 @@ class WorkspaceMembersPage extends React.Component {
/>
</View>
) : (
!_.isEmpty(policyMemberList) && (
<View style={[styles.ph5]}>
<Text style={[styles.textLabel, styles.colorMuted]}>
{this.props.translate('common.noResultsFound')}
</Text>
</View>
)
<View style={[styles.ph5]}>
<Text style={[styles.textLabel, styles.colorMuted]}>
{this.props.translate('common.noResultsFound')}
</Text>
</View>
)}
</View>
</FullPageNotFoundView>
Expand All @@ -446,4 +459,5 @@ export default compose(
key: ONYXKEYS.SESSION,
},
}),
withCurrentUserPersonalDetails,
)(WorkspaceMembersPage);