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.

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

[HOLD for payment 2024-07-26] [$250] Workspace - "Delay submissions" turns off if the "Submission frequency" is set to "Manually" #43196

Closed
1 of 6 tasks
lanitochka17 opened this issue Jun 6, 2024 · 53 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Jun 6, 2024

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:

  1. Log in with a new expensifail account
  2. Create a workspace
  3. Turn "Workflows" on
  4. Navigate to Workflows
  5. Turn "Delay submissions" on
  6. Set "Submission frequency" to "Manually"

Expected Result:

"Delay submissions" should remain on

Actual Result:

"Delay submissions" turns off 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
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6501893_1717524990298.bandicam_2024-06-04_19-14-44-441.mp4

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 OwnerCurrent Issue Owner: @dukenv0307
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 6, 2024
Copy link

melvin-bot bot commented Jun 6, 2024

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
Copy link
Author

@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
Copy link
Contributor

dominictb commented Jun 7, 2024

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
Copy link
Contributor

Tony-MK commented Jun 9, 2024

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.

/** Scheduled submit data */
harvesting?: {
/** Whether the scheduled submit is enabled */
enabled: boolean;
};

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

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

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 melvin-bot bot added the Overdue label Jun 9, 2024
@sonialiap sonialiap added the External Added to denote the issue can be worked on by a contributor label Jun 11, 2024
@melvin-bot melvin-bot bot changed the title Workspace - "Delay submissions" turns of if the "Submission frequency" is set to "Manually" [$250] Workspace - "Delay submissions" turns of if the "Submission frequency" is set to "Manually" Jun 11, 2024
Copy link

melvin-bot bot commented Jun 11, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 11, 2024
Copy link

melvin-bot bot commented Jun 11, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 11, 2024
Copy link

melvin-bot bot commented Jun 14, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jun 14, 2024
@sonialiap
Copy link
Contributor

@abdulrahuman5196 what do you think of the above proposal?

Copy link

melvin-bot bot commented Jun 18, 2024

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

@abdulrahuman5196
Copy link
Contributor

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 18, 2024
@abdulrahuman5196
Copy link
Contributor

checking now

@abdulrahuman5196
Copy link
Contributor

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?

Screenshot 2024-06-20 at 9 28 06 PM

Copy link

melvin-bot bot commented Jun 20, 2024

@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
Copy link
Contributor

@abdulrahuman5196 You need to set canUseAllBetas: true.

Copy link

melvin-bot bot commented Jun 25, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jun 25, 2024
Copy link

melvin-bot bot commented Jun 25, 2024

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

@abdulrahuman5196
Copy link
Contributor

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

@melvin-bot melvin-bot bot removed the Overdue label Jun 26, 2024
@roryabraham
Copy link
Contributor

PR is C+ approved, just waiting on review from @Beamanator, @mountiny, and/or @madmax330

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jul 19, 2024
@melvin-bot melvin-bot bot changed the title [$250] Workspace - "Delay submissions" turns off if the "Submission frequency" is set to "Manually" [HOLD for payment 2024-07-26] [$250] Workspace - "Delay submissions" turns off if the "Submission frequency" is set to "Manually" Jul 19, 2024
Copy link

melvin-bot bot commented Jul 19, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 19, 2024
Copy link

melvin-bot bot commented Jul 19, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.9-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-07-26. 🎊

For reference, here are some details about the assignees on this issue:

  • @dukenv0307 requires payment (Needs manual offer from BZ)

Copy link

melvin-bot bot commented Jul 19, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@dukenv0307] The PR that introduced the bug has been identified. Link to the PR:
  • [@dukenv0307] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@dukenv0307] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@dukenv0307] Determine if we should create a regression test for this bug.
  • [@dukenv0307] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@Tony-MK
Copy link
Contributor

Tony-MK commented Jul 24, 2024

Hey, @roryabraham, I am curious to know if I am eligible for compensation for providing a solution that was used in the recently merged PR based on your previous comment.

Thank you

@sonialiap
Copy link
Contributor

@roryabraham what do you think? #43196 (comment)

@sonialiap
Copy link
Contributor

sonialiap commented Jul 26, 2024

Payment summary:

https://www.upwork.com/ab/applicants/1816797650551015987/job-details

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 26, 2024
Copy link

melvin-bot bot commented Jul 26, 2024

Payment Summary

Upwork Job

BugZero Checklist (@sonialiap)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1800519449372778818/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@dukenv0307
Copy link
Contributor

BugZero Checklist:

  • The PR that introduced the bug has been identified. Link to the PR: N/A
  • The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
  • A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
  • Determine if we should create a regression test for this bug. Yes
  • If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression tests:

  1. Create a workspace
  2. Go to workspace settings, click more features, and enable workflows
  3. Go to workflows and toggle delay submissions
  4. Switch the reporting frequency to Daily. Verify that it works and shows Daily
  5. Switch the reporting frequency to Manually. Verify that it works and shows Manually.

Do we 👍 or 👎

@roryabraham
Copy link
Contributor

Hey, @roryabraham, I am curious to know if I am eligible for compensation for providing a solution that was used in the recently merged

Sorry, but no I don't think so. I figured out the solution by looking at back-end code, and the proposal you posted wasn't a complete and correct solution. I merely meant to acknowledge that out of all the proposals yours was the closest to being correct.

@sonialiap
Copy link
Contributor

@dukenv0307 please accept the offer in upwork 💰

@melvin-bot melvin-bot bot added the Overdue label Jul 30, 2024
Copy link

melvin-bot bot commented Jul 30, 2024

@sonialiap, @roryabraham, @dukenv0307 Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Aug 1, 2024

@sonialiap, @roryabraham, @dukenv0307 Eep! 4 days overdue now. Issues have feelings too...

@dukenv0307
Copy link
Contributor

@sonialiap Sorry I don't see any offer, could you check again and send an offer to my profile here https://www.upwork.com/freelancers/~01f5cbe690701118a2

Thank you!

@melvin-bot melvin-bot bot removed the Overdue label Aug 2, 2024
@sonialiap
Copy link
Contributor

@dukenv0307 it seems like I sent the offer to that profile, I just sent a message on the offer. Let me know if you don't see it then I'll resend :D

@sonialiap
Copy link
Contributor

sonialiap commented Aug 2, 2024

Oh I misnamed the offer with another issue number 🤦 I called it 44830, updated in upwork to 43196

@dukenv0307
Copy link
Contributor

@sonialiap No worry, I accepted it

@melvin-bot melvin-bot bot added the Overdue label Aug 5, 2024
@sonialiap
Copy link
Contributor

Paid ✔️

@melvin-bot melvin-bot bot removed the Overdue label Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
No open projects
Status: Done
Development

No branches or pull requests

7 participants