Skip to content

Replace AuthConfig with UserSettings in # #40

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

nikosdouvlis
Copy link
Member

@nikosdouvlis nikosdouvlis commented Feb 11, 2022

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Packages affected

  • @clerk/clerk-js
  • @clerk/clerk-react
  • @clerk/nextjs
  • @clerk/types
  • @clerk/clerk-expo
  • @clerk/backend-core
  • @clerk/clerk-sdk-node
  • @clerk/edge
  • build/tooling/chore

Description

  • npm test runs as expected.
  • npm run build runs as expected.

Replace authConfig with userSettings in #

@nikosdouvlis nikosdouvlis self-assigned this Feb 11, 2022
@nikosdouvlis nikosdouvlis changed the title Usersettings # Replace AuthConfig with UserSettings in # Feb 11, 2022
@nikosdouvlis nikosdouvlis marked this pull request as ready for review February 11, 2022 09:56

// TODO: # should have a field similar to SignIn's supportedFirstFactors
// listing the available strategies for this #
const emailLinkStrategyEnabled = firstFactors.includes(emailLinkStrategy);
const emailLinkStrategyEnabled =
attributes.email_address.first_factors.includes('email_link');
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ Does it make sense to also check if attributes.email_address.used_for_first_factor is true

Suggested change
attributes.email_address.first_factors.includes('email_link');
attributes.email_address.used_for_first_factor && attributes.email_address.first_factors.includes('email_link');

Copy link
Member Author

@nikosdouvlis nikosdouvlis Feb 11, 2022

Choose a reason for hiding this comment

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

It should be enough since we only care about the email_link strategy, but @gkats might be able to confirm this

const key = snakeToCamel(name) as keyof Fields;
acc[key] = desc.required ? 'required' : 'on';
return acc;
}, {} as Fields);
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this will be part of the refactoring that we will do later on, right?

@nikosdouvlis nikosdouvlis merged commit 770db0d into replace_authconfig_with_usersettings_in_signin Feb 11, 2022
@nikosdouvlis nikosdouvlis deleted the usersettings_# branch February 11, 2022 12:12
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants