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

[$250] Company cards-The card name is reset even if the cardholder has not been changed #56206

Open
4 of 8 tasks
IuliiaHerets opened this issue Feb 1, 2025 · 19 comments
Open
4 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Feb 1, 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-3
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5543601
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Windows 10/ Chrome, Samsung S23FE/ Android 14
App Component: Workspace Settings

Action Performed:

  1. In the account [email protected]
    Navigate to the workspace "Company Cards tests - DO NOT DELETE!"
  2. Tap on Company cards in the workspace editor
  3. Tap on the Feed Selector
  4. Select Visa cards
  5. Tap on Assign a card
  6. Select the user you're logged in as ([email protected])
  7. Tap Next
  8. Tap a card from the row
  9. Tap Custom start state
  10. Tap on the Date row
  11. Select a new date
  12. Tap Next
  13. Edit the Card name
  14. Tap on Cardholder
  15. Tap on Confirm

Expected Result:

The card name remains the same when the same cardholder has been selected or the selection has not been changed.

Actual Result:

The card name is reset even if the cardholder has not been changed.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6729972_1738360996916.Card_name_is_reset.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021886883012132760890
  • Upwork Job ID: 1886883012132760890
  • Last Price Increase: 2025-02-04
Issue OwnerCurrent Issue Owner: @hungvu193
@IuliiaHerets IuliiaHerets added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Feb 1, 2025
Copy link

melvin-bot bot commented Feb 1, 2025

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

@twilight2294
Copy link
Contributor

Proposal

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

The card name is reset even if the cardholder has not been changed

What is the root cause of that problem?

We set a new card name everytime a member is selected, regardless of the logic that the same member might have been selected:

const data: Partial<AssignCardData> = {
email: selectedMember,
cardName: CardUtils.getDefaultCardName(memberName),
};

This causes the cardName to get updated even when the same member got selected.

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

We need to add a condition such that if the selected member is same as assignCard?.data?.email then do not update the data / set the data to the currently existing data:

        const data: Partial<AssignCardData> =
            selectedMember === assignCard?.data?.email
                ? assignCard.data
                : {
                      email: selectedMember,
                      cardName: CardUtils.getDefaultCardName(memberName),
                  };

Or we can simply update the cardName condition to assign the same card name as assignCard?.data?.cardName if selectedMember === assignCard?.data?.email

One more optimisation i found is to send the assignCard details as prop to AssigneeStep from IssueNewCardPage

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

N/A this is a data assignment bug with the way we handle the logic to update the values for same member selection

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Overdue label Feb 3, 2025
@greg-schroeder greg-schroeder added the External Added to denote the issue can be worked on by a contributor label Feb 4, 2025
@melvin-bot melvin-bot bot changed the title Company cards-The card name is reset even if the cardholder has not been changed [$250] Company cards-The card name is reset even if the cardholder has not been changed Feb 4, 2025
Copy link

melvin-bot bot commented Feb 4, 2025

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

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

melvin-bot bot commented Feb 4, 2025

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

@greg-schroeder
Copy link
Contributor

Sending to External. I think #retain makes sense for this one given it's an admin experience bug for an established customer

@hungvu193
Copy link
Contributor

@twilight2294 How do you enable the expensify card feeed?

@cretadn22
Copy link
Contributor

Proposal

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

The card name is reset even if the cardholder has not been changed

What is the root cause of that problem?

When choosing a new assignee, we reset the cardholder to its default value

cardName: CardUtils.getDefaultCardName(memberName),

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

Need to implement an early return on the submit button if the selected member hasn't been changed

        let nextStep: AssignCardStep = CONST.COMPANY_CARD.STEP.CARD;
        if (selectedMember === assignCard?.data?.email) {
            CompanyCards.setAssignCardStepAndData({
                currentStep: isEditing ? CONST.COMPANY_CARD.STEP.CONFIRMATION : nextStep,
                isEditing: false,
            });
        }

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

It's not needed as I think

What alternative solutions did you explore? (Optional)

@cretadn22
Copy link
Contributor

@hungvu193 I don't have access to this feature, but we can reproduce the bug by modifying the local code and manually adding some data to storage

@hungvu193
Copy link
Contributor

hungvu193 commented Feb 5, 2025

Asking for help with adding a commercial card on Slack. I'll be circling back here soon.

@cretadn22
Copy link
Contributor

@hungvu193 Could you help to add a commercial card to my account too?

@hungvu193
Copy link
Contributor

@hungvu193 Could you help to add a commercial card to my account too?

I'm not sure if external contributors are allowed to add company card, but let me ask internally.

@twilight2294
Copy link
Contributor

@twilight2294 How do you enable the expensify card feeed?

I mocked data for this @hungvu193

@hungvu193
Copy link
Contributor

Thanks for the proposals, everyone. I also prefer adding an early return if we select the same person to avoid unnecessary renders. @cretadn22 's proposal here looks good to me!
🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Feb 6, 2025

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

@twilight2294
Copy link
Contributor

twilight2294 commented Feb 6, 2025

@hungvu193 I found a problem here, ASSIGN_CARD onyx key contains generic data, i.e. if we switch from workspaces this data would persist, previously I worked on the same issue for expensify cards where i converted the Onyx key to a collection to have it unique for each workspace #55125 , and i propose to do it the same for this as well, what do you think ?

Saying cause i tested it now and current implementation and suggested proposals would lead to bugs in the future, so would it be okay if we do so and update the key to collection unique for each workspace ? I would love to work on it and update the proposal as well

c.c. @MonilBhavsar as well

@cretadn22
Copy link
Contributor

@twilight2294 Could you clarify which bug might occur with my solution?

@hungvu193
Copy link
Contributor

@hungvu193 I found a problem here, ASSIGN_CARD onyx key contains generic data, i.e. if we switch from workspaces this data would persist, previously I worked on the same issue for expensify cards where i converted the Onyx key to a collection to have it unique for each workspace #55125 , and i propose to do it the same for this as well, what do you think ?

I don't think I can reproduce it, and it doesn't seem to be related to this issue 🤔

@twilight2294
Copy link
Contributor

@twilight2294 Could you clarify which bug might occur with my solution?

Not specific to yours but both of our solutions. The same data will persist for different workspaces.

I don't think I can reproduce it, and it doesn't seem to be related to this issue 🤔

We would be able to reproduce this one if we have company feeds added to 2 workspaces, same reproduction steps as the ones here #55125, just for company cards, just found it out while experimenting the solutions

@cretadn22
Copy link
Contributor

cretadn22 commented Feb 7, 2025

@twilight2294 Your explanation is a bit unclear. Could you provide a video to clarify your concern? I don't believe it's related to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
Status: No status
Development

No branches or pull requests

6 participants