Skip to content

Commit

Permalink
feat(organizations): handle user removes itself flow TASK-1354 (#5393)
Browse files Browse the repository at this point in the history
### 📣 Summary
The navigation and data flow is improved when an MMO organization admin
removes itself from the organization.


### 📖 Description
The objective of this PR is to:
- Minimize wrong API calls due to permissions and session data that
needs to be updated
- Present a better flow for the user, redirecting them to the correct
view after the removal
There's still a big margin for improving, mainly due to some work needed
in `useUsage`, but it unfolds into several other files and flows, so
it's better to be handled in a separated PR

### 👀 Preview steps
1. ℹ️ have an account and a project
2. Be the admin in an MMO
3. Navigate to Account Settings > Members
4. Remove yourself from the organization
5. 🔴 [on main] notice that the user is not redirected and there are some
failed attempts to API calls due to permissions
6. 🟢 [on PR] notice that the user is redirected and there's only 1
failed attempt to usage API
  • Loading branch information
pauloamorimbr authored Jan 9, 2025
1 parent 36b4484 commit c85150b
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 30 deletions.
14 changes: 11 additions & 3 deletions jsapp/js/account/organization/MemberActionsDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {OrganizationUserRole} from './organizationQuery';

// Styles
import styles from './memberActionsDropdown.module.scss';
import router from 'jsapp/js/router/router';
import { ROUTES } from 'jsapp/js/router/routerConstants';

interface MemberActionsDropdownProps {
targetUsername: string;
Expand Down Expand Up @@ -69,15 +71,21 @@ export default function MemberActionsDropdown(
.replace('##TEAM_OR_ORGANIZATION##', mmoLabel);
}

const onRemovalConfirmation = () => {
setIsRemoveModalVisible(false);
if (isAdminRemovingSelf) {
// Redirect to account root after leaving the organization
router.navigate(ROUTES.ACCOUNT_ROOT);
}
};

return (
<>
{isRemoveModalVisible &&
<MemberRemoveModal
username={targetUsername}
isRemovingSelf={isAdminRemovingSelf}
onConfirmDone={() => {
setIsRemoveModalVisible(false);
}}
onConfirmDone={onRemovalConfirmation}
onCancel={() => setIsRemoveModalVisible(false)}
/>
}
Expand Down
20 changes: 11 additions & 9 deletions jsapp/js/account/organization/MemberRemoveModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ export default function MemberRemoveModal(
.replaceAll('##TEAM_OR_ORGANIZATION##', mmoLabel);
}

const handleRemoveMember = async () => {
try {
await removeMember.mutateAsync(username);
} catch (error) {
notify('Failed to remove member', 'error');
} finally {
onConfirmDone();
}
};

return (
<KoboModal isOpen size='medium' onRequestClose={() => onCancel()}>
<KoboModalHeader>{textToDisplay.title}</KoboModalHeader>
Expand All @@ -87,15 +97,7 @@ export default function MemberRemoveModal(
<Button
type='danger'
size='m'
onClick={async () => {
try {
removeMember.mutateAsync(username);
} catch (error) {
notify('Failed to remove member', 'error');
} finally {
onConfirmDone();
}
}}
onClick={handleRemoveMember}
label={textToDisplay.confirmButtonLabel}
isPending={removeMember.isPending}
/>
Expand Down
49 changes: 33 additions & 16 deletions jsapp/js/account/organization/membersQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {endpoints} from 'js/api.endpoints';
import type {PaginatedResponse} from 'js/dataInterface';
import {QueryKeys} from 'js/query/queryKeys';
import type {PaginatedQueryHookParams} from 'jsapp/js/universalTable/paginatedQueryUniversalTable.component';
import {useSession} from 'jsapp/js/stores/useSession';

export interface OrganizationMember {
/**
Expand Down Expand Up @@ -49,9 +50,10 @@ export interface OrganizationMember {
}

function getMemberEndpoint(orgId: string, username: string) {
return endpoints.ORGANIZATION_MEMBER_URL
.replace(':organization_id', orgId)
.replace(':username', username);
return endpoints.ORGANIZATION_MEMBER_URL.replace(
':organization_id',
orgId
).replace(':username', username);
}

/**
Expand All @@ -65,41 +67,51 @@ export function usePatchOrganizationMember(username: string) {
const orgId = orgQuery.data?.id;

return useMutation({
mutationFn: async (data: Partial<OrganizationMember>) => (
mutationFn: async (data: Partial<OrganizationMember>) =>
// We're asserting the `orgId` is not `undefined` here, because the parent
// query (`useOrganizationMembersQuery`) wouldn't be enabled without it.
// Plus all the organization-related UI (that would use this hook) is
// accessible only to logged in users.
fetchPatch<OrganizationMember>(getMemberEndpoint(orgId!, username), data)
),
fetchPatch<OrganizationMember>(getMemberEndpoint(orgId!, username), data),
onSettled: () => {
// We invalidate query, so it will refetch (instead of refetching it
// directly, see: https://github.com/TanStack/query/discussions/2468)
queryClient.invalidateQueries({queryKey: [QueryKeys.organizationMembers]});
queryClient.invalidateQueries({
queryKey: [QueryKeys.organizationMembers],
});
},
});
}

/**
* Mutation hook for removing member from organiztion. It ensures that all
* Mutation hook for removing member from organization. It ensures that all
* related queries refetch data (are invalidated).
*/
export function useRemoveOrganizationMember() {
const queryClient = useQueryClient();

const session = useSession();

const orgQuery = useOrganizationQuery();
const orgId = orgQuery.data?.id;

return useMutation({
mutationFn: async (username: string) => (
mutationFn: async (username: string) =>
// We're asserting the `orgId` is not `undefined` here, because the parent
// query (`useOrganizationMembersQuery`) wouldn't be enabled without it.
// Plus all the organization-related UI (that would use this hook) is
// accessible only to logged in users.
fetchDelete(getMemberEndpoint(orgId!, username))
),
onSettled: () => {
queryClient.invalidateQueries({queryKey: [QueryKeys.organizationMembers]});
fetchDelete(getMemberEndpoint(orgId!, username))
,
onSuccess: (_data, username) => {
if (username === session.currentLoggedAccount?.username) {
// If user is removing themselves, we need to clear the session
session.refreshAccount();
} else {
queryClient.invalidateQueries({
queryKey: [QueryKeys.organizationMembers],
});
}
},
});
}
Expand All @@ -119,8 +131,10 @@ async function getOrganizationMembers(
offset: offset.toString(),
});

const apiUrl = endpoints.ORGANIZATION_MEMBERS_URL
.replace(':organization_id', orgId);
const apiUrl = endpoints.ORGANIZATION_MEMBERS_URL.replace(
':organization_id',
orgId
);

return fetchGet<PaginatedResponse<OrganizationMember>>(
apiUrl + '?' + params,
Expand All @@ -134,7 +148,10 @@ async function getOrganizationMembers(
* A hook that gives you paginated list of organization members. Uses
* `useOrganizationQuery` to get the id.
*/
export default function useOrganizationMembersQuery({limit, offset}: PaginatedQueryHookParams) {
export default function useOrganizationMembersQuery({
limit,
offset,
}: PaginatedQueryHookParams) {
const orgQuery = useOrganizationQuery();
const orgId = orgQuery.data?.id;

Expand Down
3 changes: 1 addition & 2 deletions jsapp/js/account/organization/organizationQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,11 @@ export const useOrganizationQuery = (params?: OrganizationQueryParams) => {
}, [params?.shouldForceInvalidation]);

const session = useSession();
const organizationUrl = session.currentLoggedAccount?.organization?.url;
const organizationUrl = !session.isPending ? session.currentLoggedAccount?.organization?.url : undefined;

// Setting the 'enabled' property so the query won't run until we have
// the session data loaded. Account data is needed to fetch the organization
// data.

const query = useQuery<Organization, FailResponse>({
staleTime: 1000 * 60 * 2,
// We're asserting the `organizationUrl` is not `undefined` here because
Expand Down
9 changes: 9 additions & 0 deletions jsapp/js/stores/useSession.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,17 @@ export const useSession = () => {
{fireImmediately: true}
);

const isPendingReactionDisposer = reaction(
() => sessionStore.isPending,
() => {
setIsPending(sessionStore.isPending);
},
{fireImmediately: true}
);

return () => {
currentAccountReactionDisposer();
isPendingReactionDisposer();
};
}, []);

Expand Down

0 comments on commit c85150b

Please sign in to comment.