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

Fix error condition for policy create #5687

Merged
merged 1 commit into from
Oct 8, 2021
Merged

Conversation

TomatoToaster
Copy link
Contributor

CC: @marcaaron @mananjadhav

Details

Fixes this an error that could create a weird experience if our app is offline.

Fixed Issues

Error was caught in PR here: https://github.com/Expensify/App/pull/4481/files#r709771608

Tests

No practical way to test that this error except for Expensify Engineers. To do this you can kill auth in the VM (sudo service auth stop) and try to click the Create New Workspace like in the QA steps (disconnect auth after step 1). You'll see an error and a workspace won't be created.

QA Steps

Make sure policy creation works as it did normally before.

  1. Log into a NewDot account without a Workspace
  2. Click on Create New Workspace after clicking on the green + at the bottom left
  3. Verify a workspace is created and you are taken to the settings page

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image
image

@TomatoToaster TomatoToaster self-assigned this Oct 6, 2021
@TomatoToaster TomatoToaster requested a review from a team as a code owner October 6, 2021 15:40
@MelvinBot MelvinBot requested review from ctkochan22 and removed request for a team October 6, 2021 15:40
@TomatoToaster TomatoToaster merged commit a860c59 into main Oct 8, 2021
@TomatoToaster TomatoToaster deleted the amal-fail-policy-create branch October 8, 2021 13:18
@OSBotify
Copy link
Contributor

OSBotify commented Oct 8, 2021

✋ 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 @TomatoToaster in version: 1.1.7-25 🚀

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

@ogumen
Copy link

ogumen commented Oct 21, 2021

The grey "Get the Expensify Card and more" text (#7D8B8F) on white (#FFFFFF) and light grey on focus (#FAFAFA) background has a contrast ratio of 3.52:1 and 3.37:1 accordingly, failing the minimum contrast requirements of 4.50:1.
Grey text on white and light grey background fails contrast requirements

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.8-9 🚀

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.

4 participants