Skip to content

Race fix attempt 2#4436

Merged
chiragsalian merged 1 commit intomainfrom
chirag-race-fix-attempt2
Aug 6, 2021
Merged

Race fix attempt 2#4436
chiragsalian merged 1 commit intomainfrom
chirag-race-fix-attempt2

Conversation

@chiragsalian
Copy link
Contributor

Details

Fixed Issues

$ #4388

Tests

  1. Add sleep(10); after this line
  2. Logout and login on ecash and confirm everything works fine and that there are no errors related to personalDetails for HeaderView, ReportActionCompose and SidebarLinks as the personalDetails take 10 seconds to load.

QA Steps

Same as the issue

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@chiragsalian chiragsalian requested a review from a team as a code owner August 5, 2021 23:39
@chiragsalian chiragsalian self-assigned this Aug 5, 2021
@MelvinBot MelvinBot requested review from madmax330 and removed request for a team August 5, 2021 23:39

/** Personal details of all the users */
personalDetails: PropTypes.objectOf(participantPropTypes).isRequired,
personalDetails: PropTypes.objectOf(participantPropTypes),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing required because these throw unnecessary errors as onyx takes a while to load in these variables. I think all async variables should just be defined as not required.


getParticipantLocalTime() {
const reportRecipientTimezone = lodashGet(this.props.participant, 'timezone', {});
const reportRecipientTimezone = lodashGet(this.props.participant, 'timezone', CONST.DEFAULT_TIME_ZONE);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

setting a proper default because if we use {} as default then it will fail 3 lines down for currentUserDay and throw an error.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 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.

@chiragsalian chiragsalian merged commit 515feb7 into main Aug 6, 2021
@chiragsalian chiragsalian deleted the chirag-race-fix-attempt2 branch August 6, 2021 00:23
@chiragsalian
Copy link
Contributor Author

self merging to get it CP'd to staging soon to hopefully fix blocker.

github-actions bot pushed a commit that referenced this pull request Aug 6, 2021
Race fix attempt 2

(cherry picked from commit 515feb7)
@AndrewGable
Copy link
Contributor

Confirmed this is working on staging web for me. I will test iOS once it's done building.

@OSBotify
Copy link
Contributor

OSBotify commented Aug 6, 2021

🚀 Deployed to staging in version: 1.0.82-6🚀

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

@chiragsalian
Copy link
Contributor Author

For my curiosity to know how long it takes for iOS to get the testflight update,
5:24pm PDT - PR was merged.
6:16pm PDT - GH actions was done (52 minutes)
6:29pm PDT - received notification of the update. Manually refreshed many times before and didn't see it

Cool, so it looks like it took an hour and 5 minutes before it was available on testflight.

@AndrewGable
Copy link
Contributor

iOS works as well 👍 Great work @chiragsalian!

@OSBotify
Copy link
Contributor

OSBotify commented Aug 6, 2021

🚀 Deployed to production in version: 1.0.82-7🚀

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

@OSBotify
Copy link
Contributor

OSBotify commented Aug 6, 2021

🚀 Deployed to staging in version: 1.0.82-8🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @francoisl in version: 1.0.83-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 failure ❌
🍎 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.

3 participants