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

#6684 Serialize google login #6781

Merged
merged 4 commits into from Oct 31, 2023
Merged

#6684 Serialize google login #6781

merged 4 commits into from Oct 31, 2023

Conversation

BLoe
Copy link
Collaborator

@BLoe BLoe commented Oct 31, 2023

What does this PR do?

Checklist

  • Add tests
  • New files added to src/tsconfig.strictNullChecks.json (if possible)
  • Designate a primary reviewer - @grahamlangford

@BLoe BLoe added this to the 1.8.2 milestone Oct 31, 2023
@BLoe BLoe self-assigned this Oct 31, 2023
* Get cached auth data for OAuth2, or login if no data found. Memoize so that multiple logins
* are not kicked off at once.
*/
const getOAuth2AuthData = memoizeUntilSettled(
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ NIT: would be good to leave a comment for why its appropriate to wrap here vs. launchOAuth2Flow. I.e., if you wrap launchOAuth2Flow instead you'll get a race condition based on whether or not the auth data has been written by the other call yet

Comment on lines +100 to +114
expect(
getOAuth2AuthData(integration, localConfig, sanitizedIntegrationConfig)
).resolves.toEqual(data),
expect(
getOAuth2AuthData(integration, localConfig, sanitizedIntegrationConfig)
).resolves.toEqual(data),
expect(
getOAuth2AuthData(integration, localConfig, sanitizedIntegrationConfig)
).resolves.toEqual(data),
expect(
getOAuth2AuthData(integration, localConfig, sanitizedIntegrationConfig)
).resolves.toEqual(data),
expect(
getOAuth2AuthData(integration, localConfig, sanitizedIntegrationConfig)
).resolves.toEqual(data),
Copy link
Collaborator Author

@BLoe BLoe Oct 31, 2023

Choose a reason for hiding this comment

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

CoPilot wanted to be very thorough here, so we're testing 5 asynchronous requests at once... 😆

Copy link

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e329c5c) 70.09% compared to head (a4110c7) 70.09%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6781   +/-   ##
=======================================
  Coverage   70.09%   70.09%           
=======================================
  Files        1193     1193           
  Lines       36910    36916    +6     
  Branches     6923     6923           
=======================================
+ Hits        25871    25878    +7     
+ Misses      11039    11038    -1     
Files Coverage Δ
src/background/auth/launchOAuth2Flow.ts 86.20% <100.00%> (ø)
src/background/partnerIntegrations.ts 53.08% <100.00%> (ø)
src/background/requests.ts 92.19% <100.00%> (+1.02%) ⬆️
src/contrib/automationanywhere/BotOptions.tsx 86.84% <ø> (ø)
src/testUtils/factories/authFactories.ts 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BLoe BLoe merged commit 73b3bf9 into main Oct 31, 2023
11 checks passed
@BLoe BLoe deleted the feature/6684-serialize-google-login branch October 31, 2023 20:32
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.

PKCE web auth flow shows multiple times simultaneously
3 participants