[$250] Workspace - "Delay submissions" turns of if the "Submission frequency" is set to "Manually"

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.

https://new.expensify.com

MIT License

3.11k stars 2.61k forks source link

[$250] Workspace - "Delay submissions" turns of if the "Submission frequency" is set to "Manually" #43196

Open lanitochka17 opened 4 weeks ago

lanitochka17 commented 4 weeks ago

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: 1.4.80-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

Log in with a new expensifail account
Create a workspace
Turn "Workflows" on
Navigate to Workflows
Turn "Delay submissions" on
Set "Submission frequency" to "Manually"

Expected Result:

"Delay submissions" should remain on

Actual Result:

"Delay submissions" turns of if the "Submission frequency" is set to "Manually"

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/a9771da7-3b1b-4576-b9fb-acc208f2f8c2

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~015bf509d8291c1690
Upwork Job ID: 1800519449372778818
Last Price Increase: 2024-06-25

Issue Owner

Current Issue Owner: @abdulrahuman5196

melvin-bot[bot] commented 4 weeks ago

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 commented 4 weeks ago

@sonialiap FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

dominictb commented 4 weeks ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Workspace - "Delay submissions" turns of if the "Submission frequency" is set to "Manually"
What is the root cause of that problem?
We do not add the correct optimistic data when set submission frequency to "Manually"
What changes do you think we should make in order to solve the problem?
We need to add the below optimistic data when calling setWorkspaceAutoReportingFrequency:

[
    {
        "key": "policyID_${policyID}",
        "onyxMethod": "merge",
        "value": {
            "harvesting": {
                "enabled": false
            },
            "autoReporting": true,
            "autoReportingFrequency": "immediate"
            "pendingFields": {autoReporting: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE},

        }
    }
]

if the frequency is manual.

Also, we need to reset the above optimistic data if API is failed.

What alternative solutions did you explore? (Optional)

Maybe the above optimistic data will be changed if there was BE error in this case and we fixed it, but in general, we need to check what data BE returns in case of manually, then we set the optimistic data based on it.

Tony-MK commented 3 weeks ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Workspace - "Delay submissions" turns of if the "Submission frequency" is set to "Manually"

What is the root cause of that problem?

The root cause of the problem is that when a user sets it to manual, the policy?.harvesting?.enabled becomes false.

The policy?.harvesting?.enabled is false because it checks whether the scheduled submission is enabled which won't be in this situation.

https://github.com/Expensify/App/blob/f3bb5a5a29aa752440190e961d1386d26bee5cf8/src/types/onyx/Policy.ts#L777-L781

Hence, the condition that makes the isActive in the WorkspaceWorkflowsPage becomes false when the auto-reporting frequency is manual.

https://github.com/Expensify/App/blob/f3bb5a5a29aa752440190e961d1386d26bee5cf8/src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx#L132

What changes do you think we should make in order to solve the problem?

Therefore, we must consider the autoReportingFrequency if it is CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MANUAL.

Just like we did with the CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT.

So, let's change the isActive logic to something simpler than the one below.

isActive: (((policy?.harvesting?.enabled && policy.autoReportingFrequency !== CONST.POLICY.AUTO_REPORTING_FREQUENCIES.INSTANT ) ?? (policy?.harvesting?.enabled && policy.autoReportingFrequency !== CONST.POLICY.AUTO_REPORTING_FREQUENCIES.MANUAL) ) && !hasDelayedSubmissionError) ?? false,

melvin-bot[bot] commented 3 weeks ago

Job added to Upwork: https://www.upwork.com/jobs/~015bf509d8291c1690

melvin-bot[bot] commented 3 weeks ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 (External)

melvin-bot[bot] commented 2 weeks ago

@sonialiap, @abdulrahuman5196 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

sonialiap commented 2 weeks ago

@abdulrahuman5196 what do you think of the above proposal?

melvin-bot[bot] commented 2 weeks ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

abdulrahuman5196 commented 2 weeks ago

Hi, I am back now since I was OOO for couple of days as posted in slack. Will work on review today.

abdulrahuman5196 commented 2 weeks ago

checking now

abdulrahuman5196 commented 2 weeks ago

Hi all, I dont see the Delay submissions option in the workflow? Do I need to be part of some beta or any specific type of workspace?

melvin-bot[bot] commented 2 weeks ago

@sonialiap @abdulrahuman5196 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

dominictb commented 1 week ago

@abdulrahuman5196 You need to set canUseAllBetas: true.

melvin-bot[bot] commented 1 week ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 1 week ago

@sonialiap, @abdulrahuman5196 Huh... This is 4 days overdue. Who can take care of this?

abdulrahuman5196 commented 1 week ago

Hi @sonialiap Sorry for the delay, I have limited availability at the moment. So unassigning myself. Kindly reassign another C+ for review.

dukenv0307 commented 1 week ago

I can take this one @sonialiap

melvin-bot[bot] commented 3 days ago

@sonialiap Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 2 days ago

❌ There was an error making the offer to @dukenv0307 for the Contributor role. The BZ member will need to manually hire the contributor.

sonialiap commented 2 days ago

@dukenv0307 added you as the C+

dukenv0307 commented 2 days ago

@Tony-MK @dominictb After choosing Manual option, harvesting.enable is false and frequency is immediate. Do you think it's BE bug? If not, why don't we just remove the manual option, since we can toggle the delay submission instead of choosing Manual?

dominictb commented 1 day ago

@sonialiap Can you check this comment?

dukenv0307 commented 1 day ago

I think we need another internal member

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 1 day ago

Triggered auto assignment to @roryabraham, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

dukenv0307 commented 1 day ago

@roryabraham Can you check my comment above? Thanks

melvin-bot[bot] commented 10 hours ago

@sonialiap @roryabraham @dukenv0307 this issue is now 4 weeks old, please consider:

Finding a contributor to fix the bug
Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!