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.

Sign up for GitHub

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

fix: lookup action roles from the params and simplify stuff #4169

Merged
merged 1 commit into from
Jan 31, 2025
Merged

fix: lookup action roles from the params and simplify stuff #4169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
74 changes: 21 additions & 53 deletions src/components/common/ColonyActionsTable/hooks/useBuildRedoEnabledActionsMap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { useEffect, useState } from 'react';

import { ColonyActionType, useGetColonyHistoricRoleRolesLazyQuery } from '~gql';
import { ColonyActionType } from '~gql';
import { type ActivityFeedColonyAction } from '~hooks/useActivityFeed/types.ts';
import { ExtendedColonyActionType } from '~types/actions.ts';
import { getHistoricRolesDatabaseId } from '~utils/databaseId.ts';
import { transformActionRolesToColonyRoles } from '~v5/common/CompletedAction/partials/SetUserRoles/utils.ts';
import { normalizeRolesForAction } from '~utils/colonyActions.ts';

export const useBuildRedoEnabledActionsMap = ({
colonyActions = [],
Expand All @@ -17,15 +16,11 @@ export const useBuildRedoEnabledActionsMap = ({
Record<ActivityFeedColonyAction['transactionHash'], boolean>
>({});

const [getHistoricRoles] = useGetColonyHistoricRoleRolesLazyQuery({
fetchPolicy: 'cache-and-network',
});

// I'm using simple stringification to help the useEffect hook with shallow comparison and avoid unnecessary rerenders
const stringifiedColonyActions = JSON.stringify(colonyActions);

useEffect(() => {
const buildRedoEnabledActionsMap = async () => {
const buildRedoEnabledActionsMap = () => {
const originalColonyActions = JSON.parse(
stringifiedColonyActions,
) as typeof colonyActions;
Expand All @@ -35,7 +30,6 @@ export const useBuildRedoEnabledActionsMap = ({
}

const updatedActionsMap: typeof redoEnabledActionsMap = {};
const promises: Promise<void>[] = [];

originalColonyActions.forEach((colonyAction) => {
switch (
Expand All @@ -44,47 +38,23 @@ export const useBuildRedoEnabledActionsMap = ({
case ColonyActionType.SetUserRoles:
case ColonyActionType.SetUserRolesMotion:
case ColonyActionType.SetUserRolesMultisig: {
const {
blockNumber,
colonyAddress,
fromDomain,
recipientAddress,
rolesAreMultiSig,
roles,
motionData,
multiSigData,
} = colonyAction;

const promise = async () => {
const result = await getHistoricRoles({
variables: {
id: getHistoricRolesDatabaseId({
blockNumber,
colonyAddress,
nativeId: fromDomain?.nativeId,
recipientAddress,
isMultiSig: rolesAreMultiSig,
}),
},
});

const dbPermissionsNew = transformActionRolesToColonyRoles(
result?.data?.getColonyHistoricRole || roles,
);

const isMotion = !!motionData;
const isMultiSig = !!multiSigData;

const shouldShowRedoItem =
!!dbPermissionsNew.length ||
(isMotion && !motionData?.isFinalized) ||
(isMultiSig && !multiSigData?.isExecuted);

updatedActionsMap[colonyAction.transactionHash] =
shouldShowRedoItem;
};

promises.push(promise());
const { roles, motionData, multiSigData } = colonyAction;

const normalizedRoles = normalizeRolesForAction(roles ?? {});
const isRemovingRoles =
normalizedRoles.filter((role) => role.setTo).length === 0;

const isMotion = !!motionData;
const isMultiSig = !!multiSigData;

const shouldShowRedoItem =
!isRemovingRoles ||
(isMotion && !motionData?.isFinalized) ||
(isMultiSig && !multiSigData?.isExecuted);

updatedActionsMap[colonyAction.transactionHash] =
shouldShowRedoItem;

break;
}

Expand Down Expand Up @@ -124,13 +94,11 @@ export const useBuildRedoEnabledActionsMap = ({
}
});

await Promise.all(promises);

setRedoEnabledActionsMap(updatedActionsMap);
};

buildRedoEnabledActionsMap();
}, [colonyActionsLoading, getHistoricRoles, stringifiedColonyActions]);
}, [colonyActionsLoading, stringifiedColonyActions]);

return redoEnabledActionsMap;
};
82 changes: 43 additions & 39 deletions src/components/v5/common/CompletedAction/partials/SetUserRoles/SetUserRoles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,14 @@ import { ColonyActionType, useGetColonyHistoricRoleRolesQuery } from '~gql';
import { getUserRolesForDomain } from '~transformers';
import { Authority } from '~types/authority.ts';
import { type ColonyAction } from '~types/graphql.ts';
import { formatRolesTitle } from '~utils/colonyActions.ts';
import { extractColonyRoles } from '~utils/colonyRoles.ts';
import {
formatRolesTitle,
normalizeRolesForAction,
} from '~utils/colonyActions.ts';
import {
extractColonyRoles,
transformApiRolesToArray,
} from '~utils/colonyRoles.ts';
import { getHistoricRolesDatabaseId } from '~utils/databaseId.ts';
import { formatText } from '~utils/intl.ts';
import { splitWalletAddress } from '~utils/splitWalletAddress.ts';
Expand Down Expand Up @@ -42,11 +48,6 @@ import {
TeamFromRow,
} from '../rows/index.ts';

import {
getIsPermissionsRemoval,
transformActionRolesToColonyRoles,
} from './utils.ts';

const displayName = 'v5.common.CompletedAction.partials.SetUserRoles';

interface Props {
Expand All @@ -72,13 +73,14 @@ const SetUserRoles = ({ action }: Props) => {
transactionHash,
fromDomain,
annotation,
rolesAreMultiSig,
blockNumber,
colonyAddress,
rolesAreMultiSig,
motionData,
multiSigData,
isMotion,
isMultiSig,
} = action;
const areRolesMultiSig = !!rolesAreMultiSig;
const isActionMotion = isMotion || isMultiSig;

const roleAuthority = rolesAreMultiSig
? Authority.ViaMultiSig
Expand Down Expand Up @@ -106,33 +108,37 @@ const SetUserRoles = ({ action }: Props) => {
constraint: 'excludeInheritedRoles',
});

const actionRoles = transformActionRolesToColonyRoles(roles);
// in case of motions, no historic roles are created so we just assume we are modifying their current roles (which contract wise we are)
const oldRoles = historicRoles?.getColonyHistoricRole
? transformApiRolesToArray(historicRoles.getColonyHistoricRole)
: currentUserRoles;

const dbPermissionsOld =
actionRoles.filter(Boolean).length > 0 // if we didnt just remove all the roles
? actionRoles
: currentUserRoles;
if (!roles) {
console.warn('No roles present in action');
return null;
}

const { role: dbRoleForDomainOld } = getRole(
dbPermissionsOld,
areRolesMultiSig,
);
const normalizedRoles = normalizeRolesForAction(roles);
const isRemovingRoles =
normalizedRoles.filter((role) => role.setTo).length === 0;

const dbPermissionsNew = transformActionRolesToColonyRoles(
historicRoles?.getColonyHistoricRole || roles,
{
isMotion: !!motionData || !!multiSigData,
},
);
/*
* This probably seems so confusing but dropping a here be dragons comment for future developers
* When you create a motion, there is no historic roles created, that's created when a setUserRoles contract passes
* when you do it via permissions, it 100% reflects the roles the user currently holds
* The roles on the action are just a diff, so if we add just 1 role, only that one is there, the same when we remove it
* Motions however modify the user's current roles in the colony, so we use those
*/
Comment on lines +124 to +130
Copy link
Contributor

Choose a reason for hiding this comment

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

Needed a couple of reads to actually understand it, but it totally makes sense 👍

const newRoles = isActionMotion
? normalizedRoles.filter((role) => role.setTo === true).map(({ id }) => id)
: oldRoles;

const { name: dbRoleNameNew, role: dbRoleForDomainNew } = getRole(
dbPermissionsNew,
const { name: newRoleName, role: newUserRole } = getRole(
newRoles,
areRolesMultiSig,
);
const { role: oldUserRole } = getRole(oldRoles, areRolesMultiSig);

const metadata = action.motionData?.motionDomain.metadata;

const rolesTitle = formatRolesTitle(roles);

return (
Expand All @@ -146,15 +152,13 @@ const SetUserRoles = ({ action }: Props) => {
[ACTION_TYPE_FIELD_NAME]: Action.ManagePermissions,
member: recipientAddress,
authority: roleAuthority,
role: getIsPermissionsRemoval(roles)
? UserRoleModifier.Remove
: dbRoleForDomainNew,
role: isRemovingRoles ? UserRoleModifier.Remove : newUserRole,
[TEAM_FIELD_NAME]: fromDomain?.nativeId,
[DECISION_METHOD_FIELD_NAME]: decisionMethod,
[DESCRIPTION_FIELD_NAME]: annotation?.message,
}}
showRedoItem={
!!dbPermissionsNew.length ||
!isRemovingRoles ||
(!!action.motionData && !action.motionData?.isFinalized) ||
(!!action.multiSigData && !action.multiSigData?.isExecuted)
}
Expand Down Expand Up @@ -231,8 +235,8 @@ const SetUserRoles = ({ action }: Props) => {
<ActionData
rowLabel={formatText({ id: 'actionSidebar.permissions' })}
rowContent={
dbPermissionsNew.length
? dbRoleNameNew
!isRemovingRoles
? newRoleName
: formatText({
id: 'actionSidebar.managePermissions.roleSelect.remove.title',
})
Expand All @@ -249,11 +253,11 @@ const SetUserRoles = ({ action }: Props) => {
<DescriptionRow description={action.annotation.message} />
)}
<PermissionsTableRow
dbPermissionsOld={dbPermissionsOld}
dbPermissionsNew={dbPermissionsNew}
dbPermissionsOld={oldRoles}
dbRoleForDomainOld={oldUserRole}
dbPermissionsNew={newRoles}
dbRoleForDomainNew={newUserRole}
domainId={action.fromDomain?.nativeId}
dbRoleForDomainNew={dbRoleForDomainNew}
dbRoleForDomainOld={dbRoleForDomainOld}
/>
</>
);
Expand Down
56 changes: 0 additions & 56 deletions src/components/v5/common/CompletedAction/partials/SetUserRoles/utils.ts
View file
Open in desktop

This file was deleted.

5 changes: 2 additions & 3 deletions src/constants/permissions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,9 @@ export const getRole = (
);
}

// Multi-sig does not use the Mod role
return (
USER_ROLES.filter((role) => role.role !== UserRole.Mod).find(
({ permissions }) => isEqual(permissions.sort(), permissionsList.sort()),
USER_ROLES.find(({ permissions }) =>
isEqual(permissions.sort(), permissionsList.sort()),
) || {
...CUSTOM_USER_ROLE,
permissions: permissionsList,
Expand Down
Loading
Loading