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-20] [$125] [Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled #51620

Closed
luacmartins opened this issue Oct 28, 2024 · 33 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@luacmartins
Copy link
Contributor

luacmartins commented Oct 28, 2024

Problem

We currently show Invoice as a Search type in the LHN for users, but that's confusing if they never enabled that feature

Solution

Avoid confusion and remove the Invoice type if users don't have this option enabled in any of their policies

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021851031006738225317
  • Upwork Job ID: 1851031006738225317
  • Last Price Increase: 2024-11-04
  • Automatic offers:
    • Krishna2323 | Contributor | 104759689
    • jaydamani | Contributor | 104770908
Issue OwnerCurrent Issue Owner: @alexpensify
@luacmartins luacmartins added Daily KSv2 NewFeature Something to build that is a new item. labels Oct 28, 2024
@luacmartins luacmartins self-assigned this Oct 28, 2024
@luacmartins luacmartins added the External Added to denote the issue can be worked on by a contributor label Oct 28, 2024
Copy link

melvin-bot bot commented Oct 28, 2024

Triggered auto assignment to @alexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added the Weekly KSv2 label Oct 28, 2024
Copy link

melvin-bot bot commented Oct 28, 2024

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

@melvin-bot melvin-bot bot removed the Daily KSv2 label Oct 28, 2024
@melvin-bot melvin-bot bot changed the title [Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled [$250] [Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled Oct 28, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 28, 2024
Copy link

melvin-bot bot commented Oct 28, 2024

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

@luacmartins luacmartins changed the title [$250] [Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled [$125] [Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled Oct 28, 2024
@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 28, 2024
Copy link

melvin-bot bot commented Oct 28, 2024

Upwork job price has been updated to $125

@Krishna2323
Copy link
Contributor

Krishna2323 commented Oct 28, 2024

Edited by proposal-police: This proposal was edited at 2024-10-28 22:58:42 UTC.

Proposal


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

[Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled

What is the root cause of that problem?

Improvement

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


  • We should create a new util function hasPolicyWithEnabledInvoice in PolicyUtils to check if there is any workspace with enabled invoice.
function hasPolicyWithEnabledInvoice() {
    return Object.values(allPolicies ?? {}).some((policy) => {
        return policy && policy.areInvoicesEnabled && policy.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
    });
}
  • We should remove the Invoices option from typeMenuItems and only push it if PolicyUtils.hasPolicyWithEnabledInvoice() is true.
  • We can also use useMemo to memoize the result of hasPolicyWithEnabledInvoice. We would nee to get allPolicies from onyx.
const isInvoiceEnabled = useMemo(() => PolicyUtils.hasPolicyWithEnabledInvoice(), [allPolicies]);

{
title: translate('workspace.common.invoices'),
type: CONST.SEARCH.DATA_TYPES.INVOICE,
icon: Expensicons.InvoiceGeneric,
getRoute: (policyID?: string) => {
const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.INVOICE, status: CONST.SEARCH.STATUS.INVOICE.ALL, policyID});
return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
},
},

    if (PolicyUtils.hasPolicyWithEnabledInvoice()) {
        typeMenuItems.push({
            title: translate('workspace.common.invoices'),
            type: CONST.SEARCH.DATA_TYPES.INVOICE,
            icon: Expensicons.InvoiceGeneric,
            getRoute: (policyID?: string) => {
                const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.INVOICE, status: CONST.SEARCH.STATUS.INVOICE.ALL, policyID});
                return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
            },
        });
    }
  • Same should be done in SearchTypeMenuNarrow file.

What alternative solutions did you explore? (Optional)

  • We can optionally add the option in the array to keep the option at the same index.
    const typeMenuItems: SearchTypeMenuItem[] = [
        {
            title: translate('common.expenses'),
            type: CONST.SEARCH.DATA_TYPES.EXPENSE,
            icon: Expensicons.Receipt,
            getRoute: (policyID?: string) => {
                const query = SearchQueryUtils.buildCannedSearchQuery({policyID});
                return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
            },
        },
        {
            title: translate('common.chats'),
            type: CONST.SEARCH.DATA_TYPES.CHAT,
            icon: Expensicons.ChatBubbles,
            getRoute: (policyID?: string) => {
                const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.CHAT, status: CONST.SEARCH.STATUS.CHAT.ALL, policyID});
                return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
            },
        },
        ...((PolicyUtils.hasPolicyWithEnabledInvoice()
            ? [
                  {
                      title: translate('workspace.common.invoices'),
                      type: CONST.SEARCH.DATA_TYPES.INVOICE,
                      icon: Expensicons.InvoiceGeneric,
                      getRoute: (policyID?: string) => {
                          const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.INVOICE, status: CONST.SEARCH.STATUS.INVOICE.ALL, policyID});
                          return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
                      },
                  },
              ]
            : []),
        {
            title: translate('travel.trips'),
            type: CONST.SEARCH.DATA_TYPES.TRIP,
            icon: Expensicons.Suitcase,
            getRoute: (policyID?: string) => {
                const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.TRIP, status: CONST.SEARCH.STATUS.TRIP.ALL, policyID});
                return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
            },
        },
    ];

Result

@luacmartins
Copy link
Contributor Author

only push it to the array if policyID from query is undefined

I think this part is incorrect. We want to push it to the array if you have any policies with invoices enabled in Onyx.

@Krishna2323
Copy link
Contributor

@luacmartins, I have updated my proposal .

@jaydamani
Copy link
Contributor

Proposal

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

Remove Invoice type from LHN in Search if it is not enabled

What is the root cause of that problem?

Invoice type is added to search LHN from here and it is always added to the LHN Menu even if user does not use invoices.

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

A user will only use invoices if it is enabled in one of their workspaces or they have received an invoice from somebody else as admins can send Invoice to users outside their workspace and the user will not be added to workspace. So, we need to update here to only add Invoices to menu items if they have a workspace with invoices or an invoice has been sent to them. The updated logic would look like this. The Implementation can be done like below:

Current:

const typeMenuItems: SearchTypeMenuItem[] = [
{
title: translate('common.expenses'),
type: CONST.SEARCH.DATA_TYPES.EXPENSE,
icon: Expensicons.Receipt,
getRoute: (policyID?: string) => {
const query = SearchQueryUtils.buildCannedSearchQuery({policyID});
return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
},
},
{
title: translate('common.chats'),
type: CONST.SEARCH.DATA_TYPES.CHAT,
icon: Expensicons.ChatBubbles,
getRoute: (policyID?: string) => {
const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.CHAT, status: CONST.SEARCH.STATUS.CHAT.ALL, policyID});
return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
},
},
{
title: translate('workspace.common.invoices'),
type: CONST.SEARCH.DATA_TYPES.INVOICE,
icon: Expensicons.InvoiceGeneric,
getRoute: (policyID?: string) => {
const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.INVOICE, status: CONST.SEARCH.STATUS.INVOICE.ALL, policyID});
return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
},
},
{
title: translate('travel.trips'),
type: CONST.SEARCH.DATA_TYPES.TRIP,
icon: Expensicons.Suitcase,
getRoute: (policyID?: string) => {
const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.TRIP, status: CONST.SEARCH.STATUS.TRIP.ALL, policyID});
return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
},
},
];

New:

const session = useOnyx(ONYXKEYS.SESSION)

const typeMenuItems = [
    ...
]

if (PolicyUtils.hasWorkspaceWithInvoices(session?.email) || ReportUtils.hasInvoiceReports()) typeMenuItems.push({
    title: translate('workspace.common.invoices'),
    type: CONST.SEARCH.DATA_TYPES.INVOICE,
    icon: Expensicons.InvoiceGeneric,
    getRoute: (policyID?: string) => {
        const query = SearchQueryUtils.buildCannedSearchQuery({type: CONST.SEARCH.DATA_TYPES.INVOICE, status: CONST.SEARCH.STATUS.INVOICE.ALL, policyID});
        return ROUTES.SEARCH_CENTRAL_PANE.getRoute({query});
    },
})

typeMenuItems.push({
    title: translate('travel.trips'),
    type: CONST.SEARCH.DATA_TYPES.TRIP,
    ...
})

Policies with Invoice

We can reuse some of the logic from the Send Invoice button for FAB, which currently looks like this:

App/src/libs/PolicyUtils.ts

Lines 597 to 612 in ce01614

/** Return active policies where current user is an admin */
function getActiveAdminWorkspaces(policies: OnyxCollection<Policy> | null, currentUserLogin: string | undefined): Policy[] {
const activePolicies = getActivePolicies(policies);
return activePolicies.filter((policy) => shouldShowPolicy(policy, NetworkStore.isOffline(), currentUserLogin) && isPolicyAdmin(policy, currentUserLogin));
}
/** Whether the user can send invoice from the workspace */
function canSendInvoiceFromWorkspace(policyID: string | undefined): boolean {
const policy = getPolicy(policyID);
return policy?.areInvoicesEnabled ?? false;
}
/** Whether the user can send invoice */
function canSendInvoice(policies: OnyxCollection<Policy> | null, currentUserLogin: string | undefined): boolean {
return getActiveAdminWorkspaces(policies, currentUserLogin).some((policy) => canSendInvoiceFromWorkspace(policy.id));
}

so, hasWorkspaceWithInvoices will look like:

function hasWorkspaceWithInvoices(currentUserLogin) {
    const activePolicies = getActivePolicies(allPolicies);
    return activePolicies.some((policy) => shouldShowPolicy(policy, NetworkStore.isOffline(), currentUserLogin) && policy.areInvoicesEnabled);
}

Invoice Report to external members

function hasInvoiceReports() {
    const allReports = ReportConnection.getAllReports();
    return allReports.some(report => isInvoiceReport(report))
}

Note: all the code samples are for explaination only and can be improved during PR

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

@alexpensify
Copy link
Contributor

@eVoloshchak - when you get a chance, can you please review if one of these proposals will fix the issue? Thanks!

@melvin-bot melvin-bot bot added the Overdue label Oct 31, 2024
Copy link

melvin-bot bot commented Nov 1, 2024

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

@alexpensify
Copy link
Contributor

@eVoloshchak any update here? Thanks!

Copy link

melvin-bot bot commented Nov 4, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@alexpensify
Copy link
Contributor

@eVoloshchak please keep me posted if you are unable to review. I will need to reassign by Wednesday if no answer.

Copy link

melvin-bot bot commented Nov 5, 2024

@alexpensify, @eVoloshchak, @luacmartins Still overdue 6 days?! Let's take care of this!

@luacmartins
Copy link
Contributor Author

@eVoloshchak any updates here?

@jaydamani
Copy link
Contributor

I am not available right now but I can work on it tomorrow so if it's urgent please assign it to @Krishna2323 Also, I think whoever implements it should be compensated so, if it's assigned to @Krishna2323 then they should be compensated.

@luacmartins
Copy link
Contributor Author

Sure, I'll reassign to you @jaydamani since you're proposal was more complete as @Krishna2323 pointed out above.

@luacmartins luacmartins assigned jaydamani and unassigned Krishna2323 Nov 6, 2024
Copy link

melvin-bot bot commented Nov 6, 2024

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

@jaydamani
Copy link
Contributor

Starting work on this

@jaydamani
Copy link
Contributor

Created PR

@luacmartins luacmartins added the Reviewing Has a PR in review label Nov 7, 2024
@garrettmknight garrettmknight moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 11, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Nov 13, 2024
@melvin-bot melvin-bot bot changed the title [$125] [Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled [HOLD for payment 2024-11-20] [$125] [Search v2.2] Remove Invoice type from LHN if workspaces don't have it enabled Nov 13, 2024
Copy link

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

melvin-bot bot commented Nov 13, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.60-3 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-20. 🎊

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

Copy link

melvin-bot bot commented Nov 13, 2024

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@rushatgabhane] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
  • [@alexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

@garrettmknight garrettmknight moved this from Bugs and Follow Up Issues to Hold for Payment in [#whatsnext] #expense Nov 14, 2024
@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 19, 2024
@alexpensify
Copy link
Contributor

alexpensify commented Nov 21, 2024

Payouts due: 2024-11-20

  • Contributor: $125 @jaydamani - paid via Upwork
  • Reviewer: $125 @rushatgabhane - Please submit a Chat request and confirm if there should be a regression test here. Thanks!

Upwork job is here.

@melvin-bot melvin-bot bot added the Overdue label Nov 25, 2024
@alexpensify
Copy link
Contributor

Closing

@melvin-bot melvin-bot bot removed the Overdue label Nov 25, 2024
@github-project-automation github-project-automation bot moved this from Hold for Payment to Done in [#whatsnext] #expense Nov 25, 2024
@garrettmknight
Copy link
Contributor

$125 approved for @rushatgabhane

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 Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
Status: Done
Development

No branches or pull requests

7 participants