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

Workspace Settings page - Fix offline behavior #5893

Merged
merged 1 commit into from
Oct 18, 2021

Conversation

jasperhuangg
Copy link
Contributor

@jasperhuangg jasperhuangg commented Oct 15, 2021

Previously, when clicking "Save" on the workspace settings page, we'd only update Onyx after the promise to the UpdatePolicy API command resolved. When this happened offline, the promise would never resolve, meaning we'd never update isPolicyUpdating for the policy in Onyx, leading to a confusing infinite spinner.

Let's update the local values directly for the workspace and queue the API request in the background. This means that the local values will be updated while offline, and the queued API command will be made once the user goes back online.

Fixed Issues

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

Tests

  1. Navigate to the "Workspace Settings" page.
  2. Turn off your internet connection for the device you're using.
  3. Update the currency and name for the workspace. Click "Save".
  4. Verify a success growl appears. Then, turn your internet connection back on.
  5. Reload the page or re-open the app.
  6. Verify that the updated currency, avatarURL, and name are displayed.

QA Steps

  1. Navigate to the "Workspace Settings" page.
  2. Turn off your internet connection for the device you're using.
  3. Update the currency and name for the workspace. Click "Save".
  4. Verify a success growl appears. Then, turn your internet connection back on, and give the app a few seconds to make any failed API calls that were queued offline.
  5. Reload the page or close/re-open the app.
  6. Verify that the updated currency, avatarURL, and name are displayed.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web.mp4

Mobile Web

mweb.mov

Desktop

desktop.mp4

iOS

ios.mp4

Android

android.mp4

@jasperhuangg jasperhuangg self-assigned this Oct 15, 2021
@jasperhuangg jasperhuangg changed the title Workspace Settings page - Update local values and queue API request in background Workspace Settings page - Fix offline behavior Oct 15, 2021
@jasperhuangg jasperhuangg marked this pull request as ready for review October 18, 2021 15:46
@jasperhuangg jasperhuangg requested a review from a team as a code owner October 18, 2021 15:46
@MelvinBot MelvinBot requested review from aldo-expensify and removed request for a team October 18, 2021 15:46
@jasperhuangg
Copy link
Contributor Author

@aldo-expensify Good for a review, thanks!

Copy link
Contributor

@aldo-expensify aldo-expensify left a comment

Choose a reason for hiding this comment

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

Tested on web, the code changes look good to me!

@aldo-expensify aldo-expensify merged commit 2e57dee into main Oct 18, 2021
@aldo-expensify aldo-expensify deleted the jasper-offlineWorkspaceSettings branch October 18, 2021 18:34
@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 @aldo-expensify in version: 1.1.8-10 🚀

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

@OSBotify
Copy link
Contributor

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

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants