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-09-26] [$250] [Workspace Feeds] Expensify Card - Policy ID is displayed under the card's name for member #48749

Closed
6 tasks done
IuliiaHerets opened this issue Sep 6, 2024 · 20 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

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 6, 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.30-9
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  • Admin and member are in the same workspace.
  1. Go to staging.new.expensify.com
  2. [Admin] Go to workspace settings > Expensify Card.
  3. [Admin] Click Issue card.
  4. [Admin] Issue a card to the member.
  5. [Member] Go to Account settings > Wallet.

Expected Result:

Policy ID will not be displayed under the card's name for member.

Actual Result:

Policy ID is displayed under the card's name for member.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6595247_1725621384137.20240906_191004.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021833064845344667979
  • Upwork Job ID: 1833064845344667979
  • Last Price Increase: 2024-09-09
  • Automatic offers:
    • allgandalf | Reviewer | 103877191
    • DylanDylann | Reviewer | 103877193
    • dominictb | Contributor | 103877195
Issue OwnerCurrent Issue Owner: @RachCHopkins
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 6, 2024
Copy link

melvin-bot bot commented Sep 6, 2024

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

@IuliiaHerets
Copy link
Author

@RachCHopkins 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

Proposal

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

  • Policy ID is displayed under the card's name for member.

What is the root cause of that problem?

  • When displaying assigned card, we always display its domain name regardless the domain name contain the policyID or not:

description: card.domainName,

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

  • We can use:
                        description: /policy([a-zA-Z0-9]+)\.exfy/.test(card.domainName) ? '' : card.domainName,

or

                        description: /policy([a-zA-Z0-9]+)\.exfy/.test(card.domainName) ? getDescriptionForPolicyDomain(card.domainName) : card.domainName,
  • In above, I introduce a new function getDescriptionForPolicyDomain which will return the policy name based on card domain:
    const getDescriptionForPolicyDomain = (domainName: string) => {
        const match = domainName.match(/policy([a-zA-Z0-9]+)\.exfy/);
        const policyID = match ? match[1] : null;
        const policy = getPolicy(policyID)
        return policy?.name // Can be improved later
    };

What alternative solutions did you explore? (Optional)

@cretadn22
Copy link
Contributor

Proposal

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

Policy ID is displayed under the card's name for member

What is the root cause of that problem?

The PolicyID is included in the domainName for Expensify Cards. As a result, when displaying the domain name, the PolicyID will also be shown.

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

Checking if this is the Expensify Card, we will customize the domain name to prevent the policyID from being displayed.

  • To identify an Expensify Card, we can use the isExpensifyCard function or create a new function to identify it using the domainName field

function isExpensifyCard(cardID?: number) {

  • We need to discuss with other stuffs which domain name to display in the subtitle of the Expensify Card, rather than showing the domain name from the backend. We will then update it to display the new subtitle.
1

What alternative solutions did you explore? (Optional)

@RachCHopkins
Copy link
Contributor

What is the expectation here? If the card is issued from the workspace, I'd expect the workspace name, not just the domain name. I will check on this.

@mountiny mountiny changed the title Expensify Card - Policy ID is displayed under the card's name for member [Workspace Feeds] Expensify Card - Policy ID is displayed under the card's name for member Sep 9, 2024
@mountiny mountiny added the External Added to denote the issue can be worked on by a contributor label Sep 9, 2024
@melvin-bot melvin-bot bot changed the title [Workspace Feeds] Expensify Card - Policy ID is displayed under the card's name for member [$250] [Workspace Feeds] Expensify Card - Policy ID is displayed under the card's name for member Sep 9, 2024
Copy link

melvin-bot bot commented Sep 9, 2024

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

@melvin-bot melvin-bot bot added Help Wanted Apply this label when an issue is open to proposals by contributors and removed Help Wanted Apply this label when an issue is open to proposals by contributors labels Sep 9, 2024
Copy link

melvin-bot bot commented Sep 9, 2024

Current assignees @allgandalf and @DylanDylann are eligible for the External assigner, not assigning anyone new.

Copy link

melvin-bot bot commented Sep 9, 2024

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

Offer link
Upwork job

Copy link

melvin-bot bot commented Sep 9, 2024

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

Offer link
Upwork job

Copy link

melvin-bot bot commented Sep 9, 2024

📣 @dominictb 🎉 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 📖

@mountiny
Copy link
Contributor

mountiny commented Sep 9, 2024

@dominictb yes, let's show the policy name there if the domain is workspace feed. Your proposal makes sense, can you create a PR? What is your ETA? Thanks!

@trjExpensify trjExpensify moved this to Release 2.5: SuiteWorld (Sept 9th) in [#whatsnext] #wave-collect Sep 9, 2024
@trjExpensify trjExpensify moved this from Release 2.5: SuiteWorld (Sept 9th) to Polish in [#whatsnext] #wave-collect Sep 9, 2024
@melvin-bot melvin-bot bot added Reviewing Has a PR in review and removed Daily KSv2 labels Sep 9, 2024
@melvin-bot melvin-bot bot added the Weekly KSv2 label Sep 9, 2024
@allgandalf allgandalf removed their assignment Sep 11, 2024
@RachCHopkins
Copy link
Contributor

With the automation borked, and this looking like it was on Prod a week ago, can you do the checklist please @DylanDylann?

The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@DylanDylann] The PR that introduced the bug has been identified. Link to the PR:
  • [@DylanDylann] 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:
  • [@DylanDylann] 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:
  • [@DylanDylann] Determine if we should create a regression test for this bug.
  • [@DylanDylann] 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.
  • [@RachCHopkins] Link the GH issue for creating/updating the regression test once above steps have been agreed upon

@DylanDylann
Copy link
Contributor

@RachCHopkins This feature is still behind beta, I don't think we need to checklist here

@mountiny
Copy link
Contributor

I agree we will include tests in the test rail as we officially release this feature

@RachCHopkins
Copy link
Contributor

Is this ready for me to pay people? I think that's the part I'm not understanding.

@DylanDylann
Copy link
Contributor

@RachCHopkins Yes. Let's pay $250 to @dominictb

cc @mountiny

@RachCHopkins
Copy link
Contributor

What about you @DylanDylann? Do you also need to be paid?

@DylanDylann
Copy link
Contributor

No need payment to me. I will get paid on the project

@mountiny mountiny added Daily KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Weekly KSv2 labels Sep 25, 2024
@mountiny mountiny changed the title [$250] [Workspace Feeds] Expensify Card - Policy ID is displayed under the card's name for member [HOLD for payment 2024-09-26] [$250] [Workspace Feeds] Expensify Card - Policy ID is displayed under the card's name for member Sep 25, 2024
@mountiny
Copy link
Contributor

Sounds good to me

@RachCHopkins
Copy link
Contributor

Contributor has been paid, the contract has been completed, and the Upwork post has been closed.

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
Status: Done
Development

No branches or pull requests

7 participants