Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6196ec2
fix logic
lakchote Feb 17, 2025
e87f5c8
fix logic in case of default approval workflow
lakchote Feb 17, 2025
fadca21
fix style
lakchote Feb 17, 2025
3678a1e
fix lint
lakchote Feb 17, 2025
6135637
improve wording
lakchote Feb 19, 2025
ec34253
improve logic for self approvals
lakchote Feb 19, 2025
22453bc
update logic for self approvals
lakchote Feb 19, 2025
49eac2d
fix style
lakchote Feb 19, 2025
e9b7d25
Merge branch 'main' into lucien/fix-prevent-self-approvals
lakchote Feb 19, 2025
2453425
fix lint
lakchote Feb 19, 2025
120d6e0
fix lint
lakchote Feb 19, 2025
8ed72e6
fix prettier
lakchote Feb 19, 2025
4505667
fix lint
lakchote Feb 19, 2025
5fdfdd6
Merge branch 'main' into lucien/fix-prevent-self-approvals
lakchote Feb 25, 2025
acca957
handle prevent self approvals
lakchote Feb 26, 2025
1d4c2a1
add keys for workflows prevent self approvals
lakchote Feb 26, 2025
c68f19f
fix style
lakchote Feb 26, 2025
32a9de8
fix lint
lakchote Feb 26, 2025
bb38640
update error for self approvals in workflows
lakchote Feb 27, 2025
646267d
fix edge case
lakchote Feb 27, 2025
c925640
fix prettier
lakchote Feb 27, 2025
fa56af6
remove unneeded method
lakchote Feb 28, 2025
80049d2
revert changes related to prevent self approvals
lakchote Feb 28, 2025
972e5a5
remove unneeded translations
lakchote Feb 28, 2025
31f24c7
remove new logic to keep previous logic
lakchote Feb 28, 2025
a9c2023
create `buildOptimisticNextStepForPreventSelfApprovalsEnabled()`
lakchote Mar 6, 2025
2a0e776
use `buildOptimisticNextStepForPreventSelfApprovalsEnabled()`
lakchote Mar 6, 2025
3a92739
fix style
lakchote Mar 6, 2025
d47518f
Merge branch 'main' into lucien/fix-prevent-self-approvals
lakchote Mar 6, 2025
8bc65f2
fix
lakchote Mar 6, 2025
f6adc07
fix
lakchote Mar 6, 2025
7346dd3
fix style
lakchote Mar 6, 2025
920eb3d
Merge branch 'main' into lucien/fix-prevent-self-approvals
lakchote Mar 7, 2025
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: 9 additions & 2 deletions src/components/MoneyReportHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import {convertToDisplayString} from '@libs/CurrencyUtils';
import Navigation from '@libs/Navigation/Navigation';
import {buildOptimisticNextStepForPreventSelfApprovalsEnabled} from '@libs/NextStepUtils';
import {getConnectedIntegration} from '@libs/PolicyUtils';
import {getOriginalMessage, isDeletedAction, isMoneyRequestAction, isTrackExpenseAction} from '@libs/ReportActionsUtils';
import {
Expand Down Expand Up @@ -207,7 +208,13 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE;
const shouldShowStatusBar =
hasAllPendingRTERViolations || shouldShowBrokenConnectionViolation || hasOnlyHeldExpenses || hasScanningReceipt || isPayAtEndExpense || hasOnlyPendingTransactions;
const shouldShowNextStep = transactions?.length !== 0 && isFromPaidPolicy && !!nextStep?.message?.length && !shouldShowStatusBar;

// When prevent self-approval is enabled, we need to show the optimistic next step
// We should always show this optimistic message for policies with preventSelfApproval
// to avoid any flicker during transitions between online/offline states
const optimisticNextStep = policy?.preventSelfApproval ? buildOptimisticNextStepForPreventSelfApprovalsEnabled() : nextStep;

const shouldShowNextStep = transactions?.length !== 0 && isFromPaidPolicy && !!optimisticNextStep?.message?.length && !shouldShowStatusBar;
Comment on lines +215 to +217

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.

Hmm shouldn't we only be doing buildOptimisticNextStepForPreventSelfApprovalsEnabled if policy?.preventSelfApproval AND if the submitter is the next receiver?

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.

More investigation in https://expensify.slack.com/archives/C06ML6X0W9L/p1741645160823569 - but yeah OldDot is correctly showing the next steps as "Waiting for to approve expenses" b/c I'm not submitting to myself 😬 I thinkkkkk i can put up a quick fix

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.

created #58128 to fix

const shouldShowAnyButton =
isDuplicate ||
shouldShowSettlementButton ||
Expand Down Expand Up @@ -510,7 +517,7 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
/>
)}
</View>
{shouldShowNextStep && <MoneyReportHeaderStatusBar nextStep={nextStep} />}
{shouldShowNextStep && <MoneyReportHeaderStatusBar nextStep={optimisticNextStep} />}
{!!statusBarProps && (
<MoneyRequestHeaderStatusBar
icon={statusBarProps.icon}
Expand Down
5 changes: 0 additions & 5 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4723,11 +4723,6 @@ const translations = {
unlockFeatureGoToSubtitle: 'Go to',
unlockFeatureEnableWorkflowsSubtitle: ({featureName}: FeatureNameParams) => `and enable workflows, then add ${featureName} to unlock this feature.`,
enableFeatureSubtitle: ({featureName}: FeatureNameParams) => `and enable ${featureName} to unlock this feature.`,
preventSelfApprovalsModalText: ({managerEmail}: {managerEmail: string}) =>
`Any members currently approving their own expenses will be removed and replaced with the default approver for this workspace (${managerEmail}).`,
preventSelfApprovalsConfirmButton: 'Prevent self-approvals',
preventSelfApprovalsModalTitle: 'Prevent self-approvals?',
preventSelfApprovalsDisabledSubtitle: "Self approvals can't be enabled until this workspace has at least two members.",
},
categoryRules: {
title: 'Category rules',
Expand Down
5 changes: 0 additions & 5 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4793,11 +4793,6 @@ const translations = {
unlockFeatureGoToSubtitle: 'Ir a',
unlockFeatureEnableWorkflowsSubtitle: ({featureName}: FeatureNameParams) => `y habilita flujos de trabajo, luego agrega ${featureName} para desbloquear esta función.`,
enableFeatureSubtitle: ({featureName}: FeatureNameParams) => `y habilita ${featureName} para desbloquear esta función.`,
preventSelfApprovalsModalText: ({managerEmail}: {managerEmail: string}) =>
`Todos los miembros que actualmente estén aprobando sus propios gastos serán eliminados y reemplazados con el aprobador predeterminado de este espacio de trabajo (${managerEmail}).`,
preventSelfApprovalsConfirmButton: 'Evitar autoaprobaciones',
preventSelfApprovalsModalTitle: '¿Evitar autoaprobaciones?',
preventSelfApprovalsDisabledSubtitle: 'Las aprobaciones propias no pueden habilitarse hasta que este espacio de trabajo tenga al menos dos miembros.',
},
categoryRules: {
title: 'Reglas de categoría',
Expand Down
30 changes: 29 additions & 1 deletion src/libs/NextStepUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,34 @@ function getNextApproverDisplayName(report: OnyxEntry<Report>, isUnapprove?: boo
return getDisplayNameForParticipant({accountID: approverAccountID}) ?? getPersonalDetailsForAccountID(approverAccountID).login;
}

function buildOptimisticNextStepForPreventSelfApprovalsEnabled() {
const optimisticNextStep: ReportNextStep = {
type: 'alert',
icon: CONST.NEXT_STEP.ICONS.HOURGLASS,
message: [
{
text: "Oops! Looks like you're submitting to ",
},
{
text: 'yourself',
type: 'strong',
},
{
text: '. Approving your own reports is ',
},
{
text: 'forbidden',
type: 'strong',
},
{
text: ' by your policy. Please submit this report to someone else or contact your admin to change the person you submit to.',
},
],
};

return optimisticNextStep;
}

/**
* Generates an optimistic nextStep based on a current report status and other properties.
*
Expand Down Expand Up @@ -416,4 +444,4 @@ function buildNextStep(report: OnyxEntry<Report>, predictedNextStatus: ValueOf<t
return optimisticNextStep;
}

export {parseMessage, buildNextStep};
export {parseMessage, buildNextStep, buildOptimisticNextStepForPreventSelfApprovalsEnabled};
31 changes: 0 additions & 31 deletions src/libs/PolicyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1325,35 +1325,6 @@ const getDescriptionForPolicyDomainCard = (domainName: string): string => {
return domainName;
};

/**
* Returns an array of user emails who are currently self-approving:
* i.e. user.submitsTo === their own email.
*/
function getAllSelfApprovers(policy: OnyxEntry<Policy>): string[] {
const defaultApprover = policy?.approver ?? policy?.owner;
if (!policy?.employeeList || !defaultApprover) {
return [];
}
return Object.keys(policy.employeeList).filter((email) => {
const employee = policy?.employeeList?.[email] ?? {};
return employee?.submitsTo === email && employee?.email !== defaultApprover;
});
}

/**
* Checks if the workspace has only one user and if there no approver for the policy.
* If so, we can't enable the "Prevent Self Approvals" feature.
*/
function canEnablePreventSelfApprovals(policy: OnyxEntry<Policy>): boolean {
if (!policy?.employeeList || !policy.approver) {
return false;
}

const employeeEmails = Object.keys(policy.employeeList);

return employeeEmails.length > 1;
}

function isPrefferedExporter(policy: Policy) {
const user = getCurrentUserEmail();
const exporters = [
Expand Down Expand Up @@ -1383,12 +1354,10 @@ function isAutoSyncEnabled(policy: Policy) {

export {
canEditTaxRate,
canEnablePreventSelfApprovals,
extractPolicyIDFromPath,
escapeTagName,
getActivePolicies,
getPerDiemCustomUnits,
getAllSelfApprovers,
getAdminEmployees,
getCleanedTagName,
getConnectedIntegration,
Expand Down
8 changes: 1 addition & 7 deletions src/pages/workspace/WorkspaceMembersPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import {getMemberAccountIDsForWorkspace, isDeletedPolicyEmployee, isExpensifyTea
import {getDisplayNameForParticipant} from '@libs/ReportUtils';
import {convertPolicyEmployeesToApprovalWorkflows, updateWorkflowDataOnApproverRemoval} from '@libs/WorkflowUtils';
import {close} from '@userActions/Modal';
import {dismissAddedWithPrimaryLoginMessages, setPolicyPreventSelfApproval} from '@userActions/Policy/Policy';
import {dismissAddedWithPrimaryLoginMessages} from '@userActions/Policy/Policy';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -239,10 +239,8 @@ function WorkspaceMembersPage({personalDetails, route, policy, currentUserPerson
return;
}

const previousEmployeesCount = Object.keys(policy?.employeeList ?? {}).length;
// Remove the admin from the list
const accountIDsToRemove = session?.accountID ? selectedEmployees.filter((id) => id !== session.accountID) : selectedEmployees;
const newEmployeesCount = previousEmployeesCount - accountIDsToRemove.length;

// Check if any of the account IDs are approvers
const hasApprovers = accountIDsToRemove.some((accountID) => isApprover(policy, accountID));
Expand Down Expand Up @@ -274,10 +272,6 @@ function WorkspaceMembersPage({personalDetails, route, policy, currentUserPerson
setRemoveMembersConfirmModalVisible(false);
InteractionManager.runAfterInteractions(() => {
removeMembers(accountIDsToRemove, route.params.policyID);
if (newEmployeesCount === 1 && policy?.preventSelfApproval) {
// We can't let the "Prevent Self Approvals" enabled if there's only one workspace user
setPolicyPreventSelfApproval(route.params.policyID, false);
}
});
};

Expand Down
Loading