Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.57k stars 2.91k forks source link

[$250] Invoices - App opens workspace description settings when clicking on invoice description text #51976

Open IuliiaHerets opened 2 weeks ago

IuliiaHerets commented 2 weeks ago

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

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to FAB > Send invoice.
  3. Send an invoice to anyone.
  4. Go to invoice chat.
  5. Click on the invoice chat header.
  6. Click Room description.
  7. Enter a description and save it.
  8. Go back to invoice chat.
  9. Click on the room description text under "Say hello!".

Expected Result:

App will open invoice room description settings.

Actual Result:

App opens workspace description settings when clicking on the invoice description text.

This issue only happens in invoice room.

Workaround:

Unknown

Platforms:

Screenshots/Videos

https://github.com/user-attachments/assets/c12026ff-68a1-461c-a2c9-dd54620a7dc1

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021854589048868933801
  • Upwork Job ID: 1854589048868933801
  • Last Price Increase: 2024-11-07
  • Automatic offers:
    • Nodebrute | Contributor | 104882470
Issue OwnerCurrent Issue Owner: @alitoshmatov
melvin-bot[bot] commented 2 weeks ago

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

nyomanjyotisa commented 2 weeks ago

Edited by proposal-police: This proposal was edited at 2024-11-04 18:00:25 UTC.

Proposal

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

App opens workspace description settings when clicking on invoice description text

What is the root cause of that problem?

We navigate to WORKSPACE_PROFILE_DESCRIPTION here https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L126-L129

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

Change to the following


                                const activeRoute = Navigation.getReportRHPActiveRoute();
                                if (ReportUtils.canEditReportDescription(report, policy)) {
                                    Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute));
                                    return;
                                }
                                Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report?.reportID ?? '-1', activeRoute));

What alternative solutions did you explore? (Optional)

daledah commented 2 weeks ago

Proposal

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

App opens workspace description settings when clicking on the invoice description text.

What is the root cause of that problem?

When we click to this description, we'll navigate to workspace description here

https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L129

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

We should navigate to the report description when it is Invoice room

ReportUtils.isInvoiceRoom(report) ? Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1')) :  Navigation.navigate(ROUTES.WORKSPACE_PROFILE_DESCRIPTION.getRoute(policy?.id ?? '-1')); 

What alternative solutions did you explore? (Optional)

NA

Nodebrute commented 2 weeks ago

Proposal

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

App opens workspace description settings when clicking on invoice description text

What is the root cause of that problem?

Here we are navigating to workspace profile description page instead of report description page https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L129

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

We don't need the canEditPolicyDescription as we are not navigating to policy description page anymore https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L126-L128

and instead of navigating to policy description page here, let's change this to

                                const activeRoute = Navigation.getReportRHPActiveRoute();
                                Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute));

[!NOTE]
We don’t need the canEditReportDescription check here since it’s already handled in RoomDescriptionPage.tsx, where users see a different view if they can’t edit the description(not the policy admin). Removing this check allows users to click on the description and navigate to the invoice room description, ensuring consistency with the behavior when accessing the room description from the header.

https://github.com/user-attachments/assets/6b6232cc-d1f4-4f08-84b9-7620c21e3b27

We should also remove these styles canEditPolicyDescription ? styles.cursorPointer : styles.cursorText

https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L131

What alternative solutions did you explore? (Optional)

If we want to prevent users from navigating to the description page when they can't edit it, we can implement the following solution:

Let's add a new check, we use the same check on RoomDescriptionPage.tsx

 const canEdit = ReportUtils.canEditReportDescription(report, policy);

then we can change these canEditPolicyDescription to canEdit https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L131 https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L126

and at last let's change this link

 const activeRoute = Navigation.getReportRHPActiveRoute();
      Navigation.navigate(ROUTES.REPORT_DESCRIPTION.getRoute(report?.reportID ?? '-1', activeRoute));

With this solution, users who cannot edit the report description will see the text cursor when hovering over it, indicating it's not editable.

zanyrenney commented 2 weeks ago

adding external!

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

@zanyrenney, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

alitoshmatov commented 1 week ago

@nyomanjyotisa Thank you for your proposal, next time adding some description would be helpful rather than just putting a code. From what I got from your solution is that it does solve the issue, but you are introducing a new behavior where if other member clicks on description it opens invoice details page.

alitoshmatov commented 1 week ago

@daledah Thank you for your proposal. Your proposal solve the issue, but I don't think there is a need to check if report is is invoice room this it is provided so here https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L121

Simply changing route is sufficient

alitoshmatov commented 1 week ago

@Nodebrute Thank you for your proposal, you are also changing current behavior by letting other members open description page even if they can't edit, currently they can't open this page at all. I think your alternative solution is overcomplicating things, which could be achieved by just changing navigation route

Nodebrute commented 1 week ago

@alitoshmatov My alternative proposal simplifies things by removing unnecessary checks. We don’t need to verify canEditPolicyDescription if we’re just navigating to the room description. Instead we'll use ReportUtils.canEditReportDescription(report, policy); and then we'll change the route https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L126-L128

Nodebrute commented 1 week ago

I believe my alternate proposal is complete. Currently, we’re navigating the user to the policy description page, so we’re checking for canEditPolicyDescription. However, with my solution, if we change the route to the report description, we should also update the check to canEditReportDescription. https://github.com/Expensify/App/blob/fcb4a5b1c3270725478d03e069be3b888d6d9004/src/components/ReportWelcomeText.tsx#L126-L129

alitoshmatov commented 1 week ago

@Nodebrute I see, I overlooked canEditReportDescription and confused it with canEditPolicyDescription

alitoshmatov commented 1 week ago

We can go with @Nodebrute 's proposal alternative solution which updated the logic to check for canEditReportDescription and update to correct route

C+ reviewed 🎀 👀 🎀

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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