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

[Due for payment 2025-02-19] [$250] Expensify Card - Step indicator does not change when returning to adding details step #56197

Open
6 of 8 tasks
vincdargento opened this issue Jan 31, 2025 · 14 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

@vincdargento
Copy link

vincdargento commented Jan 31, 2025

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.93-2
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5544667
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Mac 15.0 / Chrome
App Component: Workspace Settings

Action Performed:

Precondition:

  • Expensify Card feature is set up.
  • Private details in Profile are not filled.
  1. Go to staging.new.expensify.com
  2. Go to Workspace settings > Expensify Card.
  3. Click Issue card.
  4. Go through the flow and reach the confirmation page.
  5. Click on any field on the confirmation page to go back to the previous step.
  6. Note that the step indicator changes based on which step user is taken back.
  7. Assign a physical card to yourself.
  8. Go to workspace chat.
  9. Click Add shipping details.
  10. Go through the flow and reach the confirmation page.
  11. Click on any field on the confirmation page to go back to the previous step.

Expected Result:

The step indicator will change based on which step user is taken back.

Actual Result:

The step indicator does not change. All four steps are marked with the checkmark even when user is taken back to the first step for example.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

bug.mp4
Image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021886947168354066147
  • Upwork Job ID: 1886947168354066147
  • Last Price Increase: 2025-02-05
  • Automatic offers:
    • FitseTLT | Contributor | 106014400
Issue OwnerCurrent Issue Owner: @kadiealexander
@vincdargento vincdargento added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Jan 31, 2025
Copy link

melvin-bot bot commented Jan 31, 2025

Triggered auto assignment to @kadiealexander (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.

@FitseTLT
Copy link
Contributor

FitseTLT commented Jan 31, 2025

🚨 Edited by proposal-police: This proposal was edited at 2025-01-31 22:23:45 UTC.

Proposal

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

Expensify Card - Step indicator does not change when returning to adding details step

What is the root cause of that problem?

We are not notifying on the step change for InteractiveStepSubHeader on moveTo

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

We should first create an imperative handle here

movePrevious: () => {
setCurrentStep((actualStep) => actualStep - 1);
},

 moveTo: (step) => {
                setCurrentStep(step);
            },

and in MissingPersonalDetailsContent

 const handleMoveTo = useCallback(
        (i) => {
            ref.current.moveTo(i);
            moveTo(i);
        },
        [moveTo],
    );

pass it here

                onMove={handleMoveTo}

Then to go to the last step on confirmation return lastScreenIndex from useSubStep and utilize it here

const handleNextScreen = useCallback(() => {
if (isEditing) {
goToTheLastStep();

goToTheLastStep();
            ref.current?.moveTo(lastScreenIndex);
            return;

            ref.current?.moveTo(lastScreenIndex);

We can solve similar issues where there is a problem with InteractiveStepSubHeader usage

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/ A

What alternative solutions did you explore? (Optional)

Alternatively, we can consider refactoring InteractiveStepSubHeader to take the currentScreenIndex and its setter as a prop (instead of using imperative handle to control its internal state) and it will display based on the current step and will set the step on the selection of the step header if needed.

Optionally we can use InteractiveStepWrapper on each of the step screens giving them static startStepIndex based on their step similar to IssueNewCard

@Shahidullah-Muffakir
Copy link
Contributor

Shahidullah-Muffakir commented Jan 31, 2025

🚨 Edited by proposal-police: This proposal was edited at 2025-01-31 23:52:25 UTC.

Proposal

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

Step indicator does not change in the header when user goes to any step in the confirmation step.

What is the root cause of that problem?

  1. we are passing the startStepIndex here:
  2. And based on startStepIndex we are displaying the completed Expensify checkmark in the header:
    const isCompletedStep = currentStep > index;
  3. but when user clicks on any step in the confirmation step, the startStepIndex is not being updated:
    onPress: () => {
    onMove(PERSONAL_DETAILS_STEP_INDEXES.LEGAL_NAME);

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

  1. we have to make the startStepIndex dynamic such that when user clicks on any step in the confirmation step, update the startStepIndex to that step
  2. create a new state variable in the MissingPersonalDetailsContent and initialise it with startFrom as:
const [currentIndex, setCurrentIndex] = useState(startFrom);
  1. then pass the currentIndex instead of startFrom here:
  2. Modify the onMove to update the currentIndex , because it is being called when user clicks on any item in the confirmation page , as:
                onMove={(step) => {                 
                    setCurrentIndex(step); 
                    moveTo(step); 
                }}
  1. After that if user clicks the confirmation button, update the currentIndex to the original one as:
                onNext={(step)=>{
                    setCurrentIndex(startFrom);
                    handleNextScreen(step)
                }}
  1. In the InteractiveStepSubHeader update the currentStep whenever the startStepIndex changes as:
    useEffect(()=>{
        setCurrentStep(startStepIndex)
    },[startStepIndex])

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

NA

What alternative solutions did you explore? (Optional)

NA

@melvin-bot melvin-bot bot added the Overdue label Feb 3, 2025
Copy link

melvin-bot bot commented Feb 4, 2025

@kadiealexander Whoops! This issue is 2 days overdue. Let's get this updated quick!

@kadiealexander kadiealexander added the External Added to denote the issue can be worked on by a contributor label Feb 5, 2025
@melvin-bot melvin-bot bot changed the title Expensify Card - Step indicator does not change when returning to adding details step [$250] Expensify Card - Step indicator does not change when returning to adding details step Feb 5, 2025
Copy link

melvin-bot bot commented Feb 5, 2025

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 5, 2025
Copy link

melvin-bot bot commented Feb 5, 2025

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

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2025
@kadiealexander kadiealexander moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Feb 5, 2025
@s77rt
Copy link
Contributor

s77rt commented Feb 5, 2025

@FitseTLT Thanks for the proposal. Your RCA is correct and the solution looks good to me.

🎀 👀 🎀 C+ reviewed
Link to proposal

Copy link

melvin-bot bot commented Feb 5, 2025

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

@s77rt
Copy link
Contributor

s77rt commented Feb 5, 2025

@Shahidullah-Muffakir Thanks for the proposal. Your RCA is not correct.

And based on startStepIndex we are displaying the completed Expensify checkmark in the header

This is not true, startStepIndex is only used for initialization.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 6, 2025
Copy link

melvin-bot bot commented Feb 6, 2025

📣 @FitseTLT 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added the Awaiting Payment Auto-added when associated PR is deployed to production label Feb 12, 2025
@melvin-bot melvin-bot bot changed the title [$250] Expensify Card - Step indicator does not change when returning to adding details step [Due for payment 2025-02-19] [$250] Expensify Card - Step indicator does not change when returning to adding details step Feb 12, 2025
Copy link

melvin-bot bot commented Feb 12, 2025

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

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Feb 12, 2025
Copy link

melvin-bot bot commented Feb 12, 2025

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.96-1 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 2025-02-19. 🎊

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

Copy link

melvin-bot bot commented Feb 12, 2025

@s77rt @kadiealexander @s77rt 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]

Copy link

melvin-bot bot commented Feb 14, 2025

@s77rt @thienlnam @FitseTLT @kadiealexander 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!

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
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

6 participants