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.03k stars 2.54k forks source link

Combine report name routes #42369

Closed s77rt closed 1 day ago

s77rt commented 2 weeks ago

cc @marcaaron

Details

Part of https://github.com/Expensify/App/issues/40187

Fixed Issues

$ https://github.com/Expensify/App/issues/40187 PROPOSAL:

Tests

Test 1 (Renaming a group chat):

  1. Create or navigate to a group chat report
  2. Click on the report header
  3. Go to Settings > Name
  4. (Web Only): Verify that the url ends with /settings/name
  5. Change the name
  6. Verify that the name is changed

Test 2 (Renaming a room):

  1. Create or navigate to a room that you own
  2. Click on the report header
  3. Go to Settings > Room name
  4. (Web Only): Verify that the url ends with /settings/name
  5. Change the name
  6. Verify that the name is changed

Offline tests

Same as Tests

QA Steps

Same as Tests

PR Author Checklist

Screenshots/Videos

Android: Native https://github.com/Expensify/App/assets/16493223/f3a1399b-0622-40cd-9b66-9aad127c577b
Android: mWeb Chrome Networking issue
iOS: Native https://github.com/Expensify/App/assets/16493223/501eb8fe-9663-469d-81ae-e8a715efb289
iOS: mWeb Safari https://github.com/Expensify/App/assets/16493223/4c147f4c-f21c-4172-b3c5-50561073b516
MacOS: Chrome / Safari https://github.com/Expensify/App/assets/16493223/57b0d0a5-4c99-4731-803a-f43b43227407
MacOS: Desktop https://github.com/Expensify/App/assets/16493223/8796dde3-14a6-4413-b3b7-affab86c51f0
melvin-bot[bot] commented 2 weeks ago

Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers!

melvin-bot[bot] commented 2 weeks ago

@ikevin127 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

ikevin127 commented 2 weeks ago

Reviewer Checklist

Screenshots/Videos

Android: Native https://github.com/Expensify/App/assets/56457735/04df5908-c808-4704-b597-9ff808a312ec
Android: mWeb Chrome https://github.com/Expensify/App/assets/56457735/f7d1b69e-8212-4463-8830-2b6293a12faf
iOS: Native https://github.com/Expensify/App/assets/56457735/b3da6597-704a-461e-aa63-e719bcac5fc1
iOS: mWeb Safari https://github.com/Expensify/App/assets/56457735/ef976951-f253-4b05-bc2f-a6d111086c5c
MacOS: Chrome / Safari https://github.com/Expensify/App/assets/56457735/37a005b0-b0c1-45cf-b986-567369c3297c
MacOS: Desktop https://github.com/Expensify/App/assets/56457735/2289bcad-e189-4efe-bd31-03079dc47897
s77rt commented 2 weeks ago

The group offline behaviour is being implemented in this PR https://github.com/Expensify/App/pull/41826

as my assumption is that none of the items should be grayed-out by simply going offline

That's correct, in your case you probably already had a queued request

marcaaron commented 1 week ago

On my list to get to. Still on a merge freeze I believe but prioritizing everything as it comes in. Thanks for your patience here.

ikevin127 commented 3 days ago

I think we're off the merge freeze hence I think this is good for merge ? cc @marcaaron

marcaaron commented 3 days ago

Thank for the bump @ikevin127. Apologies, this fell off my radar!

s77rt commented 3 days ago

The typescript (and lint) error is unrelated, it should be fixed here https://github.com/Expensify/App/pull/42808

marcaaron commented 3 days ago

Looks like the PR has been merged now if we want to merge main on this so the checks pass.

s77rt commented 3 days ago

Tests 🚀

ikevin127 commented 1 day ago

cc @marcaaron

marcaaron commented 1 day ago

Thanks y'all! Nice work here ❤️

OSBotify commented 1 day ago

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.