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(billing): Invite billing members to a developer plan #80129

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
76f3967
WIP invite only billing members
jarrettscott Oct 25, 2024
648f007
WIP using react component hooks
jarrettscott Oct 29, 2024
664d855
Unit test with isOverMemberLimit
jarrettscott Oct 29, 2024
27708c8
Merge remote-tracking branch 'origin/master' into jarrett/invite-bill…
jarrettscott Nov 1, 2024
70fb92d
Allow member invitations
jarrettscott Nov 5, 2024
33c31af
Better RoleSelectControl
jarrettscott Nov 6, 2024
3e7b5ea
Formatting
jarrettscott Nov 7, 2024
bd81ad1
Merge remote-tracking branch 'origin/master' into jarrett/invite-bill…
jarrettscott Nov 7, 2024
6f16a5f
Fix test
jarrettscott Nov 7, 2024
4645503
Fix test_ignores_feature_flag
jarrettscott Nov 7, 2024
606ee70
Separating frontend and backend changes
jarrettscott Nov 7, 2024
dbc0666
Merge remote-tracking branch 'origin/master' into jarrett/invite-bill…
jarrettscott Nov 7, 2024
f08119c
Billing orgRole validation
jarrettscott Nov 10, 2024
4a577bf
Merge remote-tracking branch 'origin/master' into jarrett/invite-bill…
jarrettscott Nov 18, 2024
ba89056
Allow only one new billing member
jarrettscott Nov 18, 2024
eef92cb
Fix test
jarrettscott Nov 18, 2024
7133d23
Fix org role tests
jarrettscott Nov 18, 2024
74d2ce1
Merge remote-tracking branch 'origin/master' into jarrett/invite-bill…
jarrettscott Nov 19, 2024
dc79aba
Validate using feature flag instead of billing role
jarrettscott Nov 20, 2024
c244376
Remove owner+billing checks
jarrettscott Nov 20, 2024
2dd9a17
Validation bypass for billing members
jarrettscott Nov 20, 2024
93449f7
Cleanup FE and check for feature flag
jarrettscott Nov 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/sentry/api/endpoints/organization_member/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ def validate_role(self, role):
return self.validate_orgRole(role)

def validate_orgRole(self, role):
if role == "billing" and features.has(
"organizations:invite-billing", self.context["organization"]
):
return role
role_obj = next((r for r in self.context["allowed_roles"] if r.id == role), None)
if role_obj is None:
raise serializers.ValidationError(
Expand Down Expand Up @@ -314,13 +318,18 @@ def post(self, request: Request, organization) -> Response:
"""
Add or invite a member to an organization.
"""
if not features.has("organizations:invite-members", organization, actor=request.user):
assigned_org_role = request.data.get("orgRole") or request.data.get("role")
billing_bypass = assigned_org_role == "billing" and features.has(
"organizations:invite-billing", organization
)
if not billing_bypass and not features.has(
"organizations:invite-members", organization, actor=request.user
):
Copy link
Member

Choose a reason for hiding this comment

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

Inviting billing members would be equivalent to inviting owners on the developer plan.

I wasn't able to invite another owner on the developer plan, so that would also need to change to match the behaviour:

Screenshot 2024-11-12 at 2 34 07 PM

return Response(
{"organization": "Your organization is not allowed to invite members"}, status=403
)

allowed_roles = get_allowed_org_roles(request, organization, creating_org_invite=True)
assigned_org_role = request.data.get("orgRole") or request.data.get("role")

# We allow requests from integration tokens to invite new members as the member role only
if not allowed_roles and request.access.is_integration_token:
Expand Down
2 changes: 2 additions & 0 deletions src/sentry/features/temporary.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ def register_temporary_features(manager: FeatureManager):
manager.add("organizations:integrations-feature-flag-integration", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, api_expose=False)
# Allow tenant type installations through issue alert actions
manager.add("organizations:integrations-msteams-tenant", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Enable inviting billing members to organizations at the member limit.
manager.add("organizations:invite-billing", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, default=False, api_expose=False)
# Enable inviting members to organizations.
manager.add("organizations:invite-members", OrganizationFeature, FeatureHandlerStrategy.INTERNAL, default=True, api_expose=True)
# Enable new invite members modal.
Expand Down
10 changes: 9 additions & 1 deletion static/app/components/modals/inviteMembersModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,12 @@ function InviteMembersModal({
willInvite={willInvite}
onSendInvites={sendInvites}
>
{({sendInvites: inviteModalSendInvites, canSend, headerInfo}) => {
{({
sendInvites: inviteModalSendInvites,
canSend: canSend,
headerInfo: headerInfo,
isOverMemberLimit: isOverMemberLimit,
}) => {
return organization.features.includes('invite-members-new-modal') ? (
<InviteMembersContext.Provider
value={{
Expand Down Expand Up @@ -133,6 +138,9 @@ function InviteMembersModal({
headerInfo={headerInfo}
invites={invites}
inviteStatus={inviteStatus}
isOverMemberLimit={
isOverMemberLimit && organization.features?.includes('invite-billing')
}
member={memberResult.data}
pendingInvites={pendingInvites}
removeInviteRow={removeInviteRow}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,29 @@ describe('InviteMembersModalView', function () {
setRole: () => {},
setTeams: () => {},
willInvite: false,
isOverMemberLimit: false,
};

const overMemberLimitModalProps: ComponentProps<typeof InviteMembersModalView> = {
Footer: styledWrapper(),
addInviteRow: () => {},
canSend: true,
closeModal: () => {},
complete: false,
headerInfo: null,
inviteStatus: {},
invites: [],
member: undefined,
pendingInvites: [],
removeInviteRow: () => {},
reset: () => {},
sendInvites: () => {},
sendingInvites: false,
setEmails: () => {},
setRole: () => {},
setTeams: () => {},
willInvite: true,
isOverMemberLimit: true,
};

it('renders', function () {
Expand All @@ -45,4 +68,10 @@ describe('InviteMembersModalView', function () {
// Check that the Alert component renders with the provided error message
expect(screen.getByText('This is an error message')).toBeInTheDocument();
});

it('renders when over member limit', function () {
render(<InviteMembersModalView {...overMemberLimitModalProps} />);

expect(screen.getByText('Invite New Members')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type {ReactNode} from 'react';
import {Fragment} from 'react';
import {Fragment, type ReactNode, useEffect, useRef} from 'react';
import {css} from '@emotion/react';
import styled from '@emotion/styled';

Expand Down Expand Up @@ -30,6 +29,7 @@ interface Props {
headerInfo: ReactNode;
inviteStatus: InviteStatus;
invites: NormalizedInvite[];
isOverMemberLimit: boolean;
member: Member | undefined;
pendingInvites: InviteRow[];
removeInviteRow: (index: number) => void;
Expand All @@ -52,6 +52,7 @@ export default function InviteMembersModalView({
headerInfo,
invites,
inviteStatus,
isOverMemberLimit,
member,
pendingInvites,
removeInviteRow,
Expand All @@ -76,6 +77,16 @@ export default function InviteMembersModalView({
</Alert>
) : null;

const canSendRef = useRef(canSend);

useEffect(() => {
if (isOverMemberLimit) {
setRole('billing', 0);
setTeams([], 0);
canSendRef.current = true;
}
});

return (
<Fragment>
{errorAlert}
Expand Down Expand Up @@ -115,10 +126,10 @@ export default function InviteMembersModalView({
onChangeRole={value => setRole(value?.value, i)}
onChangeTeams={opts => setTeams(opts ? opts.map(v => v.value) : [], i)}
disableRemove={disableInputs || pendingInvites.length === 1}
isOverMemberLimit={isOverMemberLimit}
/>
))}
</Rows>

<AddButton
disabled={disableInputs}
size="sm"
Expand All @@ -128,7 +139,6 @@ export default function InviteMembersModalView({
>
{t('Add another')}
</AddButton>

<Footer>
<FooterContent>
<div>
Expand All @@ -140,7 +150,6 @@ export default function InviteMembersModalView({
willInvite={willInvite}
/>
</div>

<ButtonBar gap={1}>
{complete ? (
<Fragment>
Expand Down Expand Up @@ -172,7 +181,7 @@ export default function InviteMembersModalView({
size="sm"
data-test-id="send-invites"
priority="primary"
disabled={!canSend || !isValidInvites || disableInputs}
disabled={!canSendRef.current || !isValidInvites || disableInputs}
onClick={sendInvites}
/>
</Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type Props = {
disabled: boolean;
emails: string[];
inviteStatus: InviteStatus;
isOverMemberLimit: boolean;
onChangeEmails: (emails: SelectOption[]) => void;
onChangeRole: (role: SelectOption) => void;
onChangeTeams: (teams: SelectOption[]) => void;
Expand Down Expand Up @@ -52,6 +53,7 @@ function InviteRowControl({
onChangeRole,
onChangeTeams,
disableRemove,
isOverMemberLimit,
}: Props) {
const [inputValue, setInputValue] = useState('');

Expand Down Expand Up @@ -118,7 +120,7 @@ function InviteRowControl({
<RoleSelectControl
aria-label={t('Role')}
data-test-id="select-role"
disabled={disabled}
disabled={isOverMemberLimit ? true : disabled}
value={role}
roles={roleOptions}
disableUnallowed={roleDisabledUnallowed}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import HookOrDefault from 'sentry/components/hookOrDefault';
export const InviteModalHook = HookOrDefault({
hookName: 'member-invite-modal:customization',
defaultComponent: ({onSendInvites, children}) =>
children({sendInvites: onSendInvites, canSend: true}),
children({sendInvites: onSendInvites, canSend: true, isOverMemberLimit: false}),
});

export type InviteModalRenderFunc = React.ComponentProps<
Expand Down
5 changes: 5 additions & 0 deletions static/app/types/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,11 @@ type InviteModalCustomizationHook = () => React.ComponentType<{
* invites may currently be sent.
*/
canSend: boolean;
/**
* Indicates that the account has reached the maximum member limit. Future invitations
* are limited to Billing roles
*/
isOverMemberLimit: boolean;
/**
* Trigger sending invites
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ function OrganizationMembersList() {
refetchInviteRequests();
refetchMembers();
}}
allowedRoles={currentMember ? currentMember.roles : ORG_ROLES}
allowedRoles={currentMember?.orgRoleList ?? currentMember?.roles ?? ORG_ROLES}
/>
{inviteRequests.length > 0 && (
<Panel>
Expand All @@ -329,7 +329,7 @@ function OrganizationMembersList() {
organization={organization}
inviteRequest={inviteRequest}
inviteRequestBusy={{}}
allRoles={currentMember?.roles ?? ORG_ROLES}
allRoles={currentMember?.orgRoleList ?? currentMember?.roles ?? ORG_ROLES}
onApprove={handleInviteRequestApprove}
onDeny={handleInviteRequestDeny}
onUpdate={data => updateInviteRequest(inviteRequest.id, data)}
Expand All @@ -340,7 +340,7 @@ function OrganizationMembersList() {
)}
<SearchWrapperWithFilter>
<MembersFilter
roles={currentMember?.roles ?? ORG_ROLES}
roles={currentMember?.orgRoleList ?? currentMember?.roles ?? ORG_ROLES}
query={searchQuery}
onChange={handleQueryChange}
/>
Expand Down
23 changes: 23 additions & 0 deletions tests/sentry/api/endpoints/test_organization_member_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,29 @@ def test_invalid_team_role(self):
assert not serializer.is_valid()
assert serializer.errors == {"teamRoles": ["Invalid team-role"]}

@with_feature("organizations:invite-billing")
def test_valid_invite_billing_member(self):
context = {"organization": self.organization, "allowed_roles": [roles.get("member")]}
data = {
"email": "bill@localhost",
"orgRole": "billing",
"teamRoles": [],
}

serializer = OrganizationMemberRequestSerializer(context=context, data=data)
assert serializer.is_valid()

def test_invalid_invite_billing_member(self):
context = {"organization": self.organization, "allowed_roles": [roles.get("member")]}
data = {
"email": "bill@localhost",
"orgRole": "billing",
"teamRoles": [],
}

serializer = OrganizationMemberRequestSerializer(context=context, data=data)
assert not serializer.is_valid()


class OrganizationMemberListTestBase(APITestCase):
endpoint = "sentry-api-0-organization-member-index"
Expand Down
Loading