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.

Sign up for GitHub

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

Jump to bottom

Remove policy expense chat beta #24579

Merged
merged 9 commits into from
Aug 23, 2023
Merged

Remove policy expense chat beta #24579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c9e2d65
Remove policyExpenseChat beta
Beamanator Aug 15, 2023
1099600
Update tests
Beamanator Aug 15, 2023
6db5ae1
Merge branch 'main' of github.com:Expensify/App into beaman-removePol…
Beamanator Aug 18, 2023
bbc2cf6
Yeah pretend i remembered to remove that earlier
Beamanator Aug 18, 2023
c50119e
Remove unused betas
Beamanator Aug 18, 2023
a65f22e
Remove unused var
Beamanator Aug 18, 2023
093590f
Fix getShareDestinationsOptions test
Beamanator Aug 18, 2023
0364988
Merge branch 'main' of github.com:Expensify/App into beaman-removePol…
Beamanator Aug 21, 2023
3352cc8
Merge branch 'main' of github.com:Expensify/App into beaman-removePol…
Beamanator Aug 22, 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
1 change: 0 additions & 1 deletion src/CONST.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,6 @@ const CONST = {
INTERNATIONALIZATION: 'internationalization',
IOU_SEND: 'sendMoney',
POLICY_ROOMS: 'policyRooms',
POLICY_EXPENSE_CHAT: 'policyExpenseChat',
PASSWORDLESS: 'passwordless',
TASKS: 'tasks',
THREADS: 'threads',
Expand Down
9 changes: 0 additions & 9 deletions src/libs/Permissions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,6 @@ function canUsePolicyRooms(betas) {
return _.contains(betas, CONST.BETAS.POLICY_ROOMS) || canUseAllBetas(betas);
}

/**
* @param {Array<String>} betas
* @returns {Boolean}
*/
function canUsePolicyExpenseChat(betas) {
return _.contains(betas, CONST.BETAS.POLICY_EXPENSE_CHAT) || canUseAllBetas(betas);
}

/**
* @param {Array<String>} betas
* @returns {Boolean}
Expand Down Expand Up @@ -126,7 +118,6 @@ export default {
canUseWallet,
canUseCommentLinking,
canUsePolicyRooms,
canUsePolicyExpenseChat,
canUseTasks,
canUseScanReceipts,
canUseCustomStatus,
Expand Down
5 changes: 0 additions & 5 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2638,11 +2638,6 @@ function shouldReportBeInOptionList(report, currentReportId, isInGSDMode, betas,
return true;
}

// Exclude policy expense chats if the user isn't in the policy expense chat beta
if (isPolicyExpenseChat(report) && !Permissions.canUsePolicyExpenseChat(betas)) {
return false;
}

// Hide chats between two users that haven't been commented on from the LNH
if (excludeEmptyChats && isEmptyChat && isChatReport(report) && !isChatRoom(report) && !isPolicyExpenseChat(report)) {
return false;
Expand Down
1 change: 0 additions & 1 deletion src/libs/__mocks__/Permissions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ export default {
...jest.requireActual('../Permissions'),
canUseDefaultRooms: (betas) => _.contains(betas, CONST.BETAS.DEFAULT_ROOMS),
canUsePolicyRooms: (betas) => _.contains(betas, CONST.BETAS.POLICY_ROOMS),
canUsePolicyExpenseChat: (betas) => _.contains(betas, CONST.BETAS.POLICY_EXPENSE_CHAT),
canUseIOUSend: (betas) => _.contains(betas, CONST.BETAS.IOU_SEND),
};
14 changes: 3 additions & 11 deletions src/libs/actions/Policy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import * as ErrorUtils from '../ErrorUtils';
import * as ReportUtils from '../ReportUtils';
import * as PersonalDetailsUtils from '../PersonalDetailsUtils';
import Log from '../Log';
import Permissions from '../Permissions';

const allPolicies = {};
Onyx.connect({
Expand Down Expand Up @@ -234,22 +233,16 @@ function removeMembers(accountIDs, policyID) {
*
* @param {String} policyID
* @param {Object} invitedEmailsToAccountIDs
* @param {Array} betas
* @returns {Object} - object with onyxSuccessData, onyxOptimisticData, and optimisticReportIDs (map login to reportID)
*/
function createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs, betas) {
function createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs) {
const workspaceMembersChats = {
onyxSuccessData: [],
onyxOptimisticData: [],
onyxFailureData: [],
reportCreationData: {},
};

// If the user is not in the beta, we don't want to create any chats
if (!Permissions.canUsePolicyExpenseChat(betas)) {
return workspaceMembersChats;
}

_.each(invitedEmailsToAccountIDs, (accountID, email) => {
const cleanAccountID = Number(accountID);
const login = OptionsListUtils.addSMSDomainIfPhoneNumber(email);
Expand Down Expand Up @@ -332,16 +325,15 @@ function createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs, betas) {
* @param {Object} invitedEmailsToAccountIDs
* @param {String} welcomeNote
* @param {String} policyID
* @param {Array<String>} betas
*/
function addMembersToWorkspace(invitedEmailsToAccountIDs, welcomeNote, policyID, betas) {
function addMembersToWorkspace(invitedEmailsToAccountIDs, welcomeNote, policyID) {
const membersListKey = `${ONYXKEYS.COLLECTION.POLICY_MEMBERS}${policyID}`;
const logins = _.map(_.keys(invitedEmailsToAccountIDs), (memberLogin) => OptionsListUtils.addSMSDomainIfPhoneNumber(memberLogin));
const accountIDs = _.values(invitedEmailsToAccountIDs);
const newPersonalDetailsOnyxData = PersonalDetailsUtils.getNewPersonalDetailsOnyxData(logins, accountIDs);

// create onyx data for policy expense chats for each new member
const membersChats = createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs, betas);
const membersChats = createPolicyExpenseChats(policyID, invitedEmailsToAccountIDs);

const optimisticData = [
{
Expand Down
9 changes: 1 addition & 8 deletions src/pages/workspace/WorkspaceInviteMessagePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ const propTypes = {
/** All of the personal details for everyone */
allPersonalDetails: PropTypes.objectOf(personalDetailsPropTypes),

/** Beta features list */
betas: PropTypes.arrayOf(PropTypes.string),

invitedEmailsToAccountIDsDraft: PropTypes.objectOf(PropTypes.number),

/** URL Route params */
Expand All @@ -68,7 +65,6 @@ const propTypes = {
const defaultProps = {
...policyDefaultProps,
allPersonalDetails: {},
betas: [],
invitedEmailsToAccountIDsDraft: {},
};

Expand Down Expand Up @@ -123,7 +119,7 @@ class WorkspaceInviteMessagePage extends React.Component {

sendInvitation() {
Keyboard.dismiss();
Policy.addMembersToWorkspace(this.props.invitedEmailsToAccountIDsDraft, this.state.welcomeNote, this.props.route.params.policyID, this.props.betas);
Policy.addMembersToWorkspace(this.props.invitedEmailsToAccountIDsDraft, this.state.welcomeNote, this.props.route.params.policyID);
Policy.setWorkspaceInviteMembersDraft(this.props.route.params.policyID, {});
// Pop the invite message page before navigating to the members page.
Navigation.goBack();
Expand Down Expand Up @@ -248,9 +244,6 @@ export default compose(
allPersonalDetails: {
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
},
betas: {
key: ONYXKEYS.BETAS,
},
invitedEmailsToAccountIDsDraft: {
key: ({route}) => `${ONYXKEYS.COLLECTION.WORKSPACE_INVITE_MEMBERS_DRAFT}${route.params.policyID.toString()}`,
},
Expand Down
35 changes: 29 additions & 6 deletions tests/unit/OptionsListUtilsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'underscore';
import Onyx from 'react-native-onyx';
import * as OptionsListUtils from '../../src/libs/OptionsListUtils';
import * as ReportUtils from '../../src/libs/ReportUtils';
import ONYXKEYS from '../../src/ONYXKEYS';
import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
import CONST from '../../src/CONST';
Expand Down Expand Up @@ -103,6 +104,10 @@ describe('OptionsListUtils', () => {
oldPolicyName: "SHIELD's workspace",
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
isOwnPolicyExpenseChat: true,

// This indicates that the report is archived
stateNum: 2,
statusNum: 2,
},
};

Expand Down Expand Up @@ -567,39 +572,57 @@ describe('OptionsListUtils', () => {
});

it('getShareDestinationsOptions()', () => {
// Filter current REPORTS as we do in the component, before getting share destination options
const filteredReports = {};
_.keys(REPORTS).forEach((reportKey) => {
if (ReportUtils.shouldDisableWriteActions(REPORTS[reportKey]) || ReportUtils.isExpensifyOnlyParticipantInReport(REPORTS[reportKey])) {
return;
}
filteredReports[reportKey] = REPORTS[reportKey];
Comment on lines +577 to +581
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have to filter here b/c we do this in TaskShareDestinationSelectorModal before calling getShareDestinationOptions. So we either do this logic in the test as well, OR we extract this filtering into getShareDestinationOptions

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not able to understand why we have to do this change? Because getShareDestinationOptions is a separate function not dependent on any component right?

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 believe we need the change b/c we currently use getShareDestinationOptions by passing in filtered reports - here:

App/src/pages/tasks/TaskShareDestinationSelectorModal.js

Lines 49 to 60 in 4a7558c

const filteredReports = useMemo(() => {
const reports = {};
_.keys(props.reports).forEach((reportKey) => {
if (ReportUtils.shouldDisableWriteActions(props.reports[reportKey]) || ReportUtils.isExpensifyOnlyParticipantInReport(props.reports[reportKey])) {
return;
}
reports[reportKey] = props.reports[reportKey];
});
return reports;
}, [props.reports]);
const updateOptions = useCallback(() => {
const {recentReports} = OptionsListUtils.getShareDestinationOptions(filteredReports, props.personalDetails, props.betas, searchValue.trim(), [], CONST.EXPENSIFY_EMAILS, true);

So tests were failing when we weren't filtering out archived reports (via `ReportUtils.shouldDisableWriteActions) first

});

// When we pass an empty search value
let results = OptionsListUtils.getShareDestinationOptions(REPORTS, PERSONAL_DETAILS, [], '');
let results = OptionsListUtils.getShareDestinationOptions(filteredReports, PERSONAL_DETAILS, [], '');

// Then we should expect all the recent reports to show but exclude the archived rooms
expect(results.recentReports.length).toBe(_.size(REPORTS) - 1);

// When we pass a search value that doesn't match the group chat name
results = OptionsListUtils.getShareDestinationOptions(REPORTS, PERSONAL_DETAILS, [], 'mutants');
results = OptionsListUtils.getShareDestinationOptions(filteredReports, PERSONAL_DETAILS, [], 'mutants');

// Then we should expect no recent reports to show
expect(results.recentReports.length).toBe(0);

// When we pass a search value that matches the group chat name
results = OptionsListUtils.getShareDestinationOptions(REPORTS, PERSONAL_DETAILS, [], 'Iron Man, Fantastic');
results = OptionsListUtils.getShareDestinationOptions(filteredReports, PERSONAL_DETAILS, [], 'Iron Man, Fantastic');

// Then we should expect the group chat to show along with the contacts matching the search
expect(results.recentReports.length).toBe(1);

// Filter current REPORTS_WITH_WORKSPACE_ROOMS as we do in the component, before getting share destination options
const filteredReportsWithWorkspaceRooms = {};
_.keys(REPORTS_WITH_WORKSPACE_ROOMS).forEach((reportKey) => {
if (ReportUtils.shouldDisableWriteActions(REPORTS_WITH_WORKSPACE_ROOMS[reportKey]) || ReportUtils.isExpensifyOnlyParticipantInReport(REPORTS_WITH_WORKSPACE_ROOMS[reportKey])) {
return;
}
filteredReportsWithWorkspaceRooms[reportKey] = REPORTS_WITH_WORKSPACE_ROOMS[reportKey];
});

// When we also have a policy to return rooms in the results
results = OptionsListUtils.getShareDestinationOptions(REPORTS_WITH_WORKSPACE_ROOMS, PERSONAL_DETAILS, [], '');
results = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, PERSONAL_DETAILS, [], '');

// Then we should expect the DMS, the group chats and the workspace room to show
// We should expect all the recent reports to show, excluding the archived rooms
expect(results.recentReports.length).toBe(_.size(REPORTS_WITH_WORKSPACE_ROOMS) - 1);

// When we search for a workspace room
results = OptionsListUtils.getShareDestinationOptions(REPORTS_WITH_WORKSPACE_ROOMS, PERSONAL_DETAILS, [], 'Avengers Room');
results = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, PERSONAL_DETAILS, [], 'Avengers Room');

// Then we should expect only the workspace room to show
expect(results.recentReports.length).toBe(1);

// When we search for a workspace room that doesn't exist
results = OptionsListUtils.getShareDestinationOptions(REPORTS_WITH_WORKSPACE_ROOMS, PERSONAL_DETAILS, [], 'Mutants Lair');
results = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, PERSONAL_DETAILS, [], 'Mutants Lair');

// Then we should expect no results to show
expect(results.recentReports.length).toBe(0);
Expand Down
65 changes: 8 additions & 57 deletions tests/unit/SidebarFilterTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import wrapOnyxWithWaitForPromisesToResolve from '../utils/wrapOnyxWithWaitForPr
import CONST from '../../src/CONST';
import DateUtils from '../../src/libs/DateUtils';
import * as Localize from '../../src/libs/Localize';
import * as Report from '../../src/libs/actions/Report';

// Be sure to include the mocked permissions library or else the beta tests won't work
jest.mock('../../src/libs/Permissions');
Expand Down Expand Up @@ -127,54 +126,6 @@ describe('Sidebar', () => {
);
});

it('includes or excludes policy expensechats depending on the beta', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();

// Given a policy expense report
// and the user not being in any betas
const report = {
...LHNTestUtils.getFakeReport(),
chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT,
};

return (
waitForPromisesToResolve()
// When Onyx is updated to contain that data and the sidebar re-renders
.then(() =>
Onyx.multiSet({
[ONYXKEYS.BETAS]: [],
[ONYXKEYS.PERSONAL_DETAILS_LIST]: LHNTestUtils.fakePersonalDetails,
[ONYXKEYS.IS_LOADING_REPORT_DATA]: false,
[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]: report,
}),
)

// When the report has at least one ADDCOMMENT action to be rendered in the LNH
.then(() => Report.addComment(report.reportID, 'Hi, this is a comment'))

// Then no reports are rendered in the LHN
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.navigatesToChat');
const optionRows = screen.queryAllByAccessibilityHint(hintText);
expect(optionRows).toHaveLength(0);
})

// When the user is added to the policy expense beta and the sidebar re-renders
.then(() =>
Onyx.multiSet({
[ONYXKEYS.BETAS]: [CONST.BETAS.POLICY_EXPENSE_CHAT],
}),
)

// Then there is one report rendered in the LHN
.then(() => {
const hintText = Localize.translateLocal('accessibilityHints.navigatesToChat');
const optionRows = screen.queryAllByAccessibilityHint(hintText);
expect(optionRows).toHaveLength(1);
})
);
});

it('includes or excludes user created policy rooms depending on the beta', () => {
LHNTestUtils.getDefaultRenderedSidebarLinks();

Expand Down Expand Up @@ -335,7 +286,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

// Given there are 6 boolean variables tested in the filtering logic:
// 1. isArchived
Expand Down Expand Up @@ -539,7 +490,7 @@ describe('Sidebar', () => {
};
LHNTestUtils.getDefaultRenderedSidebarLinks();

const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
waitForPromisesToResolve()
Expand Down Expand Up @@ -602,7 +553,7 @@ describe('Sidebar', () => {
};
LHNTestUtils.getDefaultRenderedSidebarLinks();

const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
waitForPromisesToResolve()
Expand Down Expand Up @@ -660,7 +611,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

// Given there are 6 boolean variables tested in the filtering logic:
// 1. isArchived
Expand Down Expand Up @@ -753,7 +704,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
waitForPromisesToResolve()
Expand Down Expand Up @@ -804,7 +755,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
waitForPromisesToResolve()
Expand Down Expand Up @@ -853,7 +804,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
waitForPromisesToResolve()
Expand Down Expand Up @@ -898,7 +849,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];

return (
waitForPromisesToResolve()
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/SidebarOrderTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ describe('Sidebar', () => {
Report.addComment(report3.reportID, 'Hi, this is a comment');

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];
LHNTestUtils.getDefaultRenderedSidebarLinks('0');
return (
waitForPromisesToResolve()
Expand Down Expand Up @@ -666,7 +666,7 @@ describe('Sidebar', () => {
const report3 = LHNTestUtils.getFakeReport([5, 6], 1, true);

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];
LHNTestUtils.getDefaultRenderedSidebarLinks('0');
return (
waitForPromisesToResolve()
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/SidebarTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];
LHNTestUtils.getDefaultRenderedSidebarLinks('0');
return (
waitForPromisesToResolve()
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('Sidebar', () => {
};

// Given the user is in all betas
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS, CONST.BETAS.POLICY_EXPENSE_CHAT];
const betas = [CONST.BETAS.DEFAULT_ROOMS, CONST.BETAS.POLICY_ROOMS];
LHNTestUtils.getDefaultRenderedSidebarLinks('0');
return (
waitForPromisesToResolve()
Expand Down
Loading