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

feat(clerk-js,types): Supports default role on OrganizationProfile invitations #4210

Merged

Conversation

LauraBeatris
Copy link
Member

@LauraBeatris LauraBeatris commented Sep 23, 2024

Description

Resolves ORGS-211

Prepares OrganizationProfile for the upcoming default_role changes. When inviting a member, the default_role will be automatically selected, otherwise it falls back to the only available role.

Currently behind a feature flag in the Dashboard, we're now allowing updating default_role regardless of whether verified domains are enabled.


CleanShot.2024-09-24.at.07.57.19.mp4

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

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

@LauraBeatris LauraBeatris self-assigned this Sep 23, 2024
Copy link

changeset-bot bot commented Sep 23, 2024

🦋 Changeset detected

Latest commit: 8212581

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages
Name Type
@clerk/clerk-js Patch
@clerk/types Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/elements Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/localizations Patch
@clerk/nextjs Patch
@clerk/clerk-react Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
@clerk/shared Patch
@clerk/tanstack-start Patch
@clerk/testing Patch
@clerk/themes Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@LauraBeatris LauraBeatris force-pushed the laura/orgs-211-set-default-role-on-organizationprofile branch 2 times, most recently from eafc4b5 to bfe9d11 Compare September 24, 2024 10:53
@LauraBeatris LauraBeatris changed the title feat(clerk-js,types): Supports default role on OrganizationProfile feat(clerk-js,types): Supports default role on OrganizationProfile invitations Sep 24, 2024
@LauraBeatris LauraBeatris changed the title feat(clerk-js,types): Supports default role on OrganizationProfile invitations feat(clerk-js,types): Supports default role on OrganizationProfile invitations Sep 24, 2024
@LauraBeatris LauraBeatris force-pushed the laura/orgs-211-set-default-role-on-organizationprofile branch 2 times, most recently from a231b1e to df5ceb2 Compare September 24, 2024 11:05
@LauraBeatris LauraBeatris marked this pull request as ready for review September 24, 2024 11:05
@LauraBeatris LauraBeatris requested review from panteliselef and a team September 24, 2024 11:06
Copy link
Member

@panteliselef panteliselef left a comment

Choose a reason for hiding this comment

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

Left a few comments. If you feel they are not significant feel free to merge.


let defaultRole = organizationSettings.domains.defaultRole;

if (!defaultRole && options?.length === 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be >1 ?

"If default role doesn't exist and fetched roles exist, use the first fetched role"

Copy link
Member Author

Choose a reason for hiding this comment

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

We're checking if there's only one role returned from FAPI here, instead of selecting the first one. This avoids making an assumption on behalf of the user, as there could be cases where they wouldn't want that one the first one as default.

@LauraBeatris LauraBeatris force-pushed the laura/orgs-211-set-default-role-on-organizationprofile branch from 9ebf02b to 598a2f6 Compare September 24, 2024 15:53
@LauraBeatris LauraBeatris force-pushed the laura/orgs-211-set-default-role-on-organizationprofile branch 2 times, most recently from e18df9c to 99ec73b Compare September 24, 2024 17:38
key: 'admin',
name: 'Admin',
description: '',
permissions: [],
Copy link
Member Author

Choose a reason for hiding this comment

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

Those tests were mocking with a single role, therefore initiating the field with that one instead and the assertion for "Select role" started to fail.

@LauraBeatris LauraBeatris force-pushed the laura/orgs-211-set-default-role-on-organizationprofile branch from 99ec73b to d7b4569 Compare September 24, 2024 17:47
@LauraBeatris LauraBeatris force-pushed the laura/orgs-211-set-default-role-on-organizationprofile branch from d7b4569 to 8212581 Compare September 24, 2024 17:48
@LauraBeatris LauraBeatris enabled auto-merge (squash) September 24, 2024 17:48
@LauraBeatris LauraBeatris merged commit 19d3808 into main Sep 24, 2024
20 checks passed
@LauraBeatris LauraBeatris deleted the laura/orgs-211-set-default-role-on-organizationprofile branch September 24, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants