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: allow user to leave a group #88

Merged
merged 3 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
35 changes: 3 additions & 32 deletions components/groups/components/myGroups.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,27 @@ import { PiInfo } from 'react-icons/pi';
import { GroupInfo } from '../modals/groupInfo';
import { MemberManagementModal } from '../modals/memberManagementModal';
import { useChain } from '@cosmos-kit/react';
import { useGroupsByMember } from '@/hooks/useQueries';
import { MemberSDKType } from '@liftedinit/manifestjs/dist/codegen/cosmos/group/v1/types';

export function YourGroups({
groups,
proposals,
isLoading,
refetch,
}: {
groups: ExtendedQueryGroupsByMemberResponseSDKType;
proposals: { [policyAddress: string]: ProposalSDKType[] };
isLoading: boolean;
refetch: () => void;
}) {
const [searchTerm, setSearchTerm] = useState('');
const [selectedGroup, setSelectedGroup] = useState<{
policyAddress: string;
name: string;
threshold: string;
} | null>(null);
const [members, setMembers] = useState<MemberSDKType[]>([]);
const [groupId, setGroupId] = useState<string>('');
const [groupAdmin, setGroupAdmin] = useState<string>('');

const router = useRouter();
const { address } = useChain('manifest');
const { groupByMemberData } = useGroupsByMember(address ?? '');

const filteredGroups = groups.groups.filter(group =>
(group.ipfsMetadata?.title || 'Untitled Group').toLowerCase().includes(searchTerm.toLowerCase())
Expand Down Expand Up @@ -74,31 +70,6 @@ export function YourGroups({
}
}, [selectedGroup]);

useEffect(() => {
if (groupByMemberData && selectedGroup?.policyAddress) {
const group = groupByMemberData?.groups?.find(
g => g?.policies?.length > 0 && g.policies[0]?.address === selectedGroup.policyAddress
);
if (group) {
setMembers(
group.members.map(member => ({
...member.member,
address: member?.member?.address || '',
weight: member?.member?.weight || '0',
metadata: member?.member?.metadata || '',
added_at: member?.member?.added_at || new Date(),
isCoreMember: true,
isActive: true,
isAdmin: member?.member?.address === group.admin,
isPolicyAdmin: member?.member?.address === group.policies[0]?.admin,
}))
);
setGroupId(group.id.toString());
setGroupAdmin(group.admin);
}
}
}, [groupByMemberData, selectedGroup?.policyAddress]);

const handleSelectGroup = (policyAddress: string, groupName: string, threshold: string) => {
setSelectedGroup({ policyAddress, name: groupName || 'Untitled Group', threshold });
router.push(`/groups?policyAddress=${policyAddress}`, undefined, { shallow: true });
Expand Down Expand Up @@ -253,7 +224,7 @@ export function YourGroups({
group={group}
address={address ?? ''}
policyAddress={group.policies[0]?.address ?? ''}
onUpdate={() => {}}
onUpdate={refetch}
/>
<MemberManagementModal
modalId={`member-management-modal-${group.id}`}
Expand Down
64 changes: 51 additions & 13 deletions components/groups/modals/groupInfo.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { TruncatedAddressWithCopy } from '@/components/react/addressCopy';
import { useBalance } from '@/hooks/useQueries';
import { shiftDigits } from '@/utils';
import ProfileAvatar from '@/utils/identicon';
import { ExtendedGroupType } from '@/hooks/useQueries';
import { UpdateGroupModal } from './updateGroupModal';
import { ThresholdDecisionPolicySDKType } from '@liftedinit/manifestjs/dist/codegen/cosmos/group/v1/types';

import { useFeeEstimation, useTx } from '@/hooks';
import { chainName } from '@/config';
import { cosmos } from '@liftedinit/manifestjs';
interface GroupInfoProps {
modalId: string;
group: ExtendedGroupType | null;
Expand All @@ -15,6 +15,9 @@ interface GroupInfoProps {
}

export function GroupInfo({ modalId, group, policyAddress, address, onUpdate }: GroupInfoProps) {
const { tx, isSigning, setIsSigning } = useTx(chainName);
const { leaveGroup } = cosmos.group.v1.MessageComposer.withTypeUrl;
const { estimateFee } = useFeeEstimation(chainName);
Comment on lines +18 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for fee estimation

While the transaction hooks are properly set up, consider adding error handling for fee estimation failures.

- const { estimateFee } = useFeeEstimation(chainName);
+ const { estimateFee, error: feeError } = useFeeEstimation(chainName);
+ 
+ useEffect(() => {
+   if (feeError) {
+     console.error('Fee estimation error:', feeError);
+     // Consider showing user feedback
+   }
+ }, [feeError]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { tx, isSigning, setIsSigning } = useTx(chainName);
const { leaveGroup } = cosmos.group.v1.MessageComposer.withTypeUrl;
const { estimateFee } = useFeeEstimation(chainName);
const { tx, isSigning, setIsSigning } = useTx(chainName);
const { leaveGroup } = cosmos.group.v1.MessageComposer.withTypeUrl;
const { estimateFee, error: feeError } = useFeeEstimation(chainName);
useEffect(() => {
if (feeError) {
console.error('Fee estimation error:', feeError);
// Consider showing user feedback
}
}, [feeError]);

if (!group || !group.policies || group.policies.length === 0) return null;

const policy = group.policies[0];
Expand Down Expand Up @@ -95,6 +98,30 @@ export function GroupInfo({ modalId, group, policyAddress, address, onUpdate }:
return <InfoItem label="Author" value="Invalid author information" />;
}

const handleLeave = async () => {
setIsSigning(true);

try {
const msg = leaveGroup({
address: address,
groupId: group?.id,
});

const fee = await estimateFee(address, [msg]);
await tx([msg], {
fee,
onSuccess: () => {
setIsSigning(false);
onUpdate();
},
});
} catch (error) {
console.error('Error submitting proposal:', error);
} finally {
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Provide user feedback when an error occurs during leave operation

Currently, if an error occurs while leaving the group, the user is not notified. Consider displaying a user-friendly error message to inform the user of the issue.

Add code to display an error notification to the user. For example:

catch (error) {
  console.error('Error leaving group:', error);
+  // Display error to the user
+  showErrorNotification('An error occurred while leaving the group. Please try again.');
} finally {
  setIsSigning(false);
}

Ensure that showErrorNotification is an appropriate method or component in your application for displaying errors.

Committable suggestion skipped: line range outside the PR's diff.

setIsSigning(false);
}
};

return (
<dialog id={modalId} className="modal">
<div className="modal-box bg-secondary rounded-[24px] max-h-['574px'] max-w-[542px] p-6">
Expand All @@ -112,16 +139,27 @@ export function GroupInfo({ modalId, group, policyAddress, address, onUpdate }:

<div className="flex justify-between items-center mb-6">
<span className="text-xl font-semibold text-secondary-content">Info</span>
<button
aria-label={'update-btn'}
className="btn btn-gradient text-white rounded-[12px] h-[52px] w-[140px]"
onClick={() => {
const modal = document.getElementById('update-group-modal') as HTMLDialogElement;
if (modal) modal.showModal();
}}
>
Update
</button>
<div className="flex items-center space-x-4">
<button
aria-label={'update-btn'}
className="btn btn-dropdown text-white rounded-[12px] h-[52px] w-[140px]"
onClick={handleLeave}
disabled={isSigning}
>
{isSigning ? <span className="animate-pulse">Leaving</span> : 'Leave'}
</button>

<button
aria-label={'update-btn'}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Duplicate aria-label values for buttons cause accessibility issues

Both the "Leave" and "Update" buttons have the same aria-label value 'update-btn', which can cause confusion for users relying on assistive technologies.

Assign unique and descriptive aria-labels to each button. Apply this diff:

<button
-   aria-label={'update-btn'}
+   aria-label={'leave-group-btn'}
    className="btn btn-dropdown text-white rounded-[12px] h-[52px] w-[140px]"
    onClick={handleLeave}
    disabled={isSigning}
>
  {isSigning ? <span className="animate-pulse">Leaving</span> : 'Leave'}
</button>

<button
-   aria-label={'update-btn'}
+   aria-label={'update-group-btn'}
    className="btn btn-gradient text-white rounded-[12px] h-[52px] w-[140px]"
    onClick={() => {
      const modal = document.getElementById('update-group-modal') as HTMLDialogElement;
      if (modal) modal.showModal();
    }}
>
  Update
</button>

Committable suggestion skipped: line range outside the PR's diff.

className="btn btn-gradient text-white rounded-[12px] h-[52px] w-[140px]"
onClick={() => {
const modal = document.getElementById('update-group-modal') as HTMLDialogElement;
if (modal) modal.showModal();
}}
>
Update
</button>
</div>
</div>

<div className="space-y-4">
Expand Down
6 changes: 4 additions & 2 deletions pages/groups/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export default function Groups() {
const groupPolicyAddresses =
groupByMemberData?.groups?.map(group => group.policies[0].address) ?? [];

const { proposalsByPolicyAccount, isProposalsError, isProposalsLoading } =
const { proposalsByPolicyAccount, isProposalsError, isProposalsLoading, refetchProposals } =
useProposalsByPolicyAccountAll(groupPolicyAddresses ?? []);

const isLoading = isGroupByMemberLoading || isProposalsLoading;
Expand Down Expand Up @@ -82,6 +82,7 @@ export default function Groups() {
groups={groupByMemberData ?? { groups: [] }}
proposals={proposalsByPolicyAccount}
isLoading={isLoading}
refetch={refetchGroupByMember}
/>
) : isError ? (
<div className="text-center text-error">Error loading groups or proposals</div>
Expand All @@ -93,6 +94,7 @@ export default function Groups() {
groups={groupByMemberData ?? { groups: [] }}
proposals={proposalsByPolicyAccount}
isLoading={isLoading}
refetch={refetchGroupByMember}
/>
{selectedPolicyAddress && (
<GroupInfo
Expand All @@ -104,7 +106,7 @@ export default function Groups() {
) ?? null
}
address={address ?? ''}
onUpdate={() => {}}
onUpdate={refetchGroupByMember}
/>
)}
</>
Expand Down
Loading