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-11-11] [$750] Netsuite - App navigates to the first flow when refreshing the page while adding custom list #49986

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

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 1, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.42-0
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
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

  1. Log in to new dot with any account
  2. Create a new workspace > Enable "Accounting" in the "More features" page
  3. Navigate to "Accounting" > Connect to NetSuite and upgrade the workspace to Control > Enter credentials and finish
  4. After the connection syncs go to Import > Custom Lists > Add custom list
  5. Click on "Name" and select any of the options > Next > Type any ID name > Next
  6. Before making any selection refresh the page

Expected Result:

App stays in the current flow ("How should this custom list be displayed in Expensify?")

Actual Result:

App navigates to the first step of the flow ("Choose a custom list" step)

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

Bug6620920_1727773245095.2024-10-01_11_43_59.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021842185001478860112
  • Upwork Job ID: 1842185001478860112
  • Last Price Increase: 2024-11-12
  • Automatic offers:
    • rayane-djouah | Reviewer | 104455519
    • allgandalf | Contributor | 104455521
Issue OwnerCurrent Issue Owner: @sonialiap
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 1, 2024
Copy link

melvin-bot bot commented Oct 1, 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

@melvin-bot melvin-bot bot added the Overdue label Oct 3, 2024
@sonialiap sonialiap added the External Added to denote the issue can be worked on by a contributor label Oct 4, 2024
Copy link

melvin-bot bot commented Oct 4, 2024

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

@melvin-bot melvin-bot bot changed the title Netsuite - App navigates to the first flow when refreshing the page while adding custom list [$250] Netsuite - App navigates to the first flow when refreshing the page while adding custom list Oct 4, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 4, 2024
Copy link

melvin-bot bot commented Oct 4, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Oct 4, 2024
@allgandalf
Copy link
Contributor

allgandalf commented Oct 5, 2024

Proposal

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

Form navigates to first step on refresh

What is the root cause of that problem?

There are actually 2 bug in the flow:

  • We do not save the current step anywhere in the flow.
    And we always default to the first step of the flow, that is why on refresh we go to the first step:

startStepIndex={CONST.NETSUITE_CUSTOM_FIELD_SUBSTEP_INDEXES.CUSTOM_LISTS.CUSTOM_LIST_PICKER}

  • We also do not save the draft values in the flow.

So even if we fix 1, there would be a regression that values won't be saved once refreshed. So we need to fix both the bugs here.

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

Like the refactor we did in :

We need to refactor the full form to save draft values of the form as well as save the current state of the form (sub-step) using the util getInitialSubstep. We would also need to refactor NetSuiteImportAddCustomSegmentPage as both use the same substeps. and also update any similar places where this bug exists

Note

The scope of work is vast as this would be full form refactor i.e. adding draft states to Onyx, adding draft values to FORM input of both NetSuiteImportAddCustomSegmentPage and NetSuiteImportAddCustomListPage, creating new utils, changing the form structure completely, creating related types files.
So i think that for this issue a fair compensation would be $750 considering the amount of work involved

What alternative solutions did you explore? (Optional)

Copy link

melvin-bot bot commented Oct 7, 2024

@sonialiap, @rayane-djouah Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Oct 7, 2024
@rayane-djouah
Copy link
Contributor

@allgandalf's proposal looks good to me

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Oct 7, 2024

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

@allgandalf
Copy link
Contributor

thanks @rayane-djouah , @danieldoglas please take a look at the note, i have explained the scope of work and requested an increase in bounty, please consider that request 😄

@rayane-djouah
Copy link
Contributor

I agree that a payment increase would be fair given the scope of work

@allgandalf
Copy link
Contributor

bump @danieldoglas for assignment

@danieldoglas
Copy link
Contributor

I missed this issue, my bad. I've read the proposal and I initially agree with it, but I'll refer to @yuwenmemon since he was one of the people working on this functionality before moving on.

Copy link

melvin-bot bot commented Oct 11, 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 Oct 11, 2024
@rayane-djouah
Copy link
Contributor

@yuwenmemon Kind reminder

@melvin-bot melvin-bot bot removed the Overdue label Oct 11, 2024
@allgandalf
Copy link
Contributor

I checked their slack, they are out of office until tuesday, @danieldoglas how should we proceed here?

@allgandalf
Copy link
Contributor

@sonialiap can you bump the price for this issue please 🙏

@mananjadhav
Copy link
Collaborator

Thanks for picking this folks. I missed this one from the design doc. Let me know if any other help/secondary review is needed.

@allgandalf
Copy link
Contributor

@rayane-djouah PR ready for review, there might be some style changes which i am working on currently , but you can test the functionality it works smooth

@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
@allgandalf
Copy link
Contributor

@rayane-djouah fixed the style changes as well, PR should be ready for final review

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Oct 21, 2024
@rayane-djouah
Copy link
Contributor

PR on staging

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 4, 2024
@melvin-bot melvin-bot bot changed the title [$250] Netsuite - App navigates to the first flow when refreshing the page while adding custom list [HOLD for payment 2024-11-11] [$250] Netsuite - App navigates to the first flow when refreshing the page while adding custom list Nov 4, 2024
Copy link

melvin-bot bot commented Nov 4, 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 Nov 4, 2024
Copy link

melvin-bot bot commented Nov 4, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.56-9 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-11-11. 🎊

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

Copy link

melvin-bot bot commented Nov 4, 2024

@rayane-djouah / @allgandalf @sonialiap The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@allgandalf
Copy link
Contributor

@sonialiap can you bump the payment to $750 here please ;)

@rayane-djouah
Copy link
Contributor

rayane-djouah commented Nov 11, 2024

BugZero Checklist:

  • Classify the bug:

    Bug classification

    Source of bug:

    • 1a. Result of the original design (eg. a case wasn't considered)
    • 1b. Mistake during implementation
    • 1c. Backend bug
    • 1z. Other:

    Where bug was reported:

    • 2a. Reported on production
    • 2b. Reported on staging (deploy blocker)
    • 2c. Reported on a PR
    • 2z. Other:

    Who reported the bug:

    • 3a. Expensify user
    • 3b. Expensify employee
    • 3c. Contributor
    • 3d. QA
    • 3z. Other:
  • 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: [#Wave-Control: Add NetSuite]: Settings Configuration in NewDot: Import - Add Custom Records/Lists #44942 (comment)

  • If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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

  • If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

  • @sonialiap Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue: https://github.com/Expensify/Expensify/issues/443451

Regression Test Proposal

#### Precondition:

- Workspace connected to the NetSuite integration.

#### Tests:

Test 1:

1. Navigate to Accounting > NetSuite Import > Custom Lists > Add New List.
2. Enter a custom list name and click "Next"; this takes you to the Transaction ID page.
3. Refresh the page.
4. Verify that upon refresh, you remain on the Transaction ID page.
5. Navigate back to the previous page.
6. Verify that the previously selected value persists.

Test 2:

1. Navigate to Accounting > NetSuite Import > Custom Segments > Add New Segment.
2. Select a custom record and click "Next".
3. Enter a custom record name and click "Next"; this takes you to the Internal ID page.
4. Refresh the page.
5. Verify that upon refresh, you remain on the Internal ID page.
6. Navigate back to the previous page.
7. Verify that the previously entered information persists.


Do we agree 👍 or 👎

@allgandalf
Copy link
Contributor

@sonialiap friendly bump for the bounty increase 😄

@sonialiap
Copy link
Contributor

sonialiap commented Nov 12, 2024

Chatted with Daniel and confirmed increase to $750 due to scope of work

Payment summary:

  • @rayane-djouah $750 - amount updated on accepted contract and paid ✔️
  • @allgandalf $750 - original contract not yet accepted so sent a new one with updated amount - paid ✔️

@sonialiap sonialiap changed the title [HOLD for payment 2024-11-11] [$250] Netsuite - App navigates to the first flow when refreshing the page while adding custom list [HOLD for payment 2024-11-11] [$750] Netsuite - App navigates to the first flow when refreshing the page while adding custom list Nov 12, 2024
Copy link

melvin-bot bot commented Nov 12, 2024

Upwork job price has been updated to $750

@sonialiap
Copy link
Contributor

All paid 🎉

@github-project-automation github-project-automation bot moved this from Bugs and Follow Up Issues to Done in [#whatsnext] #expense Nov 12, 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. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Archived in project
Development

No branches or pull requests

7 participants