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 invite page not loading after creating new workspace #5729

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

TomatoToaster
Copy link
Contributor

@TomatoToaster TomatoToaster commented Oct 8, 2021

@Luke9389, please review when you get the chance
CC: @marcaaron

Details

Fixes a TypeError that causes a blank page from the invite modal. This happens after creating a new workspace and then

Fixed Issues

$ #5727

Tests

Same as Web QA done locally

QA Steps

  1. Launch App and login
  2. Create a workspace
  3. Navigate to the People
  4. Click on the "Invite" button
  5. Verify you can see the invite modal on the right hand side like in the screenshot below

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2021

⚠️ ⚠️ Heads up! This pull request has the CP Staging label. ⚠️ ⚠️
Merging it will cause it to be immediately deployed to staging, even if the open StagingDeployCash deploy checklist is locked.

@TomatoToaster TomatoToaster marked this pull request as ready for review October 8, 2021 16:54
@TomatoToaster TomatoToaster requested a review from a team as a code owner October 8, 2021 16:54
@MelvinBot MelvinBot requested review from Luke9389 and removed request for a team October 8, 2021 16:55
Copy link
Contributor

@Luke9389 Luke9389 left a comment

Choose a reason for hiding this comment

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

nice catch

@Luke9389
Copy link
Contributor

Luke9389 commented Oct 8, 2021

tested well
Screen Shot 2021-10-08 at 12 11 16 PM

@Luke9389 Luke9389 merged commit 13ac792 into main Oct 8, 2021
@Luke9389 Luke9389 deleted the amal-invite-lodash-fix branch October 8, 2021 18:11
@@ -107,7 +107,7 @@ class WorkspaceInvitePage extends React.Component {
* @returns {Boolean}
*/
getShouldShowAlertPrompt() {
return _.size(lodashGet(this.props.policy, 'errors', {})) > 0 || this.props.policy.alertMessage.length > 0;
return _.size(lodashGet(this.props.policy, 'errors', {})) > 0 || lodashGet(this.props.policy, 'alertMessage.length') > 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return _.size(lodashGet(this.props.policy, 'errors', {})) > 0 || lodashGet(this.props.policy, 'alertMessage.length') > 0;
return _.size(lodashGet(this.props.policy, 'errors', {})) > 0 || lodashGet(this.props.policy, 'alertMessage', '').length > 0;

NAB, maybe slightly more correct?

Otherwise we are doing undefined > 0 which does eval to false but seems weird.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, yea this is better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Made a 2-min PR to implement this real quick.
#5738

github-actions bot pushed a commit that referenced this pull request Oct 8, 2021
Fix invite page not loading after creating new workspace

(cherry picked from commit 13ac792)
@OSBotify
Copy link
Contributor

OSBotify commented Oct 8, 2021

🚀 Cherry-picked to staging by @Luke9389 in version: 1.1.7-1 🚀

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

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

@isagoico
Copy link

isagoico commented Oct 8, 2021

Issue fixed!
image

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @AndrewGable in version: 1.1.7-24 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @Luke9389 in version: 1.1.7-25 🚀

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

@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.

None yet

5 participants