Skip to content

[HOLD for payment 2024-10-16] [$250] Category rules - Approver field in category editor shows user email instead of user name #49277

Closed

Description

@IuliiaHerets

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: 9.0.35-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  • Rules is enabled.
  • User has custom name.
  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Workflows.
  3. Click on the existing approval workflow under Add approvals.
  4. Note that Approver field shows user name.
  5. Go to workspace settings > Categories.
  6. Click on any category.
  7. Click Approver.
  8. Select a user with custom name.

Expected Result:

Approver field in category editor will show user name.

Actual Result:

Approver field in category editor shows user email instead of user name.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6604520_1726417968746.20240916_002851.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021842664254232202531
  • Upwork Job ID: 1842664254232202531
  • Last Price Increase: 2024-10-05
  • Automatic offers:
    • suneox | Reviewer | 104295831
    • cretadn22 | Contributor | 104295833
Issue OwnerCurrent Issue Owner: @anmurali

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Sep 16, 2024
  2. melvin-bot commented on Sep 16, 2024

    @melvin-bot

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

  3. IuliiaHerets commented on Sep 16, 2024

    @IuliiaHerets
    Author

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

  4. nyomanjyotisa commented on Sep 16, 2024

    @nyomanjyotisa
    Member

    Proposal

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

    Approver field in category editor shows user email instead of user name

    What is the root cause of that problem?

    function getCategoryApprover(approvalRules: ApprovalRule[], categoryName: string) {
    return approvalRules?.find((rule) => rule.applyWhen.some((when) => when.value === categoryName))?.approver;
    }

    We display the approver field here which is the approver email

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

    Update getCategoryApprover to get the approver display name using the email, and return the display name

    function getCategoryApprover(approvalRules: ApprovalRule[], categoryName: string) {
        const approverEmail = approvalRules?.find((rule) => rule.applyWhen.some((when) => when.value === categoryName))?.approver;
    
        if(!approverEmail){
            return;
        }
    
        const approver = PersonalDetailsUtils.getPersonalDetailByEmail(approverEmail);
    
        return approver?.displayName ?? approverEmail;
    }
    

    What alternative solutions did you explore? (Optional)

  5. cretadn22 commented on Sep 16, 2024

    @cretadn22
    Contributor

    Proposal

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

    Approver field in category editor shows user email instead of user name

    What is the root cause of that problem?

    We show the approver field here which is email of approver

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

    We should avoid updating the getCategoryApprover function, as changes could impact other places

    const approverText = useMemo(() => {
    const categoryApprover = CategoryUtils.getCategoryApprover(policy?.rules?.approvalRules ?? [], categoryName);
    return categoryApprover ?? '';
    }, [categoryName, policy?.rules?.approvalRules]);

    We should retrieve the display name from PersonalDetails and show that instead of displaying the email address directly.

        const approverText = useMemo(() => {
            const categoryApprover = CategoryUtils.getCategoryApprover(policy?.rules?.approvalRules ?? [], categoryName);
            const approver = PersonalDetailsUtils.getPersonalDetailByEmail(categoryApprover ?? '');
            return approver.displayName ?? categoryApprover ?? '';
        }, [categoryName, policy?.rules?.approvalRules]);
    
    

    What alternative solutions did you explore? (Optional)

  6. nkdengineer commented on Sep 17, 2024

    @nkdengineer
    Contributor

    Proposal

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

    Approver field in category editor shows user email instead of user name.

    What is the root cause of that problem?

    categoryApprover here is the email of approver

    const categoryApprover = CategoryUtils.getCategoryApprover(policy?.rules?.approvalRules ?? [], categoryName);

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

    We should get the personal details of the approver based on the email then use getDisplayNameOrDefault function to get the display name of the approver

    const approverText = useMemo(() => {
        const categoryApprover = CategoryUtils.getCategoryApprover(policy?.rules?.approvalRules ?? [], categoryName);
        if (!categoryApprover) {
            return '';
        }
        const personalDetails = getPersonalDetailByEmail(categoryApprover);
        return getDisplayNameOrDefault(personalDetails) ?? '';
    }, [categoryName, policy?.rules?.approvalRules]);
    

    const categoryApprover = CategoryUtils.getCategoryApprover(policy?.rules?.approvalRules ?? [], categoryName);

    getDisplayNameOrDefault function will cover all cases of the user like remove SMS domain of phone number user, ....

    What alternative solutions did you explore? (Optional)

    We can create a new util like getCategoryApproverDisplayName and use getCategoryApprover function and the logic above to return the display name of the approver

  7. melvin-bot commented on Sep 19, 2024

    @melvin-bot

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

  8. melvin-bot commented on Sep 23, 2024

    @melvin-bot

    @anmurali Still overdue 6 days?! Let's take care of this!

  9. anmurali commented on Sep 25, 2024

    @anmurali

    I am not sure where to go to turn on rules. I'll have to circle back to this one.

  10. melvin-bot commented on Sep 30, 2024

    @melvin-bot

    @anmurali this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

  11. 39 remaining items

  12. melvin-bot commented on Oct 23, 2024

    @melvin-bot

    @francoisl, @suneox, @anmurali, @cretadn22 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

  13. melvin-bot commented on Oct 25, 2024

    @melvin-bot

    @francoisl, @suneox, @anmurali, @cretadn22 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

  14. anmurali commented on Oct 29, 2024

    @anmurali

    @suneox - it is checked off but I don’t see a regression test. Can you please add one?

  15. anmurali commented on Oct 29, 2024

    @anmurali

    Ah nm, you said we do not need one. @francoisl do you agree?

  16. suneox commented on Oct 31, 2024

    @suneox
    Contributor

    Before proposing test steps for a bug, we first need to determine whether or not we should create a test for it. In order to determine this, we look for these common traits:

    Is it easy to test for this bug? Yes
    Is the bug related to an important user flow? I think No: it doesn’t affect the user flow as users can still select/update the approver
    Is it an impactful bug? I think No: it only affects display names without breaking functionality

    @anmurali Based on the best practices for Regression Test I think we can skip regression test for this issue

  17. suneox commented on Nov 3, 2024

    @suneox
    Contributor

    Regression Test Proposal

    • Step 1: Go to workspace settings > More features.
    • Step 2: Enable Categories in Organize section
    • Step 3: Enable Workflows in Manage section
    • Step 4: Go to workspace settings > Member.
    • Step 5: Invite member has config user name into workspace
    • Step 6: Go to workspace settings > Workflows.
    • Step 7: Add approvals member has invited from step 5.
    • Step 8: Go to workspace settings > Categories.
    • Step 9: Select any category > Approver
    • Step 10: Verify Approver field in category editor will show user name at step 5

    Do we agree 👍 or 👎

    cc: @anmurali

  18. melvin-bot commented on Nov 4, 2024

    @melvin-bot

    @francoisl, @suneox, @anmurali, @cretadn22 Huh... This is 4 days overdue. Who can take care of this?

  19. francoisl commented on Nov 5, 2024

    @francoisl
    Contributor

    I agree we can skip the regression test for this issue.

  20. anmurali commented on Nov 6, 2024

    @anmurali

    Paid.

  21. moved this from Hold for Payment to Done in #expensify-bugson Nov 6, 2024
  22. melvin-bot commented on Nov 6, 2024

    @melvin-bot

    @francoisl @anmurali Be sure to fill out the Contact List!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions