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

#4517 update timezone on user activity #4799

Merged

Conversation

dklymenk
Copy link
Contributor

@dklymenk dklymenk commented Aug 24, 2021

Details

This PR replaces the original automatic timezone implementation that only checked timezone on app start with a new one, that also checks for updates on specific user activity: send chat message and open profile settings (the 2 places where you will see your current timezone impact rendering).

Fixed Issues

$ #4517

Tests

These are long, but are required to check that the timezone is updated:

  • on app start
  • on message send
  • on profile settings open
  1. Set timezone to automatic
  2. Change throttle timer to something more testable (5 seconds for example)
  3. Open a chat with a second account that is in the same timezone as your active account
  4. You should not see participant's local time because you're in the same timezone
  5. Change the current timezone in your OS
  6. Send a message
  7. Participant's local time should appear
  8. Change the timezone back
  9. Send another message after a throttle timeout passed
  10. Participant's local time should disappear
  11. Change the timezone again
  12. Refresh the page or restart the app
  13. Open chat and you see participant's local time again
  14. Change the timezone back
  15. Open your profile's settings
  16. You should see the newly selected timezone there

QA Steps

  1. Set timezone to automatic
  2. Open a chat with a second account that is in the same timezone as your active account
  3. You should not see participant's local time because you're in the same timezone
  4. Change the current timezone in your OS
  5. Send a message
  6. Participant's local time should appear
  7. Change the timezone back
  8. Send another message 10 minutes later
  9. Participant's local time should disappear
  10. Change the timezone again
  11. Refresh the page or restart the app
  12. Open chat and you see participant's local time again
  13. Change the timezone back
  14. Open your profile's settings
  15. You should see the newly selected timezone there

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

4517-web.mp4

Mobile Web

4517-mweb.mp4

Desktop

4517-desktop.mp4

iOS

4517-ios.mp4

Android

Sorry for the crash in the middle of the video, reloading with hermes results in crash like 50% of the time for me. Still trying to figure it out

4517-android.mp4

@dklymenk dklymenk marked this pull request as ready for review August 24, 2021 12:47
@dklymenk dklymenk requested a review from a team as a code owner August 24, 2021 12:47
@MelvinBot MelvinBot requested review from tylerkaraszewski and removed request for a team August 24, 2021 12:47
@dklymenk dklymenk marked this pull request as draft August 24, 2021 12:54
@tylerkaraszewski
Copy link
Contributor

This has conflicts.

@dklymenk
Copy link
Contributor Author

Hello, sorry. Conflict fixed.

@thienlnam
Copy link
Contributor

Looks great! Leaving for you @tylerkaraszewski

@tylerkaraszewski tylerkaraszewski merged commit ec9a319 into Expensify:main Aug 30, 2021
@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
Copy link
Contributor

🚀 Deployed to staging by @tylerkaraszewski in version: 1.0.89-0 🚀

platform result
🤖 android 🤖 skipped 🚫
🖥 desktop 🖥 skipped 🚫
🍎 iOS 🍎 skipped 🚫
🕸 web 🕸 skipped 🚫

@OSBotify
Copy link
Contributor

OSBotify commented Sep 1, 2021

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants