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.

Sign up for GitHub

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

Jump to bottom

Rename Policy Rooms #7217

Merged
merged 28 commits into from
Jan 24, 2022
Merged

Rename Policy Rooms #7217

merged 28 commits into from
Jan 24, 2022

Conversation

luacmartins
Copy link
Contributor

@luacmartins luacmartins commented Jan 13, 2022 •
edited
Loading

cc @TomatoToaster

Details

Adds the ability to rename a Policy Room.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/176854

Tests

  1. Create a new policy room by navigating to + > New room.
  2. Click the chat header to open up Details.
  3. Click Settings
  4. Enter a new value for Room name and click Save.
  5. Verify the following:

  • The Save button displays a loading spinner and becomes disabled.

  • A growl message shows up and reads Policy room renamed!.

  • A comment like the ones showed in the screenshots below is posted to the chat.

  • The room name is updated in the LHN and the chat header.

  • Verify that no errors appear in the JS console

QA Steps

Steps above.

  • [] Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web-pv3

Mobile Web

mweb

Desktop

desktop

iOS

ios-pv3

Android

android-pv3

@luacmartins luacmartins self-assigned this Jan 13, 2022
@luacmartins
export function and add comment
4c5ffa0
@luacmartins
add renameReport action
c0911ef
@luacmartins
add event handler to ReportSettingsPage
232836e
@luacmartins
update spanish copy
18fdd5c
@luacmartins
fix style
ed54092
@luacmartins
remove loading state
570eba1
@luacmartins
replace policyID with reportID in all function calls
5caf463
@luacmartins
update copy
edc04f2
@luacmartins
add renamed const and include renamed action in report action filter
699ebfa
@luacmartins
update reportName on LHN and header
60e05ea
@luacmartins
merge main
e0a4083
@luacmartins
add loading state to rename policy form
af3b558
@luacmartins
create RenameAction
1e46cad
@luacmartins
fix style
364a5c4
@luacmartins
change lastMessage logic
861e88d
@luacmartins
update isConsecutiveActionMadeByPreviousActor to not group renamed ac…
0fa0c2d 
…tions
@luacmartins
update comment
0080bf8
@luacmartins
Merge branch 'main' into cmartins-renamePolicyRooms
d8a1a86
@luacmartins
add comment
0b21d5b
@luacmartins
remove dot on copy
bf3ccfa
@luacmartins
fix style
05314f0
@luacmartins
rm podlock-file commit
b6c83d3
@luacmartins
rename AnnouceAction component
e6af3dd
@luacmartins
rename translattion keys
cfe9cda
@luacmartins
update comment
f0f2070
@luacmartins luacmartins changed the title Rename Policy Rooms [HOLD Web-E 32831] Rename Policy Rooms Jan 19, 2022
@luacmartins luacmartins marked this pull request as ready for review January 19, 2022 21:55
@luacmartins luacmartins requested a review from a team as a code owner January 19, 2022 21:55
@luacmartins
Copy link
Contributor Author

luacmartins commented Jan 19, 2022 •
edited
Loading

@Expensify/design would love your thoughts on the renamed action message design.

@shawnborton
Copy link
Contributor

Left a couple of very minor comments, otherwise looking good!

neil-marcellini
neil-marcellini previously approved these changes Jan 20, 2022
View reviewed changes
Copy link
Contributor

@neil-marcellini neil-marcellini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rename functionality works well. I left 2 small comments that aren't blockers. While testing this I was unable to pull up the settings page after creating a room until I refreshed. Does that happen for you? That problem also occurs on the main branch. I'll create a separate issue for it if one doesn't already exist.

@luacmartins
Copy link
Contributor Author

While testing this I was unable to pull up the settings page after creating a room until I refreshed. Does that happen for you?

I noticed some inconsistent behavior as well. It seems that after creating a Policy room the route is not updated correctly and we try to access the settings page from a different report. Definitely worth investigating! Thanks for creating the issue!

shawnborton
shawnborton previously approved these changes Jan 20, 2022
View reviewed changes
@luacmartins
Copy link
Contributor Author

Updated!

neil-marcellini
neil-marcellini approved these changes Jan 20, 2022
View reviewed changes
Copy link
Contributor

@neil-marcellini neil-marcellini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@shawnborton
Copy link
Contributor

Approved on my end, feel free to self-merge once the checks successfully pass.

@neil-marcellini neil-marcellini mentioned this pull request Jan 20, 2022
Closed
@luacmartins luacmartins changed the title [HOLD Web-E 32831] Rename Policy Rooms Rename Policy Rooms Jan 24, 2022
@luacmartins
Copy link
Contributor Author

No longer on hold! Self-merging with approval.

@luacmartins luacmartins merged commit 2f2bc06 into main Jan 24, 2022
@luacmartins luacmartins deleted the cmartins-renamePolicyRooms branch January 24, 2022 17:29
@OSBotify
Copy link
Contributor

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

@OSBotify OSBotify mentioned this pull request Jan 25, 2022
Closed
46 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @luacmartins in version: 1.1.32-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@kavimuru kavimuru mentioned this pull request Jan 25, 2022
Closed
@OSBotify
Copy link
Contributor

OSBotify commented Feb 1, 2022

🚀 Deployed to production by @roryabraham in version: 1.1.33-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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

@neil-marcellini neil-marcellini neil-marcellini approved these changes

@shawnborton shawnborton shawnborton approved these changes

Assignees

@luacmartins luacmartins

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@luacmartins @shawnborton @OSBotify @neil-marcellini