chore: proposal details rework #274
Conversation
|
Warning Rate limit exceeded@fmorency has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughThe pull request removes the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
components/groups/modals/voteDetailsModal.tsx (1)
111-119:⚠️ Potential issueAssign the useMemo result to a variable.
The useMemo hook's result is not being assigned to a variable, making it effectively useless.
- useMemo( + const voteMap = useMemo( () => votes?.reduce<VoteMap>((acc, vote) => { const voterKey = vote?.voter?.toLowerCase().trim(); acc[voterKey] = vote?.option; return acc; }, {}), [votes] );
🧹 Nitpick comments (3)
components/groups/modals/tally.tsx (2)
10-48: Consider consolidating tally calculations.The current implementation has separate useMemo hooks for each tally type and percentage, which could be consolidated into a single useMemo for better performance and maintainability.
- const tallyYes = useMemo(() => { - const yes = BigNumber(tallies.tally?.yes_count); - return yes.isFinite() ? yes : BigNumber(0); - }, [tallies]); - - const tallyNo = useMemo(() => { - const no = BigNumber(tallies.tally?.no_count); - return no.isFinite() ? no : BigNumber(0); - }, [tallies]); - - const tallyVeto = useMemo(() => { - const veto = BigNumber(tallies.tally?.no_with_veto_count); - return veto.isFinite() ? veto : BigNumber(0); - }, [tallies]); - - const tallyAbstain = useMemo(() => { - const abstain = BigNumber(tallies.tally?.abstain_count); - return abstain.isFinite() ? abstain : BigNumber(0); - }, [tallies]); + const tallyCounts = useMemo(() => { + const getTally = (value?: string) => { + const count = BigNumber(value); + return count.isFinite() ? count : BigNumber(0); + }; + + const yes = getTally(tallies.tally?.yes_count); + const no = getTally(tallies.tally?.no_count); + const veto = getTally(tallies.tally?.no_with_veto_count); + const abstain = getTally(tallies.tally?.abstain_count); + const total = BigNumber.sum(yes, no, veto, abstain); + + const getPercentage = (value: BigNumber) => + total.isZero() ? BigNumber(0) : value.div(total).multipliedBy(100); + + return { + yes, + no, + veto, + abstain, + total, + yesPercentage: getPercentage(yes), + noPercentage: getPercentage(no), + vetoPercentage: getPercentage(veto), + abstainPercentage: getPercentage(abstain) + }; + }, [tallies]);
52-69: Consider extracting color constants.The color values are hardcoded in multiple places. Consider extracting them into constants or a theme configuration.
+const TALLY_COLORS = { + yes: '#4CAF50', + no: '#E53935', + veto: '#FFB300', + abstain: '#3F51B5' +} as const; <div className="w-full bg-gray-700 rounded-lg h-6 relative mt-2 flex"> <div className={`bg-[#4CAF50] h-6 rounded-l-lg ${tallyYesPercentage.eq(100) && 'rounded-r-lg'}`} - style={{ width: `${tallyYesPercentage}%` }} + style={{ width: `${tallyYesPercentage}%`, backgroundColor: TALLY_COLORS.yes }} ></div> {/* Apply similar changes to other bars */}components/groups/modals/voteDetailsModal.tsx (1)
323-351: Consider extracting proposal state logic.The complex proposal state logic in useMemo could be moved to a separate utility function for better maintainability.
+const getProposalState = ( + proposal: ProposalSDKType, + status: ProposalStatus, + proposalExpired: boolean, + userHasVoted: boolean, + address?: string +) => { + const isWithdrawn = status === ProposalStatus.PROPOSAL_STATUS_WITHDRAWN; + const isAborted = status === ProposalStatus.PROPOSAL_STATUS_ABORTED; + // ... rest of the logic + return { action: null, label: null }; +}; -useMemo(() => { +const proposalState = useMemo(() => { - const isWithdrawn = status === ProposalStatus.PROPOSAL_STATUS_WITHDRAWN; - // ... rest of the logic + return getProposalState(proposal, status, proposalExpired, userHasVoted, address); }, [proposal, proposalExpired, status, userHasVoted, address]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/groups/components/groupControls.tsx(0 hunks)components/groups/modals/tally.tsx(1 hunks)components/groups/modals/voteDetailsModal.tsx(7 hunks)components/groups/utils.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- components/groups/components/groupControls.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
components/groups/utils.tsx
[error] 43-43: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 44-44: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 45-45: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 46-46: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 47-48: The default clause should be the last switch clause.
The following case clause is here:
Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
(lint/suspicious/useDefaultSwitchClauseLast)
[error] 65-65: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 83-83: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 99-99: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 100-100: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (2)
components/groups/modals/tally.tsx (1)
1-8: LGTM! Clean imports and well-defined interface.The component has clear dependencies and a well-typed interface using SDK types.
components/groups/modals/voteDetailsModal.tsx (1)
442-445: LGTM! Clean integration of the Tally component.The Tally component is well integrated and the conditional rendering of the proposal button is clean.
| export function getVoteOptionBadgeColor(option?: VoteOption): string { | ||
| switch (voteOptionFromJSON(option)) { | ||
| case VoteOption.VOTE_OPTION_YES: | ||
| return 'bg-success'; | ||
| case VoteOption.VOTE_OPTION_NO: | ||
| return 'bg-error'; | ||
| case VoteOption.VOTE_OPTION_ABSTAIN: | ||
| return 'bg-info'; | ||
| case VoteOption.VOTE_OPTION_NO_WITH_VETO: | ||
| return 'bg-warning'; | ||
| case VoteOption.UNRECOGNIZED: | ||
| case VoteOption.VOTE_OPTION_UNSPECIFIED: | ||
| default: | ||
| return 'bg-gray-400'; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify switch statement in getVoteOptionBadgeColor.
The function has useless case clauses that can be simplified.
export function getVoteOptionBadgeColor(option?: VoteOption): string {
switch (voteOptionFromJSON(option)) {
case VoteOption.VOTE_OPTION_YES:
return 'bg-success';
case VoteOption.VOTE_OPTION_NO:
return 'bg-error';
case VoteOption.VOTE_OPTION_ABSTAIN:
return 'bg-info';
case VoteOption.VOTE_OPTION_NO_WITH_VETO:
return 'bg-warning';
- case VoteOption.UNRECOGNIZED:
- case VoteOption.VOTE_OPTION_UNSPECIFIED:
default:
return 'bg-gray-400';
}
}📝 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.
| export function getVoteOptionBadgeColor(option?: VoteOption): string { | |
| switch (voteOptionFromJSON(option)) { | |
| case VoteOption.VOTE_OPTION_YES: | |
| return 'bg-success'; | |
| case VoteOption.VOTE_OPTION_NO: | |
| return 'bg-error'; | |
| case VoteOption.VOTE_OPTION_ABSTAIN: | |
| return 'bg-info'; | |
| case VoteOption.VOTE_OPTION_NO_WITH_VETO: | |
| return 'bg-warning'; | |
| case VoteOption.UNRECOGNIZED: | |
| case VoteOption.VOTE_OPTION_UNSPECIFIED: | |
| default: | |
| return 'bg-gray-400'; | |
| } | |
| } | |
| export function getVoteOptionBadgeColor(option?: VoteOption): string { | |
| switch (voteOptionFromJSON(option)) { | |
| case VoteOption.VOTE_OPTION_YES: | |
| return 'bg-success'; | |
| case VoteOption.VOTE_OPTION_NO: | |
| return 'bg-error'; | |
| case VoteOption.VOTE_OPTION_ABSTAIN: | |
| return 'bg-info'; | |
| case VoteOption.VOTE_OPTION_NO_WITH_VETO: | |
| return 'bg-warning'; | |
| default: | |
| return 'bg-gray-400'; | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 99-99: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 100-100: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
components/groups/utils.tsx
Outdated
| export function getProposalButton(proposal: ProposalSDKType): JSX.Element | undefined { | ||
| switch (proposalExecutorResultFromJSON(proposal.executor_result)) { | ||
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_NOT_RUN: | ||
| switch (proposalStatusFromJSON(proposal.status)) { | ||
| case ProposalStatus.PROPOSAL_STATUS_UNSPECIFIED: | ||
| case ProposalStatus.PROPOSAL_STATUS_REJECTED: | ||
| case ProposalStatus.PROPOSAL_STATUS_ABORTED: | ||
| case ProposalStatus.PROPOSAL_STATUS_WITHDRAWN: | ||
| default: | ||
| return undefined; | ||
| case ProposalStatus.PROPOSAL_STATUS_SUBMITTED: | ||
| return ( | ||
| <button className="btn w-full btn-gradient text-white rounded-[12px]">vote</button> | ||
| ); | ||
| case ProposalStatus.PROPOSAL_STATUS_ACCEPTED: | ||
| return ( | ||
| <div className="flex flex-row items-center justify-center gap-2"> | ||
| <button className="btn btn-error text-white rounded-[12px] w-1/2">withdraw</button> | ||
| <button className="btn btn-gradient text-white rounded-[12px] w-1/2">execute</button> | ||
| </div> | ||
| ); | ||
| } | ||
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_FAILURE: | ||
| return ( | ||
| <button className="btn w-full btn-gradient text-white rounded-[12px]">re-execute</button> | ||
| ); | ||
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_SUCCESS: | ||
| default: | ||
| return undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor switch statement structure.
The switch statement has useless case clauses and an incorrectly positioned default clause, which could lead to confusion.
export function getProposalButton(proposal: ProposalSDKType): JSX.Element | undefined {
switch (proposalExecutorResultFromJSON(proposal.executor_result)) {
case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_NOT_RUN:
switch (proposalStatusFromJSON(proposal.status)) {
- case ProposalStatus.PROPOSAL_STATUS_UNSPECIFIED:
- case ProposalStatus.PROPOSAL_STATUS_REJECTED:
- case ProposalStatus.PROPOSAL_STATUS_ABORTED:
- case ProposalStatus.PROPOSAL_STATUS_WITHDRAWN:
- default:
- return undefined;
case ProposalStatus.PROPOSAL_STATUS_SUBMITTED:
return (
<button className="btn w-full btn-gradient text-white rounded-[12px]">vote</button>
);
case ProposalStatus.PROPOSAL_STATUS_ACCEPTED:
return (
<div className="flex flex-row items-center justify-center gap-2">
<button className="btn btn-error text-white rounded-[12px] w-1/2">withdraw</button>
<button className="btn btn-gradient text-white rounded-[12px] w-1/2">execute</button>
</div>
);
+ default:
+ return undefined;
}
case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_FAILURE:
return (
<button className="btn w-full btn-gradient text-white rounded-[12px]">re-execute</button>
);
- case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_SUCCESS:
default:
return undefined;
}
}📝 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.
| export function getProposalButton(proposal: ProposalSDKType): JSX.Element | undefined { | |
| switch (proposalExecutorResultFromJSON(proposal.executor_result)) { | |
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_NOT_RUN: | |
| switch (proposalStatusFromJSON(proposal.status)) { | |
| case ProposalStatus.PROPOSAL_STATUS_UNSPECIFIED: | |
| case ProposalStatus.PROPOSAL_STATUS_REJECTED: | |
| case ProposalStatus.PROPOSAL_STATUS_ABORTED: | |
| case ProposalStatus.PROPOSAL_STATUS_WITHDRAWN: | |
| default: | |
| return undefined; | |
| case ProposalStatus.PROPOSAL_STATUS_SUBMITTED: | |
| return ( | |
| <button className="btn w-full btn-gradient text-white rounded-[12px]">vote</button> | |
| ); | |
| case ProposalStatus.PROPOSAL_STATUS_ACCEPTED: | |
| return ( | |
| <div className="flex flex-row items-center justify-center gap-2"> | |
| <button className="btn btn-error text-white rounded-[12px] w-1/2">withdraw</button> | |
| <button className="btn btn-gradient text-white rounded-[12px] w-1/2">execute</button> | |
| </div> | |
| ); | |
| } | |
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_FAILURE: | |
| return ( | |
| <button className="btn w-full btn-gradient text-white rounded-[12px]">re-execute</button> | |
| ); | |
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_SUCCESS: | |
| default: | |
| return undefined; | |
| } | |
| } | |
| export function getProposalButton(proposal: ProposalSDKType): JSX.Element | undefined { | |
| switch (proposalExecutorResultFromJSON(proposal.executor_result)) { | |
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_NOT_RUN: | |
| switch (proposalStatusFromJSON(proposal.status)) { | |
| case ProposalStatus.PROPOSAL_STATUS_SUBMITTED: | |
| return ( | |
| <button className="btn w-full btn-gradient text-white rounded-[12px]">vote</button> | |
| ); | |
| case ProposalStatus.PROPOSAL_STATUS_ACCEPTED: | |
| return ( | |
| <div className="flex flex-row items-center justify-center gap-2"> | |
| <button className="btn btn-error text-white rounded-[12px] w-1/2">withdraw</button> | |
| <button className="btn btn-gradient text-white rounded-[12px] w-1/2">execute</button> | |
| </div> | |
| ); | |
| default: | |
| return undefined; | |
| } | |
| case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_FAILURE: | |
| return ( | |
| <button className="btn w-full btn-gradient text-white rounded-[12px]">re-execute</button> | |
| ); | |
| default: | |
| return undefined; | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 43-43: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 44-44: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 45-45: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 46-46: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 47-48: The default clause should be the last switch clause.
The following case clause is here:
Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
(lint/suspicious/useDefaultSwitchClauseLast)
[error] 65-65: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #274 +/- ##
==========================================
- Coverage 54.88% 54.47% -0.42%
==========================================
Files 210 211 +1
Lines 17303 17021 -282
==========================================
- Hits 9497 9272 -225
+ Misses 7806 7749 -57 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (11)
components/groups/modals/tallyResults.tsx (2)
17-21: Consider adding a backdrop for better modal accessibility.The modal implementation could benefit from an overlay backdrop for better visual hierarchy and accessibility.
<Dialog open={opened} onClose={onClose} className="modal modal-open fixed flex p-0 m-0 top-0 right-0 z-[9999]" > + <div className="fixed inset-0 bg-black/30" aria-hidden="true" />
34-37: Consider responsive design for mobile views.The fixed grid column widths might not work well on smaller screens.
- <div className="grid grid-cols-[70%_30%] gap-4 font-semibold text-gray-700 dark:text-gray-300 mb-2"> + <div className="grid grid-cols-1 md:grid-cols-[70%_30%] gap-4 font-semibold text-gray-700 dark:text-gray-300 mb-2">components/groups/modals/voting/messagesModal.tsx (1)
5-10: Consider lazy loading syntax highlighter for better performance.The syntax highlighter and its styles could be lazy loaded to improve initial load time.
-import { PrismAsyncLight as SyntaxHighlighter } from 'react-syntax-highlighter'; -import json from 'react-syntax-highlighter/dist/esm/languages/prism/json'; -import oneDark from 'react-syntax-highlighter/dist/esm/styles/prism/one-dark'; -import oneLight from 'react-syntax-highlighter/dist/esm/styles/prism/one-light'; +const SyntaxHighlighter = React.lazy(() => import('react-syntax-highlighter').then(mod => ({ + default: mod.PrismAsyncLight +}))); +const json = React.lazy(() => import('react-syntax-highlighter/dist/esm/languages/prism/json')); +const oneDark = React.lazy(() => import('react-syntax-highlighter/dist/esm/styles/prism/one-dark')); +const oneLight = React.lazy(() => import('react-syntax-highlighter/dist/esm/styles/prism/one-light'));components/groups/components/__tests__/CountdownTimer.test.tsx (1)
8-10: Reset mock function between tests.Add cleanup for the mock function to ensure tests are isolated.
describe('CountdownTimer', () => { - afterEach(cleanup); + afterEach(() => { + cleanup(); + refetch.mockReset(); + });components/groups/modals/voteDetailsModal.tsx (2)
101-110: Add test coverage for query invalidation.The centralization of query invalidation is a good practice, but the function needs test coverage to ensure reliability.
Consider adding unit tests to verify that:
- All necessary queries are invalidated
- The function handles errors gracefully
- The promises resolve correctly
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 101-108: components/groups/modals/voteDetailsModal.tsx#L101-L108
Added lines #L101 - L108 were not covered by tests
112-136: Enhance error handling in vote handling logic.The error handling in the vote function could be improved to provide better user feedback.
Consider these improvements:
} catch (error) { - console.error('Failed to vote: ', error); + const errorMessage = error instanceof Error ? error.message : 'Unknown error occurred'; + console.error('Failed to vote:', errorMessage); + // Add user notification here + throw new Error(`Failed to vote: ${errorMessage}`); }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 128-128: components/groups/modals/voteDetailsModal.tsx#L128
Added line #L128 was not covered by testscomponents/groups/components/groupControls.tsx (1)
400-423: Add test coverage for proposal status handling.The switch-case structure improves code organization, but the status handling logic needs test coverage.
Consider adding unit tests for:
- Different proposal executor results
- Various proposal status combinations
- Edge cases in tally calculations
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 401-402: components/groups/components/groupControls.tsx#L401-L402
Added lines #L401 - L402 were not covered by tests
[warning] 411-417: components/groups/components/groupControls.tsx#L411-L417
Added lines #L411 - L417 were not covered by tests
[warning] 419-419: components/groups/components/groupControls.tsx#L419
Added line #L419 was not covered by testshooks/useQueries.ts (4)
278-307: Consider adding error handling for invalid proposal IDs.The new hook is well-structured, but could benefit from additional error handling.
Consider adding validation and error handling:
export const useProposalById = (proposalId: bigint, options: UseProposalByIdOptions = {}) => { const { lcdQueryClient } = useLcdQueryClient(); const fetchProposalInfo = async () => { if (!lcdQueryClient) { throw new Error('LCD Client not ready'); } + if (proposalId <= 0n) { + throw new Error('Invalid proposal ID'); + } return await lcdQueryClient.cosmos.group.v1.proposal({ proposalId: proposalId }); };
358-365: Improve the enabled condition check.The
enabledcondition should check ifproposalIdis positive.Apply this improvement to both hooks:
-enabled: !!lcdQueryClient && proposalId !== undefined, +enabled: !!lcdQueryClient && proposalId > 0n,Also applies to: 387-393
599-606: Improve error handling in token factory hooks.While the debouncing implementation is good, the error handling could be enhanced.
Consider adding validation for the denom parameter:
const fetchAuthority = async () => { if (!rpcQueryClient) { throw new Error('RPC Client not ready'); } if (!denom) { throw new Error('Denom not provided'); } + if (!denom.match(/^[a-z][a-z0-9/]{2,127}$/)) { + throw new Error('Invalid denom format'); + } return await rpcQueryClient.osmosis.tokenfactory.v1beta1.denomAuthorityMetadata({ denom: denom, }); };Also applies to: 633-640
871-878: Enhance error handling for address and pagination parameters.While the debouncing implementation is good, the error handling could be improved.
Consider adding parameter validation:
const fetchMessages = async () => { + if (!address?.match(/^[a-z0-9]{40,59}$/)) { + throw new Error('Invalid address format'); + } + if (page < 1 || pageSize < 1 || pageSize > 100) { + throw new Error('Invalid pagination parameters'); + } const baseUrl = `${indexerUrl}/rpc/get_messages_for_address?_address=${address}`;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
components/bank/components/historyBox.tsx(0 hunks)components/bank/modals/txInfo.tsx(2 hunks)components/groups/components/CountdownTimer.tsx(2 hunks)components/groups/components/__tests__/CountdownTimer.test.tsx(3 hunks)components/groups/components/__tests__/groupProposals.test.tsx(0 hunks)components/groups/components/groupControls.tsx(9 hunks)components/groups/components/myGroups.tsx(3 hunks)components/groups/modals/__tests__/voteDetailsModal.test.tsx(4 hunks)components/groups/modals/tally.tsx(1 hunks)components/groups/modals/tallyResults.tsx(1 hunks)components/groups/modals/voteDetailsModal.tsx(8 hunks)components/groups/modals/voting/messagesModal.tsx(3 hunks)components/groups/utils.tsx(1 hunks)components/index.tsx(0 hunks)components/messageSyntax.tsx(0 hunks)components/react/authSignerModal.tsx(2 hunks)hooks/useQueries.ts(14 hunks)hooks/useResponsivePageSize.ts(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (4)
- components/index.tsx
- components/bank/components/historyBox.tsx
- components/groups/components/tests/groupProposals.test.tsx
- components/messageSyntax.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- components/groups/modals/tally.tsx
🧰 Additional context used
🧠 Learnings (1)
components/bank/modals/txInfo.tsx (1)
Learnt from: chalabi2
PR: liftedinit/manifest-app#87
File: components/bank/components/historyBox.tsx:276-276
Timestamp: 2024-11-26T23:12:45.694Z
Learning: In `components/bank/components/historyBox.tsx`, the `TxInfoModal` component will always receive a valid `tx` prop because it's only rendered when there is transaction data (`selectedTx` is defined), so additional null handling within `TxInfoModal` is not necessary.
🪛 GitHub Check: codecov/patch
components/groups/components/myGroups.tsx
[warning] 208-212: components/groups/components/myGroups.tsx#L208-L212
Added lines #L208 - L212 were not covered by tests
components/react/authSignerModal.tsx
[warning] 192-192: components/react/authSignerModal.tsx#L192
Added line #L192 was not covered by tests
[warning] 194-194: components/react/authSignerModal.tsx#L194
Added line #L194 was not covered by tests
components/groups/components/groupControls.tsx
[warning] 110-110: components/groups/components/groupControls.tsx#L110
Added line #L110 was not covered by tests
[warning] 122-122: components/groups/components/groupControls.tsx#L122
Added line #L122 was not covered by tests
[warning] 131-131: components/groups/components/groupControls.tsx#L131
Added line #L131 was not covered by tests
[warning] 240-240: components/groups/components/groupControls.tsx#L240
Added line #L240 was not covered by tests
[warning] 401-402: components/groups/components/groupControls.tsx#L401-L402
Added lines #L401 - L402 were not covered by tests
[warning] 411-417: components/groups/components/groupControls.tsx#L411-L417
Added lines #L411 - L417 were not covered by tests
[warning] 419-419: components/groups/components/groupControls.tsx#L419
Added line #L419 was not covered by tests
components/groups/modals/voteDetailsModal.tsx
[warning] 75-75: components/groups/modals/voteDetailsModal.tsx#L75
Added line #L75 was not covered by tests
[warning] 78-80: components/groups/modals/voteDetailsModal.tsx#L78-L80
Added lines #L78 - L80 were not covered by tests
[warning] 88-88: components/groups/modals/voteDetailsModal.tsx#L88
Added line #L88 was not covered by tests
[warning] 101-108: components/groups/modals/voteDetailsModal.tsx#L101-L108
Added lines #L101 - L108 were not covered by tests
[warning] 128-128: components/groups/modals/voteDetailsModal.tsx#L128
Added line #L128 was not covered by tests
[warning] 146-150: components/groups/modals/voteDetailsModal.tsx#L146-L150
Added lines #L146 - L150 were not covered by tests
[warning] 168-168: components/groups/modals/voteDetailsModal.tsx#L168
Added line #L168 was not covered by tests
[warning] 228-234: components/groups/modals/voteDetailsModal.tsx#L228-L234
Added lines #L228 - L234 were not covered by tests
components/groups/utils.tsx
[warning] 26-29: components/groups/utils.tsx#L26-L29
Added lines #L26 - L29 were not covered by tests
[warning] 36-37: components/groups/utils.tsx#L36-L37
Added lines #L36 - L37 were not covered by tests
[warning] 39-40: components/groups/utils.tsx#L39-L40
Added lines #L39 - L40 were not covered by tests
[warning] 87-87: components/groups/utils.tsx#L87
Added line #L87 was not covered by tests
[warning] 97-97: components/groups/utils.tsx#L97
Added line #L97 was not covered by tests
[warning] 103-108: components/groups/utils.tsx#L103-L108
Added lines #L103 - L108 were not covered by tests
[warning] 110-110: components/groups/utils.tsx#L110
Added line #L110 was not covered by tests
[warning] 112-112: components/groups/utils.tsx#L112
Added line #L112 was not covered by tests
[warning] 123-123: components/groups/utils.tsx#L123
Added line #L123 was not covered by tests
[warning] 127-133: components/groups/utils.tsx#L127-L133
Added lines #L127 - L133 were not covered by tests
[warning] 143-150: components/groups/utils.tsx#L143-L150
Added lines #L143 - L150 were not covered by tests
[warning] 152-152: components/groups/utils.tsx#L152
Added line #L152 was not covered by tests
🪛 Biome (1.9.4)
components/groups/utils.tsx
[error] 26-26: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 27-27: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 28-28: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 29-29: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 30-31: The default clause should be the last switch clause.
The following case clause is here:
Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
(lint/suspicious/useDefaultSwitchClauseLast)
[error] 87-87: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 133-133: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 149-149: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 150-150: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (17)
hooks/useResponsivePageSize.ts (1)
2-2: LGTM! Simplified imports.The removal of the unused
useCallbackimport helps maintain clean code.components/groups/components/CountdownTimer.tsx (1)
1-41: LGTM! The countdown timer implementation is robust and well-structured.The component correctly handles the timer end event with proper debouncing using
useRefto prevent multiple calls.components/bank/modals/txInfo.tsx (1)
50-51: LGTM! The component has been simplified by removing prop drilling.The changes improve maintainability by centralizing the explorer URL configuration.
Also applies to: 54-55, 60-60, 65-65
components/groups/utils.tsx (3)
25-31: Refactor switch statement structure.The switch statement has useless case clauses and an incorrectly positioned default clause, which could lead to confusion.
Apply this diff to improve the code:
- switch (proposalStatusFromJSON(proposal.status)) { - case ProposalStatus.PROPOSAL_STATUS_UNSPECIFIED: - case ProposalStatus.PROPOSAL_STATUS_REJECTED: - case ProposalStatus.PROPOSAL_STATUS_ABORTED: - case ProposalStatus.PROPOSAL_STATUS_WITHDRAWN: - default: - return undefined; + switch (proposalStatusFromJSON(proposal.status)) { case ProposalStatus.PROPOSAL_STATUS_SUBMITTED: return ( // ... existing code ... ); case ProposalStatus.PROPOSAL_STATUS_ACCEPTED: return ( // ... existing code ... ); + default: + return undefined; } - case ProposalExecutorResult.PROPOSAL_EXECUTOR_RESULT_SUCCESS: default: return undefined;Also applies to: 87-89
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 27-27: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 28-28: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 29-29: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 30-31: The default clause should be the last switch clause.
The following case clause is here:
Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
(lint/suspicious/useDefaultSwitchClauseLast)
🪛 GitHub Check: codecov/patch
[warning] 26-29: components/groups/utils.tsx#L26-L29
Added lines #L26 - L29 were not covered by tests
139-154: Simplify switch statement in getVoteOptionBadgeColor.The function has useless case clauses that can be simplified.
Apply this diff to improve the code:
export function getVoteOptionBadgeColor(option?: VoteOption): string { switch (voteOptionFromJSON(option)) { case VoteOption.VOTE_OPTION_YES: return 'bg-success'; case VoteOption.VOTE_OPTION_NO: return 'bg-error'; case VoteOption.VOTE_OPTION_ABSTAIN: return 'bg-info'; case VoteOption.VOTE_OPTION_NO_WITH_VETO: return 'bg-warning'; - case VoteOption.UNRECOGNIZED: - case VoteOption.VOTE_OPTION_UNSPECIFIED: default: return 'bg-gray-400'; } }🧰 Tools
🪛 Biome (1.9.4)
[error] 149-149: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 150-150: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🪛 GitHub Check: codecov/patch
[warning] 143-150: components/groups/utils.tsx#L143-L150
Added lines #L143 - L150 were not covered by tests
[warning] 152-152: components/groups/utils.tsx#L152
Added line #L152 was not covered by tests
26-29: Improve test coverage for utility functions.Several code paths are not covered by tests, including:
- Various proposal status cases
- Vote option cases
- Loading states
Consider adding test cases for:
- Different proposal statuses
- Various vote options
- Loading and error states
- Edge cases for each utility function
Would you like me to help generate comprehensive test cases for these scenarios?
Also applies to: 36-37, 39-40, 87-87, 97-97, 103-108, 110-110, 112-112, 123-123, 127-133, 143-150, 152-152
🧰 Tools
🪛 Biome (1.9.4)
[error] 26-26: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 27-27: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 28-28: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 29-29: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🪛 GitHub Check: codecov/patch
[warning] 26-29: components/groups/utils.tsx#L26-L29
Added lines #L26 - L29 were not covered by testscomponents/groups/modals/__tests__/voteDetailsModal.test.tsx (1)
28-53: LGTM! Test coverage is comprehensive and well-structured.The test suite effectively covers:
- Different proposal states and user roles
- Conditional rendering of buttons
- User interactions
- Edge cases
Also applies to: 89-92, 94-98, 100-110, 138-155, 157-171
components/groups/modals/voteDetailsModal.tsx (2)
34-40: LGTM! Props interface changes look good.The updated props interface correctly reflects the component's new responsibilities, removing the
membersdependency and adding necessary props for proposal management.
218-294: LGTM! UI improvements enhance user experience.The UI changes are well-structured and improve both aesthetics and accessibility:
- Clear header section with proposal ID and status
- Centrally positioned countdown timer
- Improved layout for proposal details
- Integration of the new
Tallycomponent🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 228-234: components/groups/modals/voteDetailsModal.tsx#L228-L234
Added lines #L228 - L234 were not covered by testscomponents/react/authSignerModal.tsx (1)
187-216: LGTM! Improved JSON data formatting.The use of
SyntaxHighlighterwith theme-aware styling enhances the readability of transaction data. The implementation correctly handles different data types and provides a consistent visual experience.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 192-192: components/react/authSignerModal.tsx#L192
Added line #L192 was not covered by tests
[warning] 194-194: components/react/authSignerModal.tsx#L194
Added line #L194 was not covered by testscomponents/groups/components/groupControls.tsx (1)
77-77: LGTM! Improved state management.The change from
selectedProposaltoselectedProposalIdis a good practice as it reduces the amount of data stored in state.Also applies to: 590-598
components/groups/components/myGroups.tsx (1)
207-214: LGTM! Improved asynchronous data handling.The use of
Promise.allfor concurrent data fetching is a good practice that improves performance. The change to return promises from refetch functions allows better control over the refetch lifecycle.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 208-212: components/groups/components/myGroups.tsx#L208-L212
Added lines #L208 - L212 were not covered by testshooks/useQueries.ts (4)
2-2: LGTM! Good choice of debounce time.The imports and debounce time constant are well-structured. The 1-second debounce time is a reasonable default that balances responsiveness with performance.
Also applies to: 8-8, 20-22
115-115: LGTM! More descriptive query keys.The query keys have been updated to be more specific (
groupInfoByMemberandgroupInfoByAdmin), which improves cache management and prevents key collisions.Also applies to: 175-175
261-268: LGTM! Good performance optimizations.The changes improve performance and user experience by:
- Debouncing the policy account parameter to reduce unnecessary queries
- Using
keepPreviousDatato provide a smoother experience during refetches
321-329: LGTM! Consistent implementation of debouncing for arrays.The changes maintain consistency with other hooks while properly handling array debouncing.
package.json (1)
55-55:❓ Verification inconclusive
Verify compatibility with React 18.3.1.
The update to React 18.3.1 is significant. Please ensure all components and dependencies are compatible with this version.
Also applies to: 77-79, 102-103
🌐 Web query:
What is the latest stable version of React? Are there any known issues with version 18.3.1?💡 Result:
The latest stable version of React as of February 2025 is 19.0, released on December 5, 2024[3][4]. This version introduces features like Server Components, enhanced Suspense data fetching, and compiler optimizations[1][3].
For React 18.3.1 (the final minor release before React 19), there are several known issues:
Dependency Mismatch Errors
Developers report version conflicts when mixing React 18.3.1 with older ecosystem packages like Atlaskit components (designed for React 16), causing installation failures and runtime errors about invalid hook calls[2][5].Build Tool Compatibility
Some projects using Vite experience deployment-specific failures in production builds that don't occur locally, particularly with undefined reference errors in minified code[5].Legacy Package Interactions
Thereact-test-rendererpackage shows deprecation warnings in React 18.3.1 environments[8], and shallow rendering techniques face compatibility challenges due to internal React changes[8].
Issue Category Example Manifestations Affected Versions Dependency Conflicts npm ERR! ERESOLVEduring installs[2]18.3.x with <18.x dependencies Production Builds Cannot read properties of undefinederrors[5]Projects using Vite+React 18.3.x Testing Tools Shallow renderer deprecation warnings[8] All React 18.x versions The React team recommends upgrading to React 19 where possible, as it resolves many architectural limitations while maintaining backward compatibility through automatic compiler optimizations[1][3]. For teams needing to stay on React 18.x, ensuring all dependencies use compatible React versions (via
npm ls react) helps avoid common pitfalls[2][5].Citations:
- 1: https://dev.to/sovannaro/react-18-vs-react-19-key-differences-to-know-for-2025-1614
- 2: https://community.developer.atlassian.com/t/versions-missmatch-between-react-and-react-dom-after-switch-to-ui-kit-2-ga/78080
- 3: https://endoflife.date/react
- 4: https://react.dev/versions
- 5: https://answers.netlify.com/t/react-component-fails-to-load-on-deployed-site-but-works-locally/121422
- 6: https://www.youtube.com/watch?v=0I25Ch3h1Eg
- 7: https://legacy.reactjs.org/versions/
- 8: https://react.dev/blog/2024/04/25/react-19-upgrade-guide
- 9: https://github.com/facebook/react/blob/main/CHANGELOG.md
- 10: https://dev.to/holasoymalva/why-i-decided-to-stop-working-with-reactjs-in-2025-4d1l
Verify compatibility with React 18.3.1 and consider a migration to React 19.0 if possible.
The update to React 18.3.1 is significant. As of February 2025, React 19.0 is the latest stable version and offers important enhancements. Note the following concerns with React 18.3.1:
- Dependency Conflicts: There are reports of version mismatch errors when using older ecosystem packages (e.g., Atlaskit components) with React 18.3.1.
- Build Tool Compatibility: Some projects (especially those using Vite) have experienced production build failures and runtime issues.
- Legacy Package Interactions: Tools like
react-test-renderermay emit deprecation warnings, and certain testing techniques (like shallow rendering) might face compatibility challenges.Please verify that all components and dependencies are compatible with React 18.3.1. If these issues impact your project, consider upgrading to React 19.0 to benefit from improved stability and performance.
Applies to updates in:
- package.json: Lines 55 (dependency addition)
- package.json: Lines 77-79, 102-103 (React and related dependency updates)
| {JSON.stringify( | ||
| proposal.messages, | ||
| (_, v) => (typeof v === 'bigint' ? v.toString() : v), | ||
| 2 | ||
| )} | ||
| </SyntaxHighlighter> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON stringification.
The current implementation might throw errors if the messages contain circular references or invalid JSON.
- {JSON.stringify(
- proposal.messages,
- (_, v) => (typeof v === 'bigint' ? v.toString() : v),
- 2
- )}
+ {(() => {
+ try {
+ return JSON.stringify(
+ proposal.messages,
+ (_, v) => (typeof v === 'bigint' ? v.toString() : v),
+ 2
+ );
+ } catch (error) {
+ console.error('Failed to stringify messages:', error);
+ return 'Error: Unable to display messages';
+ }
+ })()}📝 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.
| {JSON.stringify( | |
| proposal.messages, | |
| (_, v) => (typeof v === 'bigint' ? v.toString() : v), | |
| 2 | |
| )} | |
| </SyntaxHighlighter> | |
| {(() => { | |
| try { | |
| return JSON.stringify( | |
| proposal.messages, | |
| (_, v) => (typeof v === 'bigint' ? v.toString() : v), | |
| 2 | |
| ); | |
| } catch (error) { | |
| console.error('Failed to stringify messages:', error); | |
| return 'Error: Unable to display messages'; | |
| } | |
| })()} | |
| </SyntaxHighlighter> |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
components/groups/modals/__tests__/voteDetailsModal.test.tsx (3)
94-98: Enhance test coverage for expanded messages modal.The test only verifies the presence of the modal title. Consider adding assertions for:
- The actual messages content
- Modal close functionality
- Any interactive elements within the modal
112-171: Improve spy cleanup in conditional rendering tests.The spy cleanup could be moved to the afterEach block to ensure consistent cleanup across all tests. This would prevent potential test interference.
afterEach(() => { mock.restore(); cleanup(); + jest.restoreAllMocks(); });
71-193: Consider reorganizing test cases for better readability.The test cases could be better organized using describe blocks to group related tests:
- Basic rendering tests
- Button rendering tests
- User interaction tests
Example structure:
describe('VoteDetailsModal', () => { describe('Basic Rendering', () => { test('renders the component with provided props', () => { // ... }); test('renders the tally chart', () => { // ... }); // ... }); describe('Button Rendering', () => { test('conditionally renders execute button when proposal is accepted', () => { // ... }); // ... }); describe('User Interactions', () => { test('handles vote button click and opens voting modal', () => { // ... }); // ... }); });components/groups/utils.tsx (1)
121-137: Add test coverage and simplify switch statement.
The following vote options lack test coverage:
- Line 123: Unspecified option
- Lines 127-133: No, abstain, veto, and unrecognized options
The switch statement can be simplified by removing the redundant
UNRECOGNIZEDcase.Apply this diff to simplify the switch statement:
switch (voteOptionFromJSON(option)) { case VoteOption.VOTE_OPTION_UNSPECIFIED: return 'unspecified'; case VoteOption.VOTE_OPTION_YES: return 'yes'; case VoteOption.VOTE_OPTION_NO: return 'no'; case VoteOption.VOTE_OPTION_ABSTAIN: return 'abstain'; case VoteOption.VOTE_OPTION_NO_WITH_VETO: return 'veto'; - case VoteOption.UNRECOGNIZED: default: return 'unknown'; }Would you like me to help generate test cases for the uncovered vote options?
🧰 Tools
🪛 Biome (1.9.4)
[error] 133-133: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🪛 GitHub Check: codecov/patch
[warning] 123-123: components/groups/utils.tsx#L123
Added line #L123 was not covered by tests
[warning] 127-133: components/groups/utils.tsx#L127-L133
Added lines #L127 - L133 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/groups/modals/__tests__/voteDetailsModal.test.tsx(4 hunks)components/groups/utils.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
components/groups/utils.tsx
[error] 26-26: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 27-27: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 28-28: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 29-29: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 30-31: The default clause should be the last switch clause.
The following case clause is here:
Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
(lint/suspicious/useDefaultSwitchClauseLast)
[error] 87-87: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 133-133: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 149-149: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 150-150: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🪛 GitHub Check: codecov/patch
components/groups/utils.tsx
[warning] 26-29: components/groups/utils.tsx#L26-L29
Added lines #L26 - L29 were not covered by tests
[warning] 36-37: components/groups/utils.tsx#L36-L37
Added lines #L36 - L37 were not covered by tests
[warning] 39-40: components/groups/utils.tsx#L39-L40
Added lines #L39 - L40 were not covered by tests
[warning] 87-87: components/groups/utils.tsx#L87
Added line #L87 was not covered by tests
[warning] 97-97: components/groups/utils.tsx#L97
Added line #L97 was not covered by tests
[warning] 103-108: components/groups/utils.tsx#L103-L108
Added lines #L103 - L108 were not covered by tests
[warning] 110-110: components/groups/utils.tsx#L110
Added line #L110 was not covered by tests
[warning] 112-112: components/groups/utils.tsx#L112
Added line #L112 was not covered by tests
[warning] 123-123: components/groups/utils.tsx#L123
Added line #L123 was not covered by tests
[warning] 127-133: components/groups/utils.tsx#L127-L133
Added lines #L127 - L133 were not covered by tests
[warning] 143-150: components/groups/utils.tsx#L143-L150
Added lines #L143 - L150 were not covered by tests
[warning] 152-152: components/groups/utils.tsx#L152
Added line #L152 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
components/groups/modals/__tests__/voteDetailsModal.test.tsx (2)
1-54: LGTM! Well-structured mock setup.The mock implementations are comprehensive and follow testing best practices. The default mock objects for hooks are well-structured and reusable.
55-70: LGTM! Clean test setup.The test setup is well-organized with proper cleanup and clear default props.
components/groups/utils.tsx (2)
93-119: Add test coverage for proposal status cases.The following status cases lack test coverage:
- Line 97: Unspecified status
- Lines 103-108: Withdrawn, rejected, and aborted statuses
- Line 110: Unknown status
- Line 112: Success status
Would you like me to help generate test cases for these uncovered status mappings?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 97-97: components/groups/utils.tsx#L97
Added line #L97 was not covered by tests
[warning] 103-108: components/groups/utils.tsx#L103-L108
Added lines #L103 - L108 were not covered by tests
[warning] 110-110: components/groups/utils.tsx#L110
Added line #L110 was not covered by tests
[warning] 112-112: components/groups/utils.tsx#L112
Added line #L112 was not covered by tests
139-154: Simplify switch statement and add test coverage.
The following vote options lack test coverage:
- Lines 143-150: No, abstain, veto, unspecified, and unrecognized options
- Line 152: Default case
The switch statement can be simplified by removing redundant cases.
Apply this diff to simplify the switch statement:
switch (voteOptionFromJSON(option)) { case VoteOption.VOTE_OPTION_YES: return 'bg-success'; case VoteOption.VOTE_OPTION_NO: return 'bg-error'; case VoteOption.VOTE_OPTION_ABSTAIN: return 'bg-info'; case VoteOption.VOTE_OPTION_NO_WITH_VETO: return 'bg-warning'; - case VoteOption.UNRECOGNIZED: - case VoteOption.VOTE_OPTION_UNSPECIFIED: default: return 'bg-gray-400'; }Would you like me to help generate test cases for the uncovered vote options?
🧰 Tools
🪛 Biome (1.9.4)
[error] 149-149: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 150-150: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🪛 GitHub Check: codecov/patch
[warning] 143-150: components/groups/utils.tsx#L143-L150
Added lines #L143 - L150 were not covered by tests
[warning] 152-152: components/groups/utils.tsx#L152
Added line #L152 was not covered by tests
This PR reworks the proposal detail modal.
Tallycomponent (the bar)TallyResultscomponent (the table)Fixes #241
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor