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

Fix Weekly submission frequency not selected by default #37821

Merged
merged 15 commits into from
Mar 8, 2024
Merged

Fix Weekly submission frequency not selected by default #37821

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6f92681
fix delayed submission
lakchote Mar 6, 2024
b641e6e
fix ts
lakchote Mar 6, 2024
3850cd0
fix prettier
lakchote Mar 6, 2024
d6815d2
remove unneeded check
lakchote Mar 6, 2024
d22fd3c
fix prettier
lakchote Mar 6, 2024
049f088
add optional frequency param
lakchote Mar 7, 2024
f0a7dba
remove the need for 2 API calls
lakchote Mar 7, 2024
02e1e50
pass down frequency if instant frequency scenario
lakchote Mar 7, 2024
f7df56b
keep previous frequency value on toggle off/on
lakchote Mar 7, 2024
91adf00
remove unneeded param
lakchote Mar 8, 2024
b01d14e
set optimistic auto reporting frequency
lakchote Mar 8, 2024
99a0e99
improve comment
lakchote Mar 8, 2024
c637b41
fix type check
lakchote Mar 8, 2024
531f0ca
put correct optimistic data for auto reporting
lakchote Mar 8, 2024
bf2c6cc
fix style
lakchote Mar 8, 2024
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: 1 addition & 0 deletions src/libs/API/parameters/SetWorkspaceAutoReportingParams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
type SetWorkspaceAutoReportingParams = {
policyID: string;
enabled: boolean;
frequency?: string;
};

export default SetWorkspaceAutoReportingParams;
8 changes: 5 additions & 3 deletions src/libs/actions/Policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ function buildAnnounceRoomMembersOnyxData(policyID: string, accountIDs: number[]
return announceRoomMembers;
}

function setWorkspaceAutoReporting(policyID: string, enabled: boolean) {
function setWorkspaceAutoReporting(policyID: string, enabled: boolean, frequency?: string) {
const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -413,7 +413,6 @@ function setWorkspaceAutoReporting(policyID: string, enabled: boolean) {
harvesting: {
enabled: true,
},
autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.WEEKLY,
lakchote marked this conversation as resolved.
Show resolved Hide resolved
pendingFields: {isAutoApprovalEnabled: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE},
},
},
Expand All @@ -425,7 +424,6 @@ function setWorkspaceAutoReporting(policyID: string, enabled: boolean) {
key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
autoReporting: !enabled,
autoReportingFrequency: null,
lakchote marked this conversation as resolved.
Show resolved Hide resolved
pendingFields: {isAutoApprovalEnabled: null, harvesting: null},
},
},
Expand All @@ -442,6 +440,10 @@ function setWorkspaceAutoReporting(policyID: string, enabled: boolean) {
];

const params: SetWorkspaceAutoReportingParams = {policyID, enabled};
if (frequency && frequency === CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT) {
lakchote marked this conversation as resolved.
Show resolved Hide resolved
params.frequency = frequency;
}

API.write(WRITE_COMMANDS.SET_WORKSPACE_AUTO_REPORTING, params, {optimisticData, failureData, successData});
}

Expand Down
6 changes: 4 additions & 2 deletions src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,16 @@ function WorkspaceWorkflowsPage({policy, betas, route}: WorkspaceWorkflowsPagePr
title: translate('workflowsPage.delaySubmissionTitle'),
subtitle: translate('workflowsPage.delaySubmissionDescription'),
onToggle: (isEnabled: boolean) => {
Policy.setWorkspaceAutoReporting(route.params.policyID, isEnabled);
Policy.setWorkspaceAutoReporting(route.params.policyID, isEnabled, policy?.autoReportingFrequency ?? CONST.POLICY.AUTO_REPORTING_FREQUENCIES.WEEKLY);
},
subMenuItems: (
<MenuItem
title={translate('workflowsPage.submissionFrequency')}
titleStyle={styles.textLabelSupportingNormal}
descriptionTextStyle={styles.textNormalThemeText}
onPress={onPressAutoReportingFrequency}
// We don't want to show `Instantly` as a frequency option in the dropdown list
// see https://expensify.slack.com/archives/C036QM0SLJK/p1709738406293909?thread_ts=1709738293.910139&cid=C036QM0SLJK
lakchote marked this conversation as resolved.
Show resolved Hide resolved
description={
getAutoReportingFrequencyDisplayNames(preferredLocale)[
(policy?.autoReportingFrequency as AutoReportingFrequencyKey) ?? CONST.POLICY.AUTO_REPORTING_FREQUENCIES.WEEKLY
Expand All @@ -78,7 +80,7 @@ function WorkspaceWorkflowsPage({policy, betas, route}: WorkspaceWorkflowsPagePr
hoverAndPressStyle={[styles.mr0, styles.br2]}
/>
),
isActive: policy?.harvesting?.enabled ?? false,
isActive: (policy?.harvesting?.enabled && policy.autoReportingFrequency !== CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT) ?? false,
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI we MIGHT want to use policyUtils.isInstantSubmitEnabled here? This soon will also check that policy?.autoReporting is set to true -> Because if policy?.autoReporting is false, we don't want to consider that policy "on instant submit"

Copy link
Contributor

Choose a reason for hiding this comment

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

cc @lakchote

pendingAction: policy?.pendingFields?.isAutoApprovalEnabled,
},
]
Expand Down
Loading