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.56k stars 2.9k forks source link

Split bill - Split bill details page shows inactive fields #25420

Closed lanitochka17 closed 1 year ago

lanitochka17 commented 1 year 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!


Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Tap profile icon
  3. Tap workspaces
  4. Tap new workspace
  5. Edit the workspace name and save
  6. Tap members
  7. Invite many members
  8. Tap 3 dots of newly created workspace and go to #announce room
  9. Tap plus icon to select split bill
  10. Enter any amount
  11. Tap split bill
  12. Tap on the split bill box in conversation
  13. Tap show more

Expected Result:

There should not be inactive fields in split bill details page

Actual Result:

Split bill details page shows inactive fields like date, amount, description etc

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Version Number: 1.3.55-1 Reproducible in staging?: Yes

Reproducible in production?: Yes

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

https://github.com/Expensify/App/assets/78819774/ac131587-bea3-461c-979f-8eaf92e43e25

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

melvin-bot[bot] commented 1 year ago

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot[bot] commented 1 year ago

Bug0 Triage Checklist (Main S/O)

ShogunFire commented 1 year ago

I think that's intended for amount and description when you have already comfirmed the split, in the code it's disabled if didConfirm is true , in the code there is this comment though for date and merchant:

// Note: This component is disabled until this field is editable in next PR

That was made by that PR 3 weeks ago: https://github.com/Expensify/App/pull/23648

rayane-djouah commented 1 year ago

I agree with @ShogunFire I think this is intended

DylanDylann commented 1 year ago

Proposal

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

Split bill - Split bill details page shows inactive fields

What is the root cause of that problem?

We add isReadOnly prop as true to MoneyRequestConfirmationList. It makes all field in detail split bill page disable

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

First, We need to confirm that which field we need to disable

  1. For date and merchant field https://github.com/Expensify/App/blob/2e838a1ac189767beb0fa28df82cb419df67dc9d/src/components/MoneyRequestConfirmationList.js#L408-L420 We can remove disable field to enable 2 two fields
  2. For amount and description field https://github.com/Expensify/App/blob/2e838a1ac189767beb0fa28df82cb419df67dc9d/src/components/MoneyRequestConfirmationList.js#L373-L389 We can remove props.isReadOnly in the disable condition

    What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@anmurali Eep! 4 days overdue now. Issues have feelings too...

anmurali commented 1 year ago

What is the expected outcome here? We want the person that split the bill to be able to come back later and add these fields they originally didn't fill out? @lanitochka17

lanitochka17 commented 1 year ago

@anmurali At least to be able to change the Description

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

anmurali commented 1 year ago

Still waiting for a response from @lanitochka17

melvin-bot[bot] commented 1 year ago

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

lanitochka17 commented 1 year ago

@anmurali My answer was above: At least to be able to change the Description

melvin-bot[bot] commented 1 year ago

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

anmurali commented 1 year ago

So to clarify in this video - the split bill itself cannot be edited to add a description. But the individual IOUs created from that split bill can be edited. You are saying the split bill should be editable?

https://github.com/Expensify/App/assets/14225673/8da7258e-7502-4514-9376-bb5f782a8386

@lanitochka17

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@anmurali Eep! 4 days overdue now. Issues have feelings too...

anmurali commented 1 year ago

Going to close this. @lanitochka17 please open when you respond.