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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Pass only supported properties of account to CreateAccount #2445

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

gdnmhr
Copy link
Contributor

@gdnmhr gdnmhr commented Apr 13, 2024

What does this PR do?

This PR removes unsupported properties from the accounts object upon signup with an external provider. These unsupported properties cause sign-ups to fail for example with Keycloak.

Fixes #2444

How should this be tested?

  1. Setup a Keycloak server
  2. Add the provider to the Formbricks config
  3. Try to create a user with and without the patch

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • First PR at Formbricks? Please sign the CLA! Without it we wont be able to merge it 馃檹

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

Copy link

vercel bot commented Apr 13, 2024

@gdnmhr is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the bug Something isn't working label Apr 13, 2024
Copy link
Contributor

github-actions bot commented Apr 13, 2024

Thank you for following the naming conventions for pull request titles! 馃檹

@gdnmhr gdnmhr changed the title Pass only supported properties of account to CreateAccount fix: Pass only supported properties of account to CreateAccount Apr 13, 2024
Copy link
Contributor

@ShubhamPalriwala ShubhamPalriwala left a comment

Choose a reason for hiding this comment

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

Thanks a lot! This works nicely! 馃殌 I refactored the code a bit so that since we also had our own ZAccountInput & TAccountInput zod types for easier handling. Hence I think we should use that so that we do not have to modify the fields at multiple places if in case we want to later.

Thanks a lot for the foundation of this, I'll commit my changes to your PR itself and let me know if you have any questions/feedback/ideas to improve on them. 馃馃徏

@ShubhamPalriwala ShubhamPalriwala added this pull request to the merge queue Apr 17, 2024
Merged via the queue into formbricks:main with commit 4fc8ee8 Apr 17, 2024
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] SignUp with Keycloak OCID fails due to refresh_expires_in
3 participants