-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Set "Delayed submission" to "Manually" by default on new Workspaces #56367
Comments
Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 ( |
Triggered auto assignment to @NicMendonca ( |
|
Triggered auto assignment to Design team member for new feature review - @dubielzyk-expensify ( |
I don't think this change requires design because its using 100% existing components |
🚨 Edited by proposal-police: This proposal was edited at 2025-02-04 18:26:51 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.Set "Delayed submission" to "Manually" by default on new Workspaces What is the root cause of that problem?Feature request What changes do you think we should make in order to solve the problem?Update the following: App/src/libs/actions/Policy/Policy.ts Lines 1768 to 1772 in 96af045
to autoReportingFrequency: CONST.POLICY.AUTO_REPORTING_FREQUENCIES.IMMEDIATE, / MANUAL (not sure about this value, will discuss more during PR phase, it's most probably immediate as seen in `SetWorkspaceAutoReportingFrequency`)
approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL,
harvesting: {
enabled: false,
}, Lines 1630 to 1631 in 96af045
Make the same in the below cases too: App/src/libs/actions/Policy/Policy.ts Lines 2410 to 2414 in 96af045
App/src/libs/actions/Policy/Policy.ts Lines 2097 to 2101 in 96af045
App/src/libs/actions/Policy/Policy.ts Line 1678 in 96af045
App/src/libs/actions/Policy/Policy.ts Lines 1688 to 1690 in 96af045
There might be some existing unit tests which will fail once we make these changes so we need to update those too We also need What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?We can create a policy using onyx data and either write a unit test or a UI test for workflows screen to see if the toggle for delayed submit is on and the submission frequency is set to manually What alternative solutions did you explore? (Optional) |
|
i'll take care of the backend changes needed for this one |
ProposalPlease re-state the problem that we are trying to solve in this issue.Set "Delayed submission" to "Manually" by default on new Workspaces What is the root cause of that problem?Improvement What changes do you think we should make in order to solve the problem?
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
What alternative solutions did you explore? (Optional) |
Backend PR is in review! |
@abdulrahuman5196 can you please review my proposal here |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Sorry for the confusion. I will review today |
Hi @dangrous , Currently I am able to verify the proposals only in offline mode since the backend change is not in production yet and current backend response is disabling the Delayed Submission. Both the proposals are almost same. |
@dangrous, @NicMendonca, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@abdulrahuman5196 how are we looking on proposal review? Thanks! |
Sorry for the delay checking now again. |
Hi @dangrous , After the backend change the issue is only reproducible while creating workspace in offline case. I was trying the proposal and it was working for setting Delayed Submission to manual, but one thing I noticed is the Add approvals section which is different in offline case and different after backend response. Do we need to update the approvers section as well or the below video expectation is fine? Or should that be a different issue not related to this one? Screen.Recording.2025-02-25.at.8.39.46.PM.mov |
@abdulrahuman5196 the issue you mentioned is already being fixed in this PR: |
Yeah as @twilight2294 said we're updating the optimistic behavior to match the backend, so you can just focus on the delayed submission bit. Thanks! |
Thank you for clarification. Got it. 🎀 👀 🎀 |
Current assignee @dangrous is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new. |
This seems like a duplicate as we are already doing this in #50391, but are we missing all cases @abdulrahuman5196 Can you check that PR and let us know. |
your PR is doing it for intent of |
ugh there are so many similar PRs. So if if #50391 is updating it for "manage team" and "something else" we still need it for:
It looks like @twilight2294's solution will set it to manual, for ALL new workplaces, not just in those four cases, but that's a quick update. @parasharrajat / @mkzie2 do you want to add the additional cases to your PR, or should we fix them here. The solutions themselves are pretty clear, but I want to make sure we're getting them out efficiently. |
It seems new Collect workspaces on Classic are defaulting to Instant submit. I expected us to copy the New Expensify approach of defaulting to "Manually". Was Classic intentionally left out or just a small oversight? |
@parasharrajat Since our PR only covers two case onboarding purposes, I think delay submission can handle in this issue. |
Ok, that works for me. |
This comment has been minimized.
This comment has been minimized.
Thank you, @dangrous can you assign me here? |
@dangrous, does it make sense to open a new issue for this? Slack convo: https://expensify.slack.com/archives/C03U7DCU4/p1740610255320019 |
assigned @twilight2294 @MitchExpensify - I was under the understanding that this was only for users that had gone through onboarding on New Dot, so we hadn't looked specifically at Classic. I can definitely take a look if we want to align those. A caveat - it will break a million tests, so we'll need to figure out which ones of those expected results need to be updated and which need to be updated to have different set ups. Which.... may take a long time. But I can start working on it. Is there onboarding that happens in Classic as well? Do they ask the same questions? Or do we want them all doing it by default even without any specific input from the user? TLDR yes a new issue would be great! |
Please add the tests for these changes. @twilight2294 |
I will get to this |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: v9.0.93-3
Issue reported by: @MitchExpensify
Slack conversation (hyperlinked to channel name): Internal https://expensify.slack.com/archives/C07HPDRELLD/p1738628148608039
Problem:
Data shows that the majority of our paying customers actively use workspace features that are disabled by default in New Expensify. For example, over 70% of Collect workspaces have Scheduled Submit set to Manual or Off but "Delayed submission" is disabled by default in New Expensify workspaces. This misalignment adds unnecessary friction to the onboarding process and risks undermining customer satisfaction by not meeting their expectations more directly in the window shopping phase, thus risking conversion.
Solution
Set "Delayed submission" to Manually by default on new Workspaces likely to have more than one member:
Action Performed:
Situation 1: introSelected value is newDotManageTeam
Situation 2: introSelected value is empty
Situation 3: Bottom Up
Expected Result (New Feature):
See the "Delayed submission" feature enabled and set to "Manually in the "Workflows" tab
Actual Result:
"Delayed submission" feature is disabled in the "Workflows" tab
Workaround:
Edit the setting manually
Platforms:
Which of our officially supported platforms is this issue occurring on?
View all open jobs on GitHub
Issue Owner
Current Issue Owner: @abdulrahuman5196The text was updated successfully, but these errors were encountered: