feat: update organization member abilities #27232
Conversation
zlwaterfield
changed the title
zlwaterfield
requested review from
Twixes,
raquelmsmith,
timgl,
benjackwhite and
patricio-posthog
and removed request for
Twixes
|
Size Change: 0 B Total Size: 1.11 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
There was a problem hiding this comment.
What is the reason for making it so that members cannot invite new members generally? Since we don't charge for seats I don't think this is a good idea. Getting more people into posthog is always better, and bottlenecking that through admins and owners is probably a bad thing?
There was a problem hiding this comment.
Yeah I've went back and forth on this. Generally it's not great practice to allow anyone to join your organization but I agree with you. I think I'll wind this back to how it is today and on another PR add a configurable option for organizations to set this - could be a teams add-on feature. We've had multiple customers ask for limiting who can invite.
There was a problem hiding this comment.
Updated - there are no changes on this PR for sending invites, although it still blocks deleting invites.
Co-authored-by: Raquel Smith <[email protected]>
zlwaterfield
requested review from
raquelmsmith
and removed request for
timgl,
benjackwhite,
Twixes,
patricio-posthog and
raquelmsmith
| /** Whether input field is disabled */ | ||
| disabled?: boolean |
There was a problem hiding this comment.
Our pattern is usually disabledReason to force someone to give a reason why something is disabled, which is a better user experience. Should we follow that pattern?
There was a problem hiding this comment.
Disabled reason looks to be used for buttons and such but just disabled on inputs because we don't show the tooltip on those.
There was a problem hiding this comment.
Yeah, just wondering if we should change that
| @@ -46,7 +53,7 @@ export function SSOSelect({ value, loading, onChange, samlAvailable }: SSOSelect | |||
| value={value} | |||
| options={options} | |||
| loading={loading} | |||
| disabledReason={loading ? 'Cannot change while loading' : undefined} | |||
| disabledReason={loading ? 'Cannot change while loading' : disabledReason} | |||
There was a problem hiding this comment.
Does this component not handle disabling while loading? I kinda figure it would since it has the loading prop. if not, it probably should?
There was a problem hiding this comment.
It does, but this is just adding a tooltip over it while it's loading. The loading props just disables it, doesn't give a tooltip.
There was a problem hiding this comment.
Yeah, might be worth changing, so if no reason it just says "loading..." Otherwise can override if you want a custom loading message with disabledReason
There was a problem hiding this comment.
I'm hestitant to make changes that affect such a high number of parts of the app - that would make it so anytime a button is loading there is a tooltip over it showing text (the LemonSelect uses LemonButton).
I think it's better to have a more explicit option like this for when you actually want a tooltip.
Changes
Related to upcoming RBAC changes, follow along #24512.
This PR is updating the abilities of an organization member to be more restrictive - we're moving to a model where users will need to be organization admins or owners to perform organization-level changes. This change is opinionated - it's meant to apply a new level of thinking of what a member should be able to do - which is very little at the organization level. This change is being made in line with the RBAC to allow organizations to control better who can do what within their PostHog organization.
These changes are only being made on the client side, there will be a subsequent PR to also check these serverside.
posthog.com docs change: PostHog/posthog.com#10246
Changes (things members can no longer do)
I will post this in Slack and let everyone know if they receive support requests related to this change. We can recommend users update member roles to admin for those who need to access features we've changed.
In the future:
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Does this work well for both Cloud and self-hosted?
Yes
How did you test this code?
Test manually